| Message ID | 20240318210904.2188120-2-rrichter@amd.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | ACPI/NUMA: SRAT/CEDT fixes and updates | expand |
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. Thanks, -Robert
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
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?
Hi Dan, thanks for the quick review. Yes, this is the 'old' patch. But only the subject was corrected. I will send a v2 anyway. See below. On 18.03.24 14:26:41, Dan Williams wrote: > 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. Right, the option is about just keeping it in non-init mem, but the parsing is durint __init. Will take a look. > > 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 Ok, will take a look here too. Thanks, -Robert > help > CXL is a bus that is electrically compatible with PCI Express, but > layers three protocols on that signalling (CXL.io, CXL.cache, and
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
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(+)