From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH 14/21] KVM: x86: Software disabled APIC should still deliver NMIs Date: Fri, 14 Nov 2014 16:00:02 +0100 Message-ID: <546618F2.2060108@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> 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: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Return-path: Received: from mx1.redhat.com ([209.132.183.28]:43089 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964887AbaKNPAS (ORCPT ); Fri, 14 Nov 2014 10:00:18 -0500 In-Reply-To: <20141106164534.GA15407@potion.brq.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 06/11/2014 17:45, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: > 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 >=20 > 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 ... >=20 >>> 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 >=20 > 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 A= PIC > is software enabled. >=20 >> That's not what either Bochs or QEMU do, though. (Though in the cas= e of >> Bochs I cannot find the place where reception of IPIs is prevented f= or >> software-disabled APICs, so I'm not sure how much to trust it in thi= s case). >> >> I'm not sure why software-disabled APICs could have different DFRs, = if >> the APICs can receive NMI IPIs. I'll ask Intel. >=20 > 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 sa= y. >=20 > Anyway, Paolo's patch seems to be in the right direction, I'd modify = it > a bit though: >=20 > LDR=3D0 isn't addressable in any logical mode, so we can ignore APICs= that > 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 a= nd > the rest is still in reset state. >=20 > (It gets harder if we allow nonzero LDRs with different DFR ... > we'd need to split our logical map to handle it.) >=20 > 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; >=20 I merged this patch and Nadav's. Paolo