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

[Michal Orzel <michal.orzel@xxxxxxx>]

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>

~Michal