x86: Fix an issue with invalid ACPI numa config

The addition of support to read the numa node for a PCI card specified by _PXM resulted in Martin's system not
booting. Looking at the ACPI tables it seems that there
are PXM entries for the root ports, but no SRAT table.

The absence of SRAT table results in dummy_numa_init being called. However, unlike on arm64, this doesn't then result in numa_off being set. When the PCI code later comes along and calls acpi_get_node for any PCI card below the root port, it navigates up the ACPI tree until it finds the PXM value in the root port. This value is then passed to acpi_map_pxm_to_node. If numa_off is set this returns, NUMA_NO_NODE (as it does on arm64), on x86 it instead tries to allocate a numa node from the unused set without setting up all the infrastructure that would normally accompany such a call. We have not identified exactly which driver is causing the subsequent hang for Martin.

It is invalid under the ACPI spec to specify new numa nodes using PXM if they have no presence in SRAT. Thus the simplest fix is to set numa_off when it is off due to an invalid SRAT (here not present at all).

I do not have easy access to appropriate x86 numa systems so would appreciate some testing of this one!

Known problem boards setups:

AMD Ryzen Threadripper 2950X on ASROCK X399 TAICHI MSI X399 SLI PLUS (probably - not confirmed yet)

The PCI patch has been reverted, so this fix is not critical.

Reported-by Martin Hundeb�ll <[email protected]>

Signed-off-by Jonathan Cameron <[email protected]>

Fixes bad7dcd94f39 ("ACPI/PCI: Pay attention to device-specific _PXM node values")

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

arch/x86/mm/numa.c +2 -0

index 1308f5408bf7..ce1182f953ff 100644
@@ -695,6 +695,8 @@ static int __init dummy_numa_init(void)
 	node_set(0, numa_nodes_parsed);
 	numa_add_memblk(0, 0, PFN_PHYS(max_pfn));
 
+	numa_off = true;
+
 	return 0;
 }
 

The addition of support to read the numa node for a PCI card specified by _PXM resulted in Martin's system not
booting. Looking at the ACPI tables it seems that there
are PXM entries for the root ports, but no SRAT table.

The absence of SRAT table results in dummy_numa_init being called. However, unlike on arm64, this doesn't then result in numa_off being set. When the PCI code later comes along and calls acpi_get_node for any PCI card below the root port, it navigates up the ACPI tree until it finds the PXM value in the root port. This value is then passed to acpi_map_pxm_to_node. If numa_off is set this returns, NUMA_NO_NODE (as it does on arm64), on x86 it instead tries to allocate a numa node from the unused set without setting up all the infrastructure that would normally accompany such a call. We have not identified exactly which driver is causing the subsequent hang for Martin.

It is invalid under the ACPI spec to specify new numa nodes using PXM if they have no presence in SRAT. Thus the simplest fix is to set numa_off when it is off due to an invalid SRAT (here not present at all).

I do not have easy access to appropriate x86 numa systems so would appreciate some testing of this one!

Known problem boards setups:

AMD Ryzen Threadripper 2950X on ASROCK X399 TAICHI MSI X399 SLI PLUS (probably - not confirmed yet)

The PCI patch has been reverted, so this fix is not critical.

Reported-by Martin Hundeb?ll <[email protected]>

Signed-off-by Jonathan Cameron <[email protected]>

Fixes bad7dcd94f39 ("ACPI/PCI: Pay attention to device-specific _PXM node values")

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

arch/x86/mm/numa.c +0 -0

index 1308f5408bf7..ce1182f953ff 100644

Quoted 46 lines · Jonathan Cameron

The addition of support to read the numa node for a PCI card specified by _PXM resulted in Martin's system not
booting. Looking at the ACPI tables it seems that there
are PXM entries for the root ports, but no SRAT table.

The absence of SRAT table results in dummy_numa_init being called. However, unlike on arm64, this doesn't then result in numa_off being set. When the PCI code later comes along and calls acpi_get_node for any PCI card below the root port, it navigates up the ACPI tree until it finds the PXM value in the root port. This value is then passed to acpi_map_pxm_to_node. If numa_off is set this returns, NUMA_NO_NODE (as it does on arm64), on x86 it instead tries to allocate a numa node from the unused set without setting up all the infrastructure that would normally accompany such a call. We have not identified exactly which driver is causing the subsequent hang for Martin.

