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, 6 Nov 2014 17:45:38 +0100 Message-ID: <20141106164534.GA15407@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> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Nadav Amit , Nadav Amit , kvm@vger.kernel.org To: Paolo Bonzini Return-path: Received: from mx1.redhat.com ([209.132.183.28]:58587 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752517AbaKFQpu (ORCPT ); Thu, 6 Nov 2014 11:45:50 -0500 Content-Disposition: inline In-Reply-To: <545B409E.1020500@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 2014-11-06 10:34+0100, Paolo Bonzini: > On 05/11/2014 21:45, Nadav Amit wrote: > > If I understand the SDM correctly, in such scenario (all APICs are > > software disabled) the mode is left as the default - flat mode (see APIC doesn't have any global mode (it is just KVM's simplification), so when a message lands on the system bus, it just compares MDA with LDR and DFR ... > > section 10.6.2.2 "Logical Destination Mode=E2=80=9D): "All processo= rs that > > have their APIC software enabled (using the spurious vector > > enable/disable bit) must have their DFRs (Destination Format > > Registers) programmed identically. The default mode for DFR is flat > > mode.=E2=80=9D I think the "default mode" points to reset state, which is flat DFR; and it is mentioned only because of the following sentence If you are using cluster mode, DFRs must be programmed before the API= C is software enabled. > That's not what either Bochs or QEMU do, though. (Though in the case= of > Bochs I cannot find the place where reception of IPIs is prevented fo= r > software-disabled APICs, so I'm not sure how much to trust it in this= case). >=20 > I'm not sure why software-disabled APICs could have different DFRs, i= f > the APICs can receive NMI IPIs. I'll ask Intel. When changing the mode, we can't switch DFR synchronously, so it has to happen and NMI may be needed (watchdog?) before APIC configuration. Explicit statement might have been a hint to be _extra_ careful when using logical destination for INIT, NMI, ... I wonder what they'll say. Anyway, Paolo's patch seems to be in the right direction, I'd modify it a bit though: LDR=3D0 isn't addressable in any logical mode, so we can ignore APICs t= hat don't set it and decide the mode by the last nonzero one. This works in a situation, where one part is configured for cluster and the rest is still in reset state. (It gets harder if we allow nonzero LDRs with different DFR ... we'd need to split our logical map to handle it.) diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 758f838..6da303e1 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -148,10 +148,6 @@ static void recalculate_apic_map(struct kvm *kvm) goto out; =20 new->ldr_bits =3D 8; - /* flat mode is default */ - new->cid_shift =3D 8; - new->cid_mask =3D 0; - new->lid_mask =3D 0xff; new->broadcast =3D APIC_BROADCAST; =20 kvm_for_each_vcpu(i, vcpu, kvm) { @@ -166,7 +162,7 @@ static void recalculate_apic_map(struct kvm *kvm) new->cid_mask =3D (1 << KVM_X2APIC_CID_BITS) - 1; new->lid_mask =3D 0xffff; new->broadcast =3D X2APIC_BROADCAST; - } else if (kvm_apic_hw_enabled(apic)) { + } else if (kvm_apic_get_reg(apic, APIC_LDR)) { if (kvm_apic_get_reg(apic, APIC_DFR) =3D=3D APIC_DFR_CLUSTER) { new->cid_shift =3D 4;