From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760424AbbA3Jik (ORCPT ); Fri, 30 Jan 2015 04:38:40 -0500 Received: from mail-wi0-f170.google.com ([209.85.212.170]:52664 "EHLO mail-wi0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760231AbbA3JiO (ORCPT ); Fri, 30 Jan 2015 04:38:14 -0500 Message-ID: <54CB5101.4010701@redhat.com> Date: Fri, 30 Jan 2015 10:38:09 +0100 From: Paolo Bonzini User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , linux-kernel@vger.kernel.org CC: kvm@vger.kernel.org, Nadav Amit , Gleb Natapov Subject: Re: [PATCH 7/8] KVM: x86: avoid logical_map when it is invalid References: <1422568135-28402-1-git-send-email-rkrcmar@redhat.com> <1422568135-28402-8-git-send-email-rkrcmar@redhat.com> In-Reply-To: <1422568135-28402-8-git-send-email-rkrcmar@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 29/01/2015 22:48, Radim Krčmář wrote: > We want to support mixed modes and the easiest solution is to avoid > optimizing those weird and unlikely scenarios. > > Signed-off-by: Radim Krčmář > --- > 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(+) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_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 { > > 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 *kvm) > new->ldr_bits = 32; > new->cid_shift = 16; > new->cid_mask = new->lid_mask = 0xffff; > + new->mode |= KVM_APIC_MODE_X2APIC; > } else if (kvm_apic_get_reg(apic, APIC_LDR)) { > if (kvm_apic_get_reg(apic, APIC_DFR) == > APIC_DFR_CLUSTER) { > new->cid_shift = 4; > new->cid_mask = 0xf; > new->lid_mask = 0xf; > + new->mode |= KVM_APIC_MODE_XAPIC_CLUSTER; > } else { > new->cid_shift = 8; > new->cid_mask = 0; > new->lid_mask = 0xff; > + new->mode |= KVM_APIC_MODE_XAPIC_FLAT; > } > } > > @@ -201,6 +204,13 @@ static void recalculate_apic_map(struct kvm *kvm) > > if (aid < ARRAY_SIZE(new->phys_map)) > new->phys_map[aid] = 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) != 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] = apic; > } > @@ -720,6 +730,12 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src, > if (cid >= ARRAY_SIZE(map->logical_map)) > goto out; > > + if (hweight8(map->mode) != 1) { > + /* Not deliverable with optimized map. */ > + ret = false; > + goto out; > + } Put this before the computation of cid and mda. The cid and mda are all 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 = map->logical_map[cid]; > > bitmap = 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 > > +#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 */ >