From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nadav Amit Subject: Re: [PATCH 14/21] KVM: x86: Software disabled APIC should still deliver NMIs Date: Wed, 5 Nov 2014 22:45:49 +0200 Message-ID: <5D182FD6-0365-4471-AD28-C587A624D151@gmail.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> Mime-Version: 1.0 (Mac OS X Mail 8.0 \(1990.1\)) Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Nadav Amit , kvm@vger.kernel.org To: Paolo Bonzini Return-path: Received: from mail-wi0-f171.google.com ([209.85.212.171]:62111 "EHLO mail-wi0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750951AbaKEUpy convert rfc822-to-8bit (ORCPT ); Wed, 5 Nov 2014 15:45:54 -0500 Received: by mail-wi0-f171.google.com with SMTP id r20so1194730wiv.16 for ; Wed, 05 Nov 2014 12:45:52 -0800 (PST) In-Reply-To: <545A1852.1000603@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: > On Nov 5, 2014, at 14:30, Paolo Bonzini wrote: >=20 >=20 >=20 > On 02/11/2014 10:54, Nadav Amit wrote: >> Currently, the APIC logical map does not consider VCPUs whose local-= apic is >> software-disabled. However, NMIs, INIT, etc. should still be delive= red to such >> VCPUs. Therefore, the APIC mode should first be determined, and then= the map, >> considering all VCPUs should be constructed. >>=20 >> To address this issue, first find the APIC mode, and only then const= ruct the >> logical map. [snip] >=20 > We need to take into account DFR even if all APICs are software disab= led, > in case it will be used to send NMIs. >=20 > What about this on top of your patch? >=20 > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index c98b44d0ffe6..d8b11f47f45e 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -160,29 +160,34 @@ static void recalculate_apic_map(struct kvm *kv= m) > if (!kvm_apic_present(vcpu)) > continue; >=20 > - /* > - * All APICs have to be configured in the same mode by an OS. > - * We take advatage of this while building logical id loockup > - * table. After reset APICs are in xapic/flat mode, so if we > - * find apic with different setting we assume this is the mode > - * OS wants all apics to be in; build lookup table accordingly. > - */ > if (apic_x2apic_mode(apic)) { > new->ldr_bits =3D 32; > new->cid_shift =3D 16; > new->cid_mask =3D (1 << KVM_X2APIC_CID_BITS) - 1; > new->lid_mask =3D 0xffff; > new->broadcast =3D X2APIC_BROADCAST; > - break; > - } else if (kvm_apic_sw_enabled(apic)) { > + } else if (kvm_apic_hw_enabled(apic)) { > if (kvm_apic_get_reg(apic, APIC_DFR) =3D=3D > APIC_DFR_CLUSTER) { > new->cid_shift =3D 4; > new->cid_mask =3D 0xf; > new->lid_mask =3D 0xf; > + } else { > + new->cid_shift =3D 8; > + new->cid_mask =3D 0; > + new->lid_mask =3D 0xff; > } > - break; > } > + > + /* > + * All APICs have to be configured in the same mode by an OS. > + * We take advatage of this while building logical id loockup > + * table. After reset APICs are in software disabled mode, so if > + * we find apic with different setting we assume this is the mode > + * OS wants all apics to be in; build lookup table accordingly. > + */ > + if (kvm_apic_sw_enabled(apic)) > + break; > } >=20 > kvm_for_each_vcpu(i, vcpu, kvm) { >=20 I didn=92t encounter the problem you try to fix, so I can=92t say for s= ure, yet=85 If I understand the SDM correctly, in such scenario (all APICs are soft= ware disabled) the mode is left as the default - flat mode (see section= 10.6.2.2 "Logical Destination Mode=94): "All processors that have their APIC software enabled (using the spurio= us vector enable/disable bit) must have their DFRs (Destination Format = Registers) programmed identically. The default mode for DFR is flat mod= e.=94 So I think the previous behaviour (before the additional changes) is th= e correct one. I might be able to confirm it, but anyhow only in a couple of weeks. Nadav