Re: [PATCH 2/2] KVM: x86: store IOAPIC-handled vectors in each VCPU

2015-07-30 Thread Paolo Bonzini


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 linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] KVM: x86: store IOAPIC-handled vectors in each VCPU

2015-07-29 Thread Steve Rutherford
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_ve

[PATCH 2/2] KVM: x86: store IOAPIC-handled vectors in each VCPU

2015-07-29 Thread Paolo Bonzini
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_track