Index: /trunk/include/VBox/err.h =================================================================== --- /trunk/include/VBox/err.h (revision 61543) +++ /trunk/include/VBox/err.h (revision 61544) @@ -2715,4 +2715,8 @@ /** Return to ring-3 to perform the hypercall there. */ #define VINF_GIM_R3_HYPERCALL 6316 +/** Continuing hypercall at the same RIP, continue guest execution. */ +#define VINF_GIM_HYPERCALL_CONTINUING 6317 +/** Instruction that triggers the hypercall is invalid/unrecognized. */ +#define VERR_GIM_INVALID_HYPERCALL_INSTR (-6318) /** @} */ Index: /trunk/include/VBox/vmm/gim.h =================================================================== --- /trunk/include/VBox/vmm/gim.h (revision 61543) +++ /trunk/include/VBox/vmm/gim.h (revision 61544) @@ -187,6 +187,7 @@ VMM_INT_DECL(bool) GIMIsParavirtTscEnabled(PVM pVM); VMM_INT_DECL(bool) GIMAreHypercallsEnabled(PVMCPU pVCpu); -VMM_INT_DECL(int) GIMHypercall(PVMCPU pVCpu, PCPUMCTX pCtx); -VMM_INT_DECL(int) GIMXcptUD(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis); +VMM_INT_DECL(VBOXSTRICTRC) GIMHypercall(PVMCPU pVCpu, PCPUMCTX pCtx); +VMM_INT_DECL(VBOXSTRICTRC) GIMExecHypercallInstr(PVMCPU pVCpu, PCPUMCTX pCtx, uint8_t *pcbInstr); +VMM_INT_DECL(VBOXSTRICTRC) GIMXcptUD(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis, uint8_t *pcbInstr); VMM_INT_DECL(bool) GIMShouldTrapXcptUD(PVMCPU pVCpu); VMM_INT_DECL(VBOXSTRICTRC) GIMReadMsr(PVMCPU pVCpu, uint32_t idMsr, PCCPUMMSRRANGE pRange, uint64_t *puValue); Index: /trunk/src/VBox/VMM/VMMAll/GIMAll.cpp =================================================================== --- /trunk/src/VBox/VMM/VMMAll/GIMAll.cpp (revision 61543) +++ /trunk/src/VBox/VMM/VMMAll/GIMAll.cpp (revision 61544) @@ -23,4 +23,6 @@ #include "GIMInternal.h" #include +#include /* For DISCPUSTATE */ +#include /* For EMInterpretDisasCurrent */ #include @@ -85,5 +87,16 @@ * Implements a GIM hypercall with the provider configured for the VM. * - * @returns VBox status code. + * @returns Strict VBox status code. + * @retval VINF_SUCCESS if the hypercall succeeded (even if its operation + * failed). + * @retval VINF_GIM_R3_HYPERCALL re-start the hypercall from ring-3. + * @retval VERR_GIM_HYPERCALL_ACCESS_DENIED CPL is insufficient. + * @retval VERR_GIM_HYPERCALLS_NOT_AVAILABLE hypercalls unavailable. + * @retval VERR_GIM_NOT_ENABLED GIM is not enabled (shouldn't really happen) + * @retval VERR_GIM_HYPERCALL_MEMORY_READ_FAILED hypercall failed while reading + * memory. + * @retval VERR_GIM_HYPERCALL_MEMORY_WRITE_FAILED hypercall failed while + * writing memory. + * * @param pVCpu The cross context virtual CPU structure. * @param pCtx Pointer to the guest-CPU context. @@ -91,5 +104,5 @@ * @thread EMT. */ -VMM_INT_DECL(int) GIMHypercall(PVMCPU pVCpu, PCPUMCTX pCtx) +VMM_INT_DECL(VBOXSTRICTRC) GIMHypercall(PVMCPU pVCpu, PCPUMCTX pCtx) { PVM pVM = pVCpu->CTX_SUFF(pVM); @@ -99,5 +112,4 @@ return VERR_GIM_NOT_ENABLED; - STAM_REL_COUNTER_INC(&pVM->gim.s.StatHypercalls); switch (pVM->gim.s.enmProviderId) { @@ -112,4 +124,51 @@ return VERR_GIM_HYPERCALLS_NOT_AVAILABLE; } +} + + +/** + * Disassembles the current instruction at RIP and if it's a hypercall + * instruction, performs the hypercall. + * + * @param pVCpu The cross context virtual CPU structure. + * @param pCtx Pointer to the guest-CPU context. + * @param pcbInstr Where to store the disassembled instruction length. + * Optional, can be NULL. + * + * @todo This interface should disappear when IEM/REM execution engines + * handle VMCALL/VMMCALL instrunctions to call into GIM when + * required. See @bugref{7270#c168}. + */ +VMM_INT_DECL(VBOXSTRICTRC) GIMExecHypercallInstr(PVMCPU pVCpu, PCPUMCTX pCtx, uint8_t *pcbInstr) +{ + PVM pVM = pVCpu->CTX_SUFF(pVM); + VMCPU_ASSERT_EMT(pVCpu); + + if (RT_UNLIKELY(!GIMIsEnabled(pVM))) + return VERR_GIM_NOT_ENABLED; + + unsigned cbInstr; + DISCPUSTATE Dis; + int rc = EMInterpretDisasCurrent(pVM, pVCpu, &Dis, &cbInstr); + if (RT_SUCCESS(rc)) + { + if (pcbInstr) + *pcbInstr = (uint8_t)cbInstr; + switch (pVM->gim.s.enmProviderId) + { + case GIMPROVIDERID_HYPERV: + return gimHvExecHypercallInstr(pVCpu, pCtx, &Dis); + + case GIMPROVIDERID_KVM: + return gimKvmExecHypercallInstr(pVCpu, pCtx, &Dis); + + default: + AssertMsgFailed(("GIMExecHypercallInstr: for provider %u not available/implemented\n", pVM->gim.s.enmProviderId)); + return VERR_GIM_HYPERCALLS_NOT_AVAILABLE; + } + } + + Log(("GIM: GIMExecHypercallInstr: Failed to disassemble CS:RIP=%04x:%08RX64. rc=%Rrc\n", pCtx->cs.Sel, pCtx->rip, rc)); + return rc; } @@ -177,21 +236,38 @@ * Exception handler for \#UD when requested by the GIM provider. * + * @returns Strict VBox status code. + * @retval VINF_SUCCESS if the hypercall succeeded (even if its operation + * failed). + * @retval VINF_GIM_R3_HYPERCALL restart the hypercall from ring-3. + * @retval VINF_GIM_HYPERCALL_CONTINUING continue hypercall without updating + * RIP. + * @retval VERR_GIM_HYPERCALL_ACCESS_DENIED CPL is insufficient. + * @retval VERR_GIM_INVALID_HYPERCALL_INSTR instruction at RIP is not a valid + * hypercall instruction. + * * @param pVCpu The cross context virtual CPU structure. * @param pCtx Pointer to the guest-CPU context. * @param pDis Pointer to the disassembled instruction state at RIP. - * Optional, can be NULL. - */ -VMM_INT_DECL(int) GIMXcptUD(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis) + * If NULL is passed, it implies the disassembly of the + * the instruction at RIP is the responsibility of the + * GIM provider. + * @param pcbInstr Where to store the instruction length of the hypercall + * instruction. Optional, can be NULL. + * + * @thread EMT(pVCpu). + */ +VMM_INT_DECL(VBOXSTRICTRC) GIMXcptUD(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis, uint8_t *pcbInstr) { PVM pVM = pVCpu->CTX_SUFF(pVM); Assert(GIMIsEnabled(pVM)); - - switch (pVM->gim.s.enmProviderId) - { - case GIMPROVIDERID_KVM: - return gimKvmXcptUD(pVCpu, pCtx, pDis); - - case GIMPROVIDERID_HYPERV: - return gimHvXcptUD(pVCpu, pCtx, pDis); + Assert(pDis || pcbInstr); + + switch (pVM->gim.s.enmProviderId) + { + case GIMPROVIDERID_KVM: + return gimKvmXcptUD(pVCpu, pCtx, pDis, pcbInstr); + + case GIMPROVIDERID_HYPERV: + return gimHvXcptUD(pVCpu, pCtx, pDis, pcbInstr); default: Index: /trunk/src/VBox/VMM/VMMAll/GIMAllHv.cpp =================================================================== --- /trunk/src/VBox/VMM/VMMAll/GIMAllHv.cpp (revision 61543) +++ /trunk/src/VBox/VMM/VMMAll/GIMAllHv.cpp (revision 61544) @@ -126,19 +126,33 @@ * Handles all Hyper-V hypercalls. * - * @returns VBox status code. + * @returns Strict VBox status code. + * @retval VINF_SUCCESS if the hypercall succeeded (even if its operation + * failed). + * @retval VINF_GIM_R3_HYPERCALL re-start the hypercall from ring-3. + * @retval VERR_GIM_HYPERCALLS_NOT_ENABLED hypercalls are disabled by the + * guest. + * @retval VERR_GIM_HYPERCALL_ACCESS_DENIED CPL is insufficient. + * @retval VERR_GIM_HYPERCALL_MEMORY_READ_FAILED hypercall failed while reading + * memory. + * @retval VERR_GIM_HYPERCALL_MEMORY_WRITE_FAILED hypercall failed while + * writing memory. + * * @param pVCpu The cross context virtual CPU structure. * @param pCtx Pointer to the guest-CPU context. * - * @thread EMT. - */ -VMM_INT_DECL(int) gimHvHypercall(PVMCPU pVCpu, PCPUMCTX pCtx) + * @thread EMT(pVCpu). + */ +VMM_INT_DECL(VBOXSTRICTRC) gimHvHypercall(PVMCPU pVCpu, PCPUMCTX pCtx) { + VMCPU_ASSERT_EMT(pVCpu); + #ifndef IN_RING3 return VINF_GIM_R3_HYPERCALL; #else PVM pVM = pVCpu->CTX_SUFF(pVM); + STAM_REL_COUNTER_INC(&pVM->gim.s.StatHypercalls); /* - * Verify that hypercalls are enabled. + * Verify that hypercalls are enabled by the guest. */ if (!gimHvAreHypercallsEnabled(pVCpu)) @@ -975,5 +989,50 @@ /** + * Checks the currently disassembled instrunction and executes the hypercall if + * it's a hypercall instruction. + * + * @returns Strict VBox status code. + * @param pVCpu The cross context virtual CPU structure. + * @param pCtx Pointer to the guest-CPU context. + * @param pDis Pointer to the disassembled instruction state at RIP. + * + * @thread EMT(pVCpu). + * + * @todo Make this function static when @bugref{7270#c168} is addressed. + */ +VMM_INT_DECL(VBOXSTRICTRC) gimHvExecHypercallInstr(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis) +{ + Assert(pVCpu); + Assert(pCtx); + Assert(pDis); + VMCPU_ASSERT_EMT(pVCpu); + + PVM pVM = pVCpu->CTX_SUFF(pVM); + CPUMCPUVENDOR const enmGuestCpuVendor = CPUMGetGuestCpuVendor(pVM); + if ( ( pDis->pCurInstr->uOpcode == OP_VMCALL + && ( enmGuestCpuVendor == CPUMCPUVENDOR_INTEL + || enmGuestCpuVendor == CPUMCPUVENDOR_VIA)) + || ( pDis->pCurInstr->uOpcode == OP_VMMCALL + && enmGuestCpuVendor == CPUMCPUVENDOR_AMD)) + { + return gimHvHypercall(pVCpu, pCtx); + } + + return VERR_GIM_INVALID_HYPERCALL_INSTR; +} + + +/** * Exception handler for \#UD. + * + * @returns Strict VBox status code. + * @retval VINF_SUCCESS if the hypercall succeeded (even if its operation + * failed). + * @retval VINF_GIM_R3_HYPERCALL re-start the hypercall from ring-3. + * @retval VINF_GIM_HYPERCALL_CONTINUING continue hypercall without updating + * RIP. + * @retval VERR_GIM_HYPERCALL_ACCESS_DENIED CPL is insufficient. + * @retval VERR_GIM_INVALID_HYPERCALL_INSTR instruction at RIP is not a valid + * hypercall instruction. * * @param pVCpu The cross context virtual CPU structure. @@ -981,17 +1040,19 @@ * @param pDis Pointer to the disassembled instruction state at RIP. * Optional, can be NULL. - * - * @thread EMT. - */ -VMM_INT_DECL(int) gimHvXcptUD(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis) + * @param pcbInstr Where to store the instruction length of the hypercall + * instruction. Optional, can be NULL. + * + * @thread EMT(pVCpu). + */ +VMM_INT_DECL(VBOXSTRICTRC) gimHvXcptUD(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis, uint8_t *pcbInstr) { + VMCPU_ASSERT_EMT(pVCpu); + /* * If we didn't ask for #UD to be trapped, bail. */ - PVM pVM = pVCpu->CTX_SUFF(pVM); if (!gimHvShouldTrapXcptUD(pVCpu)) - return VERR_GIM_OPERATION_FAILED; - - int rc = VINF_SUCCESS; + return VERR_GIM_IPE_1; + if (!pDis) { @@ -1000,35 +1061,19 @@ * or the AMD VMMCALL instruction and if so, handle it as a hypercall. */ + unsigned cbInstr; DISCPUSTATE Dis; - rc = EMInterpretDisasCurrent(pVM, pVCpu, &Dis, NULL /* pcbInstr */); - pDis = &Dis; - } - - if (RT_SUCCESS(rc)) - { - CPUMCPUVENDOR enmGuestCpuVendor = CPUMGetGuestCpuVendor(pVM); - if ( ( pDis->pCurInstr->uOpcode == OP_VMCALL - && ( enmGuestCpuVendor == CPUMCPUVENDOR_INTEL - || enmGuestCpuVendor == CPUMCPUVENDOR_VIA)) - || ( pDis->pCurInstr->uOpcode == OP_VMMCALL - && enmGuestCpuVendor == CPUMCPUVENDOR_AMD)) - { - /* - * Make sure guest ring-0 is the one making the hypercall. - */ - if (CPUMGetGuestCPL(pVCpu)) - return VERR_GIM_HYPERCALL_ACCESS_DENIED; - - /* - * Update RIP and perform the hypercall. - */ - /** @todo pre-incrementing of RIP will break when we implement continuing hypercalls. */ - pCtx->rip += pDis->cbInstr; - rc = gimHvHypercall(pVCpu, pCtx); - } - else - rc = VERR_GIM_OPERATION_FAILED; - } - return rc; + int rc = EMInterpretDisasCurrent(pVCpu->CTX_SUFF(pVM), pVCpu, &Dis, &cbInstr); + if (RT_SUCCESS(rc)) + { + if (pcbInstr) + *pcbInstr = (uint8_t)cbInstr; + return gimHvExecHypercallInstr(pVCpu, pCtx, &Dis); + } + + Log(("GIM: HyperV: Failed to disassemble instruction at CS:RIP=%04x:%08RX64. rc=%Rrc\n", pCtx->cs.Sel, pCtx->rip, rc)); + return rc; + } + + return gimHvExecHypercallInstr(pVCpu, pCtx, pDis); } Index: /trunk/src/VBox/VMM/VMMAll/GIMAllKvm.cpp =================================================================== --- /trunk/src/VBox/VMM/VMMAll/GIMAllKvm.cpp (revision 61543) +++ /trunk/src/VBox/VMM/VMMAll/GIMAllKvm.cpp (revision 61544) @@ -42,12 +42,22 @@ * Handles the KVM hypercall. * - * @returns VBox status code. + * @returns Strict VBox status code. + * @retval VINF_SUCCESS if the hypercall succeeded (even if its operation + * failed). + * @retval VINF_GIM_R3_HYPERCALL re-start the hypercall from ring-3. + * @retval VERR_GIM_HYPERCALL_ACCESS_DENIED CPL is insufficient. + * * @param pVCpu The cross context virtual CPU structure. * @param pCtx Pointer to the guest-CPU context. * - * @thread EMT. - */ -VMM_INT_DECL(int) gimKvmHypercall(PVMCPU pVCpu, PCPUMCTX pCtx) -{ + * @thread EMT(pVCpu). + */ +VMM_INT_DECL(VBOXSTRICTRC) gimKvmHypercall(PVMCPU pVCpu, PCPUMCTX pCtx) +{ + VMCPU_ASSERT_EMT(pVCpu); + + PVM pVM = pVCpu->CTX_SUFF(pVM); + STAM_REL_COUNTER_INC(&pVM->gim.s.StatHypercalls); + /* * Get the hypercall operation and arguments. @@ -76,8 +86,8 @@ */ uint32_t uCpl = CPUMGetGuestCPL(pVCpu); - if (uCpl) + if (RT_UNLIKELY(uCpl)) { pCtx->rax = KVM_HYPERCALL_RET_EPERM & uAndMask; - return VINF_SUCCESS; + return VERR_GIM_HYPERCALL_ACCESS_DENIED; } @@ -85,9 +95,9 @@ * Do the work. */ + int rc = VINF_SUCCESS; switch (uHyperOp) { case KVM_HYPERCALL_OP_KICK_CPU: { - PVM pVM = pVCpu->CTX_SUFF(pVM); if (uHyperArg1 < pVM->cCpus) { @@ -101,5 +111,5 @@ GVMMR0SchedWakeUpEx(pVM, pVCpuTarget->idCpu, false /* fTakeUsedLock */); #elif defined(IN_RING3) - int rc2 = SUPR3CallVMMR0(pVM->pVMR0, pVCpuTarget->idCpu, VMMR0_DO_GVMM_SCHED_WAKE_UP, NULL); + int rc2 = SUPR3CallVMMR0(pVM->pVMR0, pVCpuTarget->idCpu, VMMR0_DO_GVMM_SCHED_WAKE_UP, NULL /* pvArg */); AssertRC(rc2); #elif defined(IN_RC) @@ -109,4 +119,9 @@ uHyperRet = KVM_HYPERCALL_RET_SUCCESS; } + else + { + /* Shouldn't ever happen! If it does, throw a guru, as otherwise it'll lead to deadlocks in the guest anyway! */ + rc = VERR_GIM_HYPERCALL_FAILED; + } break; } @@ -124,5 +139,5 @@ */ pCtx->rax = uHyperRet & uAndMask; - return VINF_SUCCESS; + return rc; } @@ -349,5 +364,97 @@ /** + * Checks the currently disassembled instrunction and executes the hypercall if + * it's a hypercall instruction. + * + * @returns Strict VBox status code. + * @param pVCpu The cross context virtual CPU structure. + * @param pCtx Pointer to the guest-CPU context. + * @param pDis Pointer to the disassembled instruction state at RIP. + * + * @thread EMT(pVCpu). + * + * @todo Make this function static when @bugref{7270#c168} is addressed. + */ +VMM_INT_DECL(VBOXSTRICTRC) gimKvmExecHypercallInstr(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis) +{ + Assert(pVCpu); + Assert(pCtx); + Assert(pDis); + VMCPU_ASSERT_EMT(pVCpu); + + /* + * If the instruction at RIP is the Intel VMCALL instruction or + * the AMD VMMCALL instruction handle it as a hypercall. + * + * Linux/KVM guests always uses the Intel VMCALL instruction but we patch + * it to the host-native one whenever we encounter it so subsequent calls + * will not require disassembly (when coming from HM). + */ + if ( pDis->pCurInstr->uOpcode == OP_VMCALL + || pDis->pCurInstr->uOpcode == OP_VMMCALL) + { + /* + * Perform the hypercall. + * + * For HM, we can simply resume guest execution without performing the hypercall now and + * do it on the next VMCALL/VMMCALL exit handler on the patched instruction. + * + * For raw-mode we need to do this now anyway. So we do it here regardless with an added + * advantage is that it saves one world-switch for the HM case. + */ + VBOXSTRICTRC rcStrict = gimKvmHypercall(pVCpu, pCtx); + if (rcStrict == VINF_SUCCESS) + { + /* + * Patch the instruction to so we don't have to spend time disassembling it each time. + * Makes sense only for HM as with raw-mode we will be getting a #UD regardless. + */ + PVM pVM = pVCpu->CTX_SUFF(pVM); + PCGIMKVM pKvm = &pVM->gim.s.u.Kvm; + if ( pDis->pCurInstr->uOpcode != pKvm->uOpCodeNative + && HMIsEnabled(pVM)) + { + /** @todo r=ramshankar: we probably should be doing this in an + * EMT rendezvous and using proper AVL-tree patching to + * keep track at RIP so that non-patched VMCALL's still + * produce \#UD. */ + uint8_t abHypercall[3]; + size_t cbWritten = 0; + int rc = VMMPatchHypercall(pVM, &abHypercall, sizeof(abHypercall), &cbWritten); + AssertRC(rc); + Assert(sizeof(abHypercall) == pDis->cbInstr); + Assert(sizeof(abHypercall) == cbWritten); + + rc = PGMPhysSimpleWriteGCPtr(pVCpu, pCtx->rip, &abHypercall, sizeof(abHypercall)); + AssertRC(rc); + + /** @todo Add stats for patching. */ + } + } + else + { + /* The KVM provider doesn't have any concept of continuing hypercalls. */ + Assert(rcStrict != VINF_GIM_HYPERCALL_CONTINUING); +#ifdef IN_RING3 + Assert(rcStrict != VINF_GIM_R3_HYPERCALL); +#endif + } + return rcStrict; + } + + return VERR_GIM_INVALID_HYPERCALL_INSTR; +} + + +/** * Exception handler for \#UD. + * + * @returns Strict VBox status code. + * @retval VINF_SUCCESS if the hypercall succeeded (even if its operation + * failed). + * @retval VINF_GIM_R3_HYPERCALL re-start the hypercall from ring-3. + * @retval VERR_GIM_HYPERCALL_ACCESS_DENIED CPL is insufficient. + * @retval VERR_GIM_INVALID_HYPERCALL_INSTR instruction at RIP is not a valid + * hypercall instruction. * * @param pVCpu The cross context virtual CPU structure. @@ -355,76 +462,39 @@ * @param pDis Pointer to the disassembled instruction state at RIP. * Optional, can be NULL. - * - * @thread EMT. - */ -VMM_INT_DECL(int) gimKvmXcptUD(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis) -{ + * @param pcbInstr Where to store the instruction length of the hypercall + * instruction. Optional, can be NULL. + * + * @thread EMT(pVCpu). + */ +VMM_INT_DECL(VBOXSTRICTRC) gimKvmXcptUD(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis, uint8_t *pcbInstr) +{ + VMCPU_ASSERT_EMT(pVCpu); + /* * If we didn't ask for #UD to be trapped, bail. */ - PVM pVM = pVCpu->CTX_SUFF(pVM); - PGIMKVM pKvm = &pVM->gim.s.u.Kvm; - if (RT_UNLIKELY(!pVM->gim.s.u.Kvm.fTrapXcptUD)) - return VERR_GIM_OPERATION_FAILED; - - int rc = VINF_SUCCESS; + PVM pVM = pVCpu->CTX_SUFF(pVM); + PCGIMKVM pKvm = &pVM->gim.s.u.Kvm; + if (RT_UNLIKELY(!pKvm->fTrapXcptUD)) + return VERR_GIM_IPE_3; + + VBOXSTRICTRC rcStrict = VINF_SUCCESS; if (!pDis) { - /* - * Disassemble the instruction at RIP to figure out if it's the Intel VMCALL instruction - * or the AMD VMMCALL instruction and if so, handle it as a hypercall. - */ + unsigned cbInstr; DISCPUSTATE Dis; - rc = EMInterpretDisasCurrent(pVM, pVCpu, &Dis, NULL /* pcbInstr */); - pDis = &Dis; - } - - if (RT_SUCCESS(rc)) - { - /* - * Patch the instruction to so we don't have to spend time disassembling it each time. - * Makes sense only for HM as with raw-mode we will be getting a #UD regardless. - */ - if ( pDis->pCurInstr->uOpcode == OP_VMCALL - || pDis->pCurInstr->uOpcode == OP_VMMCALL) - { - /* - * Make sure guest ring-0 is the one making the hypercall. - */ - if (CPUMGetGuestCPL(pVCpu)) - return VERR_GIM_HYPERCALL_ACCESS_DENIED; - - if ( pDis->pCurInstr->uOpcode != pKvm->uOpCodeNative - && HMIsEnabled(pVM)) - { - uint8_t abHypercall[3]; - size_t cbWritten = 0; - rc = VMMPatchHypercall(pVM, &abHypercall, sizeof(abHypercall), &cbWritten); - AssertRC(rc); - Assert(sizeof(abHypercall) == pDis->cbInstr); - Assert(sizeof(abHypercall) == cbWritten); - - rc = PGMPhysSimpleWriteGCPtr(pVCpu, pCtx->rip, &abHypercall, sizeof(abHypercall)); - } - - /* - * Update RIP and perform the hypercall. - * - * For HM, we can simply resume guest execution without performing the hypercall now and - * do it on the next VMCALL/VMMCALL exit handler on the patched instruction. - * - * For raw-mode we need to do this now anyway. So we do it here regardless with an added - * advantage is that it saves one world-switch for the HM case. - */ - if (RT_SUCCESS(rc)) - { - pCtx->rip += pDis->cbInstr; - rc = gimKvmHypercall(pVCpu, pCtx); - } - } - else - rc = VERR_GIM_OPERATION_FAILED; - } - return rc; -} - + int rc = EMInterpretDisasCurrent(pVM, pVCpu, &Dis, &cbInstr); + if (RT_SUCCESS(rc)) + { + if (pcbInstr) + *pcbInstr = (uint8_t)cbInstr; + return gimKvmExecHypercallInstr(pVCpu, pCtx, &Dis); + } + + Log(("GIM: KVM: Failed to disassemble instruction at CS:RIP=%04x:%08RX64. rc=%Rrc\n", pCtx->cs.Sel, pCtx->rip, rc)); + return rc; + } + + return gimKvmExecHypercallInstr(pVCpu, pCtx, pDis); +} + Index: /trunk/src/VBox/VMM/VMMR0/HMSVMR0.cpp =================================================================== --- /trunk/src/VBox/VMM/VMMR0/HMSVMR0.cpp (revision 61543) +++ /trunk/src/VBox/VMM/VMMR0/HMSVMR0.cpp (revision 61544) @@ -4126,5 +4126,5 @@ } #endif - + if ( uIdtVector == X86_XCPT_BP || uIdtVector == X86_XCPT_OF) @@ -5198,16 +5198,22 @@ return VINF_SUCCESS; } - else if (rc == VERR_NOT_FOUND) + + if (rc == VERR_NOT_FOUND) { if (pVCpu->hm.s.fHypercallsEnabled) { - hmR0SvmUpdateRip(pVCpu, pCtx, 3); - - /** @todo pre-increment RIP before hypercall will break when we have to implement - * continuing hypercalls (e.g. Hyper-V). */ - rc = GIMHypercall(pVCpu, pCtx); - /* If the hypercall changes anything other than guest general-purpose registers, - we would need to reload the guest changed bits here before VM-entry. */ - return rc; + VBOXSTRICTRC rcStrict = GIMHypercall(pVCpu, pCtx); + if (RT_SUCCESS(VBOXSTRICTRC_VAL(rcStrict))) + { + if (rcStrict == VINF_SUCCESS) + hmR0SvmUpdateRip(pVCpu, pCtx, 3 /* cbInstr */); + else + Assert( rcStrict == VINF_GIM_HYPERCALL_CONTINUING + || rcStrict == VINF_GIM_R3_HYPERCALL); + + /* If the hypercall changes anything other than guest's general-purpose registers, + we would need to reload the guest changed bits here before VM-entry. */ + } + rc = VBOXSTRICTRC_VAL(rcStrict); } else @@ -5215,6 +5221,12 @@ } - hmR0SvmSetPendingXcptUD(pVCpu); - return VINF_SUCCESS; + /* If hypercalls are disabled or the hypercall failed for some reason, raise #UD and continue. */ + if (RT_FAILURE(rc)) + { + hmR0SvmSetPendingXcptUD(pVCpu); + rc = VINF_SUCCESS; + } + + return rc; } @@ -5423,6 +5435,6 @@ /** - * \#VMEXIT handler for undefined opcode (SVM_EXIT_EXCEPTION_6). Conditional - * \#VMEXIT. + * \#VMEXIT handler for undefined opcode (SVM_EXIT_EXCEPTION_6). + * Conditional \#VMEXIT. */ HMSVM_EXIT_DECL hmR0SvmExitXcptUD(PVMCPU pVCpu, PCPUMCTX pCtx, PSVMTRANSIENT pSvmTransient) @@ -5432,11 +5444,31 @@ HMSVM_CHECK_EXIT_DUE_TO_EVENT_DELIVERY(); + int rc = VERR_SVM_UNEXPECTED_XCPT_EXIT; if (pVCpu->hm.s.fGIMTrapXcptUD) - GIMXcptUD(pVCpu, pCtx, NULL /* pDis */); - else + { + uint8_t cbInstr = 0; + VBOXSTRICTRC rcStrict = GIMXcptUD(pVCpu, pCtx, NULL /* pDis */, &cbInstr); + if (rcStrict == VINF_SUCCESS) + { + hmR0SvmUpdateRip(pVCpu, pCtx, cbInstr); + rc = VINF_SUCCESS; + } + else if (rcStrict == VINF_GIM_HYPERCALL_CONTINUING) + rc = VINF_SUCCESS; + else if (rcStrict == VINF_GIM_R3_HYPERCALL) + rc = VINF_GIM_R3_HYPERCALL; + else + Assert(RT_FAILURE(VBOXSTRICTRC_VAL(rcStrict))); + } + + /* If the GIM #UD exception handler didn't succeed for some reason or wasn't needed, raise #UD. */ + if (RT_FAILURE(rc)) + { hmR0SvmSetPendingXcptUD(pVCpu); + rc = VINF_SUCCESS; + } STAM_COUNTER_INC(&pVCpu->hm.s.StatExitGuestUD); - return VINF_SUCCESS; + return rc; } Index: /trunk/src/VBox/VMM/VMMR0/HMVMXR0.cpp =================================================================== --- /trunk/src/VBox/VMM/VMMR0/HMVMXR0.cpp (revision 61543) +++ /trunk/src/VBox/VMM/VMMR0/HMVMXR0.cpp (revision 61544) @@ -11441,4 +11441,5 @@ STAM_COUNTER_INC(&pVCpu->hm.s.StatExitVmcall); + VBOXSTRICTRC rcStrict = VERR_VMX_IPE_3; if (pVCpu->hm.s.fHypercallsEnabled) { @@ -11449,24 +11450,33 @@ int rc = hmR0VmxSaveGuestRip(pVCpu, pMixedCtx); rc |= hmR0VmxSaveGuestSegmentRegs(pVCpu, pMixedCtx); /* For long-mode checks in gimKvmHypercall(). */ + AssertRCReturn(rc, rc); #endif - rc |= hmR0VmxAdvanceGuestRip(pVCpu, pMixedCtx, pVmxTransient); - AssertRCReturn(rc, rc); - - /** @todo pre-increment RIP before hypercall will break when we have to implement - * continuing hypercalls (e.g. Hyper-V). */ - /** @todo r=bird: GIMHypercall will probably have to be able to return - * informational status codes, so it should be made VBOXSTRICTRC. Not - * doing that now because the status code handling isn't clean (i.e. - * if you use RT_SUCCESS(rc) on the result of something, you don't - * return rc in the success case, you return VINF_SUCCESS). */ - rc = GIMHypercall(pVCpu, pMixedCtx); - /* If the hypercall changes anything other than guest general-purpose registers, + + /* Perform the hypercall. */ + rcStrict = GIMHypercall(pVCpu, pMixedCtx); + if (rcStrict == VINF_SUCCESS) + { + rc = hmR0VmxAdvanceGuestRip(pVCpu, pMixedCtx, pVmxTransient); + AssertRCReturn(rc, rc); + } + else + Assert( rcStrict == VINF_GIM_R3_HYPERCALL + || rcStrict == VINF_GIM_HYPERCALL_CONTINUING + || RT_FAILURE(VBOXSTRICTRC_VAL(rcStrict))); + + /* If the hypercall changes anything other than guest's general-purpose registers, we would need to reload the guest changed bits here before VM-entry. */ - return rc; - } - - Log4(("hmR0VmxExitVmcall: Hypercalls not enabled\n")); - hmR0VmxSetPendingXcptUD(pVCpu, pMixedCtx); - return VINF_SUCCESS; + } + else + Log4(("hmR0VmxExitVmcall: Hypercalls not enabled\n")); + + /* If hypercalls are disabled or the hypercall failed for some reason, raise #UD and continue. */ + if (RT_FAILURE(VBOXSTRICTRC_VAL(rcStrict))) + { + hmR0VmxSetPendingXcptUD(pVCpu, pMixedCtx); + rcStrict = VINF_SUCCESS; + } + + return rcStrict; } Index: /trunk/src/VBox/VMM/VMMR3/GIM.cpp =================================================================== --- /trunk/src/VBox/VMM/VMMR3/GIM.cpp (revision 61543) +++ /trunk/src/VBox/VMM/VMMR3/GIM.cpp (revision 61544) @@ -434,6 +434,6 @@ if (ASMAtomicReadBool(&pDbg->fDbgRecvBufRead) == true) { - STAM_COUNTER_INC(&pVM->gim.s.StatDbgRecv); - STAM_COUNTER_ADD(&pVM->gim.s.StatDbgRecvBytes, pDbg->cbDbgRecvBufRead); + STAM_REL_COUNTER_INC(&pVM->gim.s.StatDbgRecv); + STAM_REL_COUNTER_ADD(&pVM->gim.s.StatDbgRecvBytes, pDbg->cbDbgRecvBufRead); memcpy(pvRead, pDbg->pvDbgRecvBuf, pDbg->cbDbgRecvBufRead); @@ -477,6 +477,6 @@ && *pcbWrite == cbWrite) { - STAM_COUNTER_INC(&pVM->gim.s.StatDbgXmit); - STAM_COUNTER_ADD(&pVM->gim.s.StatDbgXmitBytes, *pcbWrite); + STAM_REL_COUNTER_INC(&pVM->gim.s.StatDbgXmit); + STAM_REL_COUNTER_ADD(&pVM->gim.s.StatDbgXmitBytes, *pcbWrite); } return rc; Index: /trunk/src/VBox/VMM/VMMR3/GIMHv.cpp =================================================================== --- /trunk/src/VBox/VMM/VMMR3/GIMHv.cpp (revision 61543) +++ /trunk/src/VBox/VMM/VMMR3/GIMHv.cpp (revision 61544) @@ -1712,4 +1712,6 @@ rc = VERR_GIM_HYPERCALL_MEMORY_WRITE_FAILED; } + else + Assert(rc == VINF_SUCCESS); *prcHv = rcHv; @@ -1791,4 +1793,6 @@ rc = VERR_GIM_HYPERCALL_MEMORY_WRITE_FAILED; } + else + Assert(rc == VINF_SUCCESS); *prcHv = rcHv; Index: /trunk/src/VBox/VMM/VMMRC/TRPMRCHandlers.cpp =================================================================== --- /trunk/src/VBox/VMM/VMMRC/TRPMRCHandlers.cpp (revision 61543) +++ /trunk/src/VBox/VMM/VMMRC/TRPMRCHandlers.cpp (revision 61544) @@ -643,8 +643,19 @@ { LogFlow(("TRPMGCTrap06Handler: -> GIMXcptUD\n")); - rc = GIMXcptUD(pVCpu, CPUMCTX_FROM_CORE(pRegFrame), &Cpu); - if (RT_FAILURE(rc)) + VBOXSTRICTRC rcStrict = GIMXcptUD(pVCpu, CPUMCTX_FROM_CORE(pRegFrame), &Cpu, NULL /* pcbInstr */); + if (rcStrict == VINF_SUCCESS) { - LogFlow(("TRPMGCTrap06Handler: -> GIMXcptUD -> VINF_EM_RAW_EMULATE_INSTR\n")); + /* The interrupt inhibition wrt to EIP will be handled by trpmGCExitTrap() below. */ + pRegFrame->eip += Cpu.cbInstr; + Assert(Cpu.cbInstr); + } + else if (rcStrict == VINF_GIM_HYPERCALL_CONTINUING) + rc = VINF_SUCCESS; + else if (rcStrict == VINF_GIM_R3_HYPERCALL) + rc = VINF_GIM_R3_HYPERCALL; + else + { + Assert(RT_FAILURE(VBOXSTRICTRC_VAL(rcStrict))); + LogFlow(("TRPMGCTrap06Handler: GIMXcptUD returns %Rrc -> VINF_EM_RAW_EMULATE_INSTR\n", rc)); rc = VINF_EM_RAW_EMULATE_INSTR; } Index: /trunk/src/VBox/VMM/include/EMHandleRCTmpl.h =================================================================== --- /trunk/src/VBox/VMM/include/EMHandleRCTmpl.h (revision 61543) +++ /trunk/src/VBox/VMM/include/EMHandleRCTmpl.h (revision 61544) @@ -236,6 +236,28 @@ */ case VINF_GIM_R3_HYPERCALL: - rc = GIMHypercall(pVCpu, pCtx); - break; + { + /** @todo IEM/REM need to handle VMCALL/VMMCALL, see + * @bugref{7270#c168}. */ + uint8_t cbInstr = 0; + VBOXSTRICTRC rcStrict = GIMExecHypercallInstr(pVCpu, pCtx, &cbInstr); + if (rcStrict == VINF_SUCCESS) + { + Assert(cbInstr); + pCtx->rip += cbInstr; + /* Update interrupt inhibition. */ + if ( VMCPU_FF_IS_PENDING(pVCpu, VMCPU_FF_INHIBIT_INTERRUPTS) + && pCtx->rip != EMGetInhibitInterruptsPC(pVCpu)) + VMCPU_FF_CLEAR(pVCpu, VMCPU_FF_INHIBIT_INTERRUPTS); + rc = VINF_SUCCESS; + } + else if (rcStrict == VINF_GIM_HYPERCALL_CONTINUING) + rc = VINF_SUCCESS; + else + { + Assert(rcStrict != VINF_GIM_R3_HYPERCALL); + rc = VBOXSTRICTRC_VAL(rcStrict); + } + break; + } #ifdef EMHANDLERC_WITH_HM Index: /trunk/src/VBox/VMM/include/GIMHvInternal.h =================================================================== --- /trunk/src/VBox/VMM/include/GIMHvInternal.h (revision 61543) +++ /trunk/src/VBox/VMM/include/GIMHvInternal.h (revision 61544) @@ -1139,6 +1139,7 @@ VMM_INT_DECL(bool) gimHvAreHypercallsEnabled(PVMCPU pVCpu); VMM_INT_DECL(bool) gimHvShouldTrapXcptUD(PVMCPU pVCpu); -VMM_INT_DECL(int) gimHvXcptUD(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis); -VMM_INT_DECL(int) gimHvHypercall(PVMCPU pVCpu, PCPUMCTX pCtx); +VMM_INT_DECL(VBOXSTRICTRC) gimHvXcptUD(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis, uint8_t *pcbInstr); +VMM_INT_DECL(VBOXSTRICTRC) gimHvHypercall(PVMCPU pVCpu, PCPUMCTX pCtx); +VMM_INT_DECL(VBOXSTRICTRC) gimHvExecHypercallInstr(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis); VMM_INT_DECL(VBOXSTRICTRC) gimHvReadMsr(PVMCPU pVCpu, uint32_t idMsr, PCCPUMMSRRANGE pRange, uint64_t *puValue); VMM_INT_DECL(VBOXSTRICTRC) gimHvWriteMsr(PVMCPU pVCpu, uint32_t idMsr, PCCPUMMSRRANGE pRange, uint64_t uRawValue); Index: /trunk/src/VBox/VMM/include/GIMKvmInternal.h =================================================================== --- /trunk/src/VBox/VMM/include/GIMKvmInternal.h (revision 61543) +++ /trunk/src/VBox/VMM/include/GIMKvmInternal.h (revision 61544) @@ -261,9 +261,10 @@ VMM_INT_DECL(bool) gimKvmIsParavirtTscEnabled(PVM pVM); VMM_INT_DECL(bool) gimKvmAreHypercallsEnabled(PVMCPU pVCpu); -VMM_INT_DECL(int) gimKvmHypercall(PVMCPU pVCpu, PCPUMCTX pCtx); +VMM_INT_DECL(VBOXSTRICTRC) gimKvmHypercall(PVMCPU pVCpu, PCPUMCTX pCtx); VMM_INT_DECL(VBOXSTRICTRC) gimKvmReadMsr(PVMCPU pVCpu, uint32_t idMsr, PCCPUMMSRRANGE pRange, uint64_t *puValue); VMM_INT_DECL(VBOXSTRICTRC) gimKvmWriteMsr(PVMCPU pVCpu, uint32_t idMsr, PCCPUMMSRRANGE pRange, uint64_t uRawValue); VMM_INT_DECL(bool) gimKvmShouldTrapXcptUD(PVMCPU pVCpu); -VMM_INT_DECL(int) gimKvmXcptUD(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis); +VMM_INT_DECL(VBOXSTRICTRC) gimKvmXcptUD(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis, uint8_t *pcbInstr); +VMM_INT_DECL(VBOXSTRICTRC) gimKvmExecHypercallInstr(PVMCPU pVCpu, PCPUMCTX pCtx, PDISCPUSTATE pDis);