Re: [PATCH 2/5]KVM:x86, apicv: adjust for virtual interrupt delivery
On 09/14/2012 05:15 PM, Li, Jiongxi wrote:
@@ -5293,16 +5300,27 @@ static int vcpu_enter_guest(struct kvm_vcpu
*vcpu)
}
if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
+ /* update archtecture specific hints for APIC virtual interrupt
delivery
*/
+ if (kvm_apic_vid_enabled(vcpu))
+ kvm_x86_ops-update_irq(vcpu);
+
Not defined.
This function is defined in patch 3/5. Because virtual interrupt delivery is
not enabled in this patch. So this function is not called. Since we will
enable this feature by default, so maybe we can merge PATCH 2,3,4 together
into one patch.
That will make it hard to review. You might try to build the eoi exit
bitmap in the first patch, and do the rest (including posted interrupts)
in a following patch. If you can split it further, it will be helpful.
inject_pending_event(vcpu);
/* enable NMI/IRQ window open exits if needed */
if (vcpu-arch.nmi_pending)
kvm_x86_ops-enable_nmi_window(vcpu);
- else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
+ else if (kvm_apic_vid_enabled(vcpu)) {
+ if (kvm_cpu_has_interrupt_apic_vid(vcpu))
+ kvm_x86_ops-enable_irq_window(vcpu);
+ } else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
kvm_x86_ops-enable_irq_window(vcpu);
if (kvm_lapic_enabled(vcpu)) {
- update_cr8_intercept(vcpu);
+ /* no need for tpr_threshold update if APIC virtual
+ * interrupt delivery is enabled
+ */
+ if (!kvm_apic_vid_enabled(vcpu))
+ update_cr8_intercept(vcpu);
Perhaps the arch function should do the ignoring.
You means putting the 'vid_enabled' judgement in
'kvm_x86_ops-update_cr8_intercept'? Is it just out of the reason that
reducing the code change in common code?
One option is to replace all the code above with
kvm_x86_ops-update_irq()
where
static void vmx_update_irq()
{
if (vid) {
... do vid stuff ...
} else
kvm_process_interrupt_queue(); // all the non-vid code above,
in a function
}
So instead of kvm_apic_vid_enabled() scattered throughout the code we
have just one check. svm_update_irq() can just call
kvm_process_interrupt_queue() and work as before, and later we can add
the AVIC code (the svm equivalent of APIC-V).
--
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 2/5]KVM:x86, apicv: adjust for virtual interrupt delivery
On 09/17/2012 02:28 PM, Li, Jiongxi wrote:
+} else if (kvm_apic_vid_enabled(vcpu)) {
+if (kvm_cpu_has_interrupt_apic_vid(vcpu)
+kvm_x86_ops-interrupt_allowed(vcpu)) {
+kvm_queue_interrupt(vcpu,
+kvm_cpu_get_interrupt_apic_vid(vcpu),
false);
+kvm_x86_ops-set_irq(vcpu);
+}
It may be simpler to change kvm_cpu_{has,get}_interrupt to ignore the
apic if virtual interrupt delivery is enabled.
OKs
Kvm_cpu_has_interrupt is also called in other place, no just used to judge
whether
to inject interrupt manually. For instance, it is called in
kvm_arch_vcpu_runnable.
In that case, apic can't be ingored. So for safety, I think it is
better to use another
function here other than change the original kvm_cpu_has_interrupt
function.
Good point.
We can call them *_extint(), since that's what they're accepting.
--
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 2/5]KVM:x86, apicv: adjust for virtual interrupt delivery
-Original Message-
From: Li, Jiongxi
Sent: Friday, September 14, 2012 10:16 PM
To: 'Avi Kivity'
Cc: kvm@vger.kernel.org
Subject: RE: [PATCH 2/5]KVM:x86, apicv: adjust for virtual interrupt delivery
Sorry for the late response
-Original Message-
From: Avi Kivity [mailto:a...@redhat.com]
Sent: Friday, September 07, 2012 12:22 AM
To: Li, Jiongxi
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH 2/5]KVM:x86, apicv: adjust for virtual interrupt
delivery
On 09/05/2012 08:41 AM, Li, Jiongxi wrote:
Virtual interrupt delivery avoids KVM to inject vAPIC interrupts
manually, which is fully taken care of by the hardware. This needs
some special awareness into existing interrupr injection path:
- for pending interrupt, instead of direct injection, we may need
update architecture specific indicators before resuming to guest.
- A pending interrupt, which is masked by ISR, should be also
considered in above update action, since hardware will decide
when to inject it at right time. Current has_interrupt and
get_interrupt only returns a valid vector from injection p.o.v.
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5194,6 +5194,13 @@ static void inject_pending_event(struct
kvm_vcpu *vcpu)
vcpu-arch.nmi_injected = true;
kvm_x86_ops-set_nmi(vcpu);
}
+ } else if (kvm_apic_vid_enabled(vcpu)) {
+ if (kvm_cpu_has_interrupt_apic_vid(vcpu)
+ kvm_x86_ops-interrupt_allowed(vcpu)) {
+ kvm_queue_interrupt(vcpu,
+ kvm_cpu_get_interrupt_apic_vid(vcpu), false);
+ kvm_x86_ops-set_irq(vcpu);
+ }
It may be simpler to change kvm_cpu_{has,get}_interrupt to ignore the
apic if virtual interrupt delivery is enabled.
OKs
Kvm_cpu_has_interrupt is also called in other place, no just used to judge
whether to inject interrupt manually. For instance, it is called in
kvm_arch_vcpu_runnable. In that case, apic can't be ingored. So for safety, I
think it is better to use another function here other than change the original
kvm_cpu_has_interrupt function.
@@ -5293,16 +5300,27 @@ static int vcpu_enter_guest(struct kvm_vcpu
*vcpu)
}
if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
+ /* update archtecture specific hints for APIC virtual interrupt
+delivery
*/
+ if (kvm_apic_vid_enabled(vcpu))
+ kvm_x86_ops-update_irq(vcpu);
+
Not defined.
This function is defined in patch 3/5. Because virtual interrupt delivery is
not
enabled in this patch. So this function is not called. Since we will enable
this
feature by default, so maybe we can merge PATCH 2,3,4 together into one
patch.
inject_pending_event(vcpu);
/* enable NMI/IRQ window open exits if needed */
if (vcpu-arch.nmi_pending)
kvm_x86_ops-enable_nmi_window(vcpu);
- else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
+ else if (kvm_apic_vid_enabled(vcpu)) {
+ if (kvm_cpu_has_interrupt_apic_vid(vcpu))
+ kvm_x86_ops-enable_irq_window(vcpu);
+ } else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
kvm_x86_ops-enable_irq_window(vcpu);
if (kvm_lapic_enabled(vcpu)) {
- update_cr8_intercept(vcpu);
+ /* no need for tpr_threshold update if APIC virtual
+ * interrupt delivery is enabled
+ */
+ if (!kvm_apic_vid_enabled(vcpu))
+ update_cr8_intercept(vcpu);
Perhaps the arch function should do the ignoring.
You means putting the 'vid_enabled' judgement in
'kvm_x86_ops-update_cr8_intercept'? Is it just out of the reason that
reducing the code change in common code?
kvm_lapic_sync_to_vapic(vcpu);
}
}
--
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 2/5]KVM:x86, apicv: adjust for virtual interrupt delivery
Sorry for the late response
-Original Message-
From: Avi Kivity [mailto:a...@redhat.com]
Sent: Friday, September 07, 2012 12:22 AM
To: Li, Jiongxi
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH 2/5]KVM:x86, apicv: adjust for virtual interrupt delivery
On 09/05/2012 08:41 AM, Li, Jiongxi wrote:
Virtual interrupt delivery avoids KVM to inject vAPIC interrupts
manually, which is fully taken care of by the hardware. This needs
some special awareness into existing interrupr injection path:
- for pending interrupt, instead of direct injection, we may need
update architecture specific indicators before resuming to guest.
- A pending interrupt, which is masked by ISR, should be also
considered in above update action, since hardware will decide
when to inject it at right time. Current has_interrupt and
get_interrupt only returns a valid vector from injection p.o.v.
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5194,6 +5194,13 @@ static void inject_pending_event(struct
kvm_vcpu *vcpu)
vcpu-arch.nmi_injected = true;
kvm_x86_ops-set_nmi(vcpu);
}
+ } else if (kvm_apic_vid_enabled(vcpu)) {
+ if (kvm_cpu_has_interrupt_apic_vid(vcpu)
+ kvm_x86_ops-interrupt_allowed(vcpu)) {
+ kvm_queue_interrupt(vcpu,
+ kvm_cpu_get_interrupt_apic_vid(vcpu), false);
+ kvm_x86_ops-set_irq(vcpu);
+ }
It may be simpler to change kvm_cpu_{has,get}_interrupt to ignore the apic if
virtual interrupt delivery is enabled.
OKs
@@ -5293,16 +5300,27 @@ static int vcpu_enter_guest(struct kvm_vcpu
*vcpu)
}
if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
+ /* update archtecture specific hints for APIC virtual interrupt
delivery
*/
+ if (kvm_apic_vid_enabled(vcpu))
+ kvm_x86_ops-update_irq(vcpu);
+
Not defined.
This function is defined in patch 3/5. Because virtual interrupt delivery is
not enabled in this patch. So this function is not called. Since we will enable
this feature by default, so maybe we can merge PATCH 2,3,4 together into one
patch.
inject_pending_event(vcpu);
/* enable NMI/IRQ window open exits if needed */
if (vcpu-arch.nmi_pending)
kvm_x86_ops-enable_nmi_window(vcpu);
- else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
+ else if (kvm_apic_vid_enabled(vcpu)) {
+ if (kvm_cpu_has_interrupt_apic_vid(vcpu))
+ kvm_x86_ops-enable_irq_window(vcpu);
+ } else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
kvm_x86_ops-enable_irq_window(vcpu);
if (kvm_lapic_enabled(vcpu)) {
- update_cr8_intercept(vcpu);
+ /* no need for tpr_threshold update if APIC virtual
+* interrupt delivery is enabled
+*/
+ if (!kvm_apic_vid_enabled(vcpu))
+ update_cr8_intercept(vcpu);
Perhaps the arch function should do the ignoring.
You means putting the 'vid_enabled' judgement in
'kvm_x86_ops-update_cr8_intercept'? Is it just out of the reason that reducing
the code change in common code?
kvm_lapic_sync_to_vapic(vcpu);
}
}
--
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 2/5]KVM:x86, apicv: adjust for virtual interrupt delivery
On 09/05/2012 08:41 AM, Li, Jiongxi wrote:
Virtual interrupt delivery avoids KVM to inject vAPIC interrupts
manually, which is fully taken care of by the hardware. This needs
some special awareness into existing interrupr injection path:
- for pending interrupt, instead of direct injection, we may need
update architecture specific indicators before resuming to guest.
- A pending interrupt, which is masked by ISR, should be also
considered in above update action, since hardware will decide
when to inject it at right time. Current has_interrupt and
get_interrupt only returns a valid vector from injection p.o.v.
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5194,6 +5194,13 @@ static void inject_pending_event(struct kvm_vcpu *vcpu)
vcpu-arch.nmi_injected = true;
kvm_x86_ops-set_nmi(vcpu);
}
+ } else if (kvm_apic_vid_enabled(vcpu)) {
+ if (kvm_cpu_has_interrupt_apic_vid(vcpu)
+ kvm_x86_ops-interrupt_allowed(vcpu)) {
+ kvm_queue_interrupt(vcpu,
+ kvm_cpu_get_interrupt_apic_vid(vcpu), false);
+ kvm_x86_ops-set_irq(vcpu);
+ }
It may be simpler to change kvm_cpu_{has,get}_interrupt to ignore the
apic if virtual interrupt delivery is enabled.
@@ -5293,16 +5300,27 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
}
if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
+ /* update archtecture specific hints for APIC virtual interrupt
delivery */
+ if (kvm_apic_vid_enabled(vcpu))
+ kvm_x86_ops-update_irq(vcpu);
+
Not defined.
inject_pending_event(vcpu);
/* enable NMI/IRQ window open exits if needed */
if (vcpu-arch.nmi_pending)
kvm_x86_ops-enable_nmi_window(vcpu);
- else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
+ else if (kvm_apic_vid_enabled(vcpu)) {
+ if (kvm_cpu_has_interrupt_apic_vid(vcpu))
+ kvm_x86_ops-enable_irq_window(vcpu);
+ } else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
kvm_x86_ops-enable_irq_window(vcpu);
if (kvm_lapic_enabled(vcpu)) {
- update_cr8_intercept(vcpu);
+ /* no need for tpr_threshold update if APIC virtual
+ * interrupt delivery is enabled
+ */
+ if (!kvm_apic_vid_enabled(vcpu))
+ update_cr8_intercept(vcpu);
Perhaps the arch function should do the ignoring.
kvm_lapic_sync_to_vapic(vcpu);
}
}
--
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
