Re: [PATCH 14/21] KVM: x86: Software disabled APIC should still deliver NMIs

["Radim Krčmář" <rkrcmar@redhat.com>]

2014-11-27 23:45+0200, Nadav Amit:
> Radim Krčmář <rkrcmar@redhat.com> wrote:
> > 2014-11-26 19:01+0200, Nadav Amit:
> >> Sorry for the late and long reply, but I got an issue with the new version
> >> (and my previous version as well). Indeed, the SDM states that DFR should
> >> be the same for enabled CPUs, and that the BIOS should get all CPUs in
> >> either xAPIC or x2APIC. Yet, there is nothing that says all CPUs need to be
> >> in xAPIC/x2APIC mode.
> >> 
> >> In my tests (which pass on bare-metal), I got a scenario in which some CPUs
> >> are in xAPIC mode, the BSP changed (which is currently not handled correctly
> >> by KVM) and the BSP has x2APIC enabled.
> > 
> > How many (V)CPUs were you using?
> > (We fail hard with logical destination x2APIC and 16+ VCPUs.)
> 2 at the moment. What failure do you refer to?

(I'll cover it under KVM_X2APIC_CID_BITS.)

xAPIC shouldn't have ever made it into the logical map under x2APIC ...
Were you testing with broadcasts?

> > Our x2APIC implementation is a hack that allowed faster IPI thanks to 1
> > MSR exit instead of 2 MMIO ones.  No OS, that doesn't know KVM's
> > limitations, should have enabled it because we didn't emulate interrupt
> > remapping, which is an architectural requirement for x2APIC …
> It is a shame - I was under the impression QEMU emulation of the Intel IOMMU
> would include it as well, and I now see they only did DMAR…

(and we had this x2APIC for years ...)

> > And for more concrete points:
> > - Physical x2APIC isn't affected (only broadcast, which is incorrect
> >  either way)
> > 
> > - Logical x2APIC and xAPIC don't work at the same time
> No, but it is important to determine what is the “consensus” APIC mode.

Only for our abstraction, SDM's APICs don't need it and I'd rather see
us not depend on it either ...
(Sanity check: if you do xAPIC broadcast when there is xAPIC and x2APIC
 on real hw, does the xAPIC receive it? And if x2APIC sends 0xff000000?)

> >  - Btw. logical x2APIC isn't supposed to work (see KVM_X2APIC_CID_BITS)
> Why? It is as if there is only a single cluster. You can still send an APIC
> message to multiple CPUs within the same cluster (0).

KVM_X2APIC_CID_BITS = 0 meant that all VCPUs and messages got mapped
into cluster 0.
If you had 32 VCPUs, at least half of them wouldn't have a pointer in
the map -- and those left out would most likely be within first 16 APIC
ids, so messages would go completely off.

> >  - Logical xAPIC is shifted incorrectly in x2APIC mode, so they are all
> >    going to be inaccessible (ldr = 0)
> >  - Our map isn't designed to allow x2APIC and xAPIC at the same time
> > 
> > - Your patch does not cover the case where sw-disabled x2APIC is
> >  "before" sw-enabled xAPIC, only if it is after.
> I thought I covered it. The rationale was that if any lapic is in x2APIC
> mode, then the are all in x2APIC mode. It is done similarly to the previous
> version (3.18).

True, sorry, I missed the 'break;' in x2apic path.

We can't deliver xAPIC and x2APIC broadcasts/logical messages at the
same time with current KVM and this patch just switches the working case
in favour of x2APIC, which is why I didn't think it was necessary ...
(And I didn't understand why prefer disabled x2APIC to enabled xAPIC.)

> Anyhow, I have my workarounds, so do as you find appropriately. Once I deal
> with the BSP issues, I may resubmit another version.

I don't really mind having it, guests worked even with more broken code,
and this patch helps at least one use case :)