| Message ID | 1438177055-26764-3-git-send-email-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <pbonzini@redhat.com> > --- > 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 kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30/07/2015 05:55, Steve Rutherford wrote: > > 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? IIRC I was copying what you did in your patch, but I also think it's better this way. The IOAPIC has no business in the vcpu structure's fields; it only needs it in order to pass vcpu to kvm_apic_match_dest. Paolo -- 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
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; + 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); + kvm_x86_ops->load_eoi_exitmap(vcpu); } static void kvm_vcpu_flush_tlb(struct kvm_vcpu *vcpu)
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 <pbonzini@redhat.com> --- 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(-)