Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1520424 > unrolled thread

Re: [PATCH] x86/cpuid: Deal with broken firmware once more

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] x86/cpuid: Deal with broken firmware once more "M. Vefa Bicakci" <m.v.b@runbox.com> - 2016-11-12 23:10 +0100
    Re: [PATCH] x86/cpuid: Deal with broken firmware once more Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-11-13 19:10 +0100
      Re: [PATCH] x86/cpuid: Deal with broken firmware once more "M. Vefa Bicakci" <m.v.b@runbox.com> - 2016-11-14 00:50 +0100
        Re: [PATCH] x86/cpuid: Deal with broken firmware once more Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-11-15 02:30 +0100
          Re: [PATCH] x86/cpuid: Deal with broken firmware once more Thomas Gleixner <tglx@linutronix.de> - 2016-11-18 12:20 +0100
            Re: [PATCH] x86/cpuid: Deal with broken firmware once more Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-11-18 15:30 +0100

#1520424 — Re: [PATCH] x86/cpuid: Deal with broken firmware once more

On 11/10/2016 06:31 PM, Boris Ostrovsky wrote:
> On 11/10/2016 10:05 AM, Charles (Chas) Williams wrote:
>>
>>
>> On 11/10/2016 09:02 AM, Boris Ostrovsky wrote:
>>> On 11/10/2016 06:13 AM, Thomas Gleixner wrote:
>>>> On Thu, 10 Nov 2016, M. Vefa Bicakci wrote:
>>>>
>>>>> I have found that your patch unfortunately does not improve the
>>>>> situation
>>>>> for me. Here is an excerpt obtained from the dmesg of a kernel
>>>>> compiled
>>>>> with this patch *as well as* Sebastian's patch:
>>>>> [    0.002561] CPU: Physical Processor ID: 0
>>>>> [    0.002566] CPU: Processor Core ID: 0
>>>>> [    0.002572] [Firmware Bug]: CPU0: APIC id mismatch. Firmware:
>>>>> ffff CPUID: 2
>>>> So apic->cpu_present_to_apicid() gives us a completely bogus APIC id
>>>> which
>>>> translates to a bogus package id. And looking at the XEN code:
>>>>
>>>>    xen_pv_apic.cpu_present_to_apicid = xen_cpu_present_to_apicid,
>>>>
>>>> and xen_cpu_present_to_apicid does:
>>>>
>>>> static int xen_cpu_present_to_apicid(int cpu)
>>>> {
>>>>         if (cpu_present(cpu))
>>>>                 return xen_get_apic_id(xen_apic_read(APIC_ID));
>>>>         else
>>>>                 return BAD_APICID;
>>>> }
>>>>
>>>> So independent of which present CPU we query we get just some random
>>>> information, in the above case we get BAD_APICID from
>>>> xen_apic_read() not
>>>> from the else path as this CPU _IS_ present.
>>>>
>>>> What's so wrong with storing the fricking firmware supplied APICid as
>>>> everybody else does and report it back when queried?
>>>
>>> By firmware you mean ACPI? It is most likely not available to PV guests.
>>> How about returning cpu_data(cpu).initial_apicid?
>>>
>>> And what was the original problem?
>>
>> The original issue I found was that VMware was returning a different set
>> of APIC id's in the ACPI tables than what it advertised on the CPU's.
>>
>> http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1266716.html
> 
> For Xen, we recently added a6a198bc60e6 ("xen/x86: Update topology map
> for PV VCPUs") to at least temporarily work around some topology map
> problems that PV guests have with RAPL (which I think is what Vefa's
> problem was).

Hello Boris,

(Sorry for the delay!)

It appears that the problem is a bit different compared to the one
corrected by a6a198bc60e6, because my kernel tree -- based on 4.8.6 --
already includes the -stable backport of that commit, i.e.
  88540ad0820ddfb05626e0136c0e5a79cea85fd1

The patch I included in my previous e-mail (dated 2016-11-10) corrects
root cause of the issue I am having with 4.8.6. Sebastian's original
patch adding error checking to the RAPL module prevents the RAPL module
from causing a kernel oops without my patch.

The issue I am experiencing is caused by the boot-up code in the
'init_apic_mappings' function switching the APIC ops structure from
Xen's structure to a no-op structure by calling the 'apic_disable'
function. Please let me know if I can clarify or elaborate.

For the record, using 4.8.7 without my correction patch patch does not
rectify the issue at hand. 4.8.7 changes the call site of the
'init_apic_mapping' function, so I had thought that it could be helpful.

Thank you,

Vefa

[toc] | [next] | [standalone]

#1520633

On 11/12/2016 05:05 PM, M. Vefa Bicakci wrote:
> On 11/10/2016 06:31 PM, Boris Ostrovsky wrote:
>> On 11/10/2016 10:05 AM, Charles (Chas) Williams wrote:
>>>
>>> On 11/10/2016 09:02 AM, Boris Ostrovsky wrote:
>>>> On 11/10/2016 06:13 AM, Thomas Gleixner wrote:
>>>>> On Thu, 10 Nov 2016, M. Vefa Bicakci wrote:
>>>>>
>>>>>> I have found that your patch unfortunately does not improve the
>>>>>> situation
>>>>>> for me. Here is an excerpt obtained from the dmesg of a kernel
>>>>>> compiled
>>>>>> with this patch *as well as* Sebastian's patch:
>>>>>> [    0.002561] CPU: Physical Processor ID: 0
>>>>>> [    0.002566] CPU: Processor Core ID: 0
>>>>>> [    0.002572] [Firmware Bug]: CPU0: APIC id mismatch. Firmware:
>>>>>> ffff CPUID: 2
>>>>> So apic->cpu_present_to_apicid() gives us a completely bogus APIC id
>>>>> which
>>>>> translates to a bogus package id. And looking at the XEN code:
>>>>>
>>>>>    xen_pv_apic.cpu_present_to_apicid = xen_cpu_present_to_apicid,
>>>>>
>>>>> and xen_cpu_present_to_apicid does:
>>>>>
>>>>> static int xen_cpu_present_to_apicid(int cpu)
>>>>> {
>>>>>         if (cpu_present(cpu))
>>>>>                 return xen_get_apic_id(xen_apic_read(APIC_ID));
>>>>>         else
>>>>>                 return BAD_APICID;
>>>>> }
>>>>>
>>>>> So independent of which present CPU we query we get just some random
>>>>> information, in the above case we get BAD_APICID from
>>>>> xen_apic_read() not
>>>>> from the else path as this CPU _IS_ present.
>>>>>
>>>>> What's so wrong with storing the fricking firmware supplied APICid as
>>>>> everybody else does and report it back when queried?
>>>> By firmware you mean ACPI? It is most likely not available to PV guests.
>>>> How about returning cpu_data(cpu).initial_apicid?
>>>>
>>>> And what was the original problem?
>>> The original issue I found was that VMware was returning a different set
>>> of APIC id's in the ACPI tables than what it advertised on the CPU's.
>>>
>>> http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1266716.html
>> For Xen, we recently added a6a198bc60e6 ("xen/x86: Update topology map
>> for PV VCPUs") to at least temporarily work around some topology map
>> problems that PV guests have with RAPL (which I think is what Vefa's
>> problem was).
> Hello Boris,
>
> (Sorry for the delay!)
>
> It appears that the problem is a bit different compared to the one
> corrected by a6a198bc60e6, because my kernel tree -- based on 4.8.6 --
> already includes the -stable backport of that commit, i.e.
>   88540ad0820ddfb05626e0136c0e5a79cea85fd1
>
> The patch I included in my previous e-mail (dated 2016-11-10) corrects
> root cause of the issue I am having with 4.8.6. Sebastian's original
> patch adding error checking to the RAPL module prevents the RAPL module
> from causing a kernel oops without my patch.

I don't see any messages from you on that date. Can you provide a link
to it (and to Sebastian's patch)?

(BTW, generally it's a good idea to copy xen-devel list on any
Xen-related issues).

>
> The issue I am experiencing is caused by the boot-up code in the
> 'init_apic_mappings' function switching the APIC ops structure from
> Xen's structure to a no-op structure by calling the 'apic_disable'
> function. Please let me know if I can clarify or elaborate.

apic_disable() is only invoked if there is no APIC present (i.e.
detect_init_APIC() returns a non-zero value) and I don't think this can
happen. Is your CPUID[1].edx[9] not set?

-boris

>
> For the record, using 4.8.7 without my correction patch patch does not
> rectify the issue at hand. 4.8.7 changes the call site of the
> 'init_apic_mapping' function, so I had thought that it could be helpful.
>
> Thank you,
>
> Vefa

[toc] | [prev] | [next] | [standalone]

#1520722

On 11/13/2016 09:04 PM, Boris Ostrovsky wrote:
> On 11/12/2016 05:05 PM, M. Vefa Bicakci wrote:
>> On 11/10/2016 06:31 PM, Boris Ostrovsky wrote:
>>> On 11/10/2016 10:05 AM, Charles (Chas) Williams wrote:
>>>>
>>>> On 11/10/2016 09:02 AM, Boris Ostrovsky wrote:
>>>>> On 11/10/2016 06:13 AM, Thomas Gleixner wrote:
>>>>>> On Thu, 10 Nov 2016, M. Vefa Bicakci wrote:
>>>>>>
>>>>>>> I have found that your patch unfortunately does not improve the
>>>>>>> situation
>>>>>>> for me. Here is an excerpt obtained from the dmesg of a kernel
>>>>>>> compiled
>>>>>>> with this patch *as well as* Sebastian's patch:
>>>>>>> [    0.002561] CPU: Physical Processor ID: 0
>>>>>>> [    0.002566] CPU: Processor Core ID: 0
>>>>>>> [    0.002572] [Firmware Bug]: CPU0: APIC id mismatch. Firmware:
>>>>>>> ffff CPUID: 2
>>>>>> So apic->cpu_present_to_apicid() gives us a completely bogus APIC id
>>>>>> which
>>>>>> translates to a bogus package id. And looking at the XEN code:
>>>>>>
>>>>>>    xen_pv_apic.cpu_present_to_apicid = xen_cpu_present_to_apicid,
>>>>>>
>>>>>> and xen_cpu_present_to_apicid does:
>>>>>>
>>>>>> static int xen_cpu_present_to_apicid(int cpu)
>>>>>> {
>>>>>>         if (cpu_present(cpu))
>>>>>>                 return xen_get_apic_id(xen_apic_read(APIC_ID));
>>>>>>         else
>>>>>>                 return BAD_APICID;
>>>>>> }
>>>>>>
>>>>>> So independent of which present CPU we query we get just some random
>>>>>> information, in the above case we get BAD_APICID from
>>>>>> xen_apic_read() not
>>>>>> from the else path as this CPU _IS_ present.
>>>>>>
>>>>>> What's so wrong with storing the fricking firmware supplied APICid as
>>>>>> everybody else does and report it back when queried?
>>>>> By firmware you mean ACPI? It is most likely not available to PV guests.
>>>>> How about returning cpu_data(cpu).initial_apicid?
>>>>>
>>>>> And what was the original problem?
>>>> The original issue I found was that VMware was returning a different set
>>>> of APIC id's in the ACPI tables than what it advertised on the CPU's.
>>>>
>>>> http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1266716.html
>>> For Xen, we recently added a6a198bc60e6 ("xen/x86: Update topology map
>>> for PV VCPUs") to at least temporarily work around some topology map
>>> problems that PV guests have with RAPL (which I think is what Vefa's
>>> problem was).
>> Hello Boris,
>>
>> (Sorry for the delay!)
>>
>> It appears that the problem is a bit different compared to the one
>> corrected by a6a198bc60e6, because my kernel tree -- based on 4.8.6 --
>> already includes the -stable backport of that commit, i.e.
>>   88540ad0820ddfb05626e0136c0e5a79cea85fd1
>>
>> The patch I included in my previous e-mail (dated 2016-11-10) corrects
>> root cause of the issue I am having with 4.8.6. Sebastian's original
>> patch adding error checking to the RAPL module prevents the RAPL module
>> from causing a kernel oops without my patch.
> 
> I don't see any messages from you on that date. Can you provide a link
> to it (and to Sebastian's patch)?
> 
> (BTW, generally it's a good idea to copy xen-devel list on any
> Xen-related issues).

As I explain below, it turns out that my issue was 'only' a kernel
configuration issue.

For reference, I had unknowingly solved my kernel-configuration-induced
issue via the patch at:
  https://marc.info/?l=linux-kernel&m=147875027314638&w=2

Sebastian's patch (which adds error handling to the RAPL module) is at:
  https://marc.info/?l=linux-kernel&m=147739814217598&w=2

>> The issue I am experiencing is caused by the boot-up code in the
>> 'init_apic_mappings' function switching the APIC ops structure from
>> Xen's structure to a no-op structure by calling the 'apic_disable'
>> function. Please let me know if I can clarify or elaborate.
> 
> apic_disable() is only invoked if there is no APIC present (i.e.
> detect_init_APIC() returns a non-zero value) and I don't think this can
> happen. Is your CPUID[1].edx[9] not set?

I found out that my domU kernels invoke the 'apic_disable' function
because CONFIG_X86_MPPARSE was not enabled in my kernel configuration,
which would cause the 'smp_found_config' bit to be unset at boot-up.

This would cause 'init_apic_mappings' to call 'apic_disable', which
would cause Xen's 'apic' ops structure pointer to be replaced with the
no-op APIC ops structure's pointer.

The use of the no-op APIC ops structure would in turn cause invalid
virtual CPU package identifiers to be generated. Invalid CPU package
identifiers would in turn cause the RAPL module to produce a kernel oops
due to potentially missing error handling.

It looks like I have been ignoring the following kernel warning which I
should have noticed a long time ago:

  MPS support code is not built-in.
  Using acpi=off or acpi=noirq or pci=noacpi may have problem

To all on this e-mail thread, I learned a bit through this exercise, but
I have also taken a lot of everyone's time and created quite a bit of
e-mail traffic because of a kernel configuration issue on my end.

My apologies.

Vefa

[toc] | [prev] | [next] | [standalone]

#1522224

On 11/13/2016 06:42 PM, M. Vefa Bicakci wrote:

> I found out that my domU kernels invoke the 'apic_disable' function
> because CONFIG_X86_MPPARSE was not enabled in my kernel configuration,
> which would cause the 'smp_found_config' bit to be unset at boot-up.


smp_found_config is not the problem, it is usually zero for Xen PV guests.

What is the problem is that because of your particular config selection 
acpi_mps_check() fails (with the error message that you mention below) 
and that leads to X86_FEATURE_APIC being cleared. And then we indeed 
switch to APIC noop and things go south after that.

-boris

>
> This would cause 'init_apic_mappings' to call 'apic_disable', which
> would cause Xen's 'apic' ops structure pointer to be replaced with the
> no-op APIC ops structure's pointer.
>
> The use of the no-op APIC ops structure would in turn cause invalid
> virtual CPU package identifiers to be generated. Invalid CPU package
> identifiers would in turn cause the RAPL module to produce a kernel oops
> due to potentially missing error handling.
>
> It looks like I have been ignoring the following kernel warning which I
> should have noticed a long time ago:
>
>   MPS support code is not built-in.
>   Using acpi=off or acpi=noirq or pci=noacpi may have problem
>
> To all on this e-mail thread, I learned a bit through this exercise, but
> I have also taken a lot of everyone's time and created quite a bit of
> e-mail traffic because of a kernel configuration issue on my end.
>
> My apologies.
>
> Vefa
>

[toc] | [prev] | [next] | [standalone]

#1525186

On Mon, 14 Nov 2016, Boris Ostrovsky wrote:
> On 11/13/2016 06:42 PM, M. Vefa Bicakci wrote:
> 
> > I found out that my domU kernels invoke the 'apic_disable' function
> > because CONFIG_X86_MPPARSE was not enabled in my kernel configuration,
> > which would cause the 'smp_found_config' bit to be unset at boot-up.
> 
> smp_found_config is not the problem, it is usually zero for Xen PV guests.
> 
> What is the problem is that because of your particular config selection
> acpi_mps_check() fails (with the error message that you mention below) and
> that leads to X86_FEATURE_APIC being cleared. And then we indeed switch to
> APIC noop and things go south after that.

Indeed. And what really puzzles me is that Xen manages to bring up a
secondary CPU despite APIC being disabled. 

There are quite some assumptions about no APIC == no SMP in all of x86. Can
we please make Xen behave like anything else?

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1525388

On 11/18/2016 06:16 AM, Thomas Gleixner wrote:
> On Mon, 14 Nov 2016, Boris Ostrovsky wrote:
>> On 11/13/2016 06:42 PM, M. Vefa Bicakci wrote:
>>
>>> I found out that my domU kernels invoke the 'apic_disable' function
>>> because CONFIG_X86_MPPARSE was not enabled in my kernel configuration,
>>> which would cause the 'smp_found_config' bit to be unset at boot-up.
>> smp_found_config is not the problem, it is usually zero for Xen PV guests.
>>
>> What is the problem is that because of your particular config selection
>> acpi_mps_check() fails (with the error message that you mention below) and
>> that leads to X86_FEATURE_APIC being cleared. And then we indeed switch to
>> APIC noop and things go south after that.
> Indeed. And what really puzzles me is that Xen manages to bring up a
> secondary CPU despite APIC being disabled. 

PV guests bring secondary VCPUs up using hypercalls (see xen_cpu_up()).

> There are quite some assumptions about no APIC == no SMP in all of x86. Can
> we please make Xen behave like anything else?
>

I will try to see if we can improve APIC emulation for these guests.
Unfortunately it will have to be done on kernel side since we still need
to support older Xen versions.

But as I said earlier, the right answer to this is PVH.

-boris

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web