Re: [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit bitmap
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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit bitmap
-Original Message-
From: Avi Kivity [mailto:a...@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.t...@intel.com
Signed-off-by: Jiongxi Li jiongxi...@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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord
Re: [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit bitmap
On 09/14/2012 05:19 PM, Li, Jiongxi wrote: Sorry for the late response -Original Message- From: Avi Kivity [mailto:a...@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 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/5]KVM:x86, apicv: add interface for poking EOI exit bitmap
Sorry for the late response
-Original Message-
From: Avi Kivity [mailto:a...@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.t...@intel.com
Signed-off-by: Jiongxi Li jiongxi...@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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info
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.t...@intel.com
Signed-off-by: Jiongxi Li jiongxi...@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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
