[PATCH 0/3] ACPI SRAT: Disregard reserved PXM bits

[PATCH 0/3] ACPI SRAT: Disregard reserved PXM bits

[Kurt Garloff]

[-- Attachment #1: Type: text/plain, Size: 2196 bytes --]

Hi,

this has been submitted thrice already -- and while it found a bit of
friendly feedback, it has not been merged. It should be IMVHO, and I
hope this time the right eyes see this flying by ...
I should mention that it fixes a real problem (wrong affinities reported
in sysfs and thus userspace high-perf compute code taking wrong
decisions) and that this has been part of SUSE enterprise kernels since 
a few years now.
It should also go right into stable IMVHO.

Original description to follow ...


ACPI specification says that the OS must disregard reserved bits.
The x86_64 SRAT parser does not discard the upper 24 bits of the
proximity_domain (pxm) in the acpi_srat_mem_affinity entries for
SRAT v1 tables. (v2 has 32 bits wide fields, v1 only 8 bits.)
This can lead to problems with poor BIOS implementations that failed
to set reserved bytes to zero. (The ACPI spec is a bit vague here
unfortunately.)

This was also inconsistent: On x86-64 (srat_64.c), the 
_cpu_affinity does only use the low 8 bits of pxm, while the
full 32 bits of _mem_affinity are consumed.
In srat_32.c (x86), only 8bits are used (which is OK, a 32bit system
with >256 PXMs does not seem reasonable at all).
On ia64, the support of more than 8 bits was consistent between
mem and cpu affinity entries, however it was dependent on "sn2" 
platform.

The patch series has the following goals:
* Make the kernel support consistently 8bits or 32bits for the
  proximity domain
* Make this dependent on the SRAT version; v1 => 8bits, v2 => 32bits.

Overview over the patches:
- [1/3] Store the SRAT table version value in acpi_srat_revision 
- [2/3] x86-64: Discard the upper 24 bits in mem_affinity if rev <= 1
        and use upper 24bits in cpu_affinity if rev >= 2
- [3/3] ia64: Also use upper 8/24bits if rev >= 2 (but leave logic to
        enable on sn2 as well -- I don't know if sn2 reports v1 or v2
        SRAT) Also add two __init decls in ia64 pxm accessors.

Patch is against current git.

Note: If you respond to this mail, please keep me in Cc -- I'm not
subscribed to LKML these days.

Cheers,
-- 
Kurt Garloff <kurt@garloff.de>  [Koeln/Greven]


[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]

[PATCH 1/3]: Store SRAT revision

[Kurt Garloff]

[-- Attachment #1.1: Type: text/plain, Size: 187 bytes --]

Hi,

This patch stores the SRAT table revision for later consumption
by arch specific __init functions.


This is patch 1/3.
-- 
Kurt Garloff <kurt@garloff.de>  [Koeln/Greven]


[-- Attachment #1.2: acpi_srat-pxm-rev-store.patch --]
[-- Type: text/x-patch, Size: 2074 bytes --]

From: Kurt Garloff <kurt@garloff.de>
Subject: Store SRAT table revision
References: bnc#503038

In SRAT v1, we had 8bit proximity domain (PXM) fields; SRAT v2 provides
32bits for these. The new fields were reserved before.
According to the ACPI spec, the OS must disregrard reserved fields.
In order to know whether or not, we must know what version the SRAT
table has.

This patch stores the SRAT table revision for later consumption
by arch specific __init functions.

This is patch 1/3.

Signed-off-by: Kurt Garloff <kurt@garloff.de>

---
 drivers/acpi/numa.c      |    3 +++
 include/acpi/acpi_numa.h |    1 +
 2 files changed, 4 insertions(+)

Index: linux-2.6.git/drivers/acpi/numa.c
===================================================================
--- linux-2.6.git.orig/drivers/acpi/numa.c
+++ linux-2.6.git/drivers/acpi/numa.c
@@ -44,8 +44,10 @@ static int pxm_to_node_map[MAX_PXM_DOMAI
 			= { [0 ... MAX_PXM_DOMAINS - 1] = NUMA_NO_NODE };
 static int node_to_pxm_map[MAX_NUMNODES]
 			= { [0 ... MAX_NUMNODES - 1] = PXM_INVAL };
 
+unsigned char acpi_srat_revision __initdata;
+
 int pxm_to_node(int pxm)
 {
 	if (pxm < 0)
 		return NUMA_NO_NODE;
@@ -254,11 +256,15 @@ acpi_parse_memory_affinity(struct acpi_s
 }
 
 static int __init acpi_parse_srat(struct acpi_table_header *table)
 {
+	struct acpi_table_srat *srat;
 	if (!table)
 		return -EINVAL;
 
+  	srat = (struct acpi_table_srat *)table;
+	acpi_srat_revision = srat->header.revision;
+
 	/* Real work done in acpi_table_parse_srat below. */
 
 	return 0;
 }
Index: linux-2.6.git/include/acpi/acpi_numa.h
===================================================================
--- linux-2.6.git.orig/include/acpi/acpi_numa.h
+++ linux-2.6.git/include/acpi/acpi_numa.h
@@ -14,7 +14,8 @@
 extern int pxm_to_node(int);
 extern int node_to_pxm(int);
 extern void __acpi_map_pxm_to_node(int, int);
 extern int acpi_map_pxm_to_node(int);
+extern unsigned char acpi_srat_revision;
 
 #endif				/* CONFIG_ACPI_NUMA */
 #endif				/* __ACP_NUMA_H */

[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH 1/3]: Store SRAT revision

[Greg KH]

On Sun, Jan 15, 2012 at 10:34:07PM +0100, Kurt Garloff wrote:
> Hi,
> 
> This patch stores the SRAT table revision for later consumption
> by arch specific __init functions.
> 
> 
> This is patch 1/3.
> -- 
> Kurt Garloff <kurt@garloff.de>  [Koeln/Greven]
> 

> From: Kurt Garloff <kurt@garloff.de>
> Subject: Store SRAT table revision
> References: bnc#503038
> 
> In SRAT v1, we had 8bit proximity domain (PXM) fields; SRAT v2 provides
> 32bits for these. The new fields were reserved before.
> According to the ACPI spec, the OS must disregrard reserved fields.
> In order to know whether or not, we must know what version the SRAT
> table has.
> 
> This patch stores the SRAT table revision for later consumption
> by arch specific __init functions.
> 
> This is patch 1/3.
> 
> Signed-off-by: Kurt Garloff <kurt@garloff.de>
> 
> ---
>  drivers/acpi/numa.c      |    3 +++
>  include/acpi/acpi_numa.h |    1 +
>  2 files changed, 4 insertions(+)


<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

</formletter>

[PATCH 2/3]: x86-64: Handle SRAT v1 and v2 consistently

[Kurt Garloff]

[-- Attachment #1.1: Type: text/plain, Size: 340 bytes --]

Hi,

x86-64 was rather inconsistent prior to this patch; it used 8 bits 
for the pxm field in cpu_affinity, but 32 bits in mem_affinity.
This patch makes it consistent: Either use 8 bits consistently (SRAT
rev 1 or lower) or 32 bits (SRAT rev 2 or higher).

This is patch 2/3.
-- 
Kurt Garloff <kurt@garloff.de>  [Koeln/Greven]


[-- Attachment #1.2: acpi_srat-pxm-rev-x86-64.patch --]
[-- Type: text/x-patch, Size: 1654 bytes --]

From: Kurt Garloff <kurt@garloff.de>
Subject: Use SRAT table rev to use 8bit or 32bit PXM fields (x86/x86-64)
References: bnc#503038

In SRAT v1, we had 8bit proximity domain (PXM) fields; SRAT v2 provides
32bits for these. The new fields were reserved before.
According to the ACPI spec, the OS must disregrard reserved fields.

x86/x86-64 was rather inconsistent prior to this patch; it used 8 bits 
for the pxm field in cpu_affinity, but 32 bits in mem_affinity.
This patch makes it consistent: Either use 8 bits consistently (SRAT
rev 1 or lower) or 32 bits (SRAT rev 2 or higher).

This is patch 2/3.

Signed-off-by: Kurt Garloff <kurt@garloff.de>

---
 arch/x86/mm/srat.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: linux-2.6.git/arch/x86/mm/srat.c
===================================================================
--- linux-2.6.git.orig/arch/x86/mm/srat.c
+++ linux-2.6.git/arch/x86/mm/srat.c
@@ -108,8 +108,10 @@ acpi_numa_processor_affinity_init(struct
 	}
 	if ((pa->flags & ACPI_SRAT_CPU_ENABLED) == 0)
 		return;
 	pxm = pa->proximity_domain_lo;
+	if (acpi_srat_revision >= 2)
+		pxm |= *((unsigned int*)pa->proximity_domain_hi) << 8;
 	node = setup_node(pxm);
 	if (node < 0) {
 		printk(KERN_ERR "SRAT: Too many proximity domains %x\n", pxm);
 		bad_srat();
@@ -159,8 +161,10 @@ acpi_numa_memory_affinity_init(struct ac
 		return;
 	start = ma->base_address;
 	end = start + ma->length;
 	pxm = ma->proximity_domain;
+	if (acpi_srat_revision <= 1)
+		pxm &= 0xff;
 	node = setup_node(pxm);
 	if (node < 0) {
 		printk(KERN_ERR "SRAT: Too many proximity domains.\n");
 		bad_srat();

[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]

[PATCH 3/3]: Consider SRAT rev on ia64

[Kurt Garloff]

[-- Attachment #1.1: Type: text/plain, Size: 283 bytes --]

Hi,

ia64 did handle the PXM fields almost consistently, but depending on 
sgi's sn2 platform. This patch leaves the sn2 logic in, but does also
use 16/32 bits for PXM if the SRAT has rev 2 or higher.

This is patch 3/3.
-- 
Kurt Garloff <kurt@garloff.de>  [Koeln/Greven]


[-- Attachment #1.2: acpi_srat-pxm-rev-ia64.patch --]
[-- Type: text/x-patch, Size: 2177 bytes --]

From: Kurt Garloff <kurt@garloff.de>
Subject: Use SRAT table rev to use 8bit or 16/32bit PXM fields (ia64)
References: bnc#503038

In SRAT v1, we had 8bit proximity domain (PXM) fields; SRAT v2 provides
32bits for these. The new fields were reserved before.
According to the ACPI spec, the OS must disregrard reserved fields.

ia64 did handle the PXM fields almost consistently, but depending on 
sgi's sn2 platform. This patch leaves the sn2 logic in, but does also
use 16/32 bits for PXM if the SRAT has rev 2 or higher.

The patch also adds __init to the two pxm accessor functions, as they
access __initdata now and are called from an __init function only anyway.

Note that the code only uses 16 bits for the PXM field in the processor
proximity field; the patch does not address this as 16 bits are more than
enough.

This is patch 3/3.

Signed-off-by: Kurt Garloff <kurt@garloff.de>

---
 arch/ia64/kernel/acpi.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Index: linux-2.6.git/arch/ia64/kernel/acpi.c
===================================================================
--- linux-2.6.git.orig/arch/ia64/kernel/acpi.c
+++ linux-2.6.git/arch/ia64/kernel/acpi.c
@@ -428,24 +428,26 @@ static u32 __devinitdata pxm_flag[PXM_FL
 #define pxm_bit_test(bit)	(test_bit(bit,(void *)pxm_flag))
 static struct acpi_table_slit __initdata *slit_table;
 cpumask_t early_cpu_possible_map = CPU_MASK_NONE;
 
-static int get_processor_proximity_domain(struct acpi_srat_cpu_affinity *pa)
+static int __init
+get_processor_proximity_domain(struct acpi_srat_cpu_affinity *pa)
 {
 	int pxm;
 
 	pxm = pa->proximity_domain_lo;
-	if (ia64_platform_is("sn2"))
+	if (ia64_platform_is("sn2") || acpi_srat_revision >= 2)
 		pxm += pa->proximity_domain_hi[0] << 8;
 	return pxm;
 }
 
-static int get_memory_proximity_domain(struct acpi_srat_mem_affinity *ma)
+static int __init
+get_memory_proximity_domain(struct acpi_srat_mem_affinity *ma)
 {
 	int pxm;
 
 	pxm = ma->proximity_domain;
-	if (!ia64_platform_is("sn2"))
+	if (!ia64_platform_is("sn2") && acpi_srat_revision <= 1)
 		pxm &= 0xff;
 
 	return pxm;
 }

[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]