[v4.6-rc1 Regression] x86/topology: Create logical package id

[v4.6-rc1 Regression] x86/topology: Create logical package id

[Joseph Salisbury]

Hi Thomas,

A kernel bug report was opened against Ubuntu [0].  After a kernel
bisect, it was found that reverting the following commit resolved this bug:

commit 1f12e32f4cd5243ae46d8b933181be0d022c6793
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Mon Feb 22 22:19:15 2016 +0000

    x86/topology: Create logical package id

To build successfully with this commit reverted, I also had to revert
commits: e7ee3e8,2d4de83,87f01cc and 33c3cc7.

The regression was introduced as of v4.6-rc1.
    
I was hoping to get your feedback, since you are the patch author.  Do
you think gathering any additional data will help diagnose this issue,
or would it be best to submit a revert request?
    
    
Thanks,
    
Joe

[0] http://pad.lv/1573231

Re: [v4.6-rc1 Regression] x86/topology: Create logical package id

[Thomas Gleixner]

On Fri, 6 May 2016, Joseph Salisbury wrote:
> A kernel bug report was opened against Ubuntu [0].  After a kernel
> bisect, it was found that reverting the following commit resolved this bug:
> 
> commit 1f12e32f4cd5243ae46d8b933181be0d022c6793
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date:   Mon Feb 22 22:19:15 2016 +0000
> 
>     x86/topology: Create logical package id
> 
> To build successfully with this commit reverted, I also had to revert
> commits: e7ee3e8,2d4de83,87f01cc and 33c3cc7.
> 
> The regression was introduced as of v4.6-rc1.
>     
> I was hoping to get your feedback, since you are the patch author.  Do
> you think gathering any additional data will help diagnose this issue,
> or would it be best to submit a revert request?

Yuck. That dies with a divide error. And that looks like XEN is supplying crap
data in the CPUID.

Does the patch below cure the issue?

Thanks,

        tglx

8<---------------

--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -332,6 +332,11 @@ static void __init smp_init_package_map(
 	 * primary cores.
 	 */
 	ncpus = boot_cpu_data.x86_max_cores;
+	if (!ncpus) {
+		pr_warn("x86_max_cores == zero !?!?");
+		ncpus = 1;
+	}
+
 	__max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
 
 	/*

Re: [v4.6-rc1 Regression] x86/topology: Create logical package id

[Boris Ostrovsky]

On 05/06/2016 02:48 PM, Thomas Gleixner wrote:
> On Fri, 6 May 2016, Joseph Salisbury wrote:
>> A kernel bug report was opened against Ubuntu [0].  After a kernel
>> bisect, it was found that reverting the following commit resolved this bug:
>>
>> commit 1f12e32f4cd5243ae46d8b933181be0d022c6793
>> Author: Thomas Gleixner <tglx@linutronix.de>
>> Date:   Mon Feb 22 22:19:15 2016 +0000
>>
>>     x86/topology: Create logical package id
>>
>> To build successfully with this commit reverted, I also had to revert
>> commits: e7ee3e8,2d4de83,87f01cc and 33c3cc7.
>>
>> The regression was introduced as of v4.6-rc1.
>>     
>> I was hoping to get your feedback, since you are the patch author.  Do
>> you think gathering any additional data will help diagnose this issue,
>> or would it be best to submit a revert request?
> Yuck. That dies with a divide error. And that looks like XEN is supplying crap
> data in the CPUID.

Joe, do you have

ed6069b xen/apic: Provide Xen-specific version of cpu_present_to_apicid
APIC op

-boris


>
> Does the patch below cure the issue?
>
> Thanks,
>
>         tglx
>
> 8<---------------
>
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -332,6 +332,11 @@ static void __init smp_init_package_map(
>  	 * primary cores.
>  	 */
>  	ncpus = boot_cpu_data.x86_max_cores;
> +	if (!ncpus) {
> +		pr_warn("x86_max_cores == zero !?!?");
> +		ncpus = 1;
> +	}
> +
>  	__max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
>  
>  	/*

Re: [v4.6-rc1 Regression] x86/topology: Create logical package id

[Joseph Salisbury]

On 05/06/2016 03:13 PM, Boris Ostrovsky wrote:
> On 05/06/2016 02:48 PM, Thomas Gleixner wrote:
>> On Fri, 6 May 2016, Joseph Salisbury wrote:
>>> A kernel bug report was opened against Ubuntu [0].  After a kernel
>>> bisect, it was found that reverting the following commit resolved this bug:
>>>
>>> commit 1f12e32f4cd5243ae46d8b933181be0d022c6793
>>> Author: Thomas Gleixner <tglx@linutronix.de>
>>> Date:   Mon Feb 22 22:19:15 2016 +0000
>>>
>>>     x86/topology: Create logical package id
>>>
>>> To build successfully with this commit reverted, I also had to revert
>>> commits: e7ee3e8,2d4de83,87f01cc and 33c3cc7.
>>>
>>> The regression was introduced as of v4.6-rc1.
>>>     
>>> I was hoping to get your feedback, since you are the patch author.  Do
>>> you think gathering any additional data will help diagnose this issue,
>>> or would it be best to submit a revert request?
>> Yuck. That dies with a divide error. And that looks like XEN is supplying crap
>> data in the CPUID.
> Joe, do you have
>
> ed6069b xen/apic: Provide Xen-specific version of cpu_present_to_apicid
> APIC op
>
> -boris
Yes the commit is in the 4.4 based Ubuntu kernel.  This bug also happens
with the vanilla 4.6-rc5 kernel, which also has that commit.


>
>
>> Does the patch below cure the issue?
>>
>> Thanks,
>>
>>         tglx
>>
>> 8<---------------
>>
>> --- a/arch/x86/kernel/smpboot.c
>> +++ b/arch/x86/kernel/smpboot.c
>> @@ -332,6 +332,11 @@ static void __init smp_init_package_map(
>>  	 * primary cores.
>>  	 */
>>  	ncpus = boot_cpu_data.x86_max_cores;
>> +	if (!ncpus) {
>> +		pr_warn("x86_max_cores == zero !?!?");
>> +		ncpus = 1;
>> +	}
>> +
>>  	__max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
>>  
>>  	/*
>

Re: [v4.6-rc1 Regression] x86/topology: Create logical package id

[Boris Ostrovsky]

On 05/06/2016 03:38 PM, Joseph Salisbury wrote:
> On 05/06/2016 03:13 PM, Boris Ostrovsky wrote:
>> On 05/06/2016 02:48 PM, Thomas Gleixner wrote:
>>>
>>> Yuck. That dies with a divide error. And that looks like XEN is supplying crap
>>> data in the CPUID.
>> Joe, do you have
>>
>> ed6069b xen/apic: Provide Xen-specific version of cpu_present_to_apicid
>> APIC op
>>
>> -boris
> Yes the commit is in the 4.4 based Ubuntu kernel.  This bug also happens
> with the vanilla 4.6-rc5 kernel, which also has that commit.


Can you post guest's cpuid -1 -r ? (I guess after you verify Thomas' patch)

Thanks.
-boris

Re: [v4.6-rc1 Regression] x86/topology: Create logical package id

[Joseph Salisbury]

On 05/06/2016 04:46 PM, Boris Ostrovsky wrote:
> On 05/06/2016 03:38 PM, Joseph Salisbury wrote:
>> On 05/06/2016 03:13 PM, Boris Ostrovsky wrote:
>>> On 05/06/2016 02:48 PM, Thomas Gleixner wrote:
>>>> Yuck. That dies with a divide error. And that looks like XEN is supplying crap
>>>> data in the CPUID.
>>> Joe, do you have
>>>
>>> ed6069b xen/apic: Provide Xen-specific version of cpu_present_to_apicid
>>> APIC op
>>>
>>> -boris
>> Yes the commit is in the 4.4 based Ubuntu kernel.  This bug also happens
>> with the vanilla 4.6-rc5 kernel, which also has that commit.
>
> Can you post guest's cpuid -1 -r ? (I guess after you verify Thomas' patch)
>
> Thanks.
> -boris
>
>
>
Thomas' patch does resolve the bug.  The cpuid info can be seen here:
https://launchpadlibrarian.net/258234267/cpuid_full.txt

Thanks,

Joe

Re: [v4.6-rc1 Regression] x86/topology: Create logical package id

[Boris Ostrovsky]

On 05/06/2016 04:51 PM, Joseph Salisbury wrote:
> Thomas' patch does resolve the bug.  The cpuid info can be seen here:
> https://launchpadlibrarian.net/258234267/cpuid_full.txt

Any chance you could post it raw (cpuid -1 -r)?

Thanks.
-boris

Re: [v4.6-rc1 Regression] x86/topology: Create logical package id

[Joseph Salisbury]

On 05/06/2016 02:48 PM, Thomas Gleixner wrote:
> On Fri, 6 May 2016, Joseph Salisbury wrote:
>> A kernel bug report was opened against Ubuntu [0].  After a kernel
>> bisect, it was found that reverting the following commit resolved this bug:
>>
>> commit 1f12e32f4cd5243ae46d8b933181be0d022c6793
>> Author: Thomas Gleixner <tglx@linutronix.de>
>> Date:   Mon Feb 22 22:19:15 2016 +0000
>>
>>     x86/topology: Create logical package id
>>
>> To build successfully with this commit reverted, I also had to revert
>> commits: e7ee3e8,2d4de83,87f01cc and 33c3cc7.
>>
>> The regression was introduced as of v4.6-rc1.
>>     
>> I was hoping to get your feedback, since you are the patch author.  Do
>> you think gathering any additional data will help diagnose this issue,
>> or would it be best to submit a revert request?
> Yuck. That dies with a divide error. And that looks like XEN is supplying crap
> data in the CPUID.
>
> Does the patch below cure the issue?
>
> Thanks,
>
>         tglx
>
> 8<---------------
>
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -332,6 +332,11 @@ static void __init smp_init_package_map(
>  	 * primary cores.
>  	 */
>  	ncpus = boot_cpu_data.x86_max_cores;
> +	if (!ncpus) {
> +		pr_warn("x86_max_cores == zero !?!?");
> +		ncpus = 1;
> +	}
> +
>  	__max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
>  
>  	/*
I'll have this patch tested and report back.

Thanks,

Joe

Re: [v4.6-rc1 Regression] x86/topology: Create logical package id

[Joseph Salisbury]

On 05/06/2016 03:38 PM, Joseph Salisbury wrote:
> On 05/06/2016 02:48 PM, Thomas Gleixner wrote:
>> On Fri, 6 May 2016, Joseph Salisbury wrote:
>>> A kernel bug report was opened against Ubuntu [0].  After a kernel
>>> bisect, it was found that reverting the following commit resolved this bug:
>>>
>>> commit 1f12e32f4cd5243ae46d8b933181be0d022c6793
>>> Author: Thomas Gleixner <tglx@linutronix.de>
>>> Date:   Mon Feb 22 22:19:15 2016 +0000
>>>
>>>     x86/topology: Create logical package id
>>>
>>> To build successfully with this commit reverted, I also had to revert
>>> commits: e7ee3e8,2d4de83,87f01cc and 33c3cc7.
>>>
>>> The regression was introduced as of v4.6-rc1.
>>>     
>>> I was hoping to get your feedback, since you are the patch author.  Do
>>> you think gathering any additional data will help diagnose this issue,
>>> or would it be best to submit a revert request?
>> Yuck. That dies with a divide error. And that looks like XEN is supplying crap
>> data in the CPUID.
>>
>> Does the patch below cure the issue?
>>
>> Thanks,
>>
>>         tglx
>>
>> 8<---------------
>>
>> --- a/arch/x86/kernel/smpboot.c
>> +++ b/arch/x86/kernel/smpboot.c
>> @@ -332,6 +332,11 @@ static void __init smp_init_package_map(
>>  	 * primary cores.
>>  	 */
>>  	ncpus = boot_cpu_data.x86_max_cores;
>> +	if (!ncpus) {
>> +		pr_warn("x86_max_cores == zero !?!?");
>> +		ncpus = 1;
>> +	}
>> +
>>  	__max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
>>  
>>  	/*
> I'll have this patch tested and report back.
>
> Thanks,
>
> Joe
Yes, your patch does in fact fix the bug.  Would you like any additional
information regarding the bug?

Re: [v4.6-rc1 Regression] x86/topology: Create logical package id

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> > I was hoping to get your feedback, since you are the patch author.  Do
> > you think gathering any additional data will help diagnose this issue,
> > or would it be best to submit a revert request?
> 
> Yuck. That dies with a divide error. And that looks like XEN is supplying crap
> data in the CPUID.
> 
> Does the patch below cure the issue?
> 
> Thanks,
> 
>         tglx
> 
> 8<---------------
> 
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -332,6 +332,11 @@ static void __init smp_init_package_map(
>  	 * primary cores.
>  	 */
>  	ncpus = boot_cpu_data.x86_max_cores;
> +	if (!ncpus) {
> +		pr_warn("x86_max_cores == zero !?!?");
> +		ncpus = 1;
> +	}
> +

Even if the underlying Xen bug is fixed, I think we should add this protective 
measure to defend against future CPUID mishaps.

Thanks,

	Ingo

[tip:x86/urgent] x86/topology: Handle CPUID bogosity gracefully

[tip-bot for Thomas Gleixner]

Commit-ID:  56402d63eefe22179f7311a51ff2094731420406
Gitweb:     http://git.kernel.org/tip/56402d63eefe22179f7311a51ff2094731420406
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Fri, 6 May 2016 20:48:16 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sat, 7 May 2016 10:06:55 +0200

x86/topology: Handle CPUID bogosity gracefully

Joseph reported that a XEN guest dies with a division by 0 in the package
topology setup code. This happens if cpu_info.x86_max_cores is zero.

Handle that case and emit a warning. This does not fix the underlying XEN bug,
but makes the code more robust.

Reported-and-tested-by: Joseph Salisbury <joseph.salisbury@canonical.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Link: http://lkml.kernel.org/r/alpine.DEB.2.11.1605062046270.3540@nanos
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/kernel/smpboot.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index a2065d3..0e4329e 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -332,6 +332,11 @@ static void __init smp_init_package_map(void)
 	 * primary cores.
 	 */
 	ncpus = boot_cpu_data.x86_max_cores;
+	if (!ncpus) {
+		pr_warn("x86_max_cores == zero !?!?");
+		ncpus = 1;
+	}
+
 	__max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
 
 	/*