Re: [XEN][PATCH v11 15/20] arm/asm/setup.h: Update struct map_range_data to add rangeset.

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 4 Sep 2023, Michal Orzel wrote:
> On 01/09/2023 06:59, Vikram Garhwal wrote:
> > Add rangesets for IRQs and IOMEMs. This was done to accommodate dynamic 
> > overlay
> > node addition/removal operations. With overlay operations, new IRQs and 
> > IOMEMs
> > are added in dt_host and routed. While removing overlay nodes, nodes are 
> > removed
> > from dt_host and their IRQs and IOMEMs routing is also removed. Storing 
> > IRQs and
> > IOMEMs in the rangeset will avoid re-parsing the device tree nodes to get 
> > the
> > IOMEM and IRQ ranges for overlay remove ops.
> > 
> > Dynamic overlay node add/remove will be introduced in follow-up patches.
> > 
> > Signed-off-by: Vikram Garhwal <vikram.garhwal@xxxxxxx>
> > 
> > ---
> > Changes from v10:
> >     Replace paddr_to_pfn(PAGE_ALIGN()) with paddr_to_pfn_aligned().
> >     Change data type of irq.
> >     fix function change for handle_device().
> >     Remove unnecessary change .d = d in mr_data.
> > ---
> > ---
> >  xen/arch/arm/device.c            | 43 +++++++++++++++++++++++++-------
> >  xen/arch/arm/domain_build.c      |  4 +--
> >  xen/arch/arm/include/asm/setup.h |  9 ++++---
> >  3 files changed, 42 insertions(+), 14 deletions(-)
> > 
> > diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
> > index 327e4d24fb..1f631d3274 100644
> > --- a/xen/arch/arm/device.c
> > +++ b/xen/arch/arm/device.c
> > @@ -165,6 +165,15 @@ int map_range_to_domain(const struct dt_device_node 
> > *dev,
> >      dt_dprintk("  - MMIO: %010"PRIx64" - %010"PRIx64" P2MType=%x\n",
> >                 addr, addr + len, mr_data->p2mt);
> >  
> > +    if ( mr_data->iomem_ranges )
> > +    {
> > +        res = rangeset_add_range(mr_data->iomem_ranges,
> > +                                 paddr_to_pfn(addr),
> > +                                 paddr_to_pfn_aligned(addr + len - 1));
> > +        if ( res )
> > +            return res;
> > +    }
> > +
> >      return 0;
> >  }
> >  
> > @@ -178,10 +187,11 @@ int map_range_to_domain(const struct dt_device_node 
> > *dev,
> >   */
> >  int map_device_irqs_to_domain(struct domain *d,
> >                                struct dt_device_node *dev,
> > -                              bool need_mapping)
> > +                              bool need_mapping,
> > +                              struct rangeset *irq_ranges)
> >  {
> >      unsigned int i, nirq;
> > -    int res;
> > +    int res, irq;
> You could make use of res to store irq just as it was done before without 
> introducing new local var.
> Anyway, if you think it improves readability:
> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>