|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v2 1/3] x86/hvm: Don't raise #GP behind the emulators back for MSR accesses
The current hvm_msr_{read,write}_intercept() infrastructure calls
hvm_inject_hw_exception() directly to latch a fault, and returns
X86EMUL_EXCEPTION to its caller.
This behaviour is problematic for the hvmemul_{read,write}_msr() paths, as the
fault is raised behind the back of the x86 emulator.
Alter the behaviour so hvm_msr_{read,write}_intercept() simply returns
X86EMUL_EXCEPTION, leaving the callers to actually inject the #GP fault.
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx>
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
CC: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
CC: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
v2:
* Substantial rebase
* Introduce __must_check for hvm_msr_{read,write}_intercept()
---
xen/arch/x86/hvm/emulate.c | 14 ++++++++++++--
xen/arch/x86/hvm/hvm.c | 7 ++++---
xen/arch/x86/hvm/svm/svm.c | 4 ++--
xen/arch/x86/hvm/vmx/vmx.c | 23 ++++++++++++++++++-----
xen/arch/x86/hvm/vmx/vvmx.c | 19 ++++++++++++++-----
xen/include/asm-x86/hvm/support.h | 12 +++++++++---
6 files changed, 59 insertions(+), 20 deletions(-)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 14f9b43..edcae5e 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1544,7 +1544,12 @@ static int hvmemul_read_msr(
uint64_t *val,
struct x86_emulate_ctxt *ctxt)
{
- return hvm_msr_read_intercept(reg, val);
+ int rc = hvm_msr_read_intercept(reg, val);
+
+ if ( rc == X86EMUL_EXCEPTION )
+ x86_emul_hw_exception(TRAP_gp_fault, 0, ctxt);
+
+ return rc;
}
static int hvmemul_write_msr(
@@ -1552,7 +1557,12 @@ static int hvmemul_write_msr(
uint64_t val,
struct x86_emulate_ctxt *ctxt)
{
- return hvm_msr_write_intercept(reg, val, 1);
+ int rc = hvm_msr_write_intercept(reg, val, 1);
+
+ if ( rc == X86EMUL_EXCEPTION )
+ x86_emul_hw_exception(TRAP_gp_fault, 0, ctxt);
+
+ return rc;
}
static int hvmemul_wbinvd(
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 6621d62..08855c2 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -518,7 +518,10 @@ void hvm_do_resume(struct vcpu *v)
if ( w->do_write.msr )
{
- hvm_msr_write_intercept(w->msr, w->value, 0);
+ if ( hvm_msr_write_intercept(w->msr, w->value, 0) ==
+ X86EMUL_EXCEPTION )
+ hvm_inject_hw_exception(TRAP_gp_fault, 0);
+
w->do_write.msr = 0;
}
@@ -3455,7 +3458,6 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t
*msr_content)
return ret;
gp_fault:
- hvm_inject_hw_exception(TRAP_gp_fault, 0);
ret = X86EMUL_EXCEPTION;
*msr_content = -1ull;
goto out;
@@ -3600,7 +3602,6 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t
msr_content,
return ret;
gp_fault:
- hvm_inject_hw_exception(TRAP_gp_fault, 0);
return X86EMUL_EXCEPTION;
}
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 894c457..b864535 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1744,7 +1744,6 @@ static int svm_msr_read_intercept(unsigned int msr,
uint64_t *msr_content)
return X86EMUL_OKAY;
gpf:
- hvm_inject_hw_exception(TRAP_gp_fault, 0);
return X86EMUL_EXCEPTION;
}
@@ -1897,7 +1896,6 @@ static int svm_msr_write_intercept(unsigned int msr,
uint64_t msr_content)
return result;
gpf:
- hvm_inject_hw_exception(TRAP_gp_fault, 0);
return X86EMUL_EXCEPTION;
}
@@ -1924,6 +1922,8 @@ static void svm_do_msr_access(struct cpu_user_regs *regs)
if ( rc == X86EMUL_OKAY )
__update_guest_eip(regs, inst_len);
+ else if ( rc == X86EMUL_EXCEPTION )
+ hvm_inject_hw_exception(TRAP_gp_fault, 0);
}
static void svm_vmexit_do_hlt(struct vmcb_struct *vmcb,
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 597d7ac..b5bfa05 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2734,7 +2734,6 @@ static int vmx_msr_read_intercept(unsigned int msr,
uint64_t *msr_content)
return X86EMUL_OKAY;
gp_fault:
- hvm_inject_hw_exception(TRAP_gp_fault, 0);
return X86EMUL_EXCEPTION;
}
@@ -2971,7 +2970,6 @@ static int vmx_msr_write_intercept(unsigned int msr,
uint64_t msr_content)
return X86EMUL_OKAY;
gp_fault:
- hvm_inject_hw_exception(TRAP_gp_fault, 0);
return X86EMUL_EXCEPTION;
}
@@ -3664,18 +3662,33 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
break;
case EXIT_REASON_MSR_READ:
{
- uint64_t msr_content;
- if ( hvm_msr_read_intercept(regs->_ecx, &msr_content) == X86EMUL_OKAY )
+ uint64_t msr_content = 0;
+
+ switch ( hvm_msr_read_intercept(regs->_ecx, &msr_content) )
{
+ case X86EMUL_OKAY:
msr_split(regs, msr_content);
update_guest_eip(); /* Safe: RDMSR */
+ break;
+
+ case X86EMUL_EXCEPTION:
+ hvm_inject_hw_exception(TRAP_gp_fault, 0);
+ break;
}
break;
}
case EXIT_REASON_MSR_WRITE:
- if ( hvm_msr_write_intercept(regs->_ecx, msr_fold(regs), 1) ==
X86EMUL_OKAY )
+ switch ( hvm_msr_write_intercept(regs->_ecx, msr_fold(regs), 1) )
+ {
+ case X86EMUL_OKAY:
update_guest_eip(); /* Safe: WRMSR */
+ break;
+
+ case X86EMUL_EXCEPTION:
+ hvm_inject_hw_exception(TRAP_gp_fault, 0);
+ break;
+ }
break;
case EXIT_REASON_VMXOFF:
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index f6a25a6..c830d16 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1032,6 +1032,7 @@ static void load_shadow_guest_state(struct vcpu *v)
struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
u32 control;
u64 cr_gh_mask, cr_read_shadow;
+ int rc;
static const u16 vmentry_fields[] = {
VM_ENTRY_INTR_INFO,
@@ -1053,8 +1054,12 @@ static void load_shadow_guest_state(struct vcpu *v)
if ( control & VM_ENTRY_LOAD_GUEST_PAT )
hvm_set_guest_pat(v, get_vvmcs(v, GUEST_PAT));
if ( control & VM_ENTRY_LOAD_PERF_GLOBAL_CTRL )
- hvm_msr_write_intercept(MSR_CORE_PERF_GLOBAL_CTRL,
- get_vvmcs(v, GUEST_PERF_GLOBAL_CTRL), 0);
+ {
+ rc = hvm_msr_write_intercept(MSR_CORE_PERF_GLOBAL_CTRL,
+ get_vvmcs(v, GUEST_PERF_GLOBAL_CTRL), 0);
+ if ( rc == X86EMUL_EXCEPTION )
+ hvm_inject_hw_exception(TRAP_gp_fault, 0);
+ }
hvm_funcs.set_tsc_offset(v, v->arch.hvm_vcpu.cache_tsc_offset, 0);
@@ -1222,7 +1227,7 @@ static void sync_vvmcs_ro(struct vcpu *v)
static void load_vvmcs_host_state(struct vcpu *v)
{
- int i;
+ int i, rc;
u64 r;
u32 control;
@@ -1240,8 +1245,12 @@ static void load_vvmcs_host_state(struct vcpu *v)
if ( control & VM_EXIT_LOAD_HOST_PAT )
hvm_set_guest_pat(v, get_vvmcs(v, HOST_PAT));
if ( control & VM_EXIT_LOAD_PERF_GLOBAL_CTRL )
- hvm_msr_write_intercept(MSR_CORE_PERF_GLOBAL_CTRL,
- get_vvmcs(v, HOST_PERF_GLOBAL_CTRL), 1);
+ {
+ rc = hvm_msr_write_intercept(MSR_CORE_PERF_GLOBAL_CTRL,
+ get_vvmcs(v, HOST_PERF_GLOBAL_CTRL), 1);
+ if ( rc == X86EMUL_EXCEPTION )
+ hvm_inject_hw_exception(TRAP_gp_fault, 0);
+ }
hvm_funcs.set_tsc_offset(v, v->arch.hvm_vcpu.cache_tsc_offset, 0);
diff --git a/xen/include/asm-x86/hvm/support.h
b/xen/include/asm-x86/hvm/support.h
index 262955d..5e25698 100644
--- a/xen/include/asm-x86/hvm/support.h
+++ b/xen/include/asm-x86/hvm/support.h
@@ -121,13 +121,19 @@ int hvm_set_efer(uint64_t value);
int hvm_set_cr0(unsigned long value, bool_t may_defer);
int hvm_set_cr3(unsigned long value, bool_t may_defer);
int hvm_set_cr4(unsigned long value, bool_t may_defer);
-int hvm_msr_read_intercept(unsigned int msr, uint64_t *msr_content);
-int hvm_msr_write_intercept(
- unsigned int msr, uint64_t msr_content, bool_t may_defer);
int hvm_mov_to_cr(unsigned int cr, unsigned int gpr);
int hvm_mov_from_cr(unsigned int cr, unsigned int gpr);
void hvm_ud_intercept(struct cpu_user_regs *);
+/*
+ * May return X86EMUL_EXCEPTION, at which point the caller is responsible for
+ * injecting a #GP fault. Used to support speculative reads.
+ */
+int __must_check hvm_msr_read_intercept(
+ unsigned int msr, uint64_t *msr_content);
+int __must_check hvm_msr_write_intercept(
+ unsigned int msr, uint64_t msr_content, bool_t may_defer);
+
#endif /* __ASM_X86_HVM_SUPPORT_H__ */
/*
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |