[PATCH Resend] x86/topology/amd: Evaluate SMT in CPUID leaf 0x8000001e only on family 0x17 and greater

[PATCH Resend] x86/topology/amd: Evaluate SMT in CPUID leaf 0x8000001e only on family 0x17 and greater

[Thomas Gleixner]

The new AMD/HYGON topology parser evaluates the SMT information in CPUID
leaf 0x8000001e unconditionally while the original code restricted it to
CPUs with family 0x17 and greater.

This breaks family 0x15 CPUs which advertise that leaf and have a non-zero
value in the SMT section. The machine boots, but the scheduler complains
loudly about the mismatch of the core IDs:

  WARNING: CPU: 1 PID: 0 at kernel/sched/core.c:6482 sched_cpu_starting+0x183/0x250
  WARNING: CPU: 0 PID: 1 at kernel/sched/topology.c:2408 build_sched_domains+0x76b/0x12b0

Add the condition back to cure it.

Fixes: f7fb3b2dd92c ("x86/cpu: Provide an AMD/HYGON specific topology parser")
Reported-by: Tim Teichmann <teichmanntim@outlook.de>
Bisected-by: Christian Heusel <christian@heusel.eu>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Tim Teichmann <teichmanntim@outlook.de>
Cc: regressions@lists.linux.dev
Cc: stable@vger.kernel.org
Closes: https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/issues/56
---
Resend with LKML in Cc. Sorry for the noise.
---
 arch/x86/kernel/cpu/topology_amd.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/x86/kernel/cpu/topology_amd.c
+++ b/arch/x86/kernel/cpu/topology_amd.c
@@ -84,9 +84,9 @@ static bool parse_8000_001e(struct topo_
 
 	/*
 	 * If leaf 0xb is available, then the domain shifts are set
-	 * already and nothing to do here.
+	 * already and nothing to do here. Only valid for family >= 0x17.
 	 */
-	if (!has_topoext) {
+	if (!has_topoext && c->x86 >= 0x17) {
 		/*
 		 * Leaf 0x80000008 set the CORE domain shift already.
 		 * Update the SMT domain, but do not propagate it.

[tip: x86/urgent] x86/topology/amd: Evaluate SMT in CPUID leaf 0x8000001e only on family 0x17 and greater

[tip-bot2 for Thomas Gleixner]

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

Commit-ID:     76357cc192acd78b85d4c3380d07f139d906dfe8
Gitweb:        https://git.kernel.org/tip/76357cc192acd78b85d4c3380d07f139d906dfe8
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Tue, 28 May 2024 22:21:31 +02:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Wed, 29 May 2024 11:01:20 +02:00

x86/topology/amd: Evaluate SMT in CPUID leaf 0x8000001e only on family 0x17 and greater

The new AMD/HYGON topology parser evaluates the SMT information in CPUID leaf
0x8000001e unconditionally while the original code restricted it to CPUs with
family 0x17 and greater.

This breaks family 0x15 CPUs which advertise that leaf and have a non-zero
value in the SMT section. The machine boots, but the scheduler complains loudly
about the mismatch of the core IDs:

  WARNING: CPU: 1 PID: 0 at kernel/sched/core.c:6482 sched_cpu_starting+0x183/0x250
  WARNING: CPU: 0 PID: 1 at kernel/sched/topology.c:2408 build_sched_domains+0x76b/0x12b0

Add the condition back to cure it.

  [ bp: Make it actually build because grandpa is not concerned with
    trivial stuff. :-P ]

Fixes: f7fb3b2dd92c ("x86/cpu: Provide an AMD/HYGON specific topology parser")
Closes: https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/issues/56
Reported-by: Tim Teichmann <teichmanntim@outlook.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Tested-by: Tim Teichmann <teichmanntim@outlook.de>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/7skhx6mwe4hxiul64v6azhlxnokheorksqsdbp7qw6g2jduf6c@7b5pvomauugk
---
 arch/x86/kernel/cpu/topology_amd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/topology_amd.c b/arch/x86/kernel/cpu/topology_amd.c
index d419dee..7d476fa 100644
--- a/arch/x86/kernel/cpu/topology_amd.c
+++ b/arch/x86/kernel/cpu/topology_amd.c
@@ -84,9 +84,9 @@ static bool parse_8000_001e(struct topo_scan *tscan, bool has_topoext)
 
 	/*
 	 * If leaf 0xb is available, then the domain shifts are set
-	 * already and nothing to do here.
+	 * already and nothing to do here. Only valid for family >= 0x17.
 	 */
-	if (!has_topoext) {
+	if (!has_topoext && tscan->c->x86 >= 0x17) {
 		/*
 		 * Leaf 0x80000008 set the CORE domain shift already.
 		 * Update the SMT domain, but do not propagate it.

Re: [tip: x86/urgent] x86/topology/amd: Evaluate SMT in CPUID leaf 0x8000001e only on family 0x17 and greater

[Christian Heusel]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

On 24/05/29 09:25AM, tip-bot2 for Thomas Gleixner wrote:
> The following commit has been merged into the x86/urgent branch of tip:
> 
> Commit-ID:     76357cc192acd78b85d4c3380d07f139d906dfe8
> Gitweb:        https://git.kernel.org/tip/76357cc192acd78b85d4c3380d07f139d906dfe8
> Author:        Thomas Gleixner <tglx@linutronix.de>
> AuthorDate:    Tue, 28 May 2024 22:21:31 +02:00
> Committer:     Borislav Petkov (AMD) <bp@alien8.de>
> CommitterDate: Wed, 29 May 2024 11:01:20 +02:00
> 
> [...]
> 
> Fixes: f7fb3b2dd92c ("x86/cpu: Provide an AMD/HYGON specific topology parser")
> Closes: https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/issues/56
> Reported-by: Tim Teichmann <teichmanntim@outlook.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> Tested-by: Tim Teichmann <teichmanntim@outlook.de>
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/r/7skhx6mwe4hxiul64v6azhlxnokheorksqsdbp7qw6g2jduf6c@7b5pvomauugk

Hey Thomas, hey Borislav,

it seems like somehow the patch has lost the following two trailers
compared to the list variant[0] while being applied:

    Bisected-by: Christian Heusel <christian@heusel.eu>
    Cc: regressions@lists.linux.dev

Did that happen on purpose or did some scripts fail?

Cheers,
Chris

[0]: https://lore.kernel.org/lkml/8734q1bsc4.ffs@tglx/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [tip: x86/urgent] x86/topology/amd: Evaluate SMT in CPUID leaf 0x8000001e only on family 0x17 and greater

[Borislav Petkov]

On Thu, May 30, 2024 at 12:40:50AM +0200, Christian Heusel wrote:
> it seems like somehow the patch has lost the following two trailers
> compared to the list variant[0] while being applied:
> 
>     Bisected-by: Christian Heusel <christian@heusel.eu>
>     Cc: regressions@lists.linux.dev
> 
> Did that happen on purpose or did some scripts fail?

Well, we don't add unknown tags because it was getting unwieldy and
Bisected-by is not really one we do. I can offer

	Reported-by: Christian Heusel <christian@heusel.eu>

as bisection is important work and it should be documented.

How does that sound?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [tip: x86/urgent] x86/topology/amd: Evaluate SMT in CPUID leaf 0x8000001e only on family 0x17 and greater

[Christian Heusel]

[-- Attachment #1: Type: text/plain, Size: 985 bytes --]

On 24/05/30 10:59AM, Borislav Petkov wrote:
> On Thu, May 30, 2024 at 12:40:50AM +0200, Christian Heusel wrote:
> > it seems like somehow the patch has lost the following two trailers
> > compared to the list variant[0] while being applied:
> > 
> >     Bisected-by: Christian Heusel <christian@heusel.eu>
> >     Cc: regressions@lists.linux.dev
> > 
> > Did that happen on purpose or did some scripts fail?
> 
> Well, we don't add unknown tags because it was getting unwieldy and
> Bisected-by is not really one we do. I can offer
> 
> 	Reported-by: Christian Heusel <christian@heusel.eu>
> 
> as bisection is important work and it should be documented.
> 
> How does that sound?

Sounds good! I thought it was a thing because I saw it used in a few
places, i.e. here:

https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=e37617c8e53a1f7fcba6d5e1041f4fd8a2425c27

I'll keep it in mind for future bug reports! :)

Have a nice week,
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [tip: x86/urgent] x86/topology/amd: Evaluate SMT in CPUID leaf 0x8000001e only on family 0x17 and greater

[Borislav Petkov]

On Thu, May 30, 2024 at 02:21:28PM +0200, Christian Heusel wrote:
> Sounds good! I thought it was a thing because I saw it used in a few
> places, i.e. here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=e37617c8e53a1f7fcba6d5e1041f4fd8a2425c27

Yeah, I'm polling internally whether we should add this new tag. But
Reported-by pretty much does it...

> I'll keep it in mind for future bug reports! :)
> 
> Have a nice week,

Thanks and ditto. :-)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[tip: x86/urgent] x86/topology/amd: Evaluate SMT in CPUID leaf 0x8000001e only on family 0x17 and greater

[tip-bot2 for Thomas Gleixner]

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

Commit-ID:     34bf6bae3286a58762711cfbce2cf74ecd42e1b5
Gitweb:        https://git.kernel.org/tip/34bf6bae3286a58762711cfbce2cf74ecd42e1b5
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Tue, 28 May 2024 22:21:31 +02:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Thu, 30 May 2024 15:58:55 +02:00

x86/topology/amd: Evaluate SMT in CPUID leaf 0x8000001e only on family 0x17 and greater

The new AMD/HYGON topology parser evaluates the SMT information in CPUID leaf
0x8000001e unconditionally while the original code restricted it to CPUs with
family 0x17 and greater.

This breaks family 0x15 CPUs which advertise that leaf and have a non-zero
value in the SMT section. The machine boots, but the scheduler complains loudly
about the mismatch of the core IDs:

  WARNING: CPU: 1 PID: 0 at kernel/sched/core.c:6482 sched_cpu_starting+0x183/0x250
  WARNING: CPU: 0 PID: 1 at kernel/sched/topology.c:2408 build_sched_domains+0x76b/0x12b0

Add the condition back to cure it.

  [ bp: Make it actually build because grandpa is not concerned with
    trivial stuff. :-P ]

Fixes: f7fb3b2dd92c ("x86/cpu: Provide an AMD/HYGON specific topology parser")
Closes: https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/issues/56
Reported-by: Tim Teichmann <teichmanntim@outlook.de>
Reported-by: Christian Heusel <christian@heusel.eu>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Tested-by: Tim Teichmann <teichmanntim@outlook.de>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/7skhx6mwe4hxiul64v6azhlxnokheorksqsdbp7qw6g2jduf6c@7b5pvomauugk
---
 arch/x86/kernel/cpu/topology_amd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/topology_amd.c b/arch/x86/kernel/cpu/topology_amd.c
index d419dee..7d476fa 100644
--- a/arch/x86/kernel/cpu/topology_amd.c
+++ b/arch/x86/kernel/cpu/topology_amd.c
@@ -84,9 +84,9 @@ static bool parse_8000_001e(struct topo_scan *tscan, bool has_topoext)
 
 	/*
 	 * If leaf 0xb is available, then the domain shifts are set
-	 * already and nothing to do here.
+	 * already and nothing to do here. Only valid for family >= 0x17.
 	 */
-	if (!has_topoext) {
+	if (!has_topoext && tscan->c->x86 >= 0x17) {
 		/*
 		 * Leaf 0x80000008 set the CORE domain shift already.
 		 * Update the SMT domain, but do not propagate it.