All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Laura Nao <laura.nao@collabora.com>,
	x86@kernel.org, kernel@collabora.com, regressions@leemhuis.info,
	"regressions@lists.linux.dev" <87il0p2era.ffs@tglx>
Subject: [patch 2/2] x86/cpu/amd: Make the NODEID_MSR union actually work
Date: Wed, 10 Apr 2024 21:45:28 +0200 (CEST)	[thread overview]
Message-ID: <20240410194311.596282919@linutronix.de> (raw)
In-Reply-To: 20240410194002.909839385@linutronix.de

A system with NODEID_MSR was reported to crash during early boot without
any output.

The reason is that the union which is used for accessing the bitfields in
the MSR is written wrongly and the resulting executable code accesses the
wrong part of the MSR data.

As a consequence a later division by that value results in 0 and that
result is used for another division as divisor, which obviously does not
work well.

The magic world of C, unions and bitfields:

    union {
    	  u64   bita : 3,
	        bitb : 3;
	  u64   all;
    } x;

    x.all = foo();

    a = x.bita;
    b = x.bitb;

results in the effective executable code of:

   a = b = x.bita;

because bita and bitb are treated as union members and therefore both end
up at bit offset 0.

Wrapping the bitfield into an anonymous struct:

    union {
    	  struct {
    	     u64  bita : 3,
	          bitb : 3;
          };
	  u64	  all;
    } x;

works like expected.

Rework the NODEID_MSR union in exactly that way to cure the problem.

Fixes: f7fb3b2dd92c ("x86/cpu: Provide an AMD/HYGON specific topology parser")
Reported-by: Laura Nao <laura.nao@collabora.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/cpu/topology_amd.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- a/arch/x86/kernel/cpu/topology_amd.c
+++ b/arch/x86/kernel/cpu/topology_amd.c
@@ -121,13 +121,13 @@ static bool parse_8000_001e(struct topo_
 
 static bool parse_fam10h_node_id(struct topo_scan *tscan)
 {
-	struct {
-		union {
+	union {
+		struct {
 			u64	node_id		:  3,
 				nodes_per_pkg	:  3,
 				unused		: 58;
-			u64	msr;
 		};
+		u64		msr;
 	} nid;
 
 	if (!boot_cpu_has(X86_FEATURE_NODEID_MSR))


  parent reply	other threads:[~2024-04-10 19:45 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-22 17:52 [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks Laura Nao
2024-03-28  9:44 ` Laura Nao
2024-03-28 11:50   ` Laura Nao
2024-04-04  8:24     ` Linux regression tracking (Thorsten Leemhuis)
2024-04-04  9:26       ` Laura Nao
2024-04-04 13:01       ` Thomas Gleixner
2024-04-04 13:05         ` Thomas Gleixner
2024-04-04 15:23           ` Laura Nao
2024-04-04 16:14             ` Thomas Gleixner
2024-04-04 18:06               ` Thomas Gleixner
2024-04-04 19:14                 ` Thomas Gleixner
2024-04-04 20:05                   ` Thomas Gleixner
2024-04-05  8:14                     ` Laura Nao
2024-04-05  8:42                       ` Thomas Gleixner
2024-04-05 10:32                         ` Laura Nao
2024-04-05 13:38                           ` Thomas Gleixner
2024-04-05 13:58                             ` Laura Nao
2024-04-05 14:59                               ` Thomas Gleixner
2024-04-08  8:20                                 ` Thomas Gleixner
2024-04-08 11:06                                   ` Laura Nao
2024-04-08 14:19                                     ` Thomas Gleixner
2024-04-09 10:07                                       ` Laura Nao
2024-04-09 12:25                                         ` Thomas Gleixner
2024-04-10  8:15                                           ` Laura Nao
2024-04-10 13:57                                             ` Thomas Gleixner
2024-04-10 16:11                                               ` Laura Nao
2024-04-10 19:34                                                 ` Thomas Gleixner
2024-04-10 19:45                                                   ` [patch 0/2] x86/cpu/amd: Fixup the topology rework fallout Thomas Gleixner
2024-04-10 19:45                                                     ` [patch 1/2] x86/cpu/amd: Make the CPUID 0x80000008 parser correct Thomas Gleixner
2024-04-11 12:45                                                       ` [tip: x86/urgent] " tip-bot2 for Thomas Gleixner
2024-04-12 10:12                                                       ` tip-bot2 for Thomas Gleixner
2024-04-10 19:45                                                     ` Thomas Gleixner [this message]
2024-04-11 12:45                                                       ` [tip: x86/urgent] x86/cpu/amd: Make the NODEID_MSR union actually work tip-bot2 for Thomas Gleixner
2024-04-12 10:12                                                       ` tip-bot2 for Thomas Gleixner
2024-04-11 11:27                                                     ` [patch 0/2] x86/cpu/amd: Fixup the topology rework fallout Laura Nao
2024-04-11 11:37                                                       ` Linux regression tracking (Thorsten Leemhuis)
2024-04-11 12:14                                                         ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240410194311.596282919@linutronix.de \
    --to=tglx@linutronix.de \
    --cc=87il0p2era.ffs@tglx \
    --cc=kernel@collabora.com \
    --cc=laura.nao@collabora.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=regressions@leemhuis.info \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.