Re: [PATCH 25/37] xen/arm: implement bad_srat for Arm NUMA initialization

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 24 Sep 2021, Jan Beulich wrote:
> On 24.09.2021 04:09, Stefano Stabellini wrote:
> > On Thu, 23 Sep 2021, Wei Chen wrote:
> >> NUMA initialization will parse information from firmware provided
> >> static resource affinity table (ACPI SRAT or DTB). bad_srat if a
> >> function that will be used when initialization code encounters
> >> some unexcepted errors.
> >>
> >> In this patch, we introduce Arm version bad_srat for NUMA common
> >> initialization code to invoke it.
> >>
> >> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> >> ---
> >>  xen/arch/arm/numa.c | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/xen/arch/arm/numa.c b/xen/arch/arm/numa.c
> >> index 3755b01ef4..5209d3de4d 100644
> >> --- a/xen/arch/arm/numa.c
> >> +++ b/xen/arch/arm/numa.c
> >> @@ -18,6 +18,7 @@
> >>   *
> >>   */
> >>  #include <xen/init.h>
> >> +#include <xen/nodemask.h>
> >>  #include <xen/numa.h>
> >>  
> >>  static uint8_t __read_mostly
> >> @@ -25,6 +26,12 @@ node_distance_map[MAX_NUMNODES][MAX_NUMNODES] = {
> >>      { 0 }
> >>  };
> >>  
> >> +__init void bad_srat(void)
> >> +{
> >> +    printk(KERN_ERR "NUMA: Firmware SRAT table not used.\n");
> >> +    fw_numa = -1;
> >> +}
> > 
> > I realize that the series keeps the "srat" terminology everywhere on DT
> > too. I wonder if it is worth replacing srat with something like
> > "numa_distance" everywhere as appropriate. I am adding the x86
> > maintainers for an opinion.
> > 
> > If you guys prefer to keep srat (if nothing else, it is concise), I am
> > also OK with keeping srat although it is not technically accurate.
> 
> I think we want to tell apart both things: Where we truly talk about
> the firmware's SRAT table, keeping that name is fine. But I suppose
> there no "Firmware SRAT table" (as in the log message above) when
> using DT?

No. FYI this is the DT binding:
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/numa.txt

The interesting bit is the "distance-map"


> If so, at the very least in log messages SRAT shouldn't be
> mentioned. Perhaps even functions serving both an ACPI and a DT
> purpose would better not use "srat" in their names (but I'm not as
> fussed about it there.)

I agree 100% with what you wrote.