|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH] x86/vmx: Fix cascade crash in vmx_vmentry_failure()
The VMEntry failure handling does not distinguish VMFail Valid vs Invalid. In
the latter case, vmx_vmentry_failure() will hit a BUG() when trying to look up
VM_INSTRUCTION_ERROR.
Preserving CF around PUSH_AND_CLEAR_GPRS is rather complicated. Borrow a
spare byte in the cpu_info block.
Move all the failure logic into .text.cold to keep it out of the fastpath
cachelines.
In vmx_vmentry_failure(), only collect the instruction error in the case of
VMFailValid. As well as printing the instruction error number decode them to
human readable names, except those which pertain to STM (a.k.a. SMI
dual-monitor or executive-VMCS-pointer), which is a mode Xen does not support.
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <jbeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Teddy Astie <teddy.astie@xxxxxxxxxx>
Slightly RFC - it's fairly hard to these these. It came from code inspection
rather than encountering a VMFailInvalid case in practice.
I'm surprised that Eclair wasn't logging a violation at the absence of an
asmlinkage or SAF lable considering that vmx_vmentry_failure() has no
declaration.
---
xen/arch/x86/hvm/vmx/entry.S | 43 +++++++++++++--------
xen/arch/x86/hvm/vmx/vmcs.c | 51 +++++++++++++++++++++----
xen/arch/x86/include/asm/current.h | 3 ++
xen/arch/x86/include/asm/hvm/vmx/vmcs.h | 1 +
xen/arch/x86/x86_64/asm-offsets.c | 1 +
5 files changed, 76 insertions(+), 23 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/entry.S b/xen/arch/x86/hvm/vmx/entry.S
index cebc70064048..24789341a082 100644
--- a/xen/arch/x86/hvm/vmx/entry.S
+++ b/xen/arch/x86/hvm/vmx/entry.S
@@ -154,21 +154,7 @@ UNLIKELY_END(realmode)
.Lvmx_launch:
vmlaunch
-
-.Lvmx_vmentry_fail:
- sti
- PUSH_AND_CLEAR_GPRS
-
- /*
- * SPEC_CTRL_ENTRY notes
- *
- * If we end up here, no guest code has executed. The MSR lists have
- * not been processed, so we still have Xen's choice of MSR_SPEC_CTRL
- * in context, and the RSB is unchanged.
- */
-
- call vmx_vmentry_failure
- jmp .Lvmx_process_softirqs
+ jmp .Lvmx_vmentry_fail
LABEL(vmx_asm_do_vmentry)
GET_CURRENT(bx)
@@ -189,3 +175,30 @@ LABEL(vmx_asm_do_vmentry)
call do_softirq
jmp .Lvmx_do_vmentry
END(vmx_asm_vmexit_handler)
+
+ .section .text.cold, "ax", @progbits
+
+FUNC(vmx_asm_vmexit_handler.cold)
+.Lvmx_vmentry_fail:
+ /*
+ * SPEC_CTRL_ENTRY notes
+ *
+ * If we end up here, no guest code has executed. The MSR lists have
+ * not been processed, so we still have Xen's choice of MSR_SPEC_CTRL
+ * in context, and the RSB is unchanged.
+ *
+ * The guest registers are live, and the on-stack copy is up-to-date.
+ * PUSH_AND_CLEAR_GPRS clobbers flags and can't reasonably be made not
+ * to. The Carry flag (VMFail Invalid vs Valid) needs preserving.
+ *
+ * We could opencode PUSH_AND_CLEAR_GPRS but that's fragile to stack
+ * layout changes. Instead, use a spare byte in the cpuinfo block.
+ */
+ setnc STK_REL(CPUINFO_vmx_vmfail_valid, CPUINFO_error_code)(%rsp)
+
+ PUSH_AND_CLEAR_GPRS
+ sti
+
+ call vmx_vmentry_failure
+ jmp .Lvmx_process_softirqs
+END(vmx_asm_vmexit_handler.cold)
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 8e52ef4d497a..8ff8d2426e94 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1833,18 +1833,53 @@ void vmx_destroy_vmcs(struct vcpu *v)
free_xenheap_page(v->arch.hvm.vmx.msr_bitmap);
}
-void vmx_vmentry_failure(void)
+static const char *vmx_error_str(unsigned int error)
+{
+ switch ( error )
+ {
+ case VMX_INSN_VMLAUNCH_NONCLEAR_VMCS:
+ return "VMLAUNCH with non-clear VMCS";
+
+ case VMX_INSN_VMRESUME_NONLAUNCHED_VMCS:
+ return "VMRESUME with non-launched VMCS";
+
+ case VMX_INSN_VMRESUME_AFTER_VMXOFF:
+ return "VMRESUME after VMXOFF";
+
+ case VMX_INSN_INVALID_CONTROL_STATE:
+ return "Invalid control state";
+
+ case VMX_INSN_INVALID_HOST_STATE:
+ return "Invalid host state";
+
+ case VMX_INSN_VMENTRY_BLOCKED_BY_MOV_SS:
+ return "Blocked by MOV-SS";
+
+ default:
+ return "Unknown";
+ }
+}
+
+void asmlinkage __cold vmx_vmentry_failure(void)
{
struct vcpu *curr = current;
- unsigned long error;
+ bool valid = get_cpu_info()->vmx_vmfail_valid;
- __vmread(VM_INSTRUCTION_ERROR, &error);
- gprintk(XENLOG_ERR, "VM%s error: %#lx\n",
- curr->arch.hvm.vmx.launched ? "RESUME" : "LAUNCH", error);
+ gprintk(XENLOG_ERR, "VM%s Failure, VMCS %svalid\n",
+ curr->arch.hvm.vmx.launched ? "RESUME" : "LAUNCH",
+ valid ? "" : "not ");
- if ( error == VMX_INSN_INVALID_CONTROL_STATE ||
- error == VMX_INSN_INVALID_HOST_STATE )
- vmcs_dump_vcpu(curr);
+ if ( valid )
+ {
+ unsigned int error = vmread(VM_INSTRUCTION_ERROR);
+
+ gprintk(XENLOG_ERR, " Instruction Error %u, %s\n",
+ error, vmx_error_str(error));
+
+ if ( error == VMX_INSN_INVALID_CONTROL_STATE ||
+ error == VMX_INSN_INVALID_HOST_STATE )
+ vmcs_dump_vcpu(curr);
+ }
domain_crash(curr->domain);
}
diff --git a/xen/arch/x86/include/asm/current.h
b/xen/arch/x86/include/asm/current.h
index 6139980ab115..3c53235d341a 100644
--- a/xen/arch/x86/include/asm/current.h
+++ b/xen/arch/x86/include/asm/current.h
@@ -80,6 +80,9 @@ struct cpu_info {
*/
bool use_pv_cr3;
+ /* Scratch space for the VT-x logic. See users. */
+ uint8_t vmx_vmfail_valid;
+
/* get_stack_bottom() must be 16-byte aligned */
};
diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
index 88bded5190c9..3b1c54fd055e 100644
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -619,6 +619,7 @@ enum vmx_insn_errno
VMX_INSN_VMCLEAR_WITH_VMXON_PTR = 3,
VMX_INSN_VMLAUNCH_NONCLEAR_VMCS = 4,
VMX_INSN_VMRESUME_NONLAUNCHED_VMCS = 5,
+ VMX_INSN_VMRESUME_AFTER_VMXOFF = 6,
VMX_INSN_INVALID_CONTROL_STATE = 7,
VMX_INSN_INVALID_HOST_STATE = 8,
VMX_INSN_VMPTRLD_INVALID_PHYADDR = 9,
diff --git a/xen/arch/x86/x86_64/asm-offsets.c
b/xen/arch/x86/x86_64/asm-offsets.c
index 9d4536402661..65b2b96fd528 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -163,6 +163,7 @@ void __dummy__(void)
OFFSET(CPUINFO_scf, struct cpu_info, scf);
OFFSET(CPUINFO_root_pgt_changed, struct cpu_info, root_pgt_changed);
OFFSET(CPUINFO_use_pv_cr3, struct cpu_info, use_pv_cr3);
+ OFFSET(CPUINFO_vmx_vmfail_valid, struct cpu_info, vmx_vmfail_valid);
DEFINE(CPUINFO_sizeof, sizeof(struct cpu_info));
BLANK();
base-commit: c4bf5bc5f0edbcbc5965c924db069483b2cf6049
--
2.39.5
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |