* RE: [PATCH] KVM: x86: keep EOI exit bitmap accurate before loading it.
2014-08-26 16:27 [PATCH] KVM: x86: keep EOI exit bitmap accurate before loading it Wei Wang
@ 2014-08-26 9:02 ` Wang, Wei W
2014-08-26 21:01 ` Alex Williamson
2014-08-26 9:47 ` Chen, Tiejun
2014-08-26 11:00 ` Paolo Bonzini
2 siblings, 1 reply; 6+ messages in thread
From: Wang, Wei W @ 2014-08-26 9:02 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, alex.williamson, Zhang, Yang Z
Hi Alex,
Could you have a test if the two patches have solved the bug?
Thanks,
Wei
-----Original Message-----
From: Wang, Wei W
Sent: Wednesday, August 27, 2014 12:28 AM
To: kvm@vger.kernel.org
Cc: pbonzini@redhat.com; alex.williamson@redhat.com; Wang, Wei W; Zhang, Yang Z
Subject: [PATCH] KVM: x86: keep EOI exit bitmap accurate before loading it.
Guest may mask the IOAPIC entry before issue EOI. In such case, EOI will not be intercepted by hypervisor, since the corrensponding bit in eoi exit bitmap is not set after the masking of IOAPIC entry.
The solution here is to OR EOI_exit_bitmap with tmr.
Tested-by: Rongrong Liu <rongrongx.liu@intel.com>
Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
arch/x86/kvm/lapic.c | 9 +++++++++
arch/x86/kvm/lapic.h | 2 ++
arch/x86/kvm/x86.c | 1 +
virt/kvm/ioapic.c | 6 +++---
4 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 93c2e93..759d24e 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -533,6 +533,15 @@ void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr)
}
}
+void kvm_apic_update_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
+ u32 *tmr)
+{
+ u32 i;
+
+ for (i = 0; i < 8; i++)
+ *((u32 *)eoi_exit_bitmap + i) |= tmr[i]; }
+
static void apic_update_ppr(struct kvm_lapic *apic) {
u32 tpr, isrv, ppr, old_ppr;
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h index 6a11845..eda7be7 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -55,6 +55,8 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr); void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
+void kvm_apic_update_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
+ u32 *tmr);
int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest); int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda); int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq, diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index d401684..4042bc0 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5992,6 +5992,7 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu)
kvm_ioapic_scan_entry(vcpu, eoi_exit_bitmap, tmr);
kvm_apic_update_tmr(vcpu, tmr);
+ kvm_apic_update_eoi_exitmap(vcpu, eoi_exit_bitmap, tmr);
kvm_x86_ops->load_eoi_exitmap(vcpu, eoi_exit_bitmap); }
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c index e8ce34c..ea5f697 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -254,9 +254,9 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
spin_lock(&ioapic->lock);
for (index = 0; index < IOAPIC_NUM_PINS; index++) {
e = &ioapic->redirtbl[index];
- if (e->fields.trig_mode == IOAPIC_LEVEL_TRIG ||
- kvm_irq_has_notifier(ioapic->kvm, KVM_IRQCHIP_IOAPIC, index) ||
- index == RTC_GSI) {
+ if (!e->fields.mask && e->fields.trig_mode == IOAPIC_LEVEL_TRIG
+ || kvm_irq_has_notifier(ioapic->kvm, KVM_IRQCHIP_IOAPIC,
+ index) || index == RTC_GSI) {
if (kvm_apic_match_dest(vcpu, NULL, 0,
e->fields.dest_id, e->fields.dest_mode)) {
__set_bit(e->fields.vector,
--
1.7.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: x86: keep EOI exit bitmap accurate before loading it.
2014-08-26 9:02 ` Wang, Wei W
@ 2014-08-26 21:01 ` Alex Williamson
2014-08-27 6:55 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: Alex Williamson @ 2014-08-26 21:01 UTC (permalink / raw)
To: Wang, Wei W; +Cc: kvm, pbonzini, Zhang, Yang Z
On Tue, 2014-08-26 at 09:02 +0000, Wang, Wei W wrote:
> Hi Alex,
> Could you have a test if the two patches have solved the bug?
What case was missed by the previous single patch that is accounted for
now? Some sort of comment in the update_tmr function would be nice to
explain the update, along with removing the unnecessary arrays as Paolo
pointed out, and I commented on in the previous version. Thanks,
Alex
> -----Original Message-----
> From: Wang, Wei W
> Sent: Wednesday, August 27, 2014 12:28 AM
> To: kvm@vger.kernel.org
> Cc: pbonzini@redhat.com; alex.williamson@redhat.com; Wang, Wei W; Zhang, Yang Z
> Subject: [PATCH] KVM: x86: keep EOI exit bitmap accurate before loading it.
>
> Guest may mask the IOAPIC entry before issue EOI. In such case, EOI will not be intercepted by hypervisor, since the corrensponding bit in eoi exit bitmap is not set after the masking of IOAPIC entry.
>
> The solution here is to OR EOI_exit_bitmap with tmr.
>
> Tested-by: Rongrong Liu <rongrongx.liu@intel.com>
> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> ---
> arch/x86/kvm/lapic.c | 9 +++++++++
> arch/x86/kvm/lapic.h | 2 ++
> arch/x86/kvm/x86.c | 1 +
> virt/kvm/ioapic.c | 6 +++---
> 4 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 93c2e93..759d24e 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -533,6 +533,15 @@ void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr)
> }
> }
>
> +void kvm_apic_update_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
> + u32 *tmr)
> +{
> + u32 i;
> +
> + for (i = 0; i < 8; i++)
> + *((u32 *)eoi_exit_bitmap + i) |= tmr[i]; }
> +
> static void apic_update_ppr(struct kvm_lapic *apic) {
> u32 tpr, isrv, ppr, old_ppr;
> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h index 6a11845..eda7be7 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -55,6 +55,8 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
>
> void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr); void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
> +void kvm_apic_update_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
> + u32 *tmr);
> int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest); int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda); int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq, diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index d401684..4042bc0 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5992,6 +5992,7 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu)
>
> kvm_ioapic_scan_entry(vcpu, eoi_exit_bitmap, tmr);
> kvm_apic_update_tmr(vcpu, tmr);
> + kvm_apic_update_eoi_exitmap(vcpu, eoi_exit_bitmap, tmr);
> kvm_x86_ops->load_eoi_exitmap(vcpu, eoi_exit_bitmap); }
>
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c index e8ce34c..ea5f697 100644
> --- a/virt/kvm/ioapic.c
> +++ b/virt/kvm/ioapic.c
> @@ -254,9 +254,9 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
> spin_lock(&ioapic->lock);
> for (index = 0; index < IOAPIC_NUM_PINS; index++) {
> e = &ioapic->redirtbl[index];
> - if (e->fields.trig_mode == IOAPIC_LEVEL_TRIG ||
> - kvm_irq_has_notifier(ioapic->kvm, KVM_IRQCHIP_IOAPIC, index) ||
> - index == RTC_GSI) {
> + if (!e->fields.mask && e->fields.trig_mode == IOAPIC_LEVEL_TRIG
> + || kvm_irq_has_notifier(ioapic->kvm, KVM_IRQCHIP_IOAPIC,
> + index) || index == RTC_GSI) {
> if (kvm_apic_match_dest(vcpu, NULL, 0,
> e->fields.dest_id, e->fields.dest_mode)) {
> __set_bit(e->fields.vector,
> --
> 1.7.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: x86: keep EOI exit bitmap accurate before loading it.
2014-08-26 21:01 ` Alex Williamson
@ 2014-08-27 6:55 ` Paolo Bonzini
0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2014-08-27 6:55 UTC (permalink / raw)
To: Alex Williamson, Wang, Wei W; +Cc: kvm, Zhang, Yang Z
Il 26/08/2014 23:01, Alex Williamson ha scritto:
>> > Hi Alex,
>> > Could you have a test if the two patches have solved the bug?
> What case was missed by the previous single patch that is accounted for
> now
None, I asked to split the patch into TMR cleanup and actual bugfix. :)
Though I agree that the patches are lacking comments.
Paolo
> Some sort of comment in the update_tmr function would be nice to
> explain the update, along with removing the unnecessary arrays as Paolo
> pointed out, and I commented on in the previous version. Thanks,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: keep EOI exit bitmap accurate before loading it.
2014-08-26 16:27 [PATCH] KVM: x86: keep EOI exit bitmap accurate before loading it Wei Wang
2014-08-26 9:02 ` Wang, Wei W
@ 2014-08-26 9:47 ` Chen, Tiejun
2014-08-26 11:00 ` Paolo Bonzini
2 siblings, 0 replies; 6+ messages in thread
From: Chen, Tiejun @ 2014-08-26 9:47 UTC (permalink / raw)
To: Wei Wang, kvm; +Cc: pbonzini, alex.williamson, Yang Zhang
On 2014/8/27 0:27, Wei Wang wrote:
> Guest may mask the IOAPIC entry before issue EOI. In such case,
> EOI will not be intercepted by hypervisor, since the corrensponding
Looks this is always missed :)
s/corrensponding/corresponding
Tiejun
> bit in eoi exit bitmap is not set after the masking of IOAPIC entry.
>
> The solution here is to OR EOI_exit_bitmap with tmr.
>
> Tested-by: Rongrong Liu <rongrongx.liu@intel.com>
> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> ---
> arch/x86/kvm/lapic.c | 9 +++++++++
> arch/x86/kvm/lapic.h | 2 ++
> arch/x86/kvm/x86.c | 1 +
> virt/kvm/ioapic.c | 6 +++---
> 4 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 93c2e93..759d24e 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -533,6 +533,15 @@ void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr)
> }
> }
>
> +void kvm_apic_update_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
> + u32 *tmr)
> +{
> + u32 i;
> +
> + for (i = 0; i < 8; i++)
> + *((u32 *)eoi_exit_bitmap + i) |= tmr[i];
> +}
> +
> static void apic_update_ppr(struct kvm_lapic *apic)
> {
> u32 tpr, isrv, ppr, old_ppr;
> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> index 6a11845..eda7be7 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -55,6 +55,8 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
>
> void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr);
> void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
> +void kvm_apic_update_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
> + u32 *tmr);
> int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
> int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
> int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index d401684..4042bc0 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5992,6 +5992,7 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu)
>
> kvm_ioapic_scan_entry(vcpu, eoi_exit_bitmap, tmr);
> kvm_apic_update_tmr(vcpu, tmr);
> + kvm_apic_update_eoi_exitmap(vcpu, eoi_exit_bitmap, tmr);
> kvm_x86_ops->load_eoi_exitmap(vcpu, eoi_exit_bitmap);
> }
>
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> index e8ce34c..ea5f697 100644
> --- a/virt/kvm/ioapic.c
> +++ b/virt/kvm/ioapic.c
> @@ -254,9 +254,9 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
> spin_lock(&ioapic->lock);
> for (index = 0; index < IOAPIC_NUM_PINS; index++) {
> e = &ioapic->redirtbl[index];
> - if (e->fields.trig_mode == IOAPIC_LEVEL_TRIG ||
> - kvm_irq_has_notifier(ioapic->kvm, KVM_IRQCHIP_IOAPIC, index) ||
> - index == RTC_GSI) {
> + if (!e->fields.mask && e->fields.trig_mode == IOAPIC_LEVEL_TRIG
> + || kvm_irq_has_notifier(ioapic->kvm, KVM_IRQCHIP_IOAPIC,
> + index) || index == RTC_GSI) {
> if (kvm_apic_match_dest(vcpu, NULL, 0,
> e->fields.dest_id, e->fields.dest_mode)) {
> __set_bit(e->fields.vector,
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: x86: keep EOI exit bitmap accurate before loading it.
2014-08-26 16:27 [PATCH] KVM: x86: keep EOI exit bitmap accurate before loading it Wei Wang
2014-08-26 9:02 ` Wang, Wei W
2014-08-26 9:47 ` Chen, Tiejun
@ 2014-08-26 11:00 ` Paolo Bonzini
2 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2014-08-26 11:00 UTC (permalink / raw)
To: Wei Wang, kvm; +Cc: alex.williamson, Yang Zhang
Il 26/08/2014 18:27, Wei Wang ha scritto:
> Guest may mask the IOAPIC entry before issue EOI. In such case,
> EOI will not be intercepted by hypervisor, since the corrensponding
> bit in eoi exit bitmap is not set after the masking of IOAPIC entry.
>
> The solution here is to OR EOI_exit_bitmap with tmr.
>
> Tested-by: Rongrong Liu <rongrongx.liu@intel.com>
> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> ---
> arch/x86/kvm/lapic.c | 9 +++++++++
> arch/x86/kvm/lapic.h | 2 ++
> arch/x86/kvm/x86.c | 1 +
> virt/kvm/ioapic.c | 6 +++---
> 4 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 93c2e93..759d24e 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -533,6 +533,15 @@ void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr)
> }
> }
>
> +void kvm_apic_update_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
> + u32 *tmr)
> +{
> + u32 i;
> +
> + for (i = 0; i < 8; i++)
> + *((u32 *)eoi_exit_bitmap + i) |= tmr[i];
> +}
> +
> static void apic_update_ppr(struct kvm_lapic *apic)
> {
> u32 tpr, isrv, ppr, old_ppr;
> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> index 6a11845..eda7be7 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -55,6 +55,8 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
>
> void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr);
> void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
> +void kvm_apic_update_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
> + u32 *tmr);
> int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
> int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
> int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index d401684..4042bc0 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5992,6 +5992,7 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu)
>
> kvm_ioapic_scan_entry(vcpu, eoi_exit_bitmap, tmr);
> kvm_apic_update_tmr(vcpu, tmr);
> + kvm_apic_update_eoi_exitmap(vcpu, eoi_exit_bitmap, tmr);
As mentioned before, you can read the TMR registers from the APIC,
instead of modifying the tmr array in kvm_apic_update_tmr.
Paolo
> kvm_x86_ops->load_eoi_exitmap(vcpu, eoi_exit_bitmap);
> }
>
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> index e8ce34c..ea5f697 100644
> --- a/virt/kvm/ioapic.c
> +++ b/virt/kvm/ioapic.c
> @@ -254,9 +254,9 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
> spin_lock(&ioapic->lock);
> for (index = 0; index < IOAPIC_NUM_PINS; index++) {
> e = &ioapic->redirtbl[index];
> - if (e->fields.trig_mode == IOAPIC_LEVEL_TRIG ||
> - kvm_irq_has_notifier(ioapic->kvm, KVM_IRQCHIP_IOAPIC, index) ||
> - index == RTC_GSI) {
> + if (!e->fields.mask && e->fields.trig_mode == IOAPIC_LEVEL_TRIG
> + || kvm_irq_has_notifier(ioapic->kvm, KVM_IRQCHIP_IOAPIC,
> + index) || index == RTC_GSI) {
> if (kvm_apic_match_dest(vcpu, NULL, 0,
> e->fields.dest_id, e->fields.dest_mode)) {
> __set_bit(e->fields.vector,
>
^ permalink raw reply [flat|nested] 6+ messages in thread