diff mbox series

[1/3] x86/numa: Fix SRAT lookup for CFMWS ranges with numa_fill_memblks()

Message ID 20240318210904.2188120-2-rrichter@amd.com
State Superseded
Headers show
Series ACPI/NUMA: SRAT/CEDT fixes and updates | expand

Commit Message

Robert Richter March 18, 2024, 9:09 p.m. UTC
With kconfig option NUMA_KEEP_MEMINFO disabled the SRAT lookup done
with numa_fill_memblks() fails returning NUMA_NO_MEMBLK (-1). An
existing SRAT memory range cannot be found for a CFMWS address range.
This causes the addition of a duplicate numa_memblk with a different
node id and a subsequent page fault and kernel crash during boot.

Note that the issue was initially introduced with [1]. But since
phys_to_target_node() was originally used that returned the valid node
0, an additional numa_memblk was not added. Though, the node id was
wrong too.

Fix this by enabling NUMA_KEEP_MEMINFO for x86 with ACPI and NUMA
enabled.

[1] fd49f99c1809 ("ACPI: NUMA: Add a node and memblk for each CFMWS not in SRAT")

Fixes: 8f1004679987 ("ACPI/NUMA: Apply SRAT proximity domain to entire CFMWS window")
Cc: Derick Marks <derick.w.marks@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/acpi/numa/Kconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Dan Williams March 18, 2024, 9:26 p.m. UTC | #1
Robert Richter wrote:
> With kconfig option NUMA_KEEP_MEMINFO disabled the SRAT lookup done
> with numa_fill_memblks() fails returning NUMA_NO_MEMBLK (-1). An
> existing SRAT memory range cannot be found for a CFMWS address range.
> This causes the addition of a duplicate numa_memblk with a different
> node id and a subsequent page fault and kernel crash during boot.
> 
> Note that the issue was initially introduced with [1]. But since
> phys_to_target_node() was originally used that returned the valid node
> 0, an additional numa_memblk was not added. Though, the node id was
> wrong too.
> 
> Fix this by enabling NUMA_KEEP_MEMINFO for x86 with ACPI and NUMA
> enabled.
> 
> [1] fd49f99c1809 ("ACPI: NUMA: Add a node and memblk for each CFMWS not in SRAT")
> 
> Fixes: 8f1004679987 ("ACPI/NUMA: Apply SRAT proximity domain to entire CFMWS window")
> Cc: Derick Marks <derick.w.marks@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Alison Schofield <alison.schofield@intel.com>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
>  drivers/acpi/numa/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/acpi/numa/Kconfig b/drivers/acpi/numa/Kconfig
> index 849c2bd820b9..2f4ac6ac6768 100644
> --- a/drivers/acpi/numa/Kconfig
> +++ b/drivers/acpi/numa/Kconfig
> @@ -3,6 +3,7 @@ config ACPI_NUMA
>  	bool "NUMA support"
>  	depends on NUMA
>  	depends on (X86 || ARM64 || LOONGARCH)
> +	select NUMA_KEEP_MEMINFO if X86
>  	default y if ARM64

A fix is needed, yes, but this is the wrong one. NUMA_KEEP_MEMINFO is
only about marking numa_meminfo data as not "__init". Since
numa_fill_memblks() *is* an __init function, it should have no
dependency on NUMA_KEEP_MEMINFO.

The fix here involves moving the definition of numa_fill_memblks() out
of the "#ifdef CONFIG_NUMA_KEEP_MEMINFO" in
arch/x86/include/asm/sparsemem.h so that it does not fallback to the
default definition in include/linux/numa.h.

It should also be the case that cxl_acpi needs this:

diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
index 67998dbd1d46..1bf25185c35b 100644
--- a/drivers/cxl/Kconfig
+++ b/drivers/cxl/Kconfig
@@ -6,6 +6,7 @@ menuconfig CXL_BUS
        select FW_UPLOAD
        select PCI_DOE
        select FIRMWARE_TABLE
+       select NUMA_KEEP_MEMINFO if NUMA
        help
          CXL is a bus that is electrically compatible with PCI Express, but
          layers three protocols on that signalling (CXL.io, CXL.cache, and
Dan Williams March 18, 2024, 9:31 p.m. UTC | #2
Robert Richter wrote:
> On 18.03.24 22:09:00, Robert Richter wrote:
> > With kconfig option NUMA_KEEP_MEMINFO disabled the SRAT lookup done
> > with numa_fill_memblks() fails returning NUMA_NO_MEMBLK (-1). An
> > existing SRAT memory range cannot be found for a CFMWS address range.
> > This causes the addition of a duplicate numa_memblk with a different
> > node id and a subsequent page fault and kernel crash during boot.
> > 
> > Note that the issue was initially introduced with [1]. But since
> > phys_to_target_node() was originally used that returned the valid node
> > 0, an additional numa_memblk was not added. Though, the node id was
> > wrong too.
> > 
> > Fix this by enabling NUMA_KEEP_MEMINFO for x86 with ACPI and NUMA
> > enabled.
> > 
> > [1] fd49f99c1809 ("ACPI: NUMA: Add a node and memblk for each CFMWS not in SRAT")
> > 
> > Fixes: 8f1004679987 ("ACPI/NUMA: Apply SRAT proximity domain to entire CFMWS window")
> > Cc: Derick Marks <derick.w.marks@intel.com>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Cc: Alison Schofield <alison.schofield@intel.com>
> > Signed-off-by: Robert Richter <rrichter@amd.com>
> 
> This patch should be dropped in favor of the other 1/3 patch, it is a
> leftover.

What "other" patch? Did I respond to the wrong one?
diff mbox series

Patch

diff --git a/drivers/acpi/numa/Kconfig b/drivers/acpi/numa/Kconfig
index 849c2bd820b9..2f4ac6ac6768 100644
--- a/drivers/acpi/numa/Kconfig
+++ b/drivers/acpi/numa/Kconfig
@@ -3,6 +3,7 @@  config ACPI_NUMA
 	bool "NUMA support"
 	depends on NUMA
 	depends on (X86 || ARM64 || LOONGARCH)
+	select NUMA_KEEP_MEMINFO if X86
 	default y if ARM64
 
 config ACPI_HMAT