Re: [Xen-devel] [PATCH for-4.13] xen/arm: fix duplicate memory node in DT

[Julien Grall <julien.grall@arm.com>]

Hi,

On 05/10/2019 00:09, Stefano Stabellini wrote:
> When reserved-memory regions are present in the host device tree, dom0
> is started with multiple memory nodes. Each memory node should have a
> unique name, but today they are all called "memory" leading to Linux
> printing the following warning at boot:
> 
>    OF: Duplicate name in base, renamed to "memory#1"
> 
> This patch fixes the problem by appending a "@<unit-address>" to the
> name, as per the Device Tree specification, where <unit-address> matches
> the base of address of the first region.
> 
> Reported-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> 
> ---
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 921b054520..a4c07db383 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -646,16 +646,22 @@ static int __init make_memory_node(const struct domain *d,
>       int res, i;
>       int reg_size = addrcells + sizecells;
>       int nr_cells = reg_size * mem->nr_banks;
> +    /* Placeholder for memory@ + a 32-bit number + \0 */
> +    char buf[18];
>       __be32 reg[NR_MEM_BANKS * 4 /* Worst case addrcells + sizecells */];
>       __be32 *cells;
>   
>       BUG_ON(nr_cells >= ARRAY_SIZE(reg));
> +    /* Nothing to do */

This a departure from the current solution where a node will be created with no 
"reg" property. I think this change of behavior should at least be described in 
the commit message if not implemented in a separate patch. But...

> +    if ( mem->nr_banks == 0 )
> +        return 0;

... I don't think we want to ignore it. The caller most likely messed up the 
banks and we should instead report an error.

>   
>       dt_dprintk("Create memory node (reg size %d, nr cells %d)\n",
>                  reg_size, nr_cells);
>   
>       /* ePAPR 3.4 */
> -    res = fdt_begin_node(fdt, "memory");
> +    snprintf(buf, sizeof(buf), "memory@%"PRIx64, mem->bank[0].start);
> +    res = fdt_begin_node(fdt, buf);
>       if ( res )
>           return res;
>   
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel