2024-03-18 21:10:49

by Robert Richter

[permalink] [raw]
Subject: [PATCH 0/3] ACPI/NUMA: SRAT/CEDT fixes and updates

Some fixes and updates for SRAT/CEDT parsing code. Patches can be
applied individually and are independent.

First patch fixes a page fault during boot. It should be marked stable.

2nd patch adds diagnostic printouts for CEDT.

3rd patch removes architectural code no longer needed.

Robert Richter (3):
x86/numa: Fix SRAT lookup of CFMWS ranges with numa_fill_memblks()
ACPI/NUMA: Print CXL Early Discovery Table (CEDT)
ACPI/NUMA: Remove architecture dependent remainings

drivers/acpi/numa/Kconfig | 1 +
drivers/acpi/numa/srat.c | 178 ++++++++++++++++++++++++++++----------
include/linux/acpi.h | 5 --
3 files changed, 133 insertions(+), 51 deletions(-)

--
2.39.2



2024-03-18 21:11:01

by Robert Richter

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

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 <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Alison Schofield <[email protected]>
Signed-off-by: Robert Richter <[email protected]>
---
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

config ACPI_HMAT
--
2.39.2


2024-03-18 21:11:21

by Robert Richter

[permalink] [raw]
Subject: [PATCH 1/3] x86/numa: Fix SRAT lookup of CFMWS ranges with numa_fill_memblks()

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 <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Alison Schofield <[email protected]>
Signed-off-by: Robert Richter <[email protected]>
---
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

config ACPI_HMAT
--
2.39.2


2024-03-18 21:11:40

by Robert Richter

[permalink] [raw]
Subject: [PATCH 2/3] ACPI/NUMA: Print CXL Early Discovery Table (CEDT)

The CEDT contains similar entries as the SRAT. For diagnostic reasons
print the CEDT same style as the SRAT.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/acpi/numa/srat.c | 112 +++++++++++++++++++++++++++++++++++++++
1 file changed, 112 insertions(+)

diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
index e45e64993c50..50ae8557e8d1 100644
--- a/drivers/acpi/numa/srat.c
+++ b/drivers/acpi/numa/srat.c
@@ -300,6 +300,114 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
return -EINVAL;
}

+static int __init
+__acpi_table_print_cedt_entry(union acpi_subtable_headers *__header,
+ void *arg, const unsigned long table_end)
+{
+ struct acpi_cedt_header *header = (struct acpi_cedt_header *)__header;
+
+ switch (header->type) {
+ case ACPI_CEDT_TYPE_CHBS:
+ {
+ struct acpi_cedt_chbs *p =
+ (struct acpi_cedt_chbs *)header;
+
+ if (header->length < sizeof(struct acpi_cedt_chbs)) {
+ pr_warn("CEDT: unsupported CHBS entry: size %d\n",
+ header->length);
+ break;
+ }
+
+ pr_info("CEDT: CHBS (0x%llx length 0x%llx uid %lu) %s (%d)\n",
+ (unsigned long long)p->base,
+ (unsigned long long)p->length,
+ (unsigned long)p->uid,
+ (p->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11) ?
+ "cxl11" :
+ (p->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL20) ?
+ "cxl20" :
+ "unsupported version",
+ p->cxl_version);
+ }
+ break;
+ case ACPI_CEDT_TYPE_CFMWS:
+ {
+ struct acpi_cedt_cfmws *p =
+ (struct acpi_cedt_cfmws *)header;
+ int eiw_to_ways[] = {1, 2, 4, 8, 16, 3, 6, 12};
+ int targets = -1;
+
+ if (header->length < sizeof(struct acpi_cedt_cfmws)) {
+ pr_warn("CEDT: unsupported CFMWS entry: size %d\n",
+ header->length);
+ break;
+ }
+
+ if (p->interleave_ways < ARRAY_SIZE(eiw_to_ways))
+ targets = eiw_to_ways[p->interleave_ways];
+ if (header->length < struct_size(
+ p, interleave_targets, targets))
+ targets = -1;
+
+ pr_info("CEDT: CFMWS (0x%llx length 0x%llx) with %d target%s",
+ (unsigned long long)p->base_hpa,
+ (unsigned long long)p->window_size,
+ targets, targets > 1 ? "s" : "");
+ for (int i = 0; i < targets; i++)
+ pr_cont("%s%lu", i ? ", " : " (",
+ (unsigned long)p->interleave_targets[i]);
+ pr_cont("%s%s%s%s%s%s\n",
+ targets > 0 ? ")" : "",
+ (p->restrictions & ACPI_CEDT_CFMWS_RESTRICT_TYPE2) ?
+ " type2" : "",
+ (p->restrictions & ACPI_CEDT_CFMWS_RESTRICT_TYPE3) ?
+ " type3" : "",
+ (p->restrictions & ACPI_CEDT_CFMWS_RESTRICT_VOLATILE) ?
+ " volatile" : "",
+ (p->restrictions & ACPI_CEDT_CFMWS_RESTRICT_PMEM) ?
+ " pmem" : "",
+ (p->restrictions & ACPI_CEDT_CFMWS_RESTRICT_FIXED) ?
+ " fixed" : "");
+ }
+ break;
+ case ACPI_CEDT_TYPE_CXIMS:
+ {
+ struct acpi_cedt_cxims *p =
+ (struct acpi_cedt_cxims *)header;
+
+ if (header->length < sizeof(struct acpi_cedt_cxims)) {
+ pr_warn("CEDT: unsupported CXIMS entry: size %d\n",
+ header->length);
+ break;
+ }
+
+ pr_info("CEDT: CXIMS (hbig %u nr_xormaps %u)\n",
+ (unsigned int)p->hbig,
+ (unsigned int)p->nr_xormaps);
+ }
+ break;
+ default:
+ pr_warn("CEDT: Found unsupported entry (type = 0x%x)\n",
+ header->type);
+ break;
+ }
+
+ return 0;
+}
+
+static void __init acpi_table_print_cedt_entry(enum acpi_cedt_type id)
+{
+ acpi_table_parse_cedt(id, __acpi_table_print_cedt_entry, NULL);
+}
+
+static void __init acpi_table_print_cedt(void)
+{
+ /* Print only implemented CEDT types */
+ acpi_table_print_cedt_entry(ACPI_CEDT_TYPE_CHBS);
+ acpi_table_print_cedt_entry(ACPI_CEDT_TYPE_CFMWS);
+ acpi_table_print_cedt_entry(ACPI_CEDT_TYPE_CXIMS);
+}
+
static int __init acpi_parse_cfmws(union acpi_subtable_headers *header,
void *arg, const unsigned long table_end)
{
@@ -341,6 +449,7 @@ static int __init acpi_parse_cfmws(union acpi_subtable_headers *header,
return 0;
}
#else
+static inline void acpi_table_print_cedt(void) {}
static int __init acpi_parse_cfmws(union acpi_subtable_headers *header,
void *arg, const unsigned long table_end)
{
@@ -526,6 +635,9 @@ int __init acpi_numa_init(void)
/* SLIT: System Locality Information Table */
acpi_table_parse(ACPI_SIG_SLIT, acpi_parse_slit);

+ /* CEDT: CXL Early Discovery Table */
+ acpi_table_print_cedt();
+
/*
* CXL Fixed Memory Window Structures (CFMWS) must be parsed
* after the SRAT. Create NUMA Nodes for CXL memory ranges that
--
2.39.2


2024-03-18 21:11:42

by Robert Richter

[permalink] [raw]
Subject: [PATCH 3/3] ACPI/NUMA: Remove architecture dependent remainings

With the removal of the Itanium architecture [1] the last architecture
dependent functions:

acpi_numa_slit_init(), acpi_numa_memory_affinity_init()

were removed. Remove its remainings in the header files too an make
them static.

[1] cf8e8658100d arch: Remove Itanium (IA-64) architecture

Signed-off-by: Robert Richter <[email protected]>
---
drivers/acpi/numa/srat.c | 68 +++++++++++++---------------------------
include/linux/acpi.h | 5 ---
2 files changed, 21 insertions(+), 52 deletions(-)

diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
index 50ae8557e8d1..910609a9754b 100644
--- a/drivers/acpi/numa/srat.c
+++ b/drivers/acpi/numa/srat.c
@@ -208,16 +208,21 @@ int __init srat_disabled(void)
return acpi_numa < 0;
}

-#if defined(CONFIG_X86) || defined(CONFIG_ARM64) || defined(CONFIG_LOONGARCH)
/*
* Callback for SLIT parsing. pxm_to_node() returns NUMA_NO_NODE for
* I/O localities since SRAT does not list them. I/O localities are
* not supported at this point.
*/
-void __init acpi_numa_slit_init(struct acpi_table_slit *slit)
+static int __init acpi_parse_slit(struct acpi_table_header *table)
{
+ struct acpi_table_slit *slit = (struct acpi_table_slit *)table;
int i, j;

+ if (!slit_valid(slit)) {
+ pr_info("SLIT table looks invalid. Not used.\n");
+ return -EINVAL;
+ }
+
for (i = 0; i < slit->locality_count; i++) {
const int from_node = pxm_to_node(i);

@@ -234,19 +239,25 @@ void __init acpi_numa_slit_init(struct acpi_table_slit *slit)
slit->entry[slit->locality_count * i + j]);
}
}
+
+ return 0;
}

-/*
- * Default callback for parsing of the Proximity Domain <-> Memory
- * Area mappings
- */
-int __init
-acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
+static int __initdata parsed_numa_memblks;
+
+static int __init
+acpi_parse_memory_affinity(union acpi_subtable_headers * header,
+ const unsigned long table_end)
{
+ struct acpi_srat_mem_affinity *ma;
u64 start, end;
u32 hotpluggable;
int node, pxm;

+ ma = (struct acpi_srat_mem_affinity *)header;
+
+ acpi_table_print_srat_entry(&header->common);
+
if (srat_disabled())
goto out_err;
if (ma->header.length < sizeof(struct acpi_srat_mem_affinity)) {
@@ -293,6 +304,8 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)

max_possible_pfn = max(max_possible_pfn, PFN_UP(end - 1));

+ parsed_numa_memblks++;
+
return 0;
out_err_bad_srat:
bad_srat();
@@ -448,27 +461,6 @@ static int __init acpi_parse_cfmws(union acpi_subtable_headers *header,
(*fake_pxm)++;
return 0;
}
-#else
-static inline void acpi_table_print_cedt(void) {}
-static int __init acpi_parse_cfmws(union acpi_subtable_headers *header,
- void *arg, const unsigned long table_end)
-{
- return 0;
-}
-#endif /* defined(CONFIG_X86) || defined (CONFIG_ARM64) */
-
-static int __init acpi_parse_slit(struct acpi_table_header *table)
-{
- struct acpi_table_slit *slit = (struct acpi_table_slit *)table;
-
- if (!slit_valid(slit)) {
- pr_info("SLIT table looks invalid. Not used.\n");
- return -EINVAL;
- }
- acpi_numa_slit_init(slit);
-
- return 0;
-}

void __init __weak
acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
@@ -559,24 +551,6 @@ acpi_parse_gi_affinity(union acpi_subtable_headers *header,
}
#endif /* defined(CONFIG_X86) || defined (CONFIG_ARM64) */

-static int __initdata parsed_numa_memblks;
-
-static int __init
-acpi_parse_memory_affinity(union acpi_subtable_headers * header,
- const unsigned long end)
-{
- struct acpi_srat_mem_affinity *memory_affinity;
-
- memory_affinity = (struct acpi_srat_mem_affinity *)header;
-
- acpi_table_print_srat_entry(&header->common);
-
- /* let architecture-dependent part to do it */
- if (!acpi_numa_memory_affinity_init(memory_affinity))
- parsed_numa_memblks++;
- return 0;
-}
-
static int __init acpi_parse_srat(struct acpi_table_header *table)
{
struct acpi_table_srat *srat = (struct acpi_table_srat *)table;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 2a7c4b90d589..5f6472a7997c 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -242,9 +242,6 @@ static inline bool acpi_gicc_is_usable(struct acpi_madt_generic_interrupt *gicc)
return gicc->flags & ACPI_MADT_ENABLED;
}

-/* the following numa functions are architecture-dependent */
-void acpi_numa_slit_init (struct acpi_table_slit *slit);
-
#if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH)
void acpi_numa_processor_affinity_init (struct acpi_srat_cpu_affinity *pa);
#else
@@ -267,8 +264,6 @@ static inline void
acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa) { }
#endif

-int acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
-
#ifndef PHYS_CPUID_INVALID
typedef u32 phys_cpuid_t;
#define PHYS_CPUID_INVALID (phys_cpuid_t)(-1)
--
2.39.2


2024-03-18 21:14:27

by Robert Richter

[permalink] [raw]
Subject: Re: [PATCH 1/3] x86/numa: Fix SRAT lookup for CFMWS ranges with numa_fill_memblks()

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 <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Alison Schofield <[email protected]>
> Signed-off-by: Robert Richter <[email protected]>

This patch should be dropped in favor of the other 1/3 patch, it is a
leftover.

Thanks,

-Robert

2024-03-18 21:27:04

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH 1/3] x86/numa: Fix SRAT lookup for CFMWS ranges with numa_fill_memblks()

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 <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Alison Schofield <[email protected]>
> Signed-off-by: Robert Richter <[email protected]>
> ---
> 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

2024-03-18 21:30:29

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH 2/3] ACPI/NUMA: Print CXL Early Discovery Table (CEDT)

Robert Richter wrote:
> The CEDT contains similar entries as the SRAT. For diagnostic reasons
> print the CEDT same style as the SRAT.

I will defer to Rafael here, but with acpica-tools, cxl-cli, and
/proc/iomem is there much value to adding this to the boot-up logs by
default?

2024-03-18 21:32:35

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH 1/3] x86/numa: Fix SRAT lookup for CFMWS ranges with numa_fill_memblks()

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 <[email protected]>
> > Cc: Dan Williams <[email protected]>
> > Cc: Alison Schofield <[email protected]>
> > Signed-off-by: Robert Richter <[email protected]>
>
> 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?

2024-03-18 21:50:26

by Robert Richter

[permalink] [raw]
Subject: Re: [PATCH 1/3] x86/numa: Fix SRAT lookup for CFMWS ranges with numa_fill_memblks()

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 <[email protected]>
> > Cc: Dan Williams <[email protected]>
> > Cc: Alison Schofield <[email protected]>
> > Signed-off-by: Robert Richter <[email protected]>
> > ---
> > 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

2024-03-18 21:55:53

by Robert Richter

[permalink] [raw]
Subject: Re: [PATCH 2/3] ACPI/NUMA: Print CXL Early Discovery Table (CEDT)

On 18.03.24 14:30:11, Dan Williams wrote:
> Robert Richter wrote:
> > The CEDT contains similar entries as the SRAT. For diagnostic reasons
> > print the CEDT same style as the SRAT.
>
> I will defer to Rafael here, but with acpica-tools, cxl-cli, and
> /proc/iomem is there much value to adding this to the boot-up logs by
> default?

Compared to the number of SRAT printks there are just a few CEDT
entries. I have added it as CEDT and SRAT entries have the similar
effect to NUMA, so diagnostics should be at the same level.

Thanks,

-Robert

2024-03-19 11:55:17

by Robert Richter

[permalink] [raw]
Subject: [PATCH] cxl: Fix use of phys_to_target_node() outside of init section

Hi Dan,

patch below. I have not included it into v2 of the SRAT/CEDT changes
as it is cxl specific and can be applied separately.

Thanks,

-Robert


On 18.03.24 14:26:41, Dan Williams wrote:
> 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

From be5b495980bae41d879909212db02dac0fba978e Mon Sep 17 00:00:00 2001
From: Robert Richter <[email protected]>
Date: Tue, 19 Mar 2024 09:28:33 +0100
Subject: [PATCH] cxl: Fix use of phys_to_target_node() outside of init section

The CXL driver uses both functions phys_to_target_node() and
memory_add_physaddr_to_nid(). The x86 architecture relies on the
NUMA_KEEP_MEMINFO kernel option to be set. Enable the option for the
driver accordingly.

Suggested-by: Dan Williams <[email protected]>
Signed-off-by: Robert Richter <[email protected]>
---
drivers/cxl/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
index 67998dbd1d46..6140b3529a29 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 && X86)
help
CXL is a bus that is electrically compatible with PCI Express, but
layers three protocols on that signalling (CXL.io, CXL.cache, and
--
2.39.2


2024-03-20 00:22:11

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] cxl: Fix use of phys_to_target_node() outside of init section

Robert Richter wrote:
> Hi Dan,
>
> patch below. I have not included it into v2 of the SRAT/CEDT changes
> as it is cxl specific and can be applied separately.
>
> Thanks,
>
> -Robert
>
>
> On 18.03.24 14:26:41, Dan Williams wrote:
> > 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
>
> From be5b495980bae41d879909212db02dac0fba978e Mon Sep 17 00:00:00 2001

Hi Robert,

When you send inline patches like this can you remember to include a
scissors line? That way tools like "b4 am" automatically know where to
trim things. So add a line like the following:

-- >8 --

..see "git mailinfo --help" for details.

Also note that if you reply with an updated patch in a series include
the "vX NN/MM" suffix, like "Subject: [PATCH v3 2/3] ..." so that b4 am
knows to perform a "partial reroll".

> From: Robert Richter <[email protected]>
> Date: Tue, 19 Mar 2024 09:28:33 +0100
> Subject: [PATCH] cxl: Fix use of phys_to_target_node() outside of init section
>
> The CXL driver uses both functions phys_to_target_node() and
> memory_add_physaddr_to_nid(). The x86 architecture relies on the
> NUMA_KEEP_MEMINFO kernel option to be set. Enable the option for the
> driver accordingly.
>
> Suggested-by: Dan Williams <[email protected]>
> Signed-off-by: Robert Richter <[email protected]>
> ---
> drivers/cxl/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index 67998dbd1d46..6140b3529a29 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 && X86)
> help
> CXL is a bus that is electrically compatible with PCI Express, but
> layers three protocols on that signalling (CXL.io, CXL.cache, and
> --
> 2.39.2
>



2024-03-21 12:09:28

by Robert Richter

[permalink] [raw]
Subject: Re: [PATCH] cxl: Fix use of phys_to_target_node() outside of init section

On 19.03.24 17:21:53, Dan Williams wrote:
> Robert Richter wrote:
> > Hi Dan,
> >
> > patch below. I have not included it into v2 of the SRAT/CEDT changes
> > as it is cxl specific and can be applied separately.
> >
> > Thanks,
> >
> > -Robert
> >
> >
> > On 18.03.24 14:26:41, Dan Williams wrote:
> > > 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
> >
> > From be5b495980bae41d879909212db02dac0fba978e Mon Sep 17 00:00:00 2001
>
> Hi Robert,
>
> When you send inline patches like this can you remember to include a
> scissors line? That way tools like "b4 am" automatically know where to
> trim things. So add a line like the following:
>
> -- >8 --
>
> ...see "git mailinfo --help" for details.

Thanks for the inside on your patch processing. Will use that in the
future.

>
> Also note that if you reply with an updated patch in a series include
> the "vX NN/MM" suffix, like "Subject: [PATCH v3 2/3] ..." so that b4 am
> knows to perform a "partial reroll".

This patch is in addition to the other SRAT patches and can be applied
directly to the cxl tree. That is why there is no version update here.
But I replied to this series for reference. I saw the b4 shazam
--no-parent option, would that help here?

Thanks,

-Robert

>
> > From: Robert Richter <[email protected]>
> > Date: Tue, 19 Mar 2024 09:28:33 +0100
> > Subject: [PATCH] cxl: Fix use of phys_to_target_node() outside of init section
> >
> > The CXL driver uses both functions phys_to_target_node() and
> > memory_add_physaddr_to_nid(). The x86 architecture relies on the
> > NUMA_KEEP_MEMINFO kernel option to be set. Enable the option for the
> > driver accordingly.
> >
> > Suggested-by: Dan Williams <[email protected]>
> > Signed-off-by: Robert Richter <[email protected]>
> > ---
> > drivers/cxl/Kconfig | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> > index 67998dbd1d46..6140b3529a29 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 && X86)
> > help
> > CXL is a bus that is electrically compatible with PCI Express, but
> > layers three protocols on that signalling (CXL.io, CXL.cache, and
> > --
> > 2.39.2
> >
>
>