|
[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
On Mon, Mar 14, 2022 at 12:31:06PM +0000, Luca Fancellu wrote:
>
>
> > 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?
>
Hi Luca,
Thanks for spotting this. Yeah, I messed this while porting the patches from
internal to upstream Xen.
Will address this in v4!
>
> > + 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
>
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |