[REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Laura Nao]

Hello,

KernelCI has identified a mainline boot regression [1] on the following
AMD Stoney Ridge Chromebooks (grunt family), between v6.8 (e8f897) and
v6.8-1185-g855684c7d938 (855684):
- Acer Chromebook Spin 311 R721T (codename kasumi360)
- HP Chromebook 14 (codename careena)
- HP Chromebook 11A G6 EE (codename barla)

The kernel doesn't boot at all and nothing is reported on the serial
console after "Starting kernel ...". The issue is still present on the
latest mainline revision.
The defconfig used by KernelCI for the boot tests can be found in [2].

Sending this report in order to track the regression while a fix is
identified.

Thanks,

Laura

[1] https://linux.kernelci.org/test/case/id/65fca98e3883a392524c4380/
[2]
https://storage.kernelci.org/mainline/master/v6.8-11837-g2ac2b1665d3fb/x86_64/x86_64_defconfig%2Bx86-board/gcc-10/config/kernel.config

#regzbot introduced: v6.8..v6.8-1185-g855684c7d938

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Laura Nao]

Hi Thomas,

I ran a manual bisection to track down the root cause for this 
regression and landed on the c749ce commit from this series:
https://lore.kernel.org/all/20240212153625.145745053@linutronix.de/

Do you have any insight on this issue or any suggestion on how to
effectively debug this?

Thank you!

Laura

#regzbot introduced: c749ce393b

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Laura Nao]

> Hi Thomas,
> 
> I ran a manual bisection to track down the root cause for this
> regression and landed on the c749ce commit from this series:
> https://lore.kernel.org/all/20240212153625.145745053@linutronix.de/
> 
> Do you have any insight on this issue or any suggestion on how to
> effectively debug this?
> 
> Thank you!
> 
> Laura

Hi again,

I actually forgot to quote the previous message describing the issue:

> KernelCI has identified a mainline boot regression [1] on the following
> AMD Stoney Ridge Chromebooks (grunt family), between v6.8 (e8f897) and
> v6.8-1185-g855684c7d938 (855684):
> - Acer Chromebook Spin 311 R721T (codename kasumi360)
> - HP Chromebook 14 (codename careena)
> - HP Chromebook 11A G6 EE (codename barla)
> 
> The kernel doesn't boot at all and nothing is reported on the serial
> console after "Starting kernel ...". The issue is still present on the
> latest mainline revision.
> The defconfig used by KernelCI for the boot tests can be found in [2].
> 
> Sending this report in order to track the regression while a fix is
> identified.
> 
> Thanks,
> 
> Laura
> 
> [1] https://linux.kernelci.org/test/case/id/65fca98e3883a392524c4380/
> [2]
> https://storage.kernelci.org/mainline/master/v6.8-11837-g2ac2b1665d3fb/x86_64/x86_64_defconfig%2Bx86-board/gcc-10/config/kernel.config

sorry about that!

Best,

Laura

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Linux regression tracking (Thorsten Leemhuis)]

[/me added x86 team]

On 28.03.24 12:50, Laura Nao wrote:
>>
>> I ran a manual bisection to track down the root cause for this
>> regression and landed on the c749ce
>> commit from this series:
>> https://lore.kernel.org/all/20240212153625.145745053@linutronix.de/

FWIW, that commit is c749ce393b8fe9 ("x86/cpu: Use common topology code
for AMD") from tglx that was part of the "x86/cpu: Rework topology
evaluation" series.

>> Do you have any insight on this issue or any suggestion on how to
>> effectively debug this?
>>
>> Thank you!

Hmmm, it looks like this did not make any progress. Thomas, did this
fall through the cracks due to Easter, or is this this on your todo list?

Or was there some progress and I just missed it?

Laura Nao, I assume the problem is still happening?

FWIW, this was the initial problem description:

>>> KernelCI has identified a mainline boot regression [1] on the following
>>> AMD Stoney Ridge Chromebooks (grunt family), between v6.8 (e8f897) and
>>> v6.8-1185-g855684c7d938 (855684):
>>> - Acer Chromebook Spin 311 R721T (codename kasumi360)
>>> - HP Chromebook 14 (codename careena)
>>> - HP Chromebook 11A G6 EE (codename barla)
>>>
>>> The kernel doesn't boot at all and nothing is reported on the serial
>>> console after "Starting kernel ...". The issue is still present on the
>>> latest mainline revision.
>>> The defconfig used by KernelCI for the boot tests can be found in [2].
>>>
>>> Sending this report in order to track the regression while a fix is
>>> identified.
>>>
>>> Thanks,
>>>
>>> Laura
>>>
>>> [1] https://linux.kernelci.org/test/case/id/65fca98e3883a392524c4380/
>>> [2]
>>> https://storage.kernelci.org/mainline/master/v6.8-11837-g2ac2b1665d3fb/x86_64/x86_64_defconfig%2Bx86-board/gcc-10/config/kernel.config


Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot poke

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Laura Nao]

Hi Thorsten,

On 4/4/24 10:24, Linux regression tracking (Thorsten Leemhuis) wrote:
> [/me added x86 team]
> 
> On 28.03.24 12:50, Laura Nao wrote:
>>>
>>> I ran a manual bisection to track down the root cause for this
>>> regression and landed on the c749ce
>>> commit from this series:
>>> https://lore.kernel.org/all/20240212153625.145745053@linutronix.de/
> 
> FWIW, that commit is c749ce393b8fe9 ("x86/cpu: Use common topology code
> for AMD") from tglx that was part of the "x86/cpu: Rework topology
> evaluation" series.
> 
>>> Do you have any insight on this issue or any suggestion on how to
>>> effectively debug this?
>>>
>>> Thank you!
> 
> Hmmm, it looks like this did not make any progress. Thomas, did this
> fall through the cracks due to Easter, or is this this on your todo list?
> 
> Or was there some progress and I just missed it?
> 
> Laura Nao, I assume the problem is still happening?

Yes, I confirm the issue is still happening.

See e.g. KernelCI report for next-20240403:

https://linux.kernelci.org/test/plan/id/660dbeed148e1a8a284c4372/

> 
> FWIW, this was the initial problem description:
> 
>>>> KernelCI has identified a mainline boot regression [1] on the following
>>>> AMD Stoney Ridge Chromebooks (grunt family), between v6.8 (e8f897) and
>>>> v6.8-1185-g855684c7d938 (855684):
>>>> - Acer Chromebook Spin 311 R721T (codename kasumi360)
>>>> - HP Chromebook 14 (codename careena)
>>>> - HP Chromebook 11A G6 EE (codename barla)
>>>>
>>>> The kernel doesn't boot at all and nothing is reported on the serial
>>>> console after "Starting kernel ...". The issue is still present on the
>>>> latest mainline revision.
>>>> The defconfig used by KernelCI for the boot tests can be found in [2].
>>>>
>>>> Sending this report in order to track the regression while a fix is
>>>> identified.
>>>>
>>>> Thanks,
>>>>
>>>> Laura
>>>>
>>>> [1] https://linux.kernelci.org/test/case/id/65fca98e3883a392524c4380/
>>>> [2]
>>>> https://storage.kernelci.org/mainline/master/v6.8-11837-g2ac2b1665d3fb/x86_64/x86_64_defconfig%2Bx86-board/gcc-10/config/kernel.config
> 
> 
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
> 
> #regzbot poke

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Thomas Gleixner]

On Thu, Apr 04 2024 at 10:24, Linux regression tracking wrote:
> On 28.03.24 12:50, Laura Nao wrote:
>>>
>>> I ran a manual bisection to track down the root cause for this
>>> regression and landed on the c749ce
>>> commit from this series:
>>> https://lore.kernel.org/all/20240212153625.145745053@linutronix.de/
>
> FWIW, that commit is c749ce393b8fe9 ("x86/cpu: Use common topology code
> for AMD") from tglx that was part of the "x86/cpu: Rework topology
> evaluation" series.
>
>>> Do you have any insight on this issue or any suggestion on how to
>>> effectively debug this?
>>>
>>> Thank you!
>
> Hmmm, it looks like this did not make any progress. Thomas, did this
> fall through the cracks due to Easter, or is this this on your todo
> list?
>
> Or was there some progress and I just missed it?

No. It's in my easter eggs basket, but I need to eat all the chocolate
eggs first to get to this.

I'll have a look later today.

Thanks,

        tglx

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Thomas Gleixner]

Laura!

On Thu, Apr 04 2024 at 15:01, Thomas Gleixner wrote:
> On Thu, Apr 04 2024 at 10:24, Linux regression tracking wrote:
>> On 28.03.24 12:50, Laura Nao wrote:
>>>>
>>>> I ran a manual bisection to track down the root cause for this
>>>> regression and landed on the c749ce
>>>> commit from this series:
>>>> https://lore.kernel.org/all/20240212153625.145745053@linutronix.de/
>>
>> FWIW, that commit is c749ce393b8fe9 ("x86/cpu: Use common topology code
>> for AMD") from tglx that was part of the "x86/cpu: Rework topology
>> evaluation" series.

Can you please provide a boot dmesg from a 6.8 kernel and the output of
'cpuid' ?

Thanks,

        tglx

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Laura Nao]

Hi Thomas,

On 4/4/24 15:05, Thomas Gleixner wrote:
> Laura!
> 
> On Thu, Apr 04 2024 at 15:01, Thomas Gleixner wrote:
>> On Thu, Apr 04 2024 at 10:24, Linux regression tracking wrote:
>>> On 28.03.24 12:50, Laura Nao wrote:
>>>>>
>>>>> I ran a manual bisection to track down the root cause for this
>>>>> regression and landed on the c749ce
>>>>> commit from this series:
>>>>> https://lore.kernel.org/all/20240212153625.145745053@linutronix.de/
>>>
>>> FWIW, that commit is c749ce393b8fe9 ("x86/cpu: Use common topology code
>>> for AMD") from tglx that was part of the "x86/cpu: Rework topology
>>> evaluation" series.
> 
> Can you please provide a boot dmesg from a 6.8 kernel and the output of
> 'cpuid' ?

Sure, here's the output from an Acer Chromebook Spin 311 R721T device:
- kernel log: https://pastebin.com/raw/NNbeJupE
- cpuid output: https://pastebin.com/raw/ThmGqnqG

This is with kernel v6.8 (e8f897f4afef0031fe618a8e94127a0934896aba), let
me know if you need the output from a different revision.

Thank you!

Laura

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Thomas Gleixner]

Laura!

On Thu, Apr 04 2024 at 17:23, Laura Nao wrote:
> On 4/4/24 15:05, Thomas Gleixner wrote:
>> Can you please provide a boot dmesg from a 6.8 kernel and the output of
>> 'cpuid' ?
>
> Sure, here's the output from an Acer Chromebook Spin 311 R721T device:
> - kernel log: https://pastebin.com/raw/NNbeJupE
> - cpuid output: https://pastebin.com/raw/ThmGqnqG

Thanks for the quick response.

Can you please provide 'cpuid -r' output too as 'cpuid' does a few
tweaks to the raw data and it's hard to match it back to the code.

Thanks,

        tglx

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Thomas Gleixner]

Laura!

On Thu, Apr 04 2024 at 18:14, Thomas Gleixner wrote:
> Can you please provide 'cpuid -r' output too as 'cpuid' does a few
> tweaks to the raw data and it's hard to match it back to the code.

Don't bother. I think I figured it out.

Can you please test the patch below?

Thanks,

        tglx
---
diff --git a/arch/x86/kernel/cpu/topology_amd.c b/arch/x86/kernel/cpu/topology_amd.c
index 1a8b3ad493af..0d91a04b1741 100644
--- a/arch/x86/kernel/cpu/topology_amd.c
+++ b/arch/x86/kernel/cpu/topology_amd.c
@@ -29,7 +29,17 @@ static bool parse_8000_0008(struct topo_scan *tscan)
 	if (!sft)
 		sft = get_count_order(ecx.cpu_nthreads + 1);
 
-	topology_set_dom(tscan, TOPO_SMT_DOMAIN, sft, ecx.cpu_nthreads + 1);
+	/*
+	 * cpu_nthreads describes the number of threads in the package
+	 * sft is the number of APIC ID bits per package
+	 *
+	 * As the number of actual threads per core is not described in
+	 * this leaf, just set the CORE domain shift and let the later
+	 * parsers set SMT shift. Assume one thread per core by default
+	 * which is correct if there are no other CPUID leafs to parse.
+	 */
+	topology_update_dom(tscan, TOPO_SMT_DOMAIN, 0, 1);
+	topology_set_dom(tscan, TOPO_CORE_DOMAIN, sft, ecx.cpu_nthreads + 1);
 	return true;
 }
 
@@ -73,12 +83,14 @@ static bool parse_8000_001e(struct topo_scan *tscan, bool has_0xb)
 	tscan->c->topo.initial_apicid = leaf.ext_apic_id;
 
 	/*
-	 * If leaf 0xb is available, then SMT shift is set already. If not
-	 * take it from ecx.threads_per_core and use topo_update_dom() -
-	 * topology_set_dom() would propagate and overwrite the already
-	 * propagated CORE level.
+	 * If leaf 0xb is available, then the domain shifts are set
+	 * already and nothing to do here.
 	 */
 	if (!has_0xb) {
+		/*
+		 * Leaf 0x80000008 set the CORE domain shift already.
+		 * Update the SMT domain, but do not propagate it.
+		 */
 		unsigned int nthreads = leaf.core_nthreads + 1;
 
 		topology_update_dom(tscan, TOPO_SMT_DOMAIN, get_count_order(nthreads), nthreads);

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Thomas Gleixner]

On Thu, Apr 04 2024 at 20:06, Thomas Gleixner wrote:
> On Thu, Apr 04 2024 at 18:14, Thomas Gleixner wrote:
>> Can you please provide 'cpuid -r' output too as 'cpuid' does a few
>> tweaks to the raw data and it's hard to match it back to the code.
>
> Don't bother. I think I figured it out.
>
> Can you please test the patch below?

Bah. Won't help. Let me dig deeper.

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Thomas Gleixner]

On Thu, Apr 04 2024 at 21:14, Thomas Gleixner wrote:

> On Thu, Apr 04 2024 at 20:06, Thomas Gleixner wrote:
>> On Thu, Apr 04 2024 at 18:14, Thomas Gleixner wrote:
>>> Can you please provide 'cpuid -r' output too as 'cpuid' does a few
>>> tweaks to the raw data and it's hard to match it back to the code.
>>
>> Don't bother. I think I figured it out.
>>
>> Can you please test the patch below?
>
> Bah. Won't help. Let me dig deeper.

Can I get the /proc/cpuinfo output please?

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Laura Nao]

Hi Thomas,

On 4/4/24 22:05, Thomas Gleixner wrote:
> On Thu, Apr 04 2024 at 21:14, Thomas Gleixner wrote:
> 
>> On Thu, Apr 04 2024 at 20:06, Thomas Gleixner wrote:
>>> On Thu, Apr 04 2024 at 18:14, Thomas Gleixner wrote:
>>>> Can you please provide 'cpuid -r' output too as 'cpuid' does a few
>>>> tweaks to the raw data and it's hard to match it back to the code.
>>>
>>> Don't bother. I think I figured it out.
>>>
>>> Can you please test the patch below?
>>
>> Bah. Won't help. Let me dig deeper.
> 
> Can I get the /proc/cpuinfo output please?

Here are more logs from the same device running v6.8:
- cpuid -r: https://pastebin.com/raw/YmDrnw06
- cpuinfo: https://pastebin.com/raw/Ki0vwdDE

Thanks for looking into this!

Laura

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Thomas Gleixner]

Laura!

On Fri, Apr 05 2024 at 10:14, Laura Nao wrote:
> On 4/4/24 22:05, Thomas Gleixner wrote:
>> Can I get the /proc/cpuinfo output please?
>
> Here are more logs from the same device running v6.8:
> - cpuid -r: https://pastebin.com/raw/YmDrnw06
> - cpuinfo: https://pastebin.com/raw/Ki0vwdDE

Thanks for the info. I'm still not seing why that wouldn't work.

As the machine seems to have a serial port, can you please replace
'earlycon' with 'earlyprintk=ttyS0,115200' on the command line and see
whether that gives some output. I'm not sure that the current 'earlycon'
has the same effect.

Thanks,

        Thomas

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Laura Nao]

On 4/5/24 10:42, Thomas Gleixner wrote:
> Laura!
> 
> On Fri, Apr 05 2024 at 10:14, Laura Nao wrote:
>> On 4/4/24 22:05, Thomas Gleixner wrote:
>>> Can I get the /proc/cpuinfo output please?
>>
>> Here are more logs from the same device running v6.8:
>> - cpuid -r: https://pastebin.com/raw/YmDrnw06
>> - cpuinfo: https://pastebin.com/raw/Ki0vwdDE
> 
> Thanks for the info. I'm still not seing why that wouldn't work.
> 
> As the machine seems to have a serial port, can you please replace
> 'earlycon' with 'earlyprintk=ttyS0,115200' on the command line and see
> whether that gives some output. I'm not sure that the current 'earlycon'
> has the same effect.
> 

Thanks for the hint - replacing earlycon with earlyprintk=ttyS0,115200 
doesn't seem to help unfortunately, I still get no output at all.

Not sure if it's really relevant to mention, but when booting a v6.8
kernel the first stage bootloader (i.e. coreboot in this case) spits out
a few lines before the kernel takes over:

coreboot-v1.9308_26_0.0.22-6034-gd586c7b122 Wed Jul 15 17:51:44 UTC 2020 smm starting...
SMI# #0
Chrome EC: Set SMI mask to 0x0000000000000000
Chrome EC: UHEPI supported
Clearing pending EC events. Error code 1 is expected.
EC returned error result code 9
Chrome EC: Set SCI mask to 0x00000000142609fb

When booting e.g. next-20240404 not even these lines are reported after
'Starting Kernel...'. Not sure if that suggests anything significant
though.

For the sake of completeness, I'm leaving here links to the full test
jobs run in our Collabora LAVA lab, to include everything that's reported
on the serial console after the device is powered on:
- v6.8 test job: https://lava.collabora.dev/scheduler/job/13265735#L279
- next-20240404 test job w/ 'earlyprintk=ttyS0,115200' in cmdline:
https://lava.collabora.dev/scheduler/job/13266992#L279

I also tried applying the patch you attached above on top of
next-20240404 but it doesn't seem to make any difference unfortunately.

Best,

Laura

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Thomas Gleixner]

On Fri, Apr 05 2024 at 12:32, Laura Nao wrote:
> I also tried applying the patch you attached above on top of
> next-20240404 but it doesn't seem to make any difference unfortunately.

As I said it won't help, but it fixes some other issue.

I'm still puzzled and trying to make sense of this.

Can you please boot a working kernel and provide the output of:

# rdmsr 0xc0011005

Thanks,

        tglx

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Laura Nao]

On 4/5/24 15:38, Thomas Gleixner wrote:
> 
> Can you please boot a working kernel and provide the output of:
> 
> # rdmsr 0xc0011005
> 

Sure:

# rdmsr 0xc0011005
2fabbfff2fd3fbff

Thanks,

Laura

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Thomas Gleixner]

On Fri, Apr 05 2024 at 15:58, Laura Nao wrote:
> On 4/5/24 15:38, Thomas Gleixner wrote:
>> 
>> Can you please boot a working kernel and provide the output of:
>> 
>> # rdmsr 0xc0011005
>
> # rdmsr 0xc0011005
> 2fabbfff2fd3fbff

Ok. So the CPU does not advertise FEATURE_TOPOEXT which is consistent
with CPUID.

Now I'm even more puzzled because then the patch I gave you should make
a difference. I need to run some errands now, I will provide a debug
patch later this evening.

Thanks,

        tglx

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Thomas Gleixner]

Laura!

On Fri, Apr 05 2024 at 16:59, Thomas Gleixner wrote:
> On Fri, Apr 05 2024 at 15:58, Laura Nao wrote:
>> On 4/5/24 15:38, Thomas Gleixner wrote:
>>> 
>>> Can you please boot a working kernel and provide the output of:
>>> 
>>> # rdmsr 0xc0011005
>>
>> # rdmsr 0xc0011005
>> 2fabbfff2fd3fbff
>
> Ok. So the CPU does not advertise FEATURE_TOPOEXT which is consistent
> with CPUID.
>
> Now I'm even more puzzled because then the patch I gave you should make
> a difference. I need to run some errands now, I will provide a debug
> patch later this evening.

Hmm. No real good idea yet.

Can you please checkout

    c749ce393b8f ("x86/cpu: Use common topology code for AMD")

and apply the patch further up in the thread and see whether that
boots. We'll take it from there.

Thanks,

        tglx

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Laura Nao]

Hi Thomas,

On 4/8/24 10:20, Thomas Gleixner wrote:
> Laura!
> 
> On Fri, Apr 05 2024 at 16:59, Thomas Gleixner wrote:
>> On Fri, Apr 05 2024 at 15:58, Laura Nao wrote:
>>> On 4/5/24 15:38, Thomas Gleixner wrote:
>>>>
>>>> Can you please boot a working kernel and provide the output of:
>>>>
>>>> # rdmsr 0xc0011005
>>>
>>> # rdmsr 0xc0011005
>>> 2fabbfff2fd3fbff
>>
>> Ok. So the CPU does not advertise FEATURE_TOPOEXT which is consistent
>> with CPUID.
>>
>> Now I'm even more puzzled because then the patch I gave you should make
>> a difference. I need to run some errands now, I will provide a debug
>> patch later this evening.
> 
> Hmm. No real good idea yet.
> 
> Can you please checkout
> 
>      c749ce393b8f ("x86/cpu: Use common topology code for AMD")
> 
> and apply the patch further up in the thread and see whether that
> boots. We'll take it from there.
> 

Just tried that and it doesn't boot, nothing on the serial console (I
kept earlyprintk=ttyS0,115200 in the cmdline).

Test job for reference: https://lava.collabora.dev/scheduler/job/13297561

Thanks,

Laura

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Thomas Gleixner]

Laura!

On Mon, Apr 08 2024 at 13:06, Laura Nao wrote:
> Just tried that and it doesn't boot, nothing on the serial console (I
> kept earlyprintk=ttyS0,115200 in the cmdline).

Thanks for trying. Now let's take a small step back.

Please reset the tree to:

      ace278e7eca6 ("x86/smpboot: Teach it about topo.amd_node_id")

That's the commit right before switching over and according to your
bisect it works. Now apply the patch below, which just runs the new
topology scan function but discards the result.

Thanks,

        tglx
---
--- a/arch/x86/kernel/cpu/topology_amd.c
+++ b/arch/x86/kernel/cpu/topology_amd.c
@@ -29,7 +29,17 @@ static bool parse_8000_0008(struct topo_
 	if (!sft)
 		sft = get_count_order(ecx.cpu_nthreads + 1);
 
-	topology_set_dom(tscan, TOPO_SMT_DOMAIN, sft, ecx.cpu_nthreads + 1);
+	/*
+	 * cpu_nthreads describes the number of threads in the package
+	 * sft is the number of APIC ID bits per package
+	 *
+	 * As the number of actual threads per core is not described in
+	 * this leaf, just set the CORE domain shift and let the later
+	 * parsers set SMT shift. Assume one thread per core by default
+	 * which is correct if there are no other CPUID leafs to parse.
+	 */
+	topology_update_dom(tscan, TOPO_SMT_DOMAIN, 0, 1);
+	topology_set_dom(tscan, TOPO_CORE_DOMAIN, sft, ecx.cpu_nthreads + 1);
 	return true;
 }
 
@@ -73,12 +83,14 @@ static bool parse_8000_001e(struct topo_
 	tscan->c->topo.initial_apicid = leaf.ext_apic_id;
 
 	/*
-	 * If leaf 0xb is available, then SMT shift is set already. If not
-	 * take it from ecx.threads_per_core and use topo_update_dom() -
-	 * topology_set_dom() would propagate and overwrite the already
-	 * propagated CORE level.
+	 * If leaf 0xb is available, then the domain shifts are set
+	 * already and nothing to do here.
 	 */
 	if (!has_0xb) {
+		/*
+		 * Leaf 0x80000008 set the CORE domain shift already.
+		 * Update the SMT domain, but do not propagate it.
+		 */
 		unsigned int nthreads = leaf.core_nthreads + 1;
 
 		topology_update_dom(tscan, TOPO_SMT_DOMAIN, get_count_order(nthreads), nthreads);
--- a/arch/x86/kernel/cpu/topology_common.c
+++ b/arch/x86/kernel/cpu/topology_common.c
@@ -133,6 +133,10 @@ static void parse_topology(struct topo_s
 	tscan->ebx1_nproc_shift = get_count_order(ebx.nproc);
 
 	switch (c->x86_vendor) {
+	case X86_VENDOR_AMD:
+		if (IS_ENABLED(CONFIG_CPU_SUP_AMD))
+			cpu_parse_topology_amd(tscan);
+		break;
 	case X86_VENDOR_CENTAUR:
 	case X86_VENDOR_ZHAOXIN:
 		parse_legacy(tscan);

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Laura Nao]

Hi Thomas,

On 4/8/24 16:19, Thomas Gleixner wrote:
> Laura!
> 
> On Mon, Apr 08 2024 at 13:06, Laura Nao wrote:
>> Just tried that and it doesn't boot, nothing on the serial console (I
>> kept earlyprintk=ttyS0,115200 in the cmdline).
> 
> Thanks for trying. Now let's take a small step back.
> 
> Please reset the tree to:
> 
>        ace278e7eca6 ("x86/smpboot: Teach it about topo.amd_node_id")
> 
> That's the commit right before switching over and according to your
> bisect it works. Now apply the patch below, which just runs the new
> topology scan function but discards the result.
> 

The ace278e7eca6 revision with the patch you provided boots correctly.

Reference test job: https://lava.collabora.dev/scheduler/job/13311035#L1894

Thanks,

Laura

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Thomas Gleixner]

Laura!

On Tue, Apr 09 2024 at 12:07, Laura Nao wrote:
> On 4/8/24 16:19, Thomas Gleixner wrote:
>> That's the commit right before switching over and according to your
>> bisect it works. Now apply the patch below, which just runs the new
>> topology scan function but discards the result.
>
> The ace278e7eca6 revision with the patch you provided boots correctly.

Progress :)

Can you please replace that patch with the one below?

Thanks,

        tglx
---
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1616,6 +1616,13 @@ void __init early_cpu_init(void)
 #endif
 	}
 	early_identify_cpu(&boot_cpu_data);
+
+	pr_info("Max cores: %u\n", boot_cpu_data.x86_max_cores);
+	pr_info("pkg %u die %u core %u nid %u\n", boot_cpu_data.topo.pkg_id,
+		boot_cpu_data.topo.die_id, boot_cpu_data.topo.core_id,
+		boot_cpu_data.topo.amd_node_id);
+	pr_info("SNS %u\n", smp_num_siblings);
+	pr_info("NPP %u MDPP %u\n", __amd_nodes_per_pkg, __max_die_per_package);
 }
 
 static bool detect_null_seg_behavior(void)
--- a/arch/x86/kernel/cpu/topology_amd.c
+++ b/arch/x86/kernel/cpu/topology_amd.c
@@ -29,7 +29,17 @@ static bool parse_8000_0008(struct topo_
 	if (!sft)
 		sft = get_count_order(ecx.cpu_nthreads + 1);
 
-	topology_set_dom(tscan, TOPO_SMT_DOMAIN, sft, ecx.cpu_nthreads + 1);
+	/*
+	 * cpu_nthreads describes the number of threads in the package
+	 * sft is the number of APIC ID bits per package
+	 *
+	 * As the number of actual threads per core is not described in
+	 * this leaf, just set the CORE domain shift and let the later
+	 * parsers set SMT shift. Assume one thread per core by default
+	 * which is correct if there are no other CPUID leafs to parse.
+	 */
+	topology_update_dom(tscan, TOPO_SMT_DOMAIN, 0, 1);
+	topology_set_dom(tscan, TOPO_CORE_DOMAIN, sft, ecx.cpu_nthreads + 1);
 	return true;
 }
 
@@ -73,12 +83,14 @@ static bool parse_8000_001e(struct topo_
 	tscan->c->topo.initial_apicid = leaf.ext_apic_id;
 
 	/*
-	 * If leaf 0xb is available, then SMT shift is set already. If not
-	 * take it from ecx.threads_per_core and use topo_update_dom() -
-	 * topology_set_dom() would propagate and overwrite the already
-	 * propagated CORE level.
+	 * If leaf 0xb is available, then the domain shifts are set
+	 * already and nothing to do here.
 	 */
 	if (!has_0xb) {
+		/*
+		 * Leaf 0x80000008 set the CORE domain shift already.
+		 * Update the SMT domain, but do not propagate it.
+		 */
 		unsigned int nthreads = leaf.core_nthreads + 1;
 
 		topology_update_dom(tscan, TOPO_SMT_DOMAIN, get_count_order(nthreads), nthreads);
--- a/arch/x86/kernel/cpu/topology_common.c
+++ b/arch/x86/kernel/cpu/topology_common.c
@@ -133,6 +133,10 @@ static void parse_topology(struct topo_s
 	tscan->ebx1_nproc_shift = get_count_order(ebx.nproc);
 
 	switch (c->x86_vendor) {
+	case X86_VENDOR_AMD:
+		if (IS_ENABLED(CONFIG_CPU_SUP_AMD))
+			cpu_parse_topology_amd(tscan);
+		break;
 	case X86_VENDOR_CENTAUR:
 	case X86_VENDOR_ZHAOXIN:
 		parse_legacy(tscan);
@@ -162,6 +166,9 @@ static void topo_set_ids(struct topo_sca
 
 	if (c->x86_vendor == X86_VENDOR_AMD)
 		cpu_topology_fixup_amd(tscan);
+
+	pr_info("pkg %u die %u core %u nid %u\n", c->topo.pkg_id, c->topo.die_id,
+		c->topo.core_id, c->topo.amd_node_id);
 }
 
 static void topo_set_max_cores(struct topo_scan *tscan)
@@ -175,6 +182,7 @@ static void topo_set_max_cores(struct to
 	 */
 	tscan->c->x86_max_cores = tscan->dom_ncpus[TOPO_DIEGRP_DOMAIN] >>
 		x86_topo_system.dom_shifts[TOPO_SMT_DOMAIN];
+	pr_info("Max cores: %u\n", tscan->c->x86_max_cores);
 }
 
 void cpu_parse_topology(struct cpuinfo_x86 *c)
@@ -215,18 +223,19 @@ void __init cpu_init_topology(struct cpu
 
 	parse_topology(&tscan, true);
 
-	if (!topo_is_converted(c))
-		return;
-
 	/* Copy the shift values and calculate the unit sizes. */
 	memcpy(x86_topo_system.dom_shifts, tscan.dom_shifts, sizeof(x86_topo_system.dom_shifts));
 
 	dom = TOPO_SMT_DOMAIN;
 	x86_topo_system.dom_size[dom] = 1U << x86_topo_system.dom_shifts[dom];
+	pr_info("Dom %u Sft: %u Sz: %u\n", dom, x86_topo_system.dom_size[dom],
+		x86_topo_system.dom_shifts[dom]);
 
 	for (dom++; dom < TOPO_MAX_DOMAIN; dom++) {
 		sft = x86_topo_system.dom_shifts[dom] - x86_topo_system.dom_shifts[dom - 1];
 		x86_topo_system.dom_size[dom] = 1U << sft;
+		pr_info("Dom %u Sft: %u Sz: %u\n", dom, x86_topo_system.dom_size[dom],
+			x86_topo_system.dom_shifts[dom]);
 	}
 
 	topo_set_ids(&tscan);
@@ -238,6 +247,7 @@ void __init cpu_init_topology(struct cpu
 	 * changes further down the road to get it right during early boot.
 	 */
 	smp_num_siblings = tscan.dom_ncpus[TOPO_SMT_DOMAIN];
+	pr_info("SNS %u\n", smp_num_siblings);
 
 	/*
 	 * Neither it's clear whether there are as many dies as the APIC
@@ -252,4 +262,6 @@ void __init cpu_init_topology(struct cpu
 	 */
 	if (c->x86_vendor == X86_VENDOR_AMD || c->x86_vendor == X86_VENDOR_HYGON)
 		__amd_nodes_per_pkg = __max_die_per_package = tscan.amd_nodes_per_pkg;
+
+	pr_info("NPP %u MDPP %u\n", __amd_nodes_per_pkg, __max_die_per_package);
 }

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Laura Nao]

Hi Thomas,

On 4/9/24 14:25, Thomas Gleixner wrote:
> Laura!
> 
> On Tue, Apr 09 2024 at 12:07, Laura Nao wrote:
>> On 4/8/24 16:19, Thomas Gleixner wrote:
>>> That's the commit right before switching over and according to your
>>> bisect it works. Now apply the patch below, which just runs the new
>>> topology scan function but discards the result.
>>
>> The ace278e7eca6 revision with the patch you provided boots correctly.
> 
> Progress :)
> 

Indeed :)

> Can you please replace that patch with the one below?

So, with this patch applied on top of ace278e7eca6 the kernel doesn't
boot anymore - reference test job: 
https://lava.collabora.dev/scheduler/job/13324010

I see the only change between the second and third patch you provided,
besides the debug prints, is:

-	if (!topo_is_converted(c))
-		return;
-

Printing the debug information without this probably doesn't really help, 
but just in case it's useful: I tried excluding the change above from the
patch while leaving everything else unchanged - reference test job:
https://lava.collabora.dev/scheduler/job/13324298 (also pasted the
kernel log here for easier consultation: https://pastebin.com/raw/TQBDvCah)

Hope this helps,

Laura

> 
> Thanks,
> 
>          tglx
> ---
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1616,6 +1616,13 @@ void __init early_cpu_init(void)
>   #endif
>   	}
>   	early_identify_cpu(&boot_cpu_data);
> +
> +	pr_info("Max cores: %u\n", boot_cpu_data.x86_max_cores);
> +	pr_info("pkg %u die %u core %u nid %u\n", boot_cpu_data.topo.pkg_id,
> +		boot_cpu_data.topo.die_id, boot_cpu_data.topo.core_id,
> +		boot_cpu_data.topo.amd_node_id);
> +	pr_info("SNS %u\n", smp_num_siblings);
> +	pr_info("NPP %u MDPP %u\n", __amd_nodes_per_pkg, __max_die_per_package);
>   }
>   
>   static bool detect_null_seg_behavior(void)
> --- a/arch/x86/kernel/cpu/topology_amd.c
> +++ b/arch/x86/kernel/cpu/topology_amd.c
> @@ -29,7 +29,17 @@ static bool parse_8000_0008(struct topo_
>   	if (!sft)
>   		sft = get_count_order(ecx.cpu_nthreads + 1);
>   
> -	topology_set_dom(tscan, TOPO_SMT_DOMAIN, sft, ecx.cpu_nthreads + 1);
> +	/*
> +	 * cpu_nthreads describes the number of threads in the package
> +	 * sft is the number of APIC ID bits per package
> +	 *
> +	 * As the number of actual threads per core is not described in
> +	 * this leaf, just set the CORE domain shift and let the later
> +	 * parsers set SMT shift. Assume one thread per core by default
> +	 * which is correct if there are no other CPUID leafs to parse.
> +	 */
> +	topology_update_dom(tscan, TOPO_SMT_DOMAIN, 0, 1);
> +	topology_set_dom(tscan, TOPO_CORE_DOMAIN, sft, ecx.cpu_nthreads + 1);
>   	return true;
>   }
>   
> @@ -73,12 +83,14 @@ static bool parse_8000_001e(struct topo_
>   	tscan->c->topo.initial_apicid = leaf.ext_apic_id;
>   
>   	/*
> -	 * If leaf 0xb is available, then SMT shift is set already. If not
> -	 * take it from ecx.threads_per_core and use topo_update_dom() -
> -	 * topology_set_dom() would propagate and overwrite the already
> -	 * propagated CORE level.
> +	 * If leaf 0xb is available, then the domain shifts are set
> +	 * already and nothing to do here.
>   	 */
>   	if (!has_0xb) {
> +		/*
> +		 * Leaf 0x80000008 set the CORE domain shift already.
> +		 * Update the SMT domain, but do not propagate it.
> +		 */
>   		unsigned int nthreads = leaf.core_nthreads + 1;
>   
>   		topology_update_dom(tscan, TOPO_SMT_DOMAIN, get_count_order(nthreads), nthreads);
> --- a/arch/x86/kernel/cpu/topology_common.c
> +++ b/arch/x86/kernel/cpu/topology_common.c
> @@ -133,6 +133,10 @@ static void parse_topology(struct topo_s
>   	tscan->ebx1_nproc_shift = get_count_order(ebx.nproc);
>   
>   	switch (c->x86_vendor) {
> +	case X86_VENDOR_AMD:
> +		if (IS_ENABLED(CONFIG_CPU_SUP_AMD))
> +			cpu_parse_topology_amd(tscan);
> +		break;
>   	case X86_VENDOR_CENTAUR:
>   	case X86_VENDOR_ZHAOXIN:
>   		parse_legacy(tscan);
> @@ -162,6 +166,9 @@ static void topo_set_ids(struct topo_sca
>   
>   	if (c->x86_vendor == X86_VENDOR_AMD)
>   		cpu_topology_fixup_amd(tscan);
> +
> +	pr_info("pkg %u die %u core %u nid %u\n", c->topo.pkg_id, c->topo.die_id,
> +		c->topo.core_id, c->topo.amd_node_id);
>   }
>   
>   static void topo_set_max_cores(struct topo_scan *tscan)
> @@ -175,6 +182,7 @@ static void topo_set_max_cores(struct to
>   	 */
>   	tscan->c->x86_max_cores = tscan->dom_ncpus[TOPO_DIEGRP_DOMAIN] >>
>   		x86_topo_system.dom_shifts[TOPO_SMT_DOMAIN];
> +	pr_info("Max cores: %u\n", tscan->c->x86_max_cores);
>   }
>   
>   void cpu_parse_topology(struct cpuinfo_x86 *c)
> @@ -215,18 +223,19 @@ void __init cpu_init_topology(struct cpu
>   
>   	parse_topology(&tscan, true);
>   
> -	if (!topo_is_converted(c))
> -		return;
> -
>   	/* Copy the shift values and calculate the unit sizes. */
>   	memcpy(x86_topo_system.dom_shifts, tscan.dom_shifts, sizeof(x86_topo_system.dom_shifts));
>   
>   	dom = TOPO_SMT_DOMAIN;
>   	x86_topo_system.dom_size[dom] = 1U << x86_topo_system.dom_shifts[dom];
> +	pr_info("Dom %u Sft: %u Sz: %u\n", dom, x86_topo_system.dom_size[dom],
> +		x86_topo_system.dom_shifts[dom]);
>   
>   	for (dom++; dom < TOPO_MAX_DOMAIN; dom++) {
>   		sft = x86_topo_system.dom_shifts[dom] - x86_topo_system.dom_shifts[dom - 1];
>   		x86_topo_system.dom_size[dom] = 1U << sft;
> +		pr_info("Dom %u Sft: %u Sz: %u\n", dom, x86_topo_system.dom_size[dom],
> +			x86_topo_system.dom_shifts[dom]);
>   	}
>   
>   	topo_set_ids(&tscan);
> @@ -238,6 +247,7 @@ void __init cpu_init_topology(struct cpu
>   	 * changes further down the road to get it right during early boot.
>   	 */
>   	smp_num_siblings = tscan.dom_ncpus[TOPO_SMT_DOMAIN];
> +	pr_info("SNS %u\n", smp_num_siblings);
>   
>   	/*
>   	 * Neither it's clear whether there are as many dies as the APIC
> @@ -252,4 +262,6 @@ void __init cpu_init_topology(struct cpu
>   	 */
>   	if (c->x86_vendor == X86_VENDOR_AMD || c->x86_vendor == X86_VENDOR_HYGON)
>   		__amd_nodes_per_pkg = __max_die_per_package = tscan.amd_nodes_per_pkg;
> +
> +	pr_info("NPP %u MDPP %u\n", __amd_nodes_per_pkg, __max_die_per_package);
>   }

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Thomas Gleixner]

Laura!

On Wed, Apr 10 2024 at 10:15, Laura Nao wrote:
> On 4/9/24 14:25, Thomas Gleixner wrote:
>> Can you please replace that patch with the one below?
>
> So, with this patch applied on top of ace278e7eca6 the kernel doesn't
> boot anymore - reference test job: 
> https://lava.collabora.dev/scheduler/job/13324010
>
> I see the only change between the second and third patch you provided,
> besides the debug prints, is:
>
> -	if (!topo_is_converted(c))
> -		return;
> -

Right. So this limits the area to search significantly.

> Printing the debug information without this probably doesn't really help, 
> but just in case it's useful: I tried excluding the change above from the
> patch while leaving everything else unchanged - reference test job:
> https://lava.collabora.dev/scheduler/job/13324298 (also pasted the
> kernel log here for easier consultation:
> https://pastebin.com/raw/TQBDvCah)
>
> Hope this helps,

It does. Good idea!

I just moved the exit check a bit so we should see the scan info. That
should tell me what goes south.

Thanks,

        tglx
---
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1616,6 +1616,13 @@ void __init early_cpu_init(void)
 #endif
 	}
 	early_identify_cpu(&boot_cpu_data);
+
+	pr_info("Max cores: %u\n", boot_cpu_data.x86_max_cores);
+	pr_info("pkg %u die %u core %u nid %u\n", boot_cpu_data.topo.pkg_id,
+		boot_cpu_data.topo.die_id, boot_cpu_data.topo.core_id,
+		boot_cpu_data.topo.amd_node_id);
+	pr_info("SNS %u\n", smp_num_siblings);
+	pr_info("NPP %u MDPP %u\n", __amd_nodes_per_pkg, __max_die_per_package);
 }
 
 static bool detect_null_seg_behavior(void)
--- a/arch/x86/kernel/cpu/topology_amd.c
+++ b/arch/x86/kernel/cpu/topology_amd.c
@@ -29,7 +29,17 @@ static bool parse_8000_0008(struct topo_
 	if (!sft)
 		sft = get_count_order(ecx.cpu_nthreads + 1);
 
-	topology_set_dom(tscan, TOPO_SMT_DOMAIN, sft, ecx.cpu_nthreads + 1);
+	/*
+	 * cpu_nthreads describes the number of threads in the package
+	 * sft is the number of APIC ID bits per package
+	 *
+	 * As the number of actual threads per core is not described in
+	 * this leaf, just set the CORE domain shift and let the later
+	 * parsers set SMT shift. Assume one thread per core by default
+	 * which is correct if there are no other CPUID leafs to parse.
+	 */
+	topology_update_dom(tscan, TOPO_SMT_DOMAIN, 0, 1);
+	topology_set_dom(tscan, TOPO_CORE_DOMAIN, sft, ecx.cpu_nthreads + 1);
 	return true;
 }
 
@@ -73,12 +83,14 @@ static bool parse_8000_001e(struct topo_
 	tscan->c->topo.initial_apicid = leaf.ext_apic_id;
 
 	/*
-	 * If leaf 0xb is available, then SMT shift is set already. If not
-	 * take it from ecx.threads_per_core and use topo_update_dom() -
-	 * topology_set_dom() would propagate and overwrite the already
-	 * propagated CORE level.
+	 * If leaf 0xb is available, then the domain shifts are set
+	 * already and nothing to do here.
 	 */
 	if (!has_0xb) {
+		/*
+		 * Leaf 0x80000008 set the CORE domain shift already.
+		 * Update the SMT domain, but do not propagate it.
+		 */
 		unsigned int nthreads = leaf.core_nthreads + 1;
 
 		topology_update_dom(tscan, TOPO_SMT_DOMAIN, get_count_order(nthreads), nthreads);
--- a/arch/x86/kernel/cpu/topology_common.c
+++ b/arch/x86/kernel/cpu/topology_common.c
@@ -133,6 +133,10 @@ static void parse_topology(struct topo_s
 	tscan->ebx1_nproc_shift = get_count_order(ebx.nproc);
 
 	switch (c->x86_vendor) {
+	case X86_VENDOR_AMD:
+		if (IS_ENABLED(CONFIG_CPU_SUP_AMD))
+			cpu_parse_topology_amd(tscan);
+		break;
 	case X86_VENDOR_CENTAUR:
 	case X86_VENDOR_ZHAOXIN:
 		parse_legacy(tscan);
@@ -162,6 +166,9 @@ static void topo_set_ids(struct topo_sca
 
 	if (c->x86_vendor == X86_VENDOR_AMD)
 		cpu_topology_fixup_amd(tscan);
+
+	pr_info("pkg %u die %u core %u nid %u\n", c->topo.pkg_id, c->topo.die_id,
+		c->topo.core_id, c->topo.amd_node_id);
 }
 
 static void topo_set_max_cores(struct topo_scan *tscan)
@@ -175,6 +182,7 @@ static void topo_set_max_cores(struct to
 	 */
 	tscan->c->x86_max_cores = tscan->dom_ncpus[TOPO_DIEGRP_DOMAIN] >>
 		x86_topo_system.dom_shifts[TOPO_SMT_DOMAIN];
+	pr_info("Max cores: %u\n", tscan->c->x86_max_cores);
 }
 
 void cpu_parse_topology(struct cpuinfo_x86 *c)
@@ -215,20 +223,26 @@ void __init cpu_init_topology(struct cpu
 
 	parse_topology(&tscan, true);
 
-	if (!topo_is_converted(c))
-		return;
-
 	/* Copy the shift values and calculate the unit sizes. */
 	memcpy(x86_topo_system.dom_shifts, tscan.dom_shifts, sizeof(x86_topo_system.dom_shifts));
 
 	dom = TOPO_SMT_DOMAIN;
 	x86_topo_system.dom_size[dom] = 1U << x86_topo_system.dom_shifts[dom];
+	pr_info("Dom %u Sft: %u Sz: %u\n", dom, x86_topo_system.dom_size[dom],
+		x86_topo_system.dom_shifts[dom]);
 
 	for (dom++; dom < TOPO_MAX_DOMAIN; dom++) {
 		sft = x86_topo_system.dom_shifts[dom] - x86_topo_system.dom_shifts[dom - 1];
 		x86_topo_system.dom_size[dom] = 1U << sft;
+		pr_info("Dom %u Sft: %u Sz: %u\n", dom, x86_topo_system.dom_size[dom],
+			x86_topo_system.dom_shifts[dom]);
 	}
 
+	pr_info("NPP %u\n", tscan.amd_nodes_per_pkg);
+
+	if (!topo_is_converted(c))
+		return;
+
 	topo_set_ids(&tscan);
 	topo_set_max_cores(&tscan);
 
@@ -238,6 +252,7 @@ void __init cpu_init_topology(struct cpu
 	 * changes further down the road to get it right during early boot.
 	 */
 	smp_num_siblings = tscan.dom_ncpus[TOPO_SMT_DOMAIN];
+	pr_info("SNS %u\n", smp_num_siblings);
 
 	/*
 	 * Neither it's clear whether there are as many dies as the APIC
@@ -252,4 +267,6 @@ void __init cpu_init_topology(struct cpu
 	 */
 	if (c->x86_vendor == X86_VENDOR_AMD || c->x86_vendor == X86_VENDOR_HYGON)
 		__amd_nodes_per_pkg = __max_die_per_package = tscan.amd_nodes_per_pkg;
+
+	pr_info("NPP %u MDPP %u\n", __amd_nodes_per_pkg, __max_die_per_package);
 }

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Laura Nao]

Hi Thomas,

On 4/10/24 15:57, Thomas Gleixner wrote:
> Laura!
> 
> On Wed, Apr 10 2024 at 10:15, Laura Nao wrote:
>> On 4/9/24 14:25, Thomas Gleixner wrote:
>>> Can you please replace that patch with the one below?
>>
>> So, with this patch applied on top of ace278e7eca6 the kernel doesn't
>> boot anymore - reference test job:
>> https://lava.collabora.dev/scheduler/job/13324010
>>
>> I see the only change between the second and third patch you provided,
>> besides the debug prints, is:
>>
>> -	if (!topo_is_converted(c))
>> -		return;
>> -
> 
> Right. So this limits the area to search significantly.
> 
>> Printing the debug information without this probably doesn't really help,
>> but just in case it's useful: I tried excluding the change above from the
>> patch while leaving everything else unchanged - reference test job:
>> https://lava.collabora.dev/scheduler/job/13324298 (also pasted the
>> kernel log here for easier consultation:
>> https://pastebin.com/raw/TQBDvCah)
>>
>> Hope this helps,
> 
> It does. Good idea!
> 
> I just moved the exit check a bit so we should see the scan info. That
> should tell me what goes south.
>

Here's the full kernel log with the latest patch applied: 
https://pastebin.com/raw/r2CkP396

Reference test job: https://lava.collabora.dev/scheduler/job/13328709

Thanks!

Laura

Re: [REGRESSION] mainline boot regression on AMD Stoney Ridge Chromebooks

[Thomas Gleixner]

Laura!

On Wed, Apr 10 2024 at 18:11, Laura Nao wrote:
> On 4/10/24 15:57, Thomas Gleixner wrote:
>> I just moved the exit check a bit so we should see the scan info. That
>> should tell me what goes south.
>
> Here's the full kernel log with the latest patch applied: 
> https://pastebin.com/raw/r2CkP396

Ok. Now I can see it.

	pr_info("NPP %u\n", tscan.amd_nodes_per_pkg);

The output is:

<6>[    0.000000] NPP 3

The original code then reads out 1, which is what's expected on that
machine.

Now that bogus value '3' is used later in

    a %= b / 3

as b == 2, the division result is zero, which makes the %= operation
crash with a division by zero. As there are no handlers yet, nothing to
see.

The problem causing the bogus readout sits between my keyboard and my
chair.

I'll send out the fixes with proper change logs later tonight.

Thanks for all your help!

       tglx

[patch 0/2] x86/cpu/amd: Fixup the topology rework fallout

[Thomas Gleixner]

Testing at Collabora unearthed two issues in the new AMD topology parser
code:

  1) The CPUID 0x80000008 parser initializes the wrong topology domain
     level.

  2) The NODEID_MSR parser uses bitfields in a union wrongly which results
     in reading out the wrong value and finally in a division by zero.

Many thanks to Laura for helping to debug this issue.

       tglx

[patch 1/2] x86/cpu/amd: Make the CPUID 0x80000008 parser correct

[Thomas Gleixner]

CPUID 0x80000008 ECX.cpu_nthreads describes the number of threads in the
package. The parser uses this value to initialize the SMT domain level.

That's wrong because cpu_nthreads does not describe the number of threads
per physical core. So this needs to set the CORE domain level and let the
later parsers set the SMT shift if available.

Preset the SMT domain level with the assumption of one thread per core,
which is correct ifrt here are no other CPUID leafs to parse, and propagate
cpu_nthreads and the core level APIC bitwidth into the CORE domain.

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 |   24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

--- a/arch/x86/kernel/cpu/topology_amd.c
+++ b/arch/x86/kernel/cpu/topology_amd.c
@@ -29,11 +29,21 @@ static bool parse_8000_0008(struct topo_
 	if (!sft)
 		sft = get_count_order(ecx.cpu_nthreads + 1);
 
-	topology_set_dom(tscan, TOPO_SMT_DOMAIN, sft, ecx.cpu_nthreads + 1);
+	/*
+	 * cpu_nthreads describes the number of threads in the package
+	 * sft is the number of APIC ID bits per package
+	 *
+	 * As the number of actual threads per core is not described in
+	 * this leaf, just set the CORE domain shift and let the later
+	 * parsers set SMT shift. Assume one thread per core by default
+	 * which is correct if there are no other CPUID leafs to parse.
+	 */
+	topology_update_dom(tscan, TOPO_SMT_DOMAIN, 0, 1);
+	topology_set_dom(tscan, TOPO_CORE_DOMAIN, sft, ecx.cpu_nthreads + 1);
 	return true;
 }
 
-static void store_node(struct topo_scan *tscan, unsigned int nr_nodes, u16 node_id)
+static void store_node(struct topo_scan *tscan, u16 nr_nodes, u16 node_id)
 {
 	/*
 	 * Starting with Fam 17h the DIE domain could probably be used to
@@ -73,12 +83,14 @@ static bool parse_8000_001e(struct topo_
 	tscan->c->topo.initial_apicid = leaf.ext_apic_id;
 
 	/*
-	 * If leaf 0xb is available, then SMT shift is set already. If not
-	 * take it from ecx.threads_per_core and use topo_update_dom() -
-	 * topology_set_dom() would propagate and overwrite the already
-	 * propagated CORE level.
+	 * If leaf 0xb is available, then the domain shifts are set
+	 * already and nothing to do here.
 	 */
 	if (!has_0xb) {
+		/*
+		 * Leaf 0x80000008 set the CORE domain shift already.
+		 * Update the SMT domain, but do not propagate it.
+		 */
 		unsigned int nthreads = leaf.core_nthreads + 1;
 
 		topology_update_dom(tscan, TOPO_SMT_DOMAIN, get_count_order(nthreads), nthreads);

[tip: x86/urgent] x86/cpu/amd: Make the CPUID 0x80000008 parser correct

[tip-bot2 for Thomas Gleixner]

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

Commit-ID:     3a93fe9cdfa7bb45716aecf771be053a0ce2bdf0
Gitweb:        https://git.kernel.org/tip/3a93fe9cdfa7bb45716aecf771be053a0ce2bdf0
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Wed, 10 Apr 2024 21:45:27 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Thu, 11 Apr 2024 14:18:29 +02:00

x86/cpu/amd: Make the CPUID 0x80000008 parser correct

CPUID 0x80000008 ECX.cpu_nthreads describes the number of threads in the
package. The parser uses this value to initialize the SMT domain level.

That's wrong because cpu_nthreads does not describe the number of threads
per physical core. So this needs to set the CORE domain level and let the
later parsers set the SMT shift if available.

Preset the SMT domain level with the assumption of one thread per core,
which is correct ifrt here are no other CPUID leafs to parse, and propagate
cpu_nthreads and the core level APIC bitwidth into the CORE domain.

Fixes: f7fb3b2dd92c ("x86/cpu: Provide an AMD/HYGON specific topology parser")
Reported-by: "kernelci.org bot" <bot@kernelci.org>
Reported-by: Laura Nao <laura.nao@collabora.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Laura Nao <laura.nao@collabora.com>
Link: https://lore.kernel.org/r/20240410194311.535206450@linutronix.de

---
 arch/x86/kernel/cpu/topology_amd.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/topology_amd.c b/arch/x86/kernel/cpu/topology_amd.c
index 1a8b3ad..79a85a4 100644
--- a/arch/x86/kernel/cpu/topology_amd.c
+++ b/arch/x86/kernel/cpu/topology_amd.c
@@ -29,11 +29,21 @@ static bool parse_8000_0008(struct topo_scan *tscan)
 	if (!sft)
 		sft = get_count_order(ecx.cpu_nthreads + 1);
 
-	topology_set_dom(tscan, TOPO_SMT_DOMAIN, sft, ecx.cpu_nthreads + 1);
+	/*
+	 * cpu_nthreads describes the number of threads in the package
+	 * sft is the number of APIC ID bits per package
+	 *
+	 * As the number of actual threads per core is not described in
+	 * this leaf, just set the CORE domain shift and let the later
+	 * parsers set SMT shift. Assume one thread per core by default
+	 * which is correct if there are no other CPUID leafs to parse.
+	 */
+	topology_update_dom(tscan, TOPO_SMT_DOMAIN, 0, 1);
+	topology_set_dom(tscan, TOPO_CORE_DOMAIN, sft, ecx.cpu_nthreads + 1);
 	return true;
 }
 
-static void store_node(struct topo_scan *tscan, unsigned int nr_nodes, u16 node_id)
+static void store_node(struct topo_scan *tscan, u16 nr_nodes, u16 node_id)
 {
 	/*
 	 * Starting with Fam 17h the DIE domain could probably be used to
@@ -73,12 +83,14 @@ static bool parse_8000_001e(struct topo_scan *tscan, bool has_0xb)
 	tscan->c->topo.initial_apicid = leaf.ext_apic_id;
 
 	/*
-	 * If leaf 0xb is available, then SMT shift is set already. If not
-	 * take it from ecx.threads_per_core and use topo_update_dom() -
-	 * topology_set_dom() would propagate and overwrite the already
-	 * propagated CORE level.
+	 * If leaf 0xb is available, then the domain shifts are set
+	 * already and nothing to do here.
 	 */
 	if (!has_0xb) {
+		/*
+		 * Leaf 0x80000008 set the CORE domain shift already.
+		 * Update the SMT domain, but do not propagate it.
+		 */
 		unsigned int nthreads = leaf.core_nthreads + 1;
 
 		topology_update_dom(tscan, TOPO_SMT_DOMAIN, get_count_order(nthreads), nthreads);

[tip: x86/urgent] x86/cpu/amd: Make the CPUID 0x80000008 parser correct

[tip-bot2 for Thomas Gleixner]

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

Commit-ID:     1b3108f6898ef2e03973d65255182792e94e2240
Gitweb:        https://git.kernel.org/tip/1b3108f6898ef2e03973d65255182792e94e2240
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Wed, 10 Apr 2024 21:45:27 +02:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Fri, 12 Apr 2024 12:05:54 +02:00

x86/cpu/amd: Make the CPUID 0x80000008 parser correct

CPUID 0x80000008 ECX.cpu_nthreads describes the number of threads in the
package. The parser uses this value to initialize the SMT domain level.

That's wrong because cpu_nthreads does not describe the number of threads
per physical core. So this needs to set the CORE domain level and let the
later parsers set the SMT shift if available.

Preset the SMT domain level with the assumption of one thread per core,
which is correct ifrt here are no other CPUID leafs to parse, and propagate
cpu_nthreads and the core level APIC bitwidth into the CORE domain.

Fixes: f7fb3b2dd92c ("x86/cpu: Provide an AMD/HYGON specific topology parser")
Reported-by: "kernelci.org bot" <bot@kernelci.org>
Reported-by: Laura Nao <laura.nao@collabora.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Laura Nao <laura.nao@collabora.com>
Link: https://lore.kernel.org/r/20240410194311.535206450@linutronix.de

---
 arch/x86/kernel/cpu/topology_amd.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/topology_amd.c b/arch/x86/kernel/cpu/topology_amd.c
index 1a8b3ad..79a85a4 100644
--- a/arch/x86/kernel/cpu/topology_amd.c
+++ b/arch/x86/kernel/cpu/topology_amd.c
@@ -29,11 +29,21 @@ static bool parse_8000_0008(struct topo_scan *tscan)
 	if (!sft)
 		sft = get_count_order(ecx.cpu_nthreads + 1);
 
-	topology_set_dom(tscan, TOPO_SMT_DOMAIN, sft, ecx.cpu_nthreads + 1);
+	/*
+	 * cpu_nthreads describes the number of threads in the package
+	 * sft is the number of APIC ID bits per package
+	 *
+	 * As the number of actual threads per core is not described in
+	 * this leaf, just set the CORE domain shift and let the later
+	 * parsers set SMT shift. Assume one thread per core by default
+	 * which is correct if there are no other CPUID leafs to parse.
+	 */
+	topology_update_dom(tscan, TOPO_SMT_DOMAIN, 0, 1);
+	topology_set_dom(tscan, TOPO_CORE_DOMAIN, sft, ecx.cpu_nthreads + 1);
 	return true;
 }
 
-static void store_node(struct topo_scan *tscan, unsigned int nr_nodes, u16 node_id)
+static void store_node(struct topo_scan *tscan, u16 nr_nodes, u16 node_id)
 {
 	/*
 	 * Starting with Fam 17h the DIE domain could probably be used to
@@ -73,12 +83,14 @@ static bool parse_8000_001e(struct topo_scan *tscan, bool has_0xb)
 	tscan->c->topo.initial_apicid = leaf.ext_apic_id;
 
 	/*
-	 * If leaf 0xb is available, then SMT shift is set already. If not
-	 * take it from ecx.threads_per_core and use topo_update_dom() -
-	 * topology_set_dom() would propagate and overwrite the already
-	 * propagated CORE level.
+	 * If leaf 0xb is available, then the domain shifts are set
+	 * already and nothing to do here.
 	 */
 	if (!has_0xb) {
+		/*
+		 * Leaf 0x80000008 set the CORE domain shift already.
+		 * Update the SMT domain, but do not propagate it.
+		 */
 		unsigned int nthreads = leaf.core_nthreads + 1;
 
 		topology_update_dom(tscan, TOPO_SMT_DOMAIN, get_count_order(nthreads), nthreads);

[patch 2/2] x86/cpu/amd: Make the NODEID_MSR union actually work

[Thomas Gleixner]

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))

[tip: x86/urgent] x86/cpu/amd: Make the NODEID_MSR union actually work

[tip-bot2 for Thomas Gleixner]

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

Commit-ID:     898e4996e76053bfa0f578629ec6b35baff4224b
Gitweb:        https://git.kernel.org/tip/898e4996e76053bfa0f578629ec6b35baff4224b
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Wed, 10 Apr 2024 21:45:28 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Thu, 11 Apr 2024 14:18:29 +02:00

x86/cpu/amd: Make the NODEID_MSR union actually work

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: "kernelci.org bot" <bot@kernelci.org>
Reported-by: Laura Nao <laura.nao@collabora.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Laura Nao <laura.nao@collabora.com>
Link: https://lore.kernel.org/r/20240410194311.596282919@linutronix.de
Closes: https://lore.kernel.org/all/20240322175210.124416-1-laura.nao@collabora.com/
---
 arch/x86/kernel/cpu/topology_amd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/topology_amd.c b/arch/x86/kernel/cpu/topology_amd.c
index 79a85a4..7f999ae 100644
--- 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_scan *tscan, bool has_0xb)
 
 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))

[tip: x86/urgent] x86/cpu/amd: Make the NODEID_MSR union actually work

[tip-bot2 for Thomas Gleixner]

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

Commit-ID:     c064b536a8f9ab7c8e204da8f5a22f7420d0b56c
Gitweb:        https://git.kernel.org/tip/c064b536a8f9ab7c8e204da8f5a22f7420d0b56c
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Wed, 10 Apr 2024 21:45:28 +02:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Fri, 12 Apr 2024 12:05:54 +02:00

x86/cpu/amd: Make the NODEID_MSR union actually work

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: "kernelci.org bot" <bot@kernelci.org>
Reported-by: Laura Nao <laura.nao@collabora.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Laura Nao <laura.nao@collabora.com>
Link: https://lore.kernel.org/r/20240410194311.596282919@linutronix.de
Closes: https://lore.kernel.org/all/20240322175210.124416-1-laura.nao@collabora.com/
---
 arch/x86/kernel/cpu/topology_amd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/topology_amd.c b/arch/x86/kernel/cpu/topology_amd.c
index 79a85a4..7f999ae 100644
--- 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_scan *tscan, bool has_0xb)
 
 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))

Re: [patch 0/2] x86/cpu/amd: Fixup the topology rework fallout

[Laura Nao]

Hi Thomas,

On 4/10/24 21:45, Thomas Gleixner wrote:
> Testing at Collabora unearthed two issues in the new AMD topology parser
> code:
> 
>    1) The CPUID 0x80000008 parser initializes the wrong topology domain
>       level.
> 
>    2) The NODEID_MSR parser uses bitfields in a union wrongly which results
>       in reading out the wrong value and finally in a division by zero.
> 
> Many thanks to Laura for helping to debug this issue.
> 
>         tglx
> 
> 

Thanks a lot for investigating and solving the issue!

I confirm that with this series applied the kernel boots correctly on 
all three AMD Stoney Ridge Chromebooks that were affected by the 
regression.

I tested the patches on top of c749ce39 (culprit commit identified by
the bisection) - reference test job:
https://lava.collabora.dev/scheduler/job/13339645

The series doesn't apply directly to next, but I manually applied the
changes on top of next-20240411 and can confirm the kernel boots
correctly with this revision too - reference test job:
https://lava.collabora.dev/scheduler/job/13340321

The regression was originally reported by KernelCI, so:

Reported-by: "kernelci.org bot" <bot@kernelci.org>
Tested-by: Laura Nao <laura.nao@collabora.com>

I'll make sure to update the Regzbot tag when the series is merged.

Best,

Laura

Re: [patch 0/2] x86/cpu/amd: Fixup the topology rework fallout

[Linux regression tracking (Thorsten Leemhuis)]

On 11.04.24 13:27, Laura Nao wrote:
> 
> On 4/10/24 21:45, Thomas Gleixner wrote:
>> Testing at Collabora unearthed two issues in the new AMD topology parser
>> code:
>>
>>    1) The CPUID 0x80000008 parser initializes the wrong topology domain
>>       level.
>>
>>    2) The NODEID_MSR parser uses bitfields in a union wrongly which results
>>       in reading out the wrong value and finally in a division by zero.
>>
>> Many thanks to Laura for helping to debug this issue.
>>
>>         tglx
>>
>>
> 
> Thanks a lot for investigating and solving the issue!> [...]
> 
> The regression was originally reported by KernelCI, so:
> 
> Reported-by: "kernelci.org bot" <bot@kernelci.org>
> Tested-by: Laura Nao <laura.nao@collabora.com>
> 
> I'll make sure to update the Regzbot tag when the series is merged.

No need to wait, we can do that now:

#regzbot fix: x86/cpu/amd: Make the NODEID_MSR union actually work

But ideally Thomas would add Link: or Closes: tag to the patch
description (e.g.

 Closes:
https://lore.kernel.org/all/20240322175210.124416-1-laura.nao@collabora.com/

) just like Linus asked him to do a while ago already[1], as then this
would not be necessary at all. ;) (SCNR)

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

[1]
https://lore.kernel.org/all/CAHk-=wjMmSZzMJ3Xnskdg4+GGz=5p5p+GSYyFBTh0f-DgvdBWg@mail.gmail.com/

Re: [patch 0/2] x86/cpu/amd: Fixup the topology rework fallout

[Thomas Gleixner]

On Thu, Apr 11 2024 at 13:37, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 11.04.24 13:27, Laura Nao wrote:
> No need to wait, we can do that now:
>
> #regzbot fix: x86/cpu/amd: Make the NODEID_MSR union actually work
>
> But ideally Thomas would add Link: or Closes: tag to the patch
> description (e.g.
>
>  Closes:
> https://lore.kernel.org/all/20240322175210.124416-1-laura.nao@collabora.com/
>
> ) just like Linus asked him to do a while ago already[1], as then this
> would not be necessary at all. ;) (SCNR)

Will do when applying them and I try to remember that Closes thing, but
you know at my age ....

Thanks,

        tglx