lkml.org 
[lkml]   [2019]   [Aug]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] x86/apic: reset LDR in clear_local_APIC
Date
Thomas Gleixner <tglx@linutronix.de> writes:

> Bandan,
>
> On Wed, 21 Aug 2019, Bandan Das wrote:
>> Thomas Gleixner <tglx@linutronix.de> writes:
>> So, in KVM: if we make sure that the logical destination map isn't filled up if the virtual
>> apic is not enabled by software, it really doesn't matter whether the LDR for an inactive CPU
>> has a stale value.
>>
>> In x86/apic: if we make sure that the LDR is 0 or reset,
>> recalculate_apic_map() will never consider including this cpu in the
>> logical map.
> ?
>> In short, as I mentioned in the patch description, this is really a KVM
>> bug but it doesn't hurt to clear out the LDR in the guest and then, it
>> wouldn't need a hypervisor fix.
>
> I still needs a hypervisor fix. Taking disabled APICs into account is a bug
> which has also other consequeces than that particular one. So please don't
> claim that. It's wrong.
>
> If that prevents the APIC bug from triggering on unfixed hypervisors, then
> this is a nice side effect, but not a solution.
>
Agreed and fwiw, the kvm fix has been queued already.

>> Is this better ?
>
> That's way better.
>
> So can you please create two patches:
>
> 1) Make that bogus bigsmp ldr init empty
>
> That one wants a changelog along these lines:
>
> - Setting LDR for physical destination mode is pointless
> - Setting multiple bits in the LDR is wrong
>
> Mention how this was discovered and caused the KVM APIC bug to be
> triggered. Also mention that the change is not there to paper over
> the KVM APIC bug. The change fixes a bug in the bigsmp APIC code.
>
> 2) Clear LDR in in that apic reset function
>
> That one wants a changelog along these lines:
>
> - Except for x2apic the LDR should be cleared as any other APIC
> register
>
> Mention how this was discovered. Again the change is not there to
> paper over the KVM APIC bug. It's for correctness sake and valid on
> its own.
>
> Thanks,
>
Will do as you suggested. Thank you for the review.

Bandan
> tglx
>
>

\
 
 \ /
  Last update: 2019-08-21 21:54    [W:0.039 / U:0.180 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site