Re: [PATCH] x86/apic: reset LDR in clear_local_APIC

[Thomas Gleixner]

Bandan,

On Wed, 21 Aug 2019, Bandan Das wrote:
> Thomas Gleixner <tglx@xxxxxxxxxxxxx> 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.

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

	tglx