It is invalid under the ACPI spec to specify new numa nodes using PXM if they have no presence in SRAT. Thus the simplest fix is to set numa_off when it is off due to an invalid SRAT (here not present at all).

I do not have easy access to appropriate x86 numa systems so would appreciate some testing of this one!

Known problem boards setups:

AMD Ryzen Threadripper 2950X on ASROCK X399 TAICHI MSI X399 SLI PLUS (probably - not confirmed yet)

The PCI patch has been reverted, so this fix is not critical.

Reported-by Martin Hundeb?ll <[email protected]>

Signed-off-by Jonathan Cameron <[email protected]>

Fixes bad7dcd94f39 ("ACPI/PCI: Pay attention to device-specific _PXM node values")

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

arch/x86/mm/numa.c +0 -0

index 1308f5408bf7..ce1182f953ff 100644

arch/x86/mm/numa.c static int __init dummy_numa_init(void) · line 698

Quoted 4 lines

arch/x86/mm/numa.c +1 -0

@@ -695,6 +695,8 @@ static int __init dummy_numa_init(void)
 	node_set(0, numa_nodes_parsed);
 	numa_add_memblk(0, 0, PFN_PHYS(max_pfn));
 
+	numa_off = true;  

Should we not:

	pr_err(FW_BUG "Invalid SRAT table.\n");

or something along those lines?

We should take every possibility to call out broken and non-compliant firmware.

Quoted 54 lines · On Tue, 20 Nov 2018 13:01:05 +0100 Peter Zijlstra <[email protected]> wrote:

Quoted 46 lines · Jonathan Cameron

The addition of support to read the numa node for a PCI card specified by _PXM resulted in Martin's system not
booting. Looking at the ACPI tables it seems that there
are PXM entries for the root ports, but no SRAT table.

The absence of SRAT table results in dummy_numa_init being called. However, unlike on arm64, this doesn't then result in numa_off being set. When the PCI code later comes along and calls acpi_get_node for any PCI card below the root port, it navigates up the ACPI tree until it finds the PXM value in the root port. This value is then passed to acpi_map_pxm_to_node. If numa_off is set this returns, NUMA_NO_NODE (as it does on arm64), on x86 it instead tries to allocate a numa node from the unused set without setting up all the infrastructure that would normally accompany such a call. We have not identified exactly which driver is causing the subsequent hang for Martin.

It is invalid under the ACPI spec to specify new numa nodes using PXM if they have no presence in SRAT. Thus the simplest fix is to set numa_off when it is off due to an invalid SRAT (here not present at all).

I do not have easy access to appropriate x86 numa systems so would appreciate some testing of this one!

Known problem boards setups:

AMD Ryzen Threadripper 2950X on ASROCK X399 TAICHI MSI X399 SLI PLUS (probably - not confirmed yet)

The PCI patch has been reverted, so this fix is not critical.

Reported-by Martin Hundeb?ll <[email protected]>

Signed-off-by Jonathan Cameron <[email protected]>

Fixes bad7dcd94f39 ("ACPI/PCI: Pay attention to device-specific _PXM node values")

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

arch/x86/mm/numa.c +0 -0

index 1308f5408bf7..ce1182f953ff 100644

arch/x86/mm/numa.c static int __init dummy_numa_init(void) · line 698

Quoted 4 lines

arch/x86/mm/numa.c +1 -0

@@ -695,6 +695,8 @@ static int __init dummy_numa_init(void)
 	node_set(0, numa_nodes_parsed);
 	numa_add_memblk(0, 0, PFN_PHYS(max_pfn));
 
+	numa_off = true;    

Should we not:

Quoted 6 lines

	pr_err(FW_BUG "Invalid SRAT table.\n");

or something along those lines?

We should take every possibility to call out broken and non-compliant firmware.

While I agree we should definitely be calling this out nice and loud, not having an SRAT isn't indicating a broken firmware as SRAT is optional in ACPI. The breakage only occurs much later when the DSDT contains a PXM entry that doesn't correspond to any entries in SRAT.

To report at that point, we would need some background info stashed on why numa_off was set. It could be off because we set it so via the kernel command line.

If people are happy with this general direction I'm happy to spin a patch to do that state tracking and pr_err as a follow up.

The addition of support to read the numa node for a PCI

card specified by _PXM resulted in Martin's system not
booting. Looking at the ACPI tables it seems that there
are PXM entries for the root ports, but no SRAT table.

The absence of SRAT table results in dummy_numa_init being

called. However, unlike on arm64, this doesn't then result in numa_off being set. When the PCI code later comes along and calls acpi_get_node for any PCI card below the root port, it navigates up the ACPI tree until it finds the PXM value in the root port. This value is then passed to acpi_map_pxm_to_node. If numa_off is set this returns, NUMA_NO_NODE (as it does on arm64), on x86 it instead tries

to allocate a numa node from the unused set without setting up all the infrastructure that would normally accompany such a call. We have not identified exactly which driver is causing the subsequent hang for Martin.

It is invalid under the ACPI spec to specify new numa nodes using PXM if they have no presence in SRAT. Thus the simplest fix is to set numa_off when it is off due to an invalid SRAT (here not present at all).

I do not have easy access to appropriate x86 numa systems so would appreciate some testing of this one!

Known problem boards setups:

AMD Ryzen Threadripper 2950X on ASROCK X399 TAICHI MSI X399 SLI PLUS (probably - not confirmed yet)

The PCI patch has been reverted, so this fix is not critical.

Reported-by Martin Hundeb?ll <[email protected]>

Signed-off-by Jonathan Cameron <[email protected]>

Fixes bad7dcd94f39 ("ACPI/PCI: Pay attention to device-specific _PXM node values")

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

arch/x86/mm/numa.c +2 -0

index 1308f5408bf7..ce1182f953ff 100644
@@ -695,6 +695,8 @@ static int __init dummy_numa_init(void)
 	node_set(0, numa_nodes_parsed);
 	numa_add_memblk(0, 0, PFN_PHYS(max_pfn));
 
+	numa_off = true;
+
 	return 0;
 }
 

Signature · 1 line

2.18.0

I don't see this in linux-next so I assume nobody has picked it up yet? numa.c isn't my file, but the fix is for something that went in via PCI, so I can take this patch if nobody else has already.

The changelog could all be wrapped to use more of the line. Assume 80 char width and allow for the 4 spaces added by git log.

Quoted 1 line · Jonathan Cameron

The addition of support to read the numa node for a PCI

s/numa/NUMA/

Quoted 3 lines

card specified by _PXM resulted in Martin's system not
booting. Looking at the ACPI tables it seems that there
are PXM entries for the root ports, but no SRAT table.

s/PXM/_PXM/ (and below)

Quoted 1 line

The absence of SRAT table results in dummy_numa_init being

dummy_numa_init() and similar for other functions below.

Quoted 7 lines

called. However, unlike on arm64, this doesn't then result in numa_off being set. When the PCI code later comes along and calls acpi_get_node for any PCI card below the root port, it navigates up the ACPI tree until it finds the PXM value in the root port. This value is then passed to acpi_map_pxm_to_node. If numa_off is set this returns, NUMA_NO_NODE (as it does on arm64), on x86 it instead tries

This doesn't parse quite right.

Quoted 4 lines

to allocate a numa node from the unused set without setting up all the infrastructure that would normally accompany such a call. We have not identified exactly which driver is causing the subsequent hang for Martin.

It's not clear to me why dummy_numa_init() must be different between arm64 and x86. This patch makes x86 more like arm64, which is good. Maybe a subsequent patch could go even further?

Quoted 41 lines

It is invalid under the ACPI spec to specify new numa nodes using PXM if they have no presence in SRAT. Thus the simplest fix is to set numa_off when it is off due to an invalid SRAT (here not present at all).

I do not have easy access to appropriate x86 numa systems so would appreciate some testing of this one!

Known problem boards setups:

AMD Ryzen Threadripper 2950X on ASROCK X399 TAICHI MSI X399 SLI PLUS (probably - not confirmed yet)

The PCI patch has been reverted, so this fix is not critical.

Reported-by Martin Hundeb?ll <[email protected]>

Signed-off-by Jonathan Cameron <[email protected]>

Fixes bad7dcd94f39 ("ACPI/PCI: Pay attention to device-specific _PXM node values")

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

arch/x86/mm/numa.c +2 -0

index 1308f5408bf7..ce1182f953ff 100644
@@ -695,6 +695,8 @@ static int __init dummy_numa_init(void)
 	node_set(0, numa_nodes_parsed);
 	numa_add_memblk(0, 0, PFN_PHYS(max_pfn));
 
+	numa_off = true;
+
 	return 0;
 }
 

Signature · 1 line

2.18.0