From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH 7/8] KVM: x86: avoid logical_map when it is invalid Date: Fri, 30 Jan 2015 10:38:09 +0100 Message-ID: <54CB5101.4010701@redhat.com> References: <1422568135-28402-1-git-send-email-rkrcmar@redhat.com> <1422568135-28402-8-git-send-email-rkrcmar@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, Nadav Amit , Gleb Natapov To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , linux-kernel@vger.kernel.org Return-path: In-Reply-To: <1422568135-28402-8-git-send-email-rkrcmar@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 29/01/2015 22:48, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: > We want to support mixed modes and the easiest solution is to avoid > optimizing those weird and unlikely scenarios. >=20 > Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 > --- > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/lapic.c | 16 ++++++++++++++++ > arch/x86/kvm/lapic.h | 4 ++++ > 3 files changed, 21 insertions(+) >=20 > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/k= vm_host.h > index 26d0f0f646d3..fec3188cabbb 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -554,6 +554,7 @@ struct kvm_arch_memory_slot { > =20 > struct kvm_apic_map { > struct rcu_head rcu; > + u8 mode; > u8 ldr_bits; > /* fields bellow are used to decode ldr values in different modes *= / > u32 cid_shift, cid_mask, lid_mask; > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index fab007509047..621d9df6ac63 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -162,16 +162,19 @@ static void recalculate_apic_map(struct kvm *kv= m) > new->ldr_bits =3D 32; > new->cid_shift =3D 16; > new->cid_mask =3D new->lid_mask =3D 0xffff; > + new->mode |=3D KVM_APIC_MODE_X2APIC; > } 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; > new->cid_mask =3D 0xf; > new->lid_mask =3D 0xf; > + new->mode |=3D KVM_APIC_MODE_XAPIC_CLUSTER; > } else { > new->cid_shift =3D 8; > new->cid_mask =3D 0; > new->lid_mask =3D 0xff; > + new->mode |=3D KVM_APIC_MODE_XAPIC_FLAT; > } > } > =20 > @@ -201,6 +204,13 @@ static void recalculate_apic_map(struct kvm *kvm= ) > =20 > if (aid < ARRAY_SIZE(new->phys_map)) > new->phys_map[aid] =3D apic; > + > + /* The logical map is definitely wrong if we have multiple > + * modes at the same time. Physical is still right though. > + */ > + if (hweight8(new->mode) !=3D 1) Better (more optimized): if (new->mode & (new->mode - 1)) Please add a comment to kvm_irq_delivery_to_apic_fast to explain what you are doing. > + continue; > + > if (lid && cid < ARRAY_SIZE(new->logical_map)) > new->logical_map[cid][ffs(lid) - 1] =3D apic; > } > @@ -720,6 +730,12 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *k= vm, struct kvm_lapic *src, > if (cid >=3D ARRAY_SIZE(map->logical_map)) > goto out; > =20 > + if (hweight8(map->mode) !=3D 1) { > + /* Not deliverable with optimized map. */ > + ret =3D false; > + goto out; > + } Put this before the computation of cid and mda. The cid and mda are al= l wrong with a mixed map, and the result of the "if" before is influenced by the wrong cid. Fixed by patch 8, but better get it right here. Paolo > dst =3D map->logical_map[cid]; > =20 > bitmap =3D apic_logical_id(map, mda); > diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h > index c1ef25c89508..fd0197a93862 100644 > --- a/arch/x86/kvm/lapic.h > +++ b/arch/x86/kvm/lapic.h > @@ -8,6 +8,10 @@ > #define KVM_APIC_INIT 0 > #define KVM_APIC_SIPI 1 > =20 > +#define KVM_APIC_MODE_XAPIC_FLAT (1 << 0) > +#define KVM_APIC_MODE_XAPIC_CLUSTER (1 << 1) > +#define KVM_APIC_MODE_X2APIC (1 << 2) > + > struct kvm_timer { > struct hrtimer timer; > s64 period; /* unit: ns */ >=20