RE: [XEN RFC PATCH 22/40] xen/arm: introduce a helper to parse device tree processor node

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: 2021年8月20日 16:44
> To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> sstabellini@xxxxxxxxxx; jbeulich@xxxxxxxx
> Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> Subject: Re: [XEN RFC PATCH 22/40] xen/arm: introduce a helper to parse
> device tree processor node
> 
> 
> 
> On 20/08/2021 03:23, Wei Chen wrote:
> > Hi Julien,
> 
> Hi Wei,
> 
> >> -----Original Message-----
> >> From: Julien Grall <julien@xxxxxxx>
> >> Sent: 2021年8月20日 2:13
> >> To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> >> sstabellini@xxxxxxxxxx; jbeulich@xxxxxxxx
> >> Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> >> Subject: Re: [XEN RFC PATCH 22/40] xen/arm: introduce a helper to parse
> >> device tree processor node
> >>
> >> Hi Wei,
> >>
> >> On 11/08/2021 11:24, Wei Chen wrote:
> >>> Processor NUMA ID information is stored in device tree's processor
> >>> node as "numa-node-id". We need a new helper to parse this ID from
> >>> processor node. If we get this ID from processor node, this ID's
> >>> validity still need to be checked. Once we got a invalid NUMA ID
> >>> from any processor node, the device tree will be marked as NUMA
> >>> information invalid.
> >>>
> >>> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> >>> ---
> >>>    xen/arch/arm/numa_device_tree.c | 41
> +++++++++++++++++++++++++++++++--
> >>>    1 file changed, 39 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/xen/arch/arm/numa_device_tree.c
> >> b/xen/arch/arm/numa_device_tree.c
> >>> index 1c74ad135d..37cc56acf3 100644
> >>> --- a/xen/arch/arm/numa_device_tree.c
> >>> +++ b/xen/arch/arm/numa_device_tree.c
> >>> @@ -20,16 +20,53 @@
> >>>    #include <xen/init.h>
> >>>    #include <xen/nodemask.h>
> >>>    #include <xen/numa.h>
> >>> +#include <xen/device_tree.h>
> >>
> >> Nothing in this file seems to depend on xen/device_tree.h. So why do
> you
> >> need to include it?
> >>
> >
> > I remember that without this header file, device_tree_get_u32 in this
> patch
> > will cause compiling failed.
> 
> I looked at the prototype of device_tree_get_u32() and I can't find how
> it depends on bits from device_tree.h. Can you paste the compilation error?
> 

I tested it again, this header file should be introduced in following patches:
numa_device_tree.c: In function ‘device_tree_parse_numa_distance_map_v1’:
numa_device_tree.c:243:16: error: implicit declaration of function 
‘dt_read_number’ [-Werror=implicit-function-declaration]
  243 |         from = dt_read_number(matrix, 1);
      |                ^~~~~~~~~~~~~~
numa_device_tree.c:243:16: error: nested extern declaration of ‘dt_read_number’ 
[-Werror=nested-externs]
cc1: all warnings being treated as errors

I will move it to another patch.

> >
> >>> +#include <asm/setup.h>
> >>>
> >>>    s8 device_tree_numa = 0;
> >>> +static nodemask_t processor_nodes_parsed __initdata;
> >>>
> >>> -int srat_disabled(void)
> >>> +static int srat_disabled(void)
> >>>    {
> >>>        return numa_off || device_tree_numa < 0;
> >>>    }
> >>>
> >>> -void __init bad_srat(void)
> >>> +static __init void bad_srat(void)
> >>>    {
> >>>        printk(KERN_ERR "DT: NUMA information is not used.\n");
> >>>        device_tree_numa = -1;
> >>>    }
> >>> +
> >>> +/* Callback for device tree processor affinity */
> >>> +static int __init dtb_numa_processor_affinity_init(nodeid_t node)
> >>> +{
> >>> +    if ( srat_disabled() )
> >>> +        return -EINVAL;
> >>> +    else if ( node == NUMA_NO_NODE || node >= MAX_NUMNODES ) {
> >>> +         bad_srat();
> >>> +         return -EINVAL;
> >>> + }
> >>> +
> >>> +    node_set(node, processor_nodes_parsed);
> >>> +
> >>> +    device_tree_numa = 1;
> >>> +    printk(KERN_INFO "DT: NUMA node %u processor parsed\n", node);
> >>> +
> >>> +    return 0;
> >>> +}
> >>> +
> >>> +/* Parse CPU NUMA node info */
> >>> +int __init device_tree_parse_numa_cpu_node(const void *fdt, int node)
> >>> +{
> >>> +    uint32_t nid;
> >>> +
> >>> +    nid = device_tree_get_u32(fdt, node, "numa-node-id",
> MAX_NUMNODES);
> >>> +    printk(XENLOG_WARNING "CPU on NUMA node:%u\n", nid);
> >>> +    if ( nid >= MAX_NUMNODES )
> >>> +    {
> >>> +        printk(XENLOG_WARNING "Node id %u exceeds maximum value\n",
> >> nid);
> >>> +        return -EINVAL;
> >>> +    }
> >>> +
> >>> +    return dtb_numa_processor_affinity_init(nid);
> >>> +}
> >>>
> >>
> >> --
> >> Julien Grall
> 
> Cheers,
> 
> --
> Julien Grall