Hi Jan,
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: 2023年1月11日 0:47
> To: Wei Chen <wei.c...@arm.com>
> Cc: nd <n...@arm.com>; Stefano Stabellini <sstabell...@kernel.org>; Julien
> Grall <jul...@xen.org>; Bertrand Marquis <bertrand.marq...@arm.com>;
> Volodymyr Babchuk <volodymyr_babc...@epam.com>; Andrew Cooper
> <andrew.coop...@citrix.com>; George Dunlap <george.dun...@citrix.com>; Wei
> Liu <w...@xen.org>; Roger Pau Monné <roger....@citrix.com>; xen-
> de...@lists.xenproject.org
> Subject: Re: [PATCH v2 03/17] xen/arm: implement node distance helpers for
> Arm
>
> On 10.01.2023 09:49, Wei Chen wrote:
> > --- a/xen/arch/arm/include/asm/numa.h
> > +++ b/xen/arch/arm/include/asm/numa.h
> > @@ -28,6 +28,20 @@ enum dt_numa_status {
> > DT_NUMA_OFF,
> > };
> >
> > +/*
> > + * In ACPI spec, 0-9 are the reserved values for node distance,
> > + * 10 indicates local node distance, 20 indicates remote node
> > + * distance. Set node distance map in device tree will follow
> > + * the ACPI's definition.
> > + */
> > +#define NUMA_DISTANCE_UDF_MIN 0
> > +#define NUMA_DISTANCE_UDF_MAX 9
> > +#define NUMA_LOCAL_DISTANCE 10
> > +#define NUMA_REMOTE_DISTANCE 20
>
> In the absence of a caller of numa_set_distance() it is entirely unclear
> whether this tying to ACPI used values is actually appropriate.
>
From Kernel's NUMA device tree binding, it seems DT NUMA are reusing
ACPI used values for distances [1].
> > --- a/xen/arch/arm/numa.c
> > +++ b/xen/arch/arm/numa.c
> > @@ -2,7 +2,7 @@
> > /*
> > * Arm Architecture support layer for NUMA.
> > *
> > - * Copyright (C) 2021 Arm Ltd
> > + * Copyright (C) 2022 Arm Ltd
>
> I don't think it makes sense to change such after the fact. And certainly
> not in an unrelated patch.
>
I will retore it, and add a SPDX header.
> > @@ -22,6 +22,11 @@
> >
> > static enum dt_numa_status __read_mostly device_tree_numa;
> >
> > +static unsigned char __read_mostly
> > +node_distance_map[MAX_NUMNODES][MAX_NUMNODES] = {
> > + { 0 }
> > +};
>
> __ro_after_init?
>
Yes.
> > @@ -42,3 +47,48 @@ int __init arch_numa_setup(const char *opt)
> > {
> > return -EINVAL;
> > }
> > +
> > +void __init numa_set_distance(nodeid_t from, nodeid_t to,
> > + unsigned int distance)
> > +{
> > + if ( from >= MAX_NUMNODES || to >= MAX_NUMNODES )
> > + {
> > + printk(KERN_WARNING
> > + "NUMA: invalid nodes: from=%"PRIu8" to=%"PRIu8"
> MAX=%"PRIu8"\n",
> > + from, to, MAX_NUMNODES);
> > + return;
> > + }
> > +
> > + /* NUMA defines 0xff as an unreachable node and 0-9 are undefined
> */
> > + if ( distance >= NUMA_NO_DISTANCE ||
> > + (distance >= NUMA_DISTANCE_UDF_MIN &&
>
> Nit: Indentation.
>
Ok.
> > + distance <= NUMA_DISTANCE_UDF_MAX) ||
> > + (from == to && distance != NUMA_LOCAL_DISTANCE) )
> > + {
> > + printk(KERN_WARNING
> > + "NUMA: invalid distance: from=%"PRIu8" to=%"PRIu8"
> distance=%"PRIu32"\n",
> > + from, to, distance);
> > + return;
> > + }
> > +
> > + node_distance_map[from][to] = distance;
> > +}
> > +
> > +unsigned char __node_distance(nodeid_t from, nodeid_t to)
> > +{
> > + /* When NUMA is off, any distance will be treated as remote. */
> > + if ( numa_disabled() )
> > + return NUMA_REMOTE_DISTANCE;
> > +
> > + /*
> > + * Check whether the nodes are in the matrix range.
> > + * When any node is out of range, except from and to nodes are the
> > + * same, we treat them as unreachable (return 0xFF)
> > + */
> > + if ( from >= MAX_NUMNODES || to >= MAX_NUMNODES )
>
> I guess using ARRAY_SIZE() here would be more future-proof.
>
I will use it in next version.
[1]https://www.kernel.org/doc/Documentation/devicetree/bindings/numa.txt
Thanks,
Wei Chen
> Jan
