| Message ID | 20210923120236.3692135-26-wei.chen@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add device tree based NUMA support to Arm | expand |
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.
Hi Stefano, > -----Original Message----- > From: Stefano Stabellini <sstabellini@kernel.org> > Sent: 2021年9月24日 10:10 > To: Wei Chen <Wei.Chen@arm.com> > Cc: xen-devel@lists.xenproject.org; sstabellini@kernel.org; julien@xen.org; > Bertrand Marquis <Bertrand.Marquis@arm.com>; jbeulich@suse.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 > > 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 have placed some comments in patch#23 for srat. I wouldn't like to replace srat by numa_distance. because srat not only contains distance, and it also includes some affinity information for CPU or other devices.
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? 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.) Jan
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.
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; +} + void __init numa_set_distance(nodeid_t from, nodeid_t to, uint32_t distance) { if ( from >= MAX_NUMNODES || to >= MAX_NUMNODES )
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(+)