RE: [PATCH 29/37] xen/arm: introduce a helper to parse device tree processor node

[Wei Chen <Wei.Chen@xxxxxxx>]

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Sent: 2021年9月24日 10:45
> To: Wei Chen <Wei.Chen@xxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; sstabellini@xxxxxxxxxx; julien@xxxxxxx;
> Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> Subject: Re: [PATCH 29/37] xen/arm: introduce a helper to parse device
> tree processor node
> 
> On Thu, 23 Sep 2021, 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/Makefile           |  1 +
> >  xen/arch/arm/numa_device_tree.c | 58 +++++++++++++++++++++++++++++++++
> >  2 files changed, 59 insertions(+)
> >  create mode 100644 xen/arch/arm/numa_device_tree.c
> >
> > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> > index 41ca311b6b..c50df2c25d 100644
> > --- a/xen/arch/arm/Makefile
> > +++ b/xen/arch/arm/Makefile
> > @@ -36,6 +36,7 @@ obj-y += mem_access.o
> >  obj-y += mm.o
> >  obj-y += monitor.o
> >  obj-$(CONFIG_NUMA) += numa.o
> > +obj-$(CONFIG_DEVICE_TREE_NUMA) += numa_device_tree.o
> >  obj-y += p2m.o
> >  obj-y += percpu.o
> >  obj-y += platform.o
> > diff --git a/xen/arch/arm/numa_device_tree.c
> b/xen/arch/arm/numa_device_tree.c
> > new file mode 100644
> > index 0000000000..2428fbae0b
> > --- /dev/null
> > +++ b/xen/arch/arm/numa_device_tree.c
> > @@ -0,0 +1,58 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Arm Architecture support layer for NUMA.
> > + *
> > + * Copyright (C) 2021 Arm Ltd
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> > + *
> > + */
> > +#include <xen/init.h>
> > +#include <xen/nodemask.h>
> > +#include <xen/numa.h>
> > +#include <xen/libfdt/libfdt.h>
> > +#include <xen/device_tree.h>
> > +
> > +/* Callback for device tree processor affinity */
> > +static int __init fdt_numa_processor_affinity_init(nodeid_t node)
> > +{
> > +    if ( srat_disabled() )
> > +        return -EINVAL;
> 
> fdt_numa_processor_affinity_init is called by fdt_parse_numa_cpu_node
> which is already parsing NUMA related info. Should this srat_disabled
> check be moved to fdt_parse_numa_cpu_node?
> 

Ah, yes, it's a good suggestion, I will address it in next version.

> 
> > +    else if ( node == NUMA_NO_NODE || node >= MAX_NUMNODES )
> > +    {
> > +        bad_srat();
> > +        return -EINVAL;
> > +   }
> > +
> > +    numa_set_processor_nodes_parsed(node);
> > +    fw_numa = 1;
> > +
> > +    printk(KERN_INFO "DT: NUMA node %"PRIu7" processor parsed\n", node);
> > +
> > +    return 0;
> > +}
> > +
> > +/* Parse CPU NUMA node info */
> > +static int __init fdt_parse_numa_cpu_node(const void *fdt, int node)
> > +{
> > +    uint32_t nid;
> > +
> > +    nid = device_tree_get_u32(fdt, node, "numa-node-id", MAX_NUMNODES);
> > +    if ( nid >= MAX_NUMNODES )
> > +    {
> > +        printk(XENLOG_ERR "Node id %u exceeds maximum value\n", nid);
>                                       ^ PRIu32
> 
> 
> > +        return -EINVAL;
> > +    }
> > +
> > +    return fdt_numa_processor_affinity_init(nid);
> > +}
> > --
> > 2.25.1
> >