From e6ef802173fb42ea06955032123e193387b08799 Mon Sep 17 00:00:00 2001 From: Yu Ning Date: Thu, 13 Dec 2018 16:54:37 +0800 Subject: [PATCH 1/2] Do not allow guest direct access to CR8 Currently, HAXM allows the guest to freely read/write CR8 via the MOV from/to CR8 instructions, and does not save/restore host CR8 state. But some host OSes are sensitive to unexpected CR8 changes: for instance, Windows x86_64 stores the current IRQL in CR8, and if the guest (e.g. NetBSD) overwrites CR8 with a smaller value, the host will crash (BSOD) shortly after the next VM exit. Fix this problem by making HAXM intercept every guest access to CR8: write requests are ignored (hardware/host CR8 untouched), whereas read requests are serviced using a dummy handler that always returns 0. This is a hacky approach, but since most guest OSes do not rely on CR8 (e.g. NetBSD writes 0 to CR8 during boot, but does not seem to care about the outcome), it is an acceptable expedient. Proper CR8 virtualization would probably require collaboration with user space, because CR8 is merely an alias for APIC.TPR[7:4] (cf. Intel SDM Vol. 3A 10.8.6.1), and APIC emulation is provided by user space (QEMU). Fixes #136. Signed-off-by: Yu Ning --- core/vcpu.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/core/vcpu.c b/core/vcpu.c index 2b205587..b98615dc 100644 --- a/core/vcpu.c +++ b/core/vcpu.c @@ -194,7 +194,7 @@ static uint64_t vcpu_read_cr(struct vcpu_state_t *state, uint32_t n) break; } default: { - hax_error("Unsupported CR%d access\n", n); + hax_warning("Ignored unsupported CR%d read, returning 0\n", n); break; } } @@ -1324,7 +1324,7 @@ static void fill_common_vmcs(struct vcpu_t *vcpu) pcpu_ctls = IO_BITMAP_ACTIVE | MSR_BITMAP_ACTIVE | DR_EXITING | INTERRUPT_WINDOW_EXITING | USE_TSC_OFFSETTING | HLT_EXITING | - SECONDARY_CONTROLS; + CR8_LOAD_EXITING | CR8_STORE_EXITING | SECONDARY_CONTROLS; scpu_ctls = ENABLE_EPT; @@ -2854,7 +2854,8 @@ static int exit_cr_access(struct vcpu_t *vcpu, struct hax_tunnel *htun) } if (cr == 8) { - hax_error("Unsupported CR%d access\n", cr); + // TODO: Redirect CR8 write to user space (emulated APIC.TPR) + hax_warning("Ignored guest CR8 write, val=0x%llx\n", val); break; } check_flush(vcpu, cr_ptr ^ val); @@ -2897,11 +2898,8 @@ static int exit_cr_access(struct vcpu_t *vcpu, struct hax_tunnel *htun) case 1: { // MOV CR -> GPR hax_info("cr_access R CR%d\n", cr); + // TODO: Redirect CR8 read to user space (emulated APIC.TPR) state->_regs[vmx(vcpu, exit_qualification).cr.gpr] = cr_ptr; - if (cr == 8) { - hax_info("Unsupported CR%d access\n", cr); - break; - } break; } case 2: { // CLTS From 2ed3796c7ba036f55ca5c40ceab422fc98bfbe76 Mon Sep 17 00:00:00 2001 From: Yu Ning Date: Fri, 14 Dec 2018 10:11:00 +0800 Subject: [PATCH 2/2] Avoid vcpu_read_cr() call from MOV to CR8 handler Refactor the VM exit handler for CR access to drop unnecessary vcpu_read_cr() calls. This can avoid a bogus "unsupported CR8 read" error message when the guest only writes to CR8. Signed-off-by: Yu Ning --- core/vcpu.c | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/core/vcpu.c b/core/vcpu.c index b98615dc..6bf32a22 100644 --- a/core/vcpu.c +++ b/core/vcpu.c @@ -2778,7 +2778,6 @@ static void check_flush(struct vcpu_t *vcpu, uint32_t bits) static int exit_cr_access(struct vcpu_t *vcpu, struct hax_tunnel *htun) { - uint64_t cr_ptr; int cr; struct vcpu_state_t *state = vcpu->state; bool is_ept_pae = false; @@ -2788,19 +2787,25 @@ static int exit_cr_access(struct vcpu_t *vcpu, struct hax_tunnel *htun) htun->_exit_reason = vmx(vcpu, exit_reason).basic_reason; cr = vmx(vcpu, exit_qualification).cr.creg; - cr_ptr = vcpu_read_cr(state, cr); switch (vmx(vcpu, exit_qualification).cr.type) { case 0: { // MOV CR <- GPR uint64_t val = state->_regs[(vmx(vcpu, exit_qualification).cr.gpr)]; + uint64_t old_val = 0; - hax_debug("cr_access W CR%d: %08llx -> %08llx\n", cr, cr_ptr, val); + if (cr == 8) { + // TODO: Redirect CR8 write to user space (emulated APIC.TPR) + hax_warning("Ignored guest CR8 write, val=0x%llx\n", val); + break; + } + + old_val = vcpu_read_cr(state, cr); if (cr == 0) { uint64_t cr0_pae_triggers; hax_info("Guest writing to CR0[%u]: 0x%llx -> 0x%llx," " _cr4=0x%llx, _efer=0x%x\n", vcpu->vcpu_id, - state->_cr0, val, state->_cr4, state->_efer); + old_val, val, state->_cr4, state->_efer); if ((val & CR0_PG) && !(val & CR0_PE)) { hax_inject_exception(vcpu, VECTOR_GP, 0); return HAX_RESUME; @@ -2812,7 +2817,7 @@ static int exit_cr_access(struct vcpu_t *vcpu, struct hax_tunnel *htun) return HAX_RESUME; } } - if (!hax->ug_enable_flag && (cr_ptr & CR0_PE) && + if (!hax->ug_enable_flag && (old_val & CR0_PE) && !(val & CR0_PE)) { htun->_exit_status = HAX_EXIT_REALMODE; hax_debug("Enter NON-PE from PE\n"); @@ -2823,19 +2828,17 @@ static int exit_cr_access(struct vcpu_t *vcpu, struct hax_tunnel *htun) cr0_pae_triggers = CR0_CD | CR0_NW | CR0_PG; if ((val & CR0_PG) && (state->_cr4 & CR4_PAE) && !(state->_efer & IA32_EFER_LME) && !vtlb_active(vcpu) && - ((val ^ cr_ptr) & cr0_pae_triggers)) { + ((val ^ old_val) & cr0_pae_triggers)) { hax_info("%s: vCPU #%u triggers PDPT (re)load for EPT+PAE" " mode (CR0 path)\n", __func__, vcpu->vcpu_id); is_ept_pae = true; } - } - - if (cr == 4) { + } else if (cr == 4) { uint64_t cr4_pae_triggers; hax_info("Guest writing to CR4[%u]: 0x%llx -> 0x%llx," "_cr0=0x%llx, _efer=0x%x\n", vcpu->vcpu_id, - state->_cr4, val, state->_cr0, state->_efer); + old_val, val, state->_cr0, state->_efer); if ((state->_efer & IA32_EFER_LMA) && !(val & CR4_PAE)) { hax_inject_exception(vcpu, VECTOR_GP, 0); return HAX_RESUME; @@ -2846,19 +2849,16 @@ static int exit_cr_access(struct vcpu_t *vcpu, struct hax_tunnel *htun) cr4_pae_triggers = CR4_PAE | CR4_PGE | CR4_PSE; if ((val & CR4_PAE) && (state->_cr0 & CR0_PG) && !(state->_efer & IA32_EFER_LME) && !vtlb_active(vcpu) && - ((val ^ cr_ptr) & cr4_pae_triggers)) { + ((val ^ old_val) & cr4_pae_triggers)) { hax_info("%s: vCPU #%u triggers PDPT (re)load for EPT+PAE" " mode (CR4 path)\n", __func__, vcpu->vcpu_id); is_ept_pae = true; } - } - - if (cr == 8) { - // TODO: Redirect CR8 write to user space (emulated APIC.TPR) - hax_warning("Ignored guest CR8 write, val=0x%llx\n", val); + } else { + hax_error("Unsupported CR%d write, val=0x%llx\n", cr, val); break; } - check_flush(vcpu, cr_ptr ^ val); + check_flush(vcpu, old_val ^ val); vcpu_write_cr(state, cr, val); if (is_ept_pae) { @@ -2896,10 +2896,13 @@ static int exit_cr_access(struct vcpu_t *vcpu, struct hax_tunnel *htun) break; } case 1: { // MOV CR -> GPR + uint64_t val; + hax_info("cr_access R CR%d\n", cr); + val = vcpu_read_cr(state, cr); // TODO: Redirect CR8 read to user space (emulated APIC.TPR) - state->_regs[vmx(vcpu, exit_qualification).cr.gpr] = cr_ptr; + state->_regs[vmx(vcpu, exit_qualification).cr.gpr] = val; break; } case 2: { // CLTS