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

From: Stefano Stabellini <sstabellini@kernel.org>
To: Jan Beulich <jbeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Chen <wei.chen@arm.com>,
	 xen-devel@lists.xenproject.org, julien@xen.org,
	Bertrand.Marquis@arm.com,  andrew.cooper3@citrix.com,
	roger.pau@citrix.com, wl@xen.org
Subject: Re: [PATCH 25/37] xen/arm: implement bad_srat for Arm NUMA initialization
Date: Fri, 24 Sep 2021 12:33:50 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.21.2109241231460.17979@sstabellini-ThinkPad-T480s> (raw)
In-Reply-To: <d6973324-52e2-1c78-4f57-929d3af07a80@suse.com>

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@arm.com>
> >> ---
> >>  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.