p***@xen.org
2018-11-02 11:22:04 UTC
commit 45cb9a4123b5550eb1f84846fe5482acae1c13a3
Author: Jan Beulich <***@suse.com>
AuthorDate: Fri Nov 2 12:15:33 2018 +0100
Commit: Jan Beulich <***@suse.com>
CommitDate: Fri Nov 2 12:15:33 2018 +0100
VMX: fix vmx_handle_eoi()
In commit 303066fdb1e ("VMX: fix interaction of APIC-V and Viridian
emulation") I screwed up: Instead of clearing SVI, other ISR bits
should be taken into account.
Introduce a new helper set_svi(), split out of vmx_process_isr(), and
use it also from vmx_handle_eoi().
Following the problems in vmx_intr_assist() (see the still present big
block of debugging code there) also warn (once) if EOI'd vector and
original SVI don't match.
Signed-off-by: Jan Beulich <***@suse.com>
Reviewed-by: Chao Gao <***@intel.com>
Acked-by: Kevin Tian <***@intel.com>
Acked-by: Andrew Cooper <***@citrix.com>
---
xen/arch/x86/hvm/vlapic.c | 2 +-
xen/arch/x86/hvm/vmx/vmx.c | 28 ++++++++++++++++++----------
xen/include/asm-x86/hvm/hvm.h | 2 +-
3 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 8a4a17311f..bdf946b25a 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -448,7 +448,7 @@ void vlapic_EOI_set(struct vlapic *vlapic)
vlapic_clear_vector(vector, &vlapic->regs->data[APIC_ISR]);
if ( hvm_funcs.handle_eoi )
- hvm_funcs.handle_eoi(vector);
+ hvm_funcs.handle_eoi(vector, vlapic_find_highest_isr(vlapic));
vlapic_handle_EOI(vlapic, vector);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index a6e6dc6c45..c9406d02c1 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1949,17 +1949,14 @@ static void vmx_update_eoi_exit_bitmap(struct vcpu *v, u8 vector, u8 trig)
vmx_clear_eoi_exit_bitmap(v, vector);
}
-static void vmx_process_isr(int isr, struct vcpu *v)
+static u8 set_svi(int isr)
{
unsigned long status;
u8 old;
- unsigned int i;
- const struct vlapic *vlapic = vcpu_vlapic(v);
if ( isr < 0 )
isr = 0;
- vmx_vmcs_enter(v);
__vmread(GUEST_INTR_STATUS, &status);
old = status >> VMX_GUEST_INTR_STATUS_SVI_OFFSET;
if ( isr != old )
@@ -1969,6 +1966,18 @@ static void vmx_process_isr(int isr, struct vcpu *v)
__vmwrite(GUEST_INTR_STATUS, status);
}
+ return old;
+}
+
+static void vmx_process_isr(int isr, struct vcpu *v)
+{
+ unsigned int i;
+ const struct vlapic *vlapic = vcpu_vlapic(v);
+
+ vmx_vmcs_enter(v);
+
+ set_svi(isr);
+
/*
* Theoretically, only level triggered interrupts can have their
* corresponding bits set in the eoi exit bitmap. That is, the bits
@@ -2119,14 +2128,13 @@ static bool vmx_test_pir(const struct vcpu *v, uint8_t vec)
return pi_test_pir(vec, &v->arch.hvm.vmx.pi_desc);
}
-static void vmx_handle_eoi(u8 vector)
+static void vmx_handle_eoi(uint8_t vector, int isr)
{
- unsigned long status;
+ uint8_t old_svi = set_svi(isr);
+ static bool warned;
- /* We need to clear the SVI field. */
- __vmread(GUEST_INTR_STATUS, &status);
- status &= VMX_GUEST_INTR_STATUS_SUBFIELD_BITMASK;
- __vmwrite(GUEST_INTR_STATUS, status);
+ if ( vector != old_svi && !test_and_set_bool(warned) )
+ printk(XENLOG_WARNING "EOI for %02x but SVI=%02x\n", vector, old_svi);
}
static void vmx_enable_msr_interception(struct domain *d, uint32_t msr)
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index e2cbcf53db..cd8acd93e7 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -201,7 +201,7 @@ struct hvm_function_table {
void (*deliver_posted_intr)(struct vcpu *v, u8 vector);
void (*sync_pir_to_irr)(struct vcpu *v);
bool (*test_pir)(const struct vcpu *v, uint8_t vector);
- void (*handle_eoi)(u8 vector);
+ void (*handle_eoi)(uint8_t vector, int isr);
/*Walk nested p2m */
int (*nhvm_hap_walk_L1_p2m)(struct vcpu *v, paddr_t L2_gpa,
--
generated by git-patchbot for /home/xen/git/xen.git#staging
Author: Jan Beulich <***@suse.com>
AuthorDate: Fri Nov 2 12:15:33 2018 +0100
Commit: Jan Beulich <***@suse.com>
CommitDate: Fri Nov 2 12:15:33 2018 +0100
VMX: fix vmx_handle_eoi()
In commit 303066fdb1e ("VMX: fix interaction of APIC-V and Viridian
emulation") I screwed up: Instead of clearing SVI, other ISR bits
should be taken into account.
Introduce a new helper set_svi(), split out of vmx_process_isr(), and
use it also from vmx_handle_eoi().
Following the problems in vmx_intr_assist() (see the still present big
block of debugging code there) also warn (once) if EOI'd vector and
original SVI don't match.
Signed-off-by: Jan Beulich <***@suse.com>
Reviewed-by: Chao Gao <***@intel.com>
Acked-by: Kevin Tian <***@intel.com>
Acked-by: Andrew Cooper <***@citrix.com>
---
xen/arch/x86/hvm/vlapic.c | 2 +-
xen/arch/x86/hvm/vmx/vmx.c | 28 ++++++++++++++++++----------
xen/include/asm-x86/hvm/hvm.h | 2 +-
3 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 8a4a17311f..bdf946b25a 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -448,7 +448,7 @@ void vlapic_EOI_set(struct vlapic *vlapic)
vlapic_clear_vector(vector, &vlapic->regs->data[APIC_ISR]);
if ( hvm_funcs.handle_eoi )
- hvm_funcs.handle_eoi(vector);
+ hvm_funcs.handle_eoi(vector, vlapic_find_highest_isr(vlapic));
vlapic_handle_EOI(vlapic, vector);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index a6e6dc6c45..c9406d02c1 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1949,17 +1949,14 @@ static void vmx_update_eoi_exit_bitmap(struct vcpu *v, u8 vector, u8 trig)
vmx_clear_eoi_exit_bitmap(v, vector);
}
-static void vmx_process_isr(int isr, struct vcpu *v)
+static u8 set_svi(int isr)
{
unsigned long status;
u8 old;
- unsigned int i;
- const struct vlapic *vlapic = vcpu_vlapic(v);
if ( isr < 0 )
isr = 0;
- vmx_vmcs_enter(v);
__vmread(GUEST_INTR_STATUS, &status);
old = status >> VMX_GUEST_INTR_STATUS_SVI_OFFSET;
if ( isr != old )
@@ -1969,6 +1966,18 @@ static void vmx_process_isr(int isr, struct vcpu *v)
__vmwrite(GUEST_INTR_STATUS, status);
}
+ return old;
+}
+
+static void vmx_process_isr(int isr, struct vcpu *v)
+{
+ unsigned int i;
+ const struct vlapic *vlapic = vcpu_vlapic(v);
+
+ vmx_vmcs_enter(v);
+
+ set_svi(isr);
+
/*
* Theoretically, only level triggered interrupts can have their
* corresponding bits set in the eoi exit bitmap. That is, the bits
@@ -2119,14 +2128,13 @@ static bool vmx_test_pir(const struct vcpu *v, uint8_t vec)
return pi_test_pir(vec, &v->arch.hvm.vmx.pi_desc);
}
-static void vmx_handle_eoi(u8 vector)
+static void vmx_handle_eoi(uint8_t vector, int isr)
{
- unsigned long status;
+ uint8_t old_svi = set_svi(isr);
+ static bool warned;
- /* We need to clear the SVI field. */
- __vmread(GUEST_INTR_STATUS, &status);
- status &= VMX_GUEST_INTR_STATUS_SUBFIELD_BITMASK;
- __vmwrite(GUEST_INTR_STATUS, status);
+ if ( vector != old_svi && !test_and_set_bool(warned) )
+ printk(XENLOG_WARNING "EOI for %02x but SVI=%02x\n", vector, old_svi);
}
static void vmx_enable_msr_interception(struct domain *d, uint32_t msr)
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index e2cbcf53db..cd8acd93e7 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -201,7 +201,7 @@ struct hvm_function_table {
void (*deliver_posted_intr)(struct vcpu *v, u8 vector);
void (*sync_pir_to_irr)(struct vcpu *v);
bool (*test_pir)(const struct vcpu *v, uint8_t vector);
- void (*handle_eoi)(u8 vector);
+ void (*handle_eoi)(uint8_t vector, int isr);
/*Walk nested p2m */
int (*nhvm_hap_walk_L1_p2m)(struct vcpu *v, paddr_t L2_gpa,
--
generated by git-patchbot for /home/xen/git/xen.git#staging