public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit bitmap
@ 2012-09-05  5:41 Li, Jiongxi
  2012-09-06 16:37 ` Avi Kivity
  0 siblings, 1 reply; 6+ messages in thread
From: Li, Jiongxi @ 2012-09-05  5:41 UTC (permalink / raw)
  To: kvm@vger.kernel.org; +Cc: avi@redhat.com

With APICv virtual interrupt delivery feature, EOI write from non
root mode doesn't cause VM-Exit unless set in EOI exit bitmap VMCS
field. Basically there're two methods to manipulate EOI exit bitmap:

[Option 1]
Ideally only level triggered irq requires a hook in vLAPIC EOI write,
so that vIOAPIC EOI is triggered and emulated. So the simplest
approach is to manipulate EOI exit bitmap when vLAPIC acks a new
interrupt, based on value of TMR. There're several corner cases
worthy of note though:

  - KVM has specific notifier hooks on vIOAPIC EOI path. So far two
    sources use it: INT-based device passthrough and PIT pending
    timers. For the former, it's virtually wired to vIOAPIC and
    thus TMR already covers it. PIT is special here, which is an
    edge triggered source. But since other timer sources like
    vLAPIC timer don't require this notifier hook, possibly PIT
    can be relaxed in the future too.

  - posted interrupt will update TMR directly, w/o chance for KVM
    to update EOI exit bitmap accordingly. This becomes a gap

[Option 2]
Indicate EOI exit bitmap requirement ('need_eoi') directly from
every interrupt source device, and then check this requirement
when vLAPIC acks a new pending interrupt. This requires more
intrusive changes to current vLAPIC/vIOAPIC logic, so that the
"irq_source_id" indicating source of interrupt is passed through
from origination point to vLAPIC ack point. For natual requirement
like vIOAPIC level triggered entries, it can be implicitly deduced.
On the other hand for non-natural requirements like aformentioned
PIT or posted interrupt, this approach can handle it efficiently.

For simplicity reason, now option 1 is used which should be
enough to test MSI-based device passthrough.

Signed-off-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
---
 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/lapic.c            |    7 ++++++-
 arch/x86/kvm/vmx.c              |   37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index ef74df5..4e06a82 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -671,6 +671,7 @@ struct kvm_x86_ops {
 	void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr);
 	int (*has_virtual_interrupt_delivery)(struct kvm_vcpu *vcpu);
 	void (*update_irq)(struct kvm_vcpu *vcpu);
+	void (*set_eoi_exitmap)(struct kvm_vcpu *vcpu, int vector, int need_eoi);
 	int (*set_tss_addr)(struct kvm *kvm, unsigned int addr);
 	int (*get_tdp_level)(void);
 	u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio);
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index d203501..4058384 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -499,8 +499,13 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 		if (trig_mode) {
 			apic_debug("level trig mode for vector %d", vector);
 			apic_set_vector(vector, apic->regs + APIC_TMR);
-		} else
+			if (kvm_apic_vid_enabled(vcpu))
+				kvm_x86_ops->set_eoi_exitmap(vcpu, vector, 1);
+		} else {
 			apic_clear_vector(vector, apic->regs + APIC_TMR);
+			if (kvm_apic_vid_enabled(vcpu))
+				kvm_x86_ops->set_eoi_exitmap(vcpu, vector, 0);
+		}
 
 		result = !apic_test_and_set_irr(vector, apic);
 		trace_kvm_apic_accept_irq(vcpu->vcpu_id, delivery_mode,
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 424a09d..73ff537 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -433,6 +433,7 @@ struct vcpu_vmx {
 
 	bool rdtscp_enabled;
 
+	u32 eoi_exitmap_changed;
 	u64 eoi_exit_bitmap[4];
 
 	/* Support for a guest hypervisor (nested VMX) */
@@ -6128,6 +6129,7 @@ static void vmx_update_irq(struct kvm_vcpu *vcpu)
 	u16 status;
 	u8 old;
 	int vector;
+	struct vcpu_vmx *vmx = to_vmx(vcpu);
 
 	vector = kvm_apic_get_highest_irr(vcpu);
 	if (vector == -1)
@@ -6140,6 +6142,40 @@ static void vmx_update_irq(struct kvm_vcpu *vcpu)
 		status |= (u8)vector;
 		vmcs_write16(GUEST_INTR_STATUS, status);
 	}
+
+	if (vmx->eoi_exitmap_changed) {
+#define UPDATE_EOI_EXITMAP(v, e) {				\
+	if (test_and_clear_bit(e, (void *)&(v)->eoi_exitmap_changed))	\
+		vmcs_write64(EOI_EXIT_BITMAP##e, (v)->eoi_exit_bitmap[e]);}
+
+		UPDATE_EOI_EXITMAP(vmx, 0);
+		UPDATE_EOI_EXITMAP(vmx, 1);
+		UPDATE_EOI_EXITMAP(vmx, 2);
+		UPDATE_EOI_EXITMAP(vmx, 3);
+	}
+}
+
+static void vmx_set_eoi_exitmap(struct kvm_vcpu *vcpu,
+				int vector,
+				int need_eoi)
+{
+	struct vcpu_vmx *vmx = to_vmx(vcpu);
+	int index, offset, changed;
+
+	if (WARN_ONCE((vector < 0) || (vector > 255),
+		"KVM VMX: vector (%d) out of range\n", vector))
+		return;
+
+	index = vector >> 6;
+	offset = vector & 63;
+	if (need_eoi)
+		changed = !test_and_set_bit(offset,
+				(void *)&vmx->eoi_exit_bitmap[index]);
+	else
+		changed = test_and_clear_bit(offset,
+				(void *)&vmx->eoi_exit_bitmap[index]);
+	if (changed)
+		set_bit(index, (void *)&vmx->eoi_exitmap_changed);
 }
 
 static void vmx_complete_atomic_exit(struct vcpu_vmx *vmx)
@@ -7403,6 +7439,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.update_cr8_intercept = update_cr8_intercept,
 	.has_virtual_interrupt_delivery = vmx_has_virtual_interrupt_delivery,
 	.update_irq = vmx_update_irq,
+	.set_eoi_exitmap = vmx_set_eoi_exitmap,
 
 	.set_tss_addr = vmx_set_tss_addr,
 	.get_tdp_level = get_ept_level,
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit bitmap
  2012-09-05  5:41 [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit bitmap Li, Jiongxi
@ 2012-09-06 16:37 ` Avi Kivity
  2012-09-14 14:19   ` Li, Jiongxi
  2012-09-17 11:28   ` Li, Jiongxi
  0 siblings, 2 replies; 6+ messages in thread
From: Avi Kivity @ 2012-09-06 16:37 UTC (permalink / raw)
  To: Li, Jiongxi; +Cc: kvm@vger.kernel.org

On 09/05/2012 08:41 AM, Li, Jiongxi wrote:
> With APICv virtual interrupt delivery feature, EOI write from non
> root mode doesn't cause VM-Exit unless set in EOI exit bitmap VMCS
> field. Basically there're two methods to manipulate EOI exit bitmap:

Should be folded into the previous patch, otherwise the previous patch
breaks level interrupts.

> 
> [Option 1]
> Ideally only level triggered irq requires a hook in vLAPIC EOI write,
> so that vIOAPIC EOI is triggered and emulated. So the simplest
> approach is to manipulate EOI exit bitmap when vLAPIC acks a new
> interrupt, based on value of TMR. There're several corner cases
> worthy of note though:
> 
>   - KVM has specific notifier hooks on vIOAPIC EOI path. So far two
>     sources use it: INT-based device passthrough and PIT pending
>     timers. For the former, it's virtually wired to vIOAPIC and
>     thus TMR already covers it. PIT is special here, which is an
>     edge triggered source. But since other timer sources like
>     vLAPIC timer don't require this notifier hook, possibly PIT
>     can be relaxed in the future too.

I would like to switch to changing the timer frequency when we need to
catch up.  But that can be done later.

> 
>   - posted interrupt will update TMR directly, w/o chance for KVM
>     to update EOI exit bitmap accordingly. This becomes a gap

Why not? we know what vector the PIT is wired to.

> 
> [Option 2]
> Indicate EOI exit bitmap requirement ('need_eoi') directly from
> every interrupt source device, and then check this requirement
> when vLAPIC acks a new pending interrupt. This requires more
> intrusive changes to current vLAPIC/vIOAPIC logic, so that the
> "irq_source_id" indicating source of interrupt is passed through
> from origination point to vLAPIC ack point. For natual requirement
> like vIOAPIC level triggered entries, it can be implicitly deduced.
> On the other hand for non-natural requirements like aformentioned
> PIT or posted interrupt, this approach can handle it efficiently.
> 
> For simplicity reason, now option 1 is used which should be
> enough to test MSI-based device passthrough.

You can change kvm_register_irq_ack_notifier() to call the ioapic and
pic to find out what vectors need EOI exits.

(alternatively, if we fix the PIT, then we only need ack notifiers for
level interrupts).

> 
> Signed-off-by: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> ---
>  arch/x86/include/asm/kvm_host.h |    1 +
>  arch/x86/kvm/lapic.c            |    7 ++++++-
>  arch/x86/kvm/vmx.c              |   37 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 44 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index ef74df5..4e06a82 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -671,6 +671,7 @@ struct kvm_x86_ops {
>  	void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr);
>  	int (*has_virtual_interrupt_delivery)(struct kvm_vcpu *vcpu);
>  	void (*update_irq)(struct kvm_vcpu *vcpu);
> +	void (*set_eoi_exitmap)(struct kvm_vcpu *vcpu, int vector, int need_eoi);
>  	int (*set_tss_addr)(struct kvm *kvm, unsigned int addr);
>  	int (*get_tdp_level)(void);
>  	u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio);
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index d203501..4058384 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -499,8 +499,13 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
>  		if (trig_mode) {
>  			apic_debug("level trig mode for vector %d", vector);
>  			apic_set_vector(vector, apic->regs + APIC_TMR);
> -		} else
> +			if (kvm_apic_vid_enabled(vcpu))
> +				kvm_x86_ops->set_eoi_exitmap(vcpu, vector, 1);
> +		} else {
>  			apic_clear_vector(vector, apic->regs + APIC_TMR);
> +			if (kvm_apic_vid_enabled(vcpu))
> +				kvm_x86_ops->set_eoi_exitmap(vcpu, vector, 0);
> +		}

This is way too late.  The flow should come from the IOAPIC and PIC,
when setting up an irq, to the local APIC.



-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit bitmap
  2012-09-06 16:37 ` Avi Kivity
@ 2012-09-14 14:19   ` Li, Jiongxi
  2012-09-16  9:53     ` Avi Kivity
  2012-09-17 11:28   ` Li, Jiongxi
  1 sibling, 1 reply; 6+ messages in thread
From: Li, Jiongxi @ 2012-09-14 14:19 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm@vger.kernel.org

Sorry for the late response

> -----Original Message-----
> From: Avi Kivity [mailto:avi@redhat.com]
> Sent: Friday, September 07, 2012 12:38 AM
> To: Li, Jiongxi
> Cc: kvm@vger.kernel.org
> Subject: Re: [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit
> bitmap
> 
> On 09/05/2012 08:41 AM, Li, Jiongxi wrote:
> > With APICv virtual interrupt delivery feature, EOI write from non root
> > mode doesn't cause VM-Exit unless set in EOI exit bitmap VMCS field.
> > Basically there're two methods to manipulate EOI exit bitmap:
> 
> Should be folded into the previous patch, otherwise the previous patch breaks
> level interrupts.
OK
> 
> >
> > [Option 1]
> > Ideally only level triggered irq requires a hook in vLAPIC EOI write,
> > so that vIOAPIC EOI is triggered and emulated. So the simplest
> > approach is to manipulate EOI exit bitmap when vLAPIC acks a new
> > interrupt, based on value of TMR. There're several corner cases worthy
> > of note though:
> >
> >   - KVM has specific notifier hooks on vIOAPIC EOI path. So far two
> >     sources use it: INT-based device passthrough and PIT pending
> >     timers. For the former, it's virtually wired to vIOAPIC and
> >     thus TMR already covers it. PIT is special here, which is an
> >     edge triggered source. But since other timer sources like
> >     vLAPIC timer don't require this notifier hook, possibly PIT
> >     can be relaxed in the future too.
> 
> I would like to switch to changing the timer frequency when we need to catch
> up.  But that can be done later.
> 
> >
> >   - posted interrupt will update TMR directly, w/o chance for KVM
> >     to update EOI exit bitmap accordingly. This becomes a gap
> 
> Why not? we know what vector the PIT is wired to.
What PIT case are you talking about? Can you elaborate it?
> 
> >
> > [Option 2]
> > Indicate EOI exit bitmap requirement ('need_eoi') directly from every
> > interrupt source device, and then check this requirement when vLAPIC
> > acks a new pending interrupt. This requires more intrusive changes to
> > current vLAPIC/vIOAPIC logic, so that the "irq_source_id" indicating
> > source of interrupt is passed through from origination point to vLAPIC
> > ack point. For natual requirement like vIOAPIC level triggered
> > entries, it can be implicitly deduced.
> > On the other hand for non-natural requirements like aformentioned PIT
> > or posted interrupt, this approach can handle it efficiently.
> >
> > For simplicity reason, now option 1 is used which should be enough to
> > test MSI-based device passthrough.
> 
> You can change kvm_register_irq_ack_notifier() to call the ioapic and pic to find
> out what vectors need EOI exits.
We will look into that, thanks for pointing out. And we will use option 1 here first
> 
> (alternatively, if we fix the PIT, then we only need ack notifiers for level
> interrupts).
> 
> >
> > Signed-off-by: Kevin Tian <kevin.tian@intel.com>
> > Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> > ---
> >  arch/x86/include/asm/kvm_host.h |    1 +
> >  arch/x86/kvm/lapic.c            |    7 ++++++-
> >  arch/x86/kvm/vmx.c              |   37
> +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 44 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h
> > b/arch/x86/include/asm/kvm_host.h index ef74df5..4e06a82 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -671,6 +671,7 @@ struct kvm_x86_ops {
> >  	void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr);
> >  	int (*has_virtual_interrupt_delivery)(struct kvm_vcpu *vcpu);
> >  	void (*update_irq)(struct kvm_vcpu *vcpu);
> > +	void (*set_eoi_exitmap)(struct kvm_vcpu *vcpu, int vector, int
> > +need_eoi);
> >  	int (*set_tss_addr)(struct kvm *kvm, unsigned int addr);
> >  	int (*get_tdp_level)(void);
> >  	u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio);
> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index
> > d203501..4058384 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -499,8 +499,13 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
> int delivery_mode,
> >  		if (trig_mode) {
> >  			apic_debug("level trig mode for vector %d", vector);
> >  			apic_set_vector(vector, apic->regs + APIC_TMR);
> > -		} else
> > +			if (kvm_apic_vid_enabled(vcpu))
> > +				kvm_x86_ops->set_eoi_exitmap(vcpu, vector, 1);
> > +		} else {
> >  			apic_clear_vector(vector, apic->regs + APIC_TMR);
> > +			if (kvm_apic_vid_enabled(vcpu))
> > +				kvm_x86_ops->set_eoi_exitmap(vcpu, vector, 0);
> > +		}
> 
> This is way too late.  The flow should come from the IOAPIC and PIC, when
> setting up an irq, to the local APIC.
Ok, we will look into that, if it is the case, we will correct it
> 
> 
> 
> --
> error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit bitmap
  2012-09-14 14:19   ` Li, Jiongxi
@ 2012-09-16  9:53     ` Avi Kivity
  0 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2012-09-16  9:53 UTC (permalink / raw)
  To: Li, Jiongxi; +Cc: kvm@vger.kernel.org

On 09/14/2012 05:19 PM, Li, Jiongxi wrote:
> Sorry for the late response
> 
>> -----Original Message-----
>> From: Avi Kivity [mailto:avi@redhat.com]
>> Sent: Friday, September 07, 2012 12:38 AM
>> To: Li, Jiongxi
>> Cc: kvm@vger.kernel.org
>> Subject: Re: [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit
>> bitmap
>> 
>> On 09/05/2012 08:41 AM, Li, Jiongxi wrote:
>> > With APICv virtual interrupt delivery feature, EOI write from non root
>> > mode doesn't cause VM-Exit unless set in EOI exit bitmap VMCS field.
>> > Basically there're two methods to manipulate EOI exit bitmap:
>> 
>> Should be folded into the previous patch, otherwise the previous patch breaks
>> level interrupts.
> OK
>> 
>> >
>> > [Option 1]
>> > Ideally only level triggered irq requires a hook in vLAPIC EOI write,
>> > so that vIOAPIC EOI is triggered and emulated. So the simplest
>> > approach is to manipulate EOI exit bitmap when vLAPIC acks a new
>> > interrupt, based on value of TMR. There're several corner cases worthy
>> > of note though:
>> >
>> >   - KVM has specific notifier hooks on vIOAPIC EOI path. So far two
>> >     sources use it: INT-based device passthrough and PIT pending
>> >     timers. For the former, it's virtually wired to vIOAPIC and
>> >     thus TMR already covers it. PIT is special here, which is an
>> >     edge triggered source. But since other timer sources like
>> >     vLAPIC timer don't require this notifier hook, possibly PIT
>> >     can be relaxed in the future too.
>> 
>> I would like to switch to changing the timer frequency when we need to catch
>> up.  But that can be done later.
>> 
>> >
>> >   - posted interrupt will update TMR directly, w/o chance for KVM
>> >     to update EOI exit bitmap accordingly. This becomes a gap
>> 
>> Why not? we know what vector the PIT is wired to.
> What PIT case are you talking about? Can you elaborate it?
>> 

The PIT output is wired to the IOAPIC, and has an ack notifier set so we
are notified on EOI (and can reinject missing ticks).

All other interrupts for which we need EOI notification are always level
triggered, and we have ack notifiers for them too.




-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit bitmap
  2012-09-06 16:37 ` Avi Kivity
  2012-09-14 14:19   ` Li, Jiongxi
@ 2012-09-17 11:28   ` Li, Jiongxi
  2012-09-19 14:45     ` Avi Kivity
  1 sibling, 1 reply; 6+ messages in thread
From: Li, Jiongxi @ 2012-09-17 11:28 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm@vger.kernel.org



> -----Original Message-----
> From: Avi Kivity [mailto:avi@redhat.com]
> Sent: Friday, September 07, 2012 12:38 AM
> To: Li, Jiongxi
> Cc: kvm@vger.kernel.org
> Subject: Re: [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit
> bitmap
> 
> On 09/05/2012 08:41 AM, Li, Jiongxi wrote:
> > With APICv virtual interrupt delivery feature, EOI write from non root
> > mode doesn't cause VM-Exit unless set in EOI exit bitmap VMCS field.
> > Basically there're two methods to manipulate EOI exit bitmap:
> 
> Should be folded into the previous patch, otherwise the previous patch breaks
> level interrupts.
> 
> >
> > [Option 1]
> > Ideally only level triggered irq requires a hook in vLAPIC EOI write,
> > so that vIOAPIC EOI is triggered and emulated. So the simplest
> > approach is to manipulate EOI exit bitmap when vLAPIC acks a new
> > interrupt, based on value of TMR. There're several corner cases worthy
> > of note though:
> >
> >   - KVM has specific notifier hooks on vIOAPIC EOI path. So far two
> >     sources use it: INT-based device passthrough and PIT pending
> >     timers. For the former, it's virtually wired to vIOAPIC and
> >     thus TMR already covers it. PIT is special here, which is an
> >     edge triggered source. But since other timer sources like
> >     vLAPIC timer don't require this notifier hook, possibly PIT
> >     can be relaxed in the future too.
> 
> I would like to switch to changing the timer frequency when we need to catch
> up.  But that can be done later.
> 
> >
> >   - posted interrupt will update TMR directly, w/o chance for KVM
> >     to update EOI exit bitmap accordingly. This becomes a gap
> 
> Why not? we know what vector the PIT is wired to.
> 
> >
> > [Option 2]
> > Indicate EOI exit bitmap requirement ('need_eoi') directly from every
> > interrupt source device, and then check this requirement when vLAPIC
> > acks a new pending interrupt. This requires more intrusive changes to
> > current vLAPIC/vIOAPIC logic, so that the "irq_source_id" indicating
> > source of interrupt is passed through from origination point to vLAPIC
> > ack point. For natual requirement like vIOAPIC level triggered
> > entries, it can be implicitly deduced.
> > On the other hand for non-natural requirements like aformentioned PIT
> > or posted interrupt, this approach can handle it efficiently.
> >
> > For simplicity reason, now option 1 is used which should be enough to
> > test MSI-based device passthrough.
> 
> You can change kvm_register_irq_ack_notifier() to call the ioapic and pic to find
> out what vectors need EOI exits.
> 
> (alternatively, if we fix the PIT, then we only need ack notifiers for level
> interrupts).
> 
> >
> > Signed-off-by: Kevin Tian <kevin.tian@intel.com>
> > Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> > ---
> >  arch/x86/include/asm/kvm_host.h |    1 +
> >  arch/x86/kvm/lapic.c            |    7 ++++++-
> >  arch/x86/kvm/vmx.c              |   37
> +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 44 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h
> > b/arch/x86/include/asm/kvm_host.h index ef74df5..4e06a82 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -671,6 +671,7 @@ struct kvm_x86_ops {
> >  	void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr);
> >  	int (*has_virtual_interrupt_delivery)(struct kvm_vcpu *vcpu);
> >  	void (*update_irq)(struct kvm_vcpu *vcpu);
> > +	void (*set_eoi_exitmap)(struct kvm_vcpu *vcpu, int vector, int
> > +need_eoi);
> >  	int (*set_tss_addr)(struct kvm *kvm, unsigned int addr);
> >  	int (*get_tdp_level)(void);
> >  	u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio);
> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index
> > d203501..4058384 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -499,8 +499,13 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
> int delivery_mode,
> >  		if (trig_mode) {
> >  			apic_debug("level trig mode for vector %d", vector);
> >  			apic_set_vector(vector, apic->regs + APIC_TMR);
> > -		} else
> > +			if (kvm_apic_vid_enabled(vcpu))
> > +				kvm_x86_ops->set_eoi_exitmap(vcpu, vector, 1);
> > +		} else {
> >  			apic_clear_vector(vector, apic->regs + APIC_TMR);
> > +			if (kvm_apic_vid_enabled(vcpu))
> > +				kvm_x86_ops->set_eoi_exitmap(vcpu, vector, 0);
> > +		}
> 
> This is way too late.  The flow should come from the IOAPIC and PIC, when
> setting up an irq, to the local APIC.
> 
I just wonder why you think it is too late to do that. Out of the reason that If post interrupt is enabled, here won't be called? Or because we can't set eoi bitmap for PIT here just according to TMR and needs to set it in a place where can recognize the PIT vector?
> 
> 
> --
> error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit bitmap
  2012-09-17 11:28   ` Li, Jiongxi
@ 2012-09-19 14:45     ` Avi Kivity
  0 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2012-09-19 14:45 UTC (permalink / raw)
  To: Li, Jiongxi; +Cc: kvm@vger.kernel.org

On 09/17/2012 02:28 PM, Li, Jiongxi wrote:
>> > +++ b/arch/x86/kvm/lapic.c
>> > @@ -499,8 +499,13 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
>> int delivery_mode,
>> >  		if (trig_mode) {
>> >  			apic_debug("level trig mode for vector %d", vector);
>> >  			apic_set_vector(vector, apic->regs + APIC_TMR);
>> > -		} else
>> > +			if (kvm_apic_vid_enabled(vcpu))
>> > +				kvm_x86_ops->set_eoi_exitmap(vcpu, vector, 1);
>> > +		} else {
>> >  			apic_clear_vector(vector, apic->regs + APIC_TMR);
>> > +			if (kvm_apic_vid_enabled(vcpu))
>> > +				kvm_x86_ops->set_eoi_exitmap(vcpu, vector, 0);
>> > +		}
>> 
>> This is way too late.  The flow should come from the IOAPIC and PIC, when
>> setting up an irq, to the local APIC.
>> 
> I just wonder why you think it is too late to do that. Out of the reason
> that If post interrupt is enabled, here won't be called?

That was not the reason, but it is true as well.  With posted interrupts
this path is bypassed completely.

> Or because we
> can't set eoi bitmap for PIT here just according to TMR and needs to set
> it in a place where can recognize the PIT vector?

Yes.  When we register an irq ack notifier we know the gsi and therefore
the vector, and can update the eoi exit bitmap accordingly.  This works
with posted interrupts as well.  Note we have to rebuild the table when
the ioapic redirection bitmap is updated (or the PIC irq base, or ELCR
change, etc.)

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-09-19 14:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-05  5:41 [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit bitmap Li, Jiongxi
2012-09-06 16:37 ` Avi Kivity
2012-09-14 14:19   ` Li, Jiongxi
2012-09-16  9:53     ` Avi Kivity
2012-09-17 11:28   ` Li, Jiongxi
2012-09-19 14:45     ` Avi Kivity

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox