From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [PATCH 14/21] KVM: x86: Software disabled APIC should still deliver NMIs Date: Thu, 27 Nov 2014 23:26:10 +0100 Message-ID: <20141127222610.GD7770@potion.brq.redhat.com> References: <1414922101-17626-1-git-send-email-namit@cs.technion.ac.il> <1414922101-17626-15-git-send-email-namit@cs.technion.ac.il> <545A1852.1000603@redhat.com> <5D182FD6-0365-4471-AD28-C587A624D151@gmail.com> <545B409E.1020500@redhat.com> <20141106164534.GA15407@potion.brq.redhat.com> <546618F2.2060108@redhat.com> <02C5C22B-2809-438B-BD42-8C235F1E5CEE@gmail.com> <20141127133957.GA383@potion.brq.redhat.com> <12B8F1E7-A4D1-46D1-948D-EAC44B88CA0D@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Paolo Bonzini , kvm list To: Nadav Amit Return-path: Received: from mx1.redhat.com ([209.132.183.28]:40015 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750985AbaK0W0R (ORCPT ); Thu, 27 Nov 2014 17:26:17 -0500 Content-Disposition: inline In-Reply-To: <12B8F1E7-A4D1-46D1-948D-EAC44B88CA0D@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: 2014-11-27 23:45+0200, Nadav Amit: > Radim Kr=C4=8Dm=C3=A1=C5=99 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 CPU= s in > >> either xAPIC or x2APIC. Yet, there is nothing that says all CPUs n= eed to be > >> in xAPIC/x2APIC mode. > >>=20 > >> 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. > >=20 > > 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 inter= rupt > > remapping, which is an architectural requirement for x2APIC =E2=80=A6 > It is a shame - I was under the impression QEMU emulation of the Inte= l IOMMU > would include it as well, and I now see they only did DMAR=E2=80=A6 (and we had this x2APIC for years ...) > > And for more concrete points: > > - Physical x2APIC isn't affected (only broadcast, which is incorrec= t > > either way) > >=20 > > - Logical x2APIC and xAPIC don't work at the same time > No, but it is important to determine what is the =E2=80=9Cconsensus=E2= =80=9D 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_B= ITS) > Why? It is as if there is only a single cluster. You can still send a= n APIC > message to multiple CPUs within the same cluster (0). KVM_X2APIC_CID_BITS =3D 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 =3D 0) > > - Our map isn't designed to allow x2APIC and xAPIC at the same tim= e > >=20 > > - 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 x2A= PIC > mode, then the are all in x2APIC mode. It is done similarly to the pr= evious > 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 cas= e 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 :)