[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN][RFC PATCH v3 01/14] xen/arm/device: Remove __init from function type


  • To: Vikram Garhwal <fnu.vikram@xxxxxxxxxx>
  • From: Luca Fancellu <Luca.Fancellu@xxxxxxx>
  • Date: Mon, 14 Mar 2022 12:31:06 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=8bM1Gm+RvVmUA+1zyEjvmswn5Rv6KWFgmLnYkWdQqsU=; b=CAxLmuflo/s3NxfZUlU97AnKWMdr+HC7Cn3Z5I5EXjBsFThupBzLoJFvy/mATDIcRO2mJkaVOPG6UJJnVgj9kAGFPjBqiIVt+P8+UZJPRpw8RyhZvLUKWWXnt9Ynq/zRjxJqZlrhgOXgspXumsAKKf2f50uNlHfIjhi9ICuCSo5B1CfSwrWpqNRNs55TCK5rV/mAZ6JtCsGqltyeB+N4NZTIjFTmVc9fD4DRLBEhRCyiSP80dVIEIeR5qWrYh7oPdWnj4JY/VrHXYXglh0o28uF6RKJNwJKxmjAutBo1HOz7qPBXuvTo2N4AUEKcwP73mNpskb8b3mzobimpsUx34w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=KcvYXCJx6ewEwJI+74QGa/Yvt2ddw5Qh6ThrmpVTF81mvdYAsZTx/5oCpGEtbOwKCqV9h6vn2DIHqD3LtpoES0qKz4uayMewmyCVglQO9OO1kzggLziKnETNyqgV1gB3F4naU2rXXqs5bJ9iVvB9NqKCNJZSuRCdkxF06dRg15WzBWc3MoWowfn258SLHrTBO93Bol1y1rZY5njxkZanjrBgXP4mLpWDMcY76XylYQ1mbbvHZ6kYEDD36iw/EQ3iqHShzjdv4gRuHgISGEG2Sfk+lOKPd5rWRdk9oOqw1kZw6jVZmo8a7OwoJ79+mba/IoK80d2T1BKnxkHs/TuqCA==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "sstabellini@xxxxxxxxxx" <sstabellini@xxxxxxxxxx>, "julien@xxxxxxx" <julien@xxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, "volodymyr_babchuk@xxxxxxxx" <volodymyr_babchuk@xxxxxxxx>
  • Delivery-date: Mon, 14 Mar 2022 12:31:27 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHYMyVyyZZSb95l5UaUheP8eaC2Lqy+2HMA
  • Thread-topic: [XEN][RFC PATCH v3 01/14] xen/arm/device: Remove __init from function type


> On 8 Mar 2022, at 19:46, Vikram Garhwal <fnu.vikram@xxxxxxxxxx> wrote:
> 
> Change function type of following function to access during runtime:
>    1. map_irq_to_domain()
>    2. handle_device_interrupt()
>    3. map_range_to_domain()
>    4. unflatten_dt_node()
>    5. unflatten_device_tree()
> 
> Move map_irq_to_domain(), handle_device_interrupt() and map_range_to_domain() 
> to
> device.c.
> 
> These changes are done to support the dynamic programming of a nodes where an
> overlay node will be added to fdt and unflattened node will be added to 
> dt_host.
> Furthermore, IRQ and mmio mapping will be done for the added node.
> 
> Signed-off-by: Vikram Garhwal <fnu.vikram@xxxxxxxxxx>
> ---
> xen/arch/arm/device.c            | 136 +++++++++++++++++++++++++++++
> xen/arch/arm/domain_build.c      | 142 -------------------------------
> xen/arch/arm/include/asm/setup.h |   3 +
> xen/common/device_tree.c         |  20 ++---
> xen/include/xen/device_tree.h    |   5 ++
> 5 files changed, 154 insertions(+), 152 deletions(-)
> 
> diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
> index 70cd6c1a19..0dfd33b33e 100644
> --- a/xen/arch/arm/device.c
> +++ b/xen/arch/arm/device.c
> @@ -21,6 +21,9 @@
> #include <xen/errno.h>
> #include <xen/init.h>
> #include <xen/lib.h>
> +#include <xen/iocap.h>
> +#include <asm/domain_build.h>
> +#include <asm/setup.h>
> 
> extern const struct device_desc _sdevice[], _edevice[];
> extern const struct acpi_device_desc _asdevice[], _aedevice[];
> @@ -84,6 +87,139 @@ enum device_class device_get_class(const struct 
> dt_device_node *dev)
>     return DEVICE_UNKNOWN;
> }
> 
> +int map_irq_to_domain(struct domain *d, unsigned int irq,
> +                      bool need_mapping, const char *devname)
> +{
> +    int res;
> +
> +    res = irq_permit_access(d, irq);
> +    if ( res )
> +    {
> +        printk(XENLOG_ERR "Unable to permit to dom%u access to IRQ %u\n",
> +               d->domain_id, irq);
> +        return res;
> +    }
> +
> +    if ( need_mapping )
> +    {
> +        /*
> +         * Checking the return of vgic_reserve_virq is not
> +         * necessary. It should not fail except when we try to map
> +         * the IRQ twice. This can legitimately happen if the IRQ is shared
> +         */
> +        vgic_reserve_virq(d, irq);
> +
> +        res = route_irq_to_guest(d, irq, irq, devname);
> +        if ( res < 0 )
> +        {
> +            printk(XENLOG_ERR "Unable to map IRQ%"PRId32" to dom%d\n",
> +                   irq, d->domain_id);
> +            return res;
> +        }
> +    }
> +
> +    dt_dprintk("  - IRQ: %u\n", irq);
> +    return 0;
> +}
> +
> +int map_range_to_domain(const struct dt_device_node *dev,
> +                        u64 addr, u64 len, void *data)
> +{
> +    struct map_range_data *mr_data = data;
> +    struct domain *d = mr_data->d;
> +    int res;
> +
> +    res = iomem_permit_access(d, paddr_to_pfn(addr),
> +            paddr_to_pfn(PAGE_ALIGN(addr + len - 1)));

Hi Vikram,

Why the if ( strncasecmp(dt_node_full_name(dev), "/reserved-memory/",
strlen("/reserved-memory/")) != 0 ) was dropped?


> +    if ( res )
> +    {
> +        printk(XENLOG_ERR "Unable to permit to dom%d access to"
> +                " 0x%"PRIx64" - 0x%"PRIx64"\n",
> +                d->domain_id,
> +                addr & PAGE_MASK, PAGE_ALIGN(addr + len) - 1);
> +        return res;
> +    }
> +
> +    if ( !mr_data->skip_mapping )
> +    {
> +        res = map_regions_p2mt(d,
> +                               gaddr_to_gfn(addr),
> +                               PFN_UP(len),
> +                               maddr_to_mfn(addr),
> +                               mr_data->p2mt);
> +
> +        if ( res < 0 )
> +        {
> +            printk(XENLOG_ERR "Unable to map 0x%"PRIx64
> +                   " - 0x%"PRIx64" in domain %d\n",
> +                   addr & PAGE_MASK, PAGE_ALIGN(addr + len) - 1,
> +                   d->domain_id);
> +            return res;
> +        }
> +    }
> +
> +    dt_dprintk("  - MMIO: %010"PRIx64" - %010"PRIx64" P2MType=%x\n",
> +               addr, addr + len, mr_data->p2mt);
> +
> +    return 0;
> +}
> +
> +/*
> + * handle_device_interrupts retrieves the interrupts configuration from
> + * a device tree node and maps those interrupts to the target domain.
> + *
> + * Returns:
> + *   < 0 error
> + *   0   success
> + */
> +int handle_device_interrupts(struct domain *d,
> +                             struct dt_device_node *dev,
> +                             bool need_mapping)
> +{
> +    unsigned int i, nirq;
> +    int res;
> +    struct dt_raw_irq rirq;
> +
> +    nirq = dt_number_of_irq(dev);
> +
> +    /* Give permission and map IRQs */
> +    for ( i = 0; i < nirq; i++ )
> +    {
> +        res = dt_device_get_raw_irq(dev, i, &rirq);
> +        if ( res )
> +        {
> +            printk(XENLOG_ERR "Unable to retrieve irq %u for %s\n",
> +                   i, dt_node_full_name(dev));
> +            return res;
> +        }
> +
> +        /*
> +         * Don't map IRQ that have no physical meaning
> +         * ie: IRQ whose controller is not the GIC
> +         */
> +        if ( rirq.controller != dt_interrupt_controller )
> +        {
> +            dt_dprintk("irq %u not connected to primary controller. 
> Connected to %s\n",
> +                      i, dt_node_full_name(rirq.controller));
> +            continue;
> +        }
> +
> +        res = platform_get_irq(dev, i);
> +        if ( res < 0 )
> +        {
> +            printk(XENLOG_ERR "Unable to get irq %u for %s\n",
> +                   i, dt_node_full_name(dev));
> +            return res;
> +        }
> +
> +        res = map_irq_to_domain(d, res, need_mapping, dt_node_name(dev));
> +        if ( res )
> +            return res;
> +    }
> +
> +    return 0;
> +}
> +
> /*
>  * Local variables:
>  * mode: C
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 8be01678de..b06770a2af 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1794,41 +1794,6 @@ int __init make_chosen_node(const struct kernel_info 
> *kinfo)
>     return res;
> }
> 
> -int __init map_irq_to_domain(struct domain *d, unsigned int irq,
> -                             bool need_mapping, const char *devname)
> -{
> -    int res;
> -
> -    res = irq_permit_access(d, irq);
> -    if ( res )
> -    {
> -        printk(XENLOG_ERR "Unable to permit to dom%u access to IRQ %u\n",
> -               d->domain_id, irq);
> -        return res;
> -    }
> -
> -    if ( need_mapping )
> -    {
> -        /*
> -         * Checking the return of vgic_reserve_virq is not
> -         * necessary. It should not fail except when we try to map
> -         * the IRQ twice. This can legitimately happen if the IRQ is shared
> -         */
> -        vgic_reserve_virq(d, irq);
> -
> -        res = route_irq_to_guest(d, irq, irq, devname);
> -        if ( res < 0 )
> -        {
> -            printk(XENLOG_ERR "Unable to map IRQ%"PRId32" to dom%d\n",
> -                   irq, d->domain_id);
> -            return res;
> -        }
> -    }
> -
> -    dt_dprintk("  - IRQ: %u\n", irq);
> -    return 0;
> -}
> -
> static int __init map_dt_irq_to_domain(const struct dt_device_node *dev,
>                                        const struct dt_irq *dt_irq,
>                                        void *data)
> @@ -1860,57 +1825,6 @@ static int __init map_dt_irq_to_domain(const struct 
> dt_device_node *dev,
>     return 0;
> }
> 
> -int __init map_range_to_domain(const struct dt_device_node *dev,
> -                               u64 addr, u64 len, void *data)
> -{
> -    struct map_range_data *mr_data = data;
> -    struct domain *d = mr_data->d;
> -    int res;
> -
> -    /*
> -     * reserved-memory regions are RAM carved out for a special purpose.
> -     * They are not MMIO and therefore a domain should not be able to
> -     * manage them via the IOMEM interface.
> -     */
> -    if ( strncasecmp(dt_node_full_name(dev), "/reserved-memory/",
> -                     strlen("/reserved-memory/")) != 0 )
> -    {
> -        res = iomem_permit_access(d, paddr_to_pfn(addr),
> -                paddr_to_pfn(PAGE_ALIGN(addr + len - 1)));
> -        if ( res )
> -        {
> -            printk(XENLOG_ERR "Unable to permit to dom%d access to"
> -                    " 0x%"PRIx64" - 0x%"PRIx64"\n",
> -                    d->domain_id,
> -                    addr & PAGE_MASK, PAGE_ALIGN(addr + len) - 1);
> -            return res;
> -        }
> -    }
> -
> -    if ( !mr_data->skip_mapping )
> -    {
> -        res = map_regions_p2mt(d,
> -                               gaddr_to_gfn(addr),
> -                               PFN_UP(len),
> -                               maddr_to_mfn(addr),
> -                               mr_data->p2mt);
> -
> -        if ( res < 0 )
> -        {
> -            printk(XENLOG_ERR "Unable to map 0x%"PRIx64
> -                   " - 0x%"PRIx64" in domain %d\n",
> -                   addr & PAGE_MASK, PAGE_ALIGN(addr + len) - 1,
> -                   d->domain_id);
> -            return res;
> -        }
> -    }
> -
> -    dt_dprintk("  - MMIO: %010"PRIx64" - %010"PRIx64" P2MType=%x\n",
> -               addr, addr + len, mr_data->p2mt);
> -
> -    return 0;
> -}
> -
> /*
>  * For a node which describes a discoverable bus (such as a PCI bus)
>  * then we may need to perform additional mappings in order to make
> @@ -1938,62 +1852,6 @@ static int __init map_device_children(const struct 
> dt_device_node *dev,
>     return 0;
> }
> 
> -/*
> - * handle_device_interrupts retrieves the interrupts configuration from
> - * a device tree node and maps those interrupts to the target domain.
> - *
> - * Returns:
> - *   < 0 error
> - *   0   success
> - */
> -static int __init handle_device_interrupts(struct domain *d,
> -                                           struct dt_device_node *dev,
> -                                           bool need_mapping)
> -{
> -    unsigned int i, nirq;
> -    int res;
> -    struct dt_raw_irq rirq;
> -
> -    nirq = dt_number_of_irq(dev);
> -
> -    /* Give permission and map IRQs */
> -    for ( i = 0; i < nirq; i++ )
> -    {
> -        res = dt_device_get_raw_irq(dev, i, &rirq);
> -        if ( res )
> -        {
> -            printk(XENLOG_ERR "Unable to retrieve irq %u for %s\n",
> -                   i, dt_node_full_name(dev));
> -            return res;
> -        }
> -
> -        /*
> -         * Don't map IRQ that have no physical meaning
> -         * ie: IRQ whose controller is not the GIC
> -         */
> -        if ( rirq.controller != dt_interrupt_controller )
> -        {
> -            dt_dprintk("irq %u not connected to primary controller. 
> Connected to %s\n",
> -                      i, dt_node_full_name(rirq.controller));
> -            continue;
> -        }
> -
> -        res = platform_get_irq(dev, i);
> -        if ( res < 0 )
> -        {
> -            printk(XENLOG_ERR "Unable to get irq %u for %s\n",
> -                   i, dt_node_full_name(dev));
> -            return res;
> -        }
> -
> -        res = map_irq_to_domain(d, res, need_mapping, dt_node_name(dev));
> -        if ( res )
> -            return res;
> -    }
> -
> -    return 0;
> -}
> -
> /*
>  * For a given device node:
>  *  - Give permission to the guest to manage IRQ and MMIO range
> diff --git a/xen/arch/arm/include/asm/setup.h 
> b/xen/arch/arm/include/asm/setup.h
> index 7a1e1d6798..8a26f1845c 100644
> --- a/xen/arch/arm/include/asm/setup.h
> +++ b/xen/arch/arm/include/asm/setup.h
> @@ -134,6 +134,9 @@ void device_tree_get_reg(const __be32 **cell, u32 
> address_cells,
> u32 device_tree_get_u32(const void *fdt, int node,
>                         const char *prop_name, u32 dflt);
> 
> +int handle_device_interrupts(struct domain *d, struct dt_device_node *dev,
> +                             bool need_mapping);
> +
> int map_range_to_domain(const struct dt_device_node *dev,
>                         u64 addr, u64 len, void *data);
> 
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 4aae281e89..f43d66a501 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -1811,12 +1811,12 @@ int dt_count_phandle_with_args(const struct 
> dt_device_node *np,
>  * @allnextpp: pointer to ->allnext from last allocated device_node
>  * @fpsize: Size of the node path up at the current depth.
>  */
> -static unsigned long __init unflatten_dt_node(const void *fdt,
> -                                              unsigned long mem,
> -                                              unsigned long *p,
> -                                              struct dt_device_node *dad,
> -                                              struct dt_device_node 
> ***allnextpp,
> -                                              unsigned long fpsize)
> +static unsigned long unflatten_dt_node(const void *fdt,
> +                                       unsigned long mem,
> +                                       unsigned long *p,
> +                                       struct dt_device_node *dad,
> +                                       struct dt_device_node ***allnextpp,
> +                                       unsigned long fpsize)
> {
>     struct dt_device_node *np;
>     struct dt_property *pp, **prev_pp = NULL;
> @@ -2047,7 +2047,7 @@ static unsigned long __init unflatten_dt_node(const 
> void *fdt,
> }
> 
> /**
> - * __unflatten_device_tree - create tree of device_nodes from flat blob
> + * unflatten_device_tree - create tree of device_nodes from flat blob
>  *
>  * unflattens a device-tree, creating the
>  * tree of struct device_node. It also fills the "name" and "type"
> @@ -2056,8 +2056,8 @@ static unsigned long __init unflatten_dt_node(const 
> void *fdt,
>  * @fdt: The fdt to expand
>  * @mynodes: The device_node tree created by the call
>  */
> -static void __init __unflatten_device_tree(const void *fdt,
> -                                           struct dt_device_node **mynodes)
> +void unflatten_device_tree(const void *fdt,
> +                           struct dt_device_node **mynodes)
> {
>     unsigned long start, mem, size;
>     struct dt_device_node **allnextp = mynodes;
> @@ -2179,7 +2179,7 @@ dt_find_interrupt_controller(const struct 
> dt_device_match *matches)
> 
> void __init dt_unflatten_host_device_tree(void)
> {
> -    __unflatten_device_tree(device_tree_flattened, &dt_host);
> +    unflatten_device_tree(device_tree_flattened, &dt_host);
>     dt_alias_scan();
> }
> 
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index fd6cd00b43..06d7866c10 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -177,6 +177,11 @@ int device_tree_for_each_node(const void *fdt, int node,
>  */
> void dt_unflatten_host_device_tree(void);
> 
> +/*
> + * unflatten any device tree.
> + */
> +void unflatten_device_tree(const void *fdt, struct dt_device_node **mynodes);
> +
> /**
>  * IRQ translation callback
>  * TODO: For the moment we assume that we only have ONE

NIT: there are some minor code style issues, like indentation that could be 
fixed

Cheers,
Luca




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.