2024-04-02 14:47:12

by Saurabh Singh Sengar

[permalink] [raw]
Subject: [PATCH v2 0/4] Fixing SMP config for DeviceTree platforms

Original motivation for these changes was to fix an SMT warning observed
in Hyper-V VTL platform. Check V1 for more details:
https://lore.kernel.org/lkml/[email protected]/

But on further review it was observed that x86 DeviceTree config has
modified recently, and there is opportunity to improve the existing
smp parsing for DeviceTree.

https://lore.kernel.org/lkml/[email protected]/

Changes are small but I have logically separated them into four patches.
I am willing to merge them if there are too many

[V2]
- Modified the approch how to fix the SMT issue, this is patch 3
in this series. V1 for it is mentioned in above link.
- Added 2 more patches which are related to new SMP config parsing.
These are patches 2 and 4 in this series.
- Fixed an issue with hv_vtl platform how it should use the smp
parsing for DeviceTree with a fixes tag.

Saurabh Sengar (4):
x86/hyperv/vtl: Correct parse_smp_cfg assignment
x86/of: Set the parse_smp_cfg for all the DeviceTree platforms by
default
x86/of: Map NUMA node to CPUs as per DeviceTree
x86/of: Change x86_dtb_parse_smp_config to static

arch/x86/hyperv/hv_vtl.c | 1 -
arch/x86/include/asm/prom.h | 9 ++-------
arch/x86/kernel/devicetree.c | 24 ++++++++++++++----------
arch/x86/platform/ce4100/ce4100.c | 1 -
4 files changed, 16 insertions(+), 19 deletions(-)

--
2.34.1



2024-04-02 14:47:13

by Saurabh Singh Sengar

[permalink] [raw]
Subject: [PATCH v2 2/4] x86/of: Set the parse_smp_cfg for all the DeviceTree platforms by default

x86_dtb_parse_smp_config must be set by DeviceTree platform for
parsing SMP configuration. Set the parse_smp_cfg pointer to
x86_dtb_parse_smp_config by default so that all the dtb platforms
need not to assign it explicitly. Today there are only two platforms
using DeviceTree in x86, ce4100 and hv_vtl. Remove the explicit
assignment of x86_dtb_parse_smp_config function from these.

Signed-off-by: Saurabh Sengar <[email protected]>
---
arch/x86/hyperv/hv_vtl.c | 1 -
arch/x86/include/asm/prom.h | 7 ++-----
arch/x86/kernel/devicetree.c | 6 ++++--
arch/x86/platform/ce4100/ce4100.c | 1 -
4 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index 3efd0e03b916..04775346369c 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -34,7 +34,6 @@ void __init hv_vtl_init_platform(void)
/* Avoid searching for BIOS MP tables */
x86_init.mpparse.find_mptable = x86_init_noop;
x86_init.mpparse.early_parse_smp_cfg = x86_init_noop;
- x86_init.mpparse.parse_smp_cfg = x86_dtb_parse_smp_config;

x86_platform.get_wallclock = get_rtc_noop;
x86_platform.set_wallclock = set_rtc_noop;
diff --git a/arch/x86/include/asm/prom.h b/arch/x86/include/asm/prom.h
index 043758a2e627..02644e010514 100644
--- a/arch/x86/include/asm/prom.h
+++ b/arch/x86/include/asm/prom.h
@@ -24,18 +24,15 @@ extern u64 initial_dtb;
extern void add_dtb(u64 data);
void x86_of_pci_init(void);
void x86_dtb_parse_smp_config(void);
+void x86_flattree_get_config(void);
#else
static inline void add_dtb(u64 data) { }
static inline void x86_of_pci_init(void) { }
static inline void x86_dtb_parse_smp_config(void) { }
+static inline void x86_flattree_get_config(void) { }
#define of_ioapic 0
#endif

-#ifdef CONFIG_OF_EARLY_FLATTREE
-void x86_flattree_get_config(void);
-#else
-static inline void x86_flattree_get_config(void) { }
-#endif
extern char cmd_line[COMMAND_LINE_SIZE];

#endif /* __ASSEMBLY__ */
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 003e0298f46a..0d3a50e8395d 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -277,9 +277,9 @@ static void __init dtb_apic_setup(void)
dtb_ioapic_setup();
}

-#ifdef CONFIG_OF_EARLY_FLATTREE
void __init x86_flattree_get_config(void)
{
+#ifdef CONFIG_OF_EARLY_FLATTREE
u32 size, map_len;
void *dt;

@@ -301,8 +301,10 @@ void __init x86_flattree_get_config(void)

if (initial_dtb)
early_memunmap(dt, map_len);
-}
#endif
+ if (of_have_populated_dt())
+ x86_init.mpparse.parse_smp_cfg = x86_dtb_parse_smp_config;
+}

void __init x86_dtb_parse_smp_config(void)
{
diff --git a/arch/x86/platform/ce4100/ce4100.c b/arch/x86/platform/ce4100/ce4100.c
index f32451bdcfdd..f8126821a94d 100644
--- a/arch/x86/platform/ce4100/ce4100.c
+++ b/arch/x86/platform/ce4100/ce4100.c
@@ -139,7 +139,6 @@ void __init x86_ce4100_early_setup(void)
x86_init.resources.probe_roms = x86_init_noop;
x86_init.mpparse.find_mptable = x86_init_noop;
x86_init.mpparse.early_parse_smp_cfg = x86_init_noop;
- x86_init.mpparse.parse_smp_cfg = x86_dtb_parse_smp_config;
x86_init.pci.init = ce4100_pci_init;
x86_init.pci.init_irq = sdv_pci_init;

--
2.34.1


2024-04-02 15:02:29

by Saurabh Singh Sengar

[permalink] [raw]
Subject: [PATCH v2 1/4] x86/hyperv/vtl: Correct parse_smp_cfg assignment

VTL platform uses DeviceTree for fetching SMP configuration, assign
the correct parsing function 'x86_dtb_parse_smp_config' for it to
parse_smp_cfg.

Fixes: c22e19cd2c8a ("x86/hyperv/vtl: Prepare for separate mpparse callbacks")
Signed-off-by: Saurabh Sengar <[email protected]>
---
arch/x86/hyperv/hv_vtl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index 5c7de79423b8..3efd0e03b916 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -34,7 +34,7 @@ void __init hv_vtl_init_platform(void)
/* Avoid searching for BIOS MP tables */
x86_init.mpparse.find_mptable = x86_init_noop;
x86_init.mpparse.early_parse_smp_cfg = x86_init_noop;
- x86_init.mpparse.parse_smp_cfg = x86_init_noop;
+ x86_init.mpparse.parse_smp_cfg = x86_dtb_parse_smp_config;

x86_platform.get_wallclock = get_rtc_noop;
x86_platform.set_wallclock = set_rtc_noop;
--
2.34.1


2024-04-02 15:04:12

by Saurabh Singh Sengar

[permalink] [raw]
Subject: [PATCH v2 3/4] x86/of: Map NUMA node to CPUs as per DeviceTree

Currently for DeviceTree bootup, x86 code does the default mapping of
CPUs to NUMA, which is wrong. This can cause incorrect mapping and WARN
on a SMT enabled systems like below:

CPU #1's smt-sibling CPU #0 is not on the same node! [node: 1 != 0]. Ignoring dependency.
WARNING: CPU: 1 PID: 0 at topology_sane.isra.0+0x5c/0x6d
match_smt+0xf6/0xfc
set_cpu_sibling_map.cold+0x24f/0x512
start_secondary+0x5c/0x110

Add the set_apicid_to_node() function in dtb_cpu_setup() for allowing
the NUMA to CPU mapping for DeviceTree platforms.

Signed-off-by: Saurabh Sengar <[email protected]>
---
arch/x86/kernel/devicetree.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 0d3a50e8395d..b93ce8a39ff7 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -24,6 +24,7 @@
#include <asm/pci_x86.h>
#include <asm/setup.h>
#include <asm/i8259.h>
+#include <asm/numa.h>
#include <asm/prom.h>

__initdata u64 initial_dtb;
@@ -137,6 +138,7 @@ static void __init dtb_cpu_setup(void)
continue;
}
topology_register_apic(apic_id, CPU_ACPIID_INVALID, true);
+ set_apicid_to_node(apic_id, of_node_to_nid(dn));
}
}

--
2.34.1


2024-04-02 15:15:29

by Saurabh Singh Sengar

[permalink] [raw]
Subject: [PATCH v2 4/4] x86/of: Change x86_dtb_parse_smp_config to static

x86_dtb_parse_smp_config is called locally only, change it to static.

Signed-off-by: Saurabh Sengar <[email protected]>
---
arch/x86/include/asm/prom.h | 2 --
arch/x86/kernel/devicetree.c | 18 +++++++++---------
2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/prom.h b/arch/x86/include/asm/prom.h
index 02644e010514..365798cb4408 100644
--- a/arch/x86/include/asm/prom.h
+++ b/arch/x86/include/asm/prom.h
@@ -23,12 +23,10 @@ extern int of_ioapic;
extern u64 initial_dtb;
extern void add_dtb(u64 data);
void x86_of_pci_init(void);
-void x86_dtb_parse_smp_config(void);
void x86_flattree_get_config(void);
#else
static inline void add_dtb(u64 data) { }
static inline void x86_of_pci_init(void) { }
-static inline void x86_dtb_parse_smp_config(void) { }
static inline void x86_flattree_get_config(void) { }
#define of_ioapic 0
#endif
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index b93ce8a39ff7..8e3c53b4d070 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -279,6 +279,15 @@ static void __init dtb_apic_setup(void)
dtb_ioapic_setup();
}

+static void __init x86_dtb_parse_smp_config(void)
+{
+ if (!of_have_populated_dt())
+ return;
+
+ dtb_setup_hpet();
+ dtb_apic_setup();
+}
+
void __init x86_flattree_get_config(void)
{
#ifdef CONFIG_OF_EARLY_FLATTREE
@@ -307,12 +316,3 @@ void __init x86_flattree_get_config(void)
if (of_have_populated_dt())
x86_init.mpparse.parse_smp_cfg = x86_dtb_parse_smp_config;
}
-
-void __init x86_dtb_parse_smp_config(void)
-{
- if (!of_have_populated_dt())
- return;
-
- dtb_setup_hpet();
- dtb_apic_setup();
-}
--
2.34.1


Subject: [tip: x86/platform] x86/of: Change x86_dtb_parse_smp_config() to static

The following commit has been merged into the x86/platform branch of tip:

Commit-ID: f87136c05714836f1b659365443caccc1bbfce2d
Gitweb: https://git.kernel.org/tip/f87136c05714836f1b659365443caccc1bbfce2d
Author: Saurabh Sengar <[email protected]>
AuthorDate: Tue, 02 Apr 2024 07:40:30 -07:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 03 Apr 2024 08:49:56 +02:00

x86/of: Change x86_dtb_parse_smp_config() to static

x86_dtb_parse_smp_config() is called locally only, change it to static.

Signed-off-by: Saurabh Sengar <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/prom.h | 2 --
arch/x86/kernel/devicetree.c | 18 +++++++++---------
2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/prom.h b/arch/x86/include/asm/prom.h
index 02644e0..365798c 100644
--- a/arch/x86/include/asm/prom.h
+++ b/arch/x86/include/asm/prom.h
@@ -23,12 +23,10 @@ extern int of_ioapic;
extern u64 initial_dtb;
extern void add_dtb(u64 data);
void x86_of_pci_init(void);
-void x86_dtb_parse_smp_config(void);
void x86_flattree_get_config(void);
#else
static inline void add_dtb(u64 data) { }
static inline void x86_of_pci_init(void) { }
-static inline void x86_dtb_parse_smp_config(void) { }
static inline void x86_flattree_get_config(void) { }
#define of_ioapic 0
#endif
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index b93ce8a..8e3c53b 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -279,6 +279,15 @@ static void __init dtb_apic_setup(void)
dtb_ioapic_setup();
}

+static void __init x86_dtb_parse_smp_config(void)
+{
+ if (!of_have_populated_dt())
+ return;
+
+ dtb_setup_hpet();
+ dtb_apic_setup();
+}
+
void __init x86_flattree_get_config(void)
{
#ifdef CONFIG_OF_EARLY_FLATTREE
@@ -307,12 +316,3 @@ void __init x86_flattree_get_config(void)
if (of_have_populated_dt())
x86_init.mpparse.parse_smp_cfg = x86_dtb_parse_smp_config;
}
-
-void __init x86_dtb_parse_smp_config(void)
-{
- if (!of_have_populated_dt())
- return;
-
- dtb_setup_hpet();
- dtb_apic_setup();
-}

Subject: [tip: x86/platform] x86/of: Map NUMA node to CPUs as per DeviceTree

The following commit has been merged into the x86/platform branch of tip:

Commit-ID: 85900d061884de85f557a06cf56ff69dfae07e26
Gitweb: https://git.kernel.org/tip/85900d061884de85f557a06cf56ff69dfae07e26
Author: Saurabh Sengar <[email protected]>
AuthorDate: Tue, 02 Apr 2024 07:40:29 -07:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 03 Apr 2024 08:49:15 +02:00

x86/of: Map NUMA node to CPUs as per DeviceTree

Currently for DeviceTree bootup, x86 code does the default mapping of
CPUs to NUMA, which is wrong. This can cause incorrect mapping and WARNs
on SMT enabled systems:

CPU #1's smt-sibling CPU #0 is not on the same node! [node: 1 != 0]. Ignoring dependency.
WARNING: CPU: 1 PID: 0 at topology_sane.isra.0+0x5c/0x6d
match_smt+0xf6/0xfc
set_cpu_sibling_map.cold+0x24f/0x512
start_secondary+0x5c/0x110

Call the set_apicid_to_node() function in dtb_cpu_setup() for setting
the NUMA to CPU mapping for DeviceTree platforms.

Signed-off-by: Saurabh Sengar <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/devicetree.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 0d3a50e..b93ce8a 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -24,6 +24,7 @@
#include <asm/pci_x86.h>
#include <asm/setup.h>
#include <asm/i8259.h>
+#include <asm/numa.h>
#include <asm/prom.h>

__initdata u64 initial_dtb;
@@ -137,6 +138,7 @@ static void __init dtb_cpu_setup(void)
continue;
}
topology_register_apic(apic_id, CPU_ACPIID_INVALID, true);
+ set_apicid_to_node(apic_id, of_node_to_nid(dn));
}
}


Subject: [tip: x86/platform] x86/of: Set the parse_smp_cfg for all the DeviceTree platforms by default

The following commit has been merged into the x86/platform branch of tip:

Commit-ID: 222408cde4d0ab17e54d4db26751c2b5cab9ac2b
Gitweb: https://git.kernel.org/tip/222408cde4d0ab17e54d4db26751c2b5cab9ac2b
Author: Saurabh Sengar <[email protected]>
AuthorDate: Tue, 02 Apr 2024 07:40:28 -07:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 03 Apr 2024 08:46:08 +02:00

x86/of: Set the parse_smp_cfg for all the DeviceTree platforms by default

x86_dtb_parse_smp_config() must be set by DeviceTree platform for
parsing SMP configuration. Set the parse_smp_cfg pointer to
x86_dtb_parse_smp_config() by default so that all the dtb platforms
need not to assign it explicitly. Today there are only two platforms
using DeviceTree in x86, ce4100 and hv_vtl. Remove the explicit
assignment of x86_dtb_parse_smp_config() function from these.

Signed-off-by: Saurabh Sengar <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/hyperv/hv_vtl.c | 1 -
arch/x86/include/asm/prom.h | 7 ++-----
arch/x86/kernel/devicetree.c | 6 ++++--
arch/x86/platform/ce4100/ce4100.c | 1 -
4 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index 3efd0e0..0477534 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -34,7 +34,6 @@ void __init hv_vtl_init_platform(void)
/* Avoid searching for BIOS MP tables */
x86_init.mpparse.find_mptable = x86_init_noop;
x86_init.mpparse.early_parse_smp_cfg = x86_init_noop;
- x86_init.mpparse.parse_smp_cfg = x86_dtb_parse_smp_config;

x86_platform.get_wallclock = get_rtc_noop;
x86_platform.set_wallclock = set_rtc_noop;
diff --git a/arch/x86/include/asm/prom.h b/arch/x86/include/asm/prom.h
index 043758a..02644e0 100644
--- a/arch/x86/include/asm/prom.h
+++ b/arch/x86/include/asm/prom.h
@@ -24,18 +24,15 @@ extern u64 initial_dtb;
extern void add_dtb(u64 data);
void x86_of_pci_init(void);
void x86_dtb_parse_smp_config(void);
+void x86_flattree_get_config(void);
#else
static inline void add_dtb(u64 data) { }
static inline void x86_of_pci_init(void) { }
static inline void x86_dtb_parse_smp_config(void) { }
+static inline void x86_flattree_get_config(void) { }
#define of_ioapic 0
#endif

-#ifdef CONFIG_OF_EARLY_FLATTREE
-void x86_flattree_get_config(void);
-#else
-static inline void x86_flattree_get_config(void) { }
-#endif
extern char cmd_line[COMMAND_LINE_SIZE];

#endif /* __ASSEMBLY__ */
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 003e029..0d3a50e 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -277,9 +277,9 @@ static void __init dtb_apic_setup(void)
dtb_ioapic_setup();
}

-#ifdef CONFIG_OF_EARLY_FLATTREE
void __init x86_flattree_get_config(void)
{
+#ifdef CONFIG_OF_EARLY_FLATTREE
u32 size, map_len;
void *dt;

@@ -301,8 +301,10 @@ void __init x86_flattree_get_config(void)

if (initial_dtb)
early_memunmap(dt, map_len);
-}
#endif
+ if (of_have_populated_dt())
+ x86_init.mpparse.parse_smp_cfg = x86_dtb_parse_smp_config;
+}

void __init x86_dtb_parse_smp_config(void)
{
diff --git a/arch/x86/platform/ce4100/ce4100.c b/arch/x86/platform/ce4100/ce4100.c
index f32451b..f812682 100644
--- a/arch/x86/platform/ce4100/ce4100.c
+++ b/arch/x86/platform/ce4100/ce4100.c
@@ -139,7 +139,6 @@ void __init x86_ce4100_early_setup(void)
x86_init.resources.probe_roms = x86_init_noop;
x86_init.mpparse.find_mptable = x86_init_noop;
x86_init.mpparse.early_parse_smp_cfg = x86_init_noop;
- x86_init.mpparse.parse_smp_cfg = x86_dtb_parse_smp_config;
x86_init.pci.init = ce4100_pci_init;
x86_init.pci.init_irq = sdv_pci_init;


Subject: [tip: x86/platform] x86/hyperv/vtl: Correct x86_init.mpparse.parse_smp_cfg assignment

The following commit has been merged into the x86/platform branch of tip:

Commit-ID: fe5e6b599fbc417662c549c04d278a13098eb52a
Gitweb: https://git.kernel.org/tip/fe5e6b599fbc417662c549c04d278a13098eb52a
Author: Saurabh Sengar <[email protected]>
AuthorDate: Tue, 02 Apr 2024 07:40:27 -07:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 03 Apr 2024 08:46:08 +02:00

x86/hyperv/vtl: Correct x86_init.mpparse.parse_smp_cfg assignment

VTL platform uses DeviceTree for fetching SMP configuration, assign
the correct parsing function x86_dtb_parse_smp_config() for it to
parse_smp_cfg.

Fixes: c22e19cd2c8a ("x86/hyperv/vtl: Prepare for separate mpparse callbacks")
Signed-off-by: Saurabh Sengar <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/hyperv/hv_vtl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index 5c7de79..3efd0e0 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -34,7 +34,7 @@ void __init hv_vtl_init_platform(void)
/* Avoid searching for BIOS MP tables */
x86_init.mpparse.find_mptable = x86_init_noop;
x86_init.mpparse.early_parse_smp_cfg = x86_init_noop;
- x86_init.mpparse.parse_smp_cfg = x86_init_noop;
+ x86_init.mpparse.parse_smp_cfg = x86_dtb_parse_smp_config;

x86_platform.get_wallclock = get_rtc_noop;
x86_platform.set_wallclock = set_rtc_noop;

2024-04-04 23:58:28

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] x86/hyperv/vtl: Correct parse_smp_cfg assignment

On Tue, Apr 02, 2024 at 07:40:27AM -0700, Saurabh Sengar wrote:
> VTL platform uses DeviceTree for fetching SMP configuration, assign
> the correct parsing function 'x86_dtb_parse_smp_config' for it to
> parse_smp_cfg.
>
> Fixes: c22e19cd2c8a ("x86/hyperv/vtl: Prepare for separate mpparse callbacks")
> Signed-off-by: Saurabh Sengar <[email protected]>

Acked-by: Wei Liu <[email protected]>