Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752420AbbG3Dzt (ORCPT ); Wed, 29 Jul 2015 23:55:49 -0400 Received: from mail-pa0-f47.google.com ([209.85.220.47]:35043 "EHLO mail-pa0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750866AbbG3Dzr (ORCPT ); Wed, 29 Jul 2015 23:55:47 -0400 Date: Wed, 29 Jul 2015 20:55:43 -0700 From: Steve Rutherford To: Paolo Bonzini Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, alex.williamson@redhat.com, yang.z.zhang@intel.com, srutherford@intel.com Subject: Re: [PATCH 2/2] KVM: x86: store IOAPIC-handled vectors in each VCPU Message-ID: <20150730035543.GE15229@google.com> References: <1438177055-26764-1-git-send-email-pbonzini@redhat.com> <1438177055-26764-3-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1438177055-26764-3-git-send-email-pbonzini@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8738 Lines: 230 On Wed, Jul 29, 2015 at 03:37:35PM +0200, Paolo Bonzini wrote: > We can reuse the algorithm that computes the EOI exit bitmap to figure > out which vectors are handled by the IOAPIC. The only difference > between the two is for edge-triggered interrupts other than IRQ8 > that have no notifiers active; however, the IOAPIC does not have to > do anything special for these interrupts anyway. > > This again limits the interactions between the IOAPIC and the LAPIC, > making it easier to move the former to userspace. > > Inspired by a patch from Steve Rutherford. > > Signed-off-by: Paolo Bonzini > --- > arch/x86/include/asm/kvm_host.h | 3 ++- > arch/x86/kvm/ioapic.c | 18 ++---------------- > arch/x86/kvm/ioapic.h | 8 -------- > arch/x86/kvm/lapic.c | 10 ++++++++-- > arch/x86/kvm/svm.c | 2 +- > arch/x86/kvm/vmx.c | 3 ++- > arch/x86/kvm/x86.c | 8 +++----- > 7 files changed, 18 insertions(+), 34 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 2f9e504f9f0c..d0e991ef6ef0 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -383,6 +383,7 @@ struct kvm_vcpu_arch { > u64 efer; > u64 apic_base; > struct kvm_lapic *apic; /* kernel irqchip context */ > + u64 eoi_exit_bitmap[4]; > unsigned long apic_attention; > int32_t apic_arb_prio; > int mp_state; > @@ -808,7 +809,7 @@ struct kvm_x86_ops { > int (*vm_has_apicv)(struct kvm *kvm); > void (*hwapic_irr_update)(struct kvm_vcpu *vcpu, int max_irr); > void (*hwapic_isr_update)(struct kvm *kvm, int isr); > - void (*load_eoi_exitmap)(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap); > + void (*load_eoi_exitmap)(struct kvm_vcpu *vcpu); > void (*set_virtual_x2apic_mode)(struct kvm_vcpu *vcpu, bool set); > void (*set_apic_access_page_addr)(struct kvm_vcpu *vcpu, hpa_t hpa); > void (*deliver_posted_interrupt)(struct kvm_vcpu *vcpu, int vector); > diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c > index eaf4ec38d980..2dcda0f188ba 100644 > --- a/arch/x86/kvm/ioapic.c > +++ b/arch/x86/kvm/ioapic.c > @@ -233,19 +233,6 @@ static void kvm_ioapic_inject_all(struct kvm_ioapic *ioapic, unsigned long irr) > } > > > -static void update_handled_vectors(struct kvm_ioapic *ioapic) > -{ > - DECLARE_BITMAP(handled_vectors, 256); > - int i; > - > - memset(handled_vectors, 0, sizeof(handled_vectors)); > - for (i = 0; i < IOAPIC_NUM_PINS; ++i) > - __set_bit(ioapic->redirtbl[i].fields.vector, handled_vectors); > - memcpy(ioapic->handled_vectors, handled_vectors, > - sizeof(handled_vectors)); > - smp_wmb(); > -} > - > void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap) > { > struct kvm_ioapic *ioapic = vcpu->kvm->arch.vioapic; > @@ -310,7 +297,6 @@ static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val) > e->bits |= (u32) val; > e->fields.remote_irr = 0; > } > - update_handled_vectors(ioapic); > mask_after = e->fields.mask; > if (mask_before != mask_after) > kvm_fire_mask_notifiers(ioapic->kvm, KVM_IRQCHIP_IOAPIC, index, mask_after); > @@ -594,7 +580,6 @@ static void kvm_ioapic_reset(struct kvm_ioapic *ioapic) > ioapic->id = 0; > memset(ioapic->irq_eoi, 0x00, IOAPIC_NUM_PINS); > rtc_irq_eoi_tracking_reset(ioapic); > - update_handled_vectors(ioapic); > } > > static const struct kvm_io_device_ops ioapic_mmio_ops = { > @@ -623,8 +608,10 @@ int kvm_ioapic_init(struct kvm *kvm) > if (ret < 0) { > kvm->arch.vioapic = NULL; > kfree(ioapic); > + return ret; > } > > + kvm_vcpu_request_scan_ioapic(kvm); > return ret; > } > > @@ -661,7 +648,6 @@ int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state) > memcpy(ioapic, state, sizeof(struct kvm_ioapic_state)); > ioapic->irr = 0; > ioapic->irr_delivered = 0; > - update_handled_vectors(ioapic); > kvm_vcpu_request_scan_ioapic(kvm); > kvm_ioapic_inject_all(ioapic, state->irr); > spin_unlock(&ioapic->lock); > diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h > index 3dbd0e2aac4e..bf36d66a1951 100644 > --- a/arch/x86/kvm/ioapic.h > +++ b/arch/x86/kvm/ioapic.h > @@ -73,7 +73,6 @@ struct kvm_ioapic { > struct kvm *kvm; > void (*ack_notifier)(void *opaque, int irq); > spinlock_t lock; > - DECLARE_BITMAP(handled_vectors, 256); > struct rtc_status rtc_status; > struct delayed_work eoi_inject; > u32 irq_eoi[IOAPIC_NUM_PINS]; > @@ -98,13 +97,6 @@ static inline struct kvm_ioapic *ioapic_irqchip(struct kvm *kvm) > return kvm->arch.vioapic; > } > > -static inline bool kvm_ioapic_handles_vector(struct kvm *kvm, int vector) > -{ > - struct kvm_ioapic *ioapic = kvm->arch.vioapic; > - smp_rmb(); > - return test_bit(vector, ioapic->handled_vectors); > -} > - > void kvm_rtc_eoi_tracking_restore_one(struct kvm_vcpu *vcpu); > bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source, > int short_hand, unsigned int dest, int dest_mode); > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index 9be64c77d6db..c158f4667aa3 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -869,14 +869,20 @@ int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2) > return vcpu1->arch.apic_arb_prio - vcpu2->arch.apic_arb_prio; > } > > +static bool kvm_ioapic_handles_vector(struct kvm_lapic *apic, int vector) > +{ > + return test_bit(vector, (ulong *)apic->vcpu->arch.eoi_exit_bitmap); > +} > + > static void kvm_ioapic_send_eoi(struct kvm_lapic *apic, int vector) > { > - if (kvm_ioapic_handles_vector(apic->vcpu->kvm, vector)) { > + if (kvm_ioapic_handles_vector(apic, vector)) { > int trigger_mode; > if (apic_test_vector(vector, apic->regs + APIC_TMR)) > trigger_mode = IOAPIC_LEVEL_TRIG; > else > trigger_mode = IOAPIC_EDGE_TRIG; > + Nit: Not sure if you meant to add this. > kvm_ioapic_update_eoi(apic->vcpu, vector, trigger_mode); > } > } > @@ -1922,7 +1928,7 @@ static void apic_sync_pv_eoi_to_guest(struct kvm_vcpu *vcpu, > /* Cache not set: could be safe but we don't bother. */ > apic->highest_isr_cache == -1 || > /* Need EOI to update ioapic. */ > - kvm_ioapic_handles_vector(vcpu->kvm, apic->highest_isr_cache)) { > + kvm_ioapic_handles_vector(apic, apic->highest_isr_cache)) { > /* > * PV EOI was disabled by apic_sync_pv_eoi_from_guest > * so we need not do anything here. > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index d1a114d8d22b..676c9eb10174 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -3765,7 +3765,7 @@ static int svm_vm_has_apicv(struct kvm *kvm) > return 0; > } > > -static void svm_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap) > +static void svm_load_eoi_exitmap(struct kvm_vcpu *vcpu) > { > return; > } > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 4014a82ae846..7323ea8fba88 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -8111,8 +8111,9 @@ static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr) > } > } > > -static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap) > +static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu) > { > + u64 *eoi_exit_bitmap = vcpu->arch.eoi_exit_bitmap; > if (!vmx_vm_has_apicv(vcpu->kvm)) > return; > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 48dc954542db..4ad7334aa604 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6148,15 +6148,13 @@ static void process_smi(struct kvm_vcpu *vcpu) > > static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu) > { > - u64 eoi_exit_bitmap[4]; > - > if (!kvm_apic_hw_enabled(vcpu->arch.apic)) > return; > > - memset(eoi_exit_bitmap, 0, 32); > + memset(vcpu->arch.eoi_exit_bitmap, 0, 256 / 8); > > - kvm_ioapic_scan_entry(vcpu, eoi_exit_bitmap); > - kvm_x86_ops->load_eoi_exitmap(vcpu, eoi_exit_bitmap); > + kvm_ioapic_scan_entry(vcpu, vcpu->arch.eoi_exit_bitmap); Nit: Does scan entry still need to carry around the pointer to the bitmap? > + kvm_x86_ops->load_eoi_exitmap(vcpu); > } > > static void kvm_vcpu_flush_tlb(struct kvm_vcpu *vcpu) > -- > 1.8.3.1 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Looks good. Just picked a few nits. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/