Re: [PATCH] xen/arm: Create GIC node using the node name from host dt

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 19 Feb 2025, Michal Orzel wrote:
> At the moment the GIC node we create for hwdom has a name
> "interrupt-controller". Change it so that we use the same name as the
> GIC node from host device tree. This is done for at least 2 purposes:
> 1) The convention in DT spec is that a node name with "reg" property
> is formed "node-name@unit-address".
> 2) With DT overlay feature, many overlays refer to the GIC node using
> the symbol under __symbols__ that we copy to hwdom 1:1. With the name
> changed, the symbol is no longer valid and requires error prone manual
> change by the user.
> 
> The unit-address part of the node name always refers to the first
> address in the "reg" property which in case of GIC, always refers to
> GICD and hwdom uses host memory layout.
> 
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

While this fix changes behavior for everyone, so it is risky at RC5, it
also fixes bugs with DT overlays, but that is an experimental feature. I
am in two minds whether it should go in right now or not. Maybe I would
wait until 4.20 is out and commit when the tree reopens.


> ---
>  xen/arch/arm/domain_build.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 7b47abade196..e760198d8609 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1615,6 +1615,7 @@ static int __init make_gic_node(const struct domain *d, 
> void *fdt,
>      int res = 0;
>      const void *addrcells, *sizecells;
>      u32 addrcells_len, sizecells_len;
> +    const char *name;
>  
>      /*
>       * Xen currently supports only a single GIC. Discard any secondary
> @@ -1628,7 +1629,11 @@ static int __init make_gic_node(const struct domain 
> *d, void *fdt,
>  
>      dt_dprintk("Create gic node\n");
>  
> -    res = fdt_begin_node(fdt, "interrupt-controller");
> +    /* Use the same name as the GIC node in host device tree */
> +    name = strrchr(gic->full_name, '/');
> +    name = name ? name + 1 : gic->full_name;
> +
> +    res = fdt_begin_node(fdt, name);
>      if ( res )
>          return res;
>  
> -- 
> 2.25.1
>