RE: [PATCH 32/37] xen/arm: unified entry to parse all NUMA data from device tree

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Stefano,

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Sent: 2021年9月24日 11:17
> To: Wei Chen <Wei.Chen@xxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; sstabellini@xxxxxxxxxx; julien@xxxxxxx;
> Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> Subject: Re: [PATCH 32/37] xen/arm: unified entry to parse all NUMA data
> from device tree
> 
> On Thu, 23 Sep 2021, Wei Chen wrote:
> > In this API, we scan whole device tree to parse CPU node id, memory
>           ^ function   ^ the whole
> 
> > node id and distance-map. Though early_scan_node will invoke has a
> > handler to process memory nodes. If we want to parse memory node id
> > in this handler, we have to embeded NUMA parse code in this handler.
>                               ^ embed
> 
> > But we still need to scan whole device tree to find CPU NUMA id and
> > distance-map. In this case, we include memory NUMA id parse in this
> > API too. Another benefit is that we have a unique entry for device
>   ^ function
> 
> > tree NUMA data parse.
> 
> Ah, that's the explanation I was asking for earlier!
> 

The question about device_tree_get_meminfo?

> 
> > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> > ---
> >  xen/arch/arm/numa_device_tree.c | 30 ++++++++++++++++++++++++++++++
> >  xen/include/asm-arm/numa.h      |  1 +
> >  2 files changed, 31 insertions(+)
> >
> > diff --git a/xen/arch/arm/numa_device_tree.c
> b/xen/arch/arm/numa_device_tree.c
> > index e7fa84df4c..6a3fed0002 100644
> > --- a/xen/arch/arm/numa_device_tree.c
> > +++ b/xen/arch/arm/numa_device_tree.c
> > @@ -242,3 +242,33 @@ static int __init
> fdt_parse_numa_distance_map_v1(const void *fdt, int node)
> >
> >      return 0;
> >  }
> > +
> > +static int __init fdt_scan_numa_nodes(const void *fdt,
> > +                int node, const char *uname, int depth,
> > +                u32 address_cells, u32 size_cells, void *data)
> 
> Please align parameters
> 

OK

> 
> > +{
> > +    int len, ret = 0;
> > +    const void *prop;
> > +
> > +    prop = fdt_getprop(fdt, node, "device_type", &len);
> > +    if (prop)
> 
> code style
> 

OK

> 
> > +    {
> > +        len += 1;
> > +        if ( memcmp(prop, "cpu", len) == 0 )
> > +            ret = fdt_parse_numa_cpu_node(fdt, node);
> > +        else if ( memcmp(prop, "memory", len) == 0 )
> > +            ret = fdt_parse_numa_memory_node(fdt, node, uname,
> > +                                address_cells, size_cells);
> 
> I realize that with the inclusion of '\0' in the check, the usage of
> memcmp should be safe, but I would prefer if we used strncmp instead.
> 

Ok, I will use strncmp in next version.

> 
> > +    }
> > +    else if ( fdt_node_check_compatible(fdt, node,
> > +                                "numa-distance-map-v1") == 0 )
> > +        ret = fdt_parse_numa_distance_map_v1(fdt, node);
> > +
> > +    return ret;
> > +}
> > +
> > +/* Initialize NUMA from device tree */
> > +int __init numa_device_tree_init(const void *fdt)
> > +{
> > +    return device_tree_for_each_node(fdt, 0, fdt_scan_numa_nodes, NULL);
> > +}
> > diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
> > index 7675012cb7..f46e8e2935 100644
> > --- a/xen/include/asm-arm/numa.h
> > +++ b/xen/include/asm-arm/numa.h
> > @@ -23,6 +23,7 @@ typedef u8 nodeid_t;
> >  #define NR_NODE_MEMBLKS NR_MEM_BANKS
> >
> >  extern void numa_set_distance(nodeid_t from, nodeid_t to, uint32_t
> distance);
> > +extern int numa_device_tree_init(const void *fdt);
> >
> >  #else
> >
> > --
> > 2.25.1
> >