VirtualBox

Changeset 58118 in vbox for trunk


Ignore:
Timestamp:
Oct 8, 2015 4:04:59 PM (9 years ago)
Author:
vboxsync
Message:

VMM,GIM: Made the order of RIP update on hypercalls consistent.
Removed redundant DBGFIsStepping() in VMX/SVM R0 code and fixed a couple of places where pending debug
exceptions weren't being handled properly.

Location:
trunk/src/VBox/VMM
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/VBox/VMM/VMMAll/GIMAllHv.cpp

    r58116 r58118  
    753753
    754754            /*
    755              * Perform the hypercall and update RIP.
     755             * Update RIP and perform the hypercall.
    756756             */
     757            pCtx->rip += pDis->cbInstr;
    757758            rc = gimHvHypercall(pVCpu, pCtx);
    758             pCtx->rip += pDis->cbInstr;
    759             return rc;
    760         }
    761         return VERR_GIM_OPERATION_FAILED;
    762     }
    763     return VERR_GIM_OPERATION_FAILED;
    764 }
    765 
     759        }
     760        else
     761            rc = VERR_GIM_OPERATION_FAILED;
     762    }
     763    return rc;
     764}
     765

  • trunk/src/VBox/VMM/VMMAll/GIMAllKvm.cpp

    r58116 r58118  
    409409
    410410            /*
    411              * Perform the hypercall and update RIP.
     411             * Update RIP and perform the hypercall.
    412412             *
    413413             * For HM, we can simply resume guest execution without performing the hypercall now and
    … …  
    419419            if (RT_SUCCESS(rc))
    420420            {
    421                 int rc2 = gimKvmHypercall(pVCpu, pCtx);
    422                 AssertRC(rc2);
    423421                pCtx->rip += pDis->cbInstr;
    424             }
    425             return rc;
    426         }
    427     }
    428 
    429     return VERR_GIM_OPERATION_FAILED;
    430 }
    431 
     422                rc = gimKvmHypercall(pVCpu, pCtx);
     423            }
     424        }
     425        else
     426            rc = VERR_GIM_OPERATION_FAILED;
     427    }
     428    return rc;
     429}
     430

  • trunk/src/VBox/VMM/VMMR0/HMSVMR0.cpp

    r58116 r58118  
    14441444     * the VMM level like the VT-x implementations does.
    14451445     */
    1446     bool const fStepping = pVCpu->hm.s.fSingleInstruction || DBGFIsStepping(pVCpu);
     1446    bool const fStepping = pVCpu->hm.s.fSingleInstruction;
    14471447    if (fStepping)
    14481448    {
    … …  
    14521452        fInterceptMovDRx = true; /* Need clean DR6, no guest mess. */
    14531453    }
     1454    else
     1455        Assert(!DBGFIsStepping(pVCpu));
    14541456
    14551457    if (   fStepping
    … …  
    34743476
    34753477    int rc;
    3476     if (!pVCpu->hm.s.fSingleInstruction && !DBGFIsStepping(pVCpu))
     3478    if (!pVCpu->hm.s.fSingleInstruction)
    34773479        rc = hmR0SvmRunGuestCodeNormal(pVM, pVCpu, pCtx);
    34783480    else
    … …  
    51575159        if (pVCpu->hm.s.fHypercallsEnabled)
    51585160        {
     5161            hmR0SvmUpdateRip(pVCpu, pCtx, 3);
    51595162            rc = GIMHypercall(pVCpu, pCtx);
    5160             if (   rc == VINF_SUCCESS
    5161                 || rc == VINF_GIM_R3_HYPERCALL)
    5162             {
    5163                 /* If the hypercall changes anything other than guest general-purpose registers,
    5164                    we would need to reload the guest changed bits here before VM-reentry. */
    5165                 hmR0SvmUpdateRip(pVCpu, pCtx, 3);
    5166                 return rc;
    5167             }
     5163            /* If the hypercall changes anything other than guest general-purpose registers,
     5164               we would need to reload the guest changed bits here before VM-entry. */
     5165            return rc;
    51685166        }
    51695167        else

  • trunk/src/VBox/VMM/VMMR0/HMVMXR0.cpp

    r58116 r58118  
    40084008    bool fInterceptDB     = false;
    40094009    bool fInterceptMovDRx = false;
    4010     if (   pVCpu->hm.s.fSingleInstruction
    4011         || DBGFIsStepping(pVCpu))
     4010    if (pVCpu->hm.s.fSingleInstruction)
    40124011    {
    40134012        /* If the CPU supports the monitor trap flag, use it for single stepping in DBGF and avoid intercepting #DB. */
    … …  
    73597358             && !pVCpu->hm.s.fSingleInstruction)
    73607359    {
     7360        Assert(!DBGFIsStepping(pVCpu));
    73617361        int rc = hmR0VmxSaveGuestRflags(pVCpu, pMixedCtx);
    73627362        AssertRC(rc);
    … …  
    73927392/**
    73937393 * Sets a pending-debug exception to be delivered to the guest if the guest is
    7394  * single-stepping.
     7394 * single-stepping in the VMCS.
    73957395 *
    73967396 * @param   pVCpu           Pointer to the VMCPU.
    7397  * @param   pMixedCtx       Pointer to the guest-CPU context. The data may be
    7398  *                          out-of-sync. Make sure to update the required fields
    7399  *                          before using them.
    7400  */
    7401 DECLINLINE(void) hmR0VmxSetPendingDebugXcpt(PVMCPU pVCpu, PCPUMCTX pMixedCtx)
     7397 */
     7398DECLINLINE(void) hmR0VmxSetPendingDebugXcptVmcs(PVMCPU pVCpu)
    74027399{
    74037400    Assert(HMVMXCPU_GST_IS_UPDATED(pVCpu, HMVMX_UPDATED_GUEST_RFLAGS)); NOREF(pVCpu);
    7404     if (pMixedCtx->eflags.Bits.u1TF)    /* We don't have any IA32_DEBUGCTL MSR for guests. Treat as all bits 0. */
    7405     {
    7406         int rc = VMXWriteVmcs32(VMX_VMCS_GUEST_PENDING_DEBUG_EXCEPTIONS, VMX_VMCS_GUEST_DEBUG_EXCEPTIONS_BS);
    7407         AssertRC(rc);
    7408     }
     7401    int rc = VMXWriteVmcs32(VMX_VMCS_GUEST_PENDING_DEBUG_EXCEPTIONS, VMX_VMCS_GUEST_DEBUG_EXCEPTIONS_BS);
     7402    AssertRC(rc);
    74097403}
    74107404
    … …  
    74867480        || fBlockMovSS)
    74877481    {
    7488         if (   !pVCpu->hm.s.fSingleInstruction
    7489             && !DBGFIsStepping(pVCpu))
     7482        if (!pVCpu->hm.s.fSingleInstruction)
    74907483        {
    74917484            /*
    … …  
    74947487             * See Intel spec. 27.3.4 "Saving Non-Register State".
    74957488             */
     7489            Assert(!DBGFIsStepping(pVCpu));
    74967490            int rc2 = hmR0VmxSaveGuestRflags(pVCpu, pMixedCtx);
    74977491            AssertRCReturn(rc2, rc2);
    7498             hmR0VmxSetPendingDebugXcpt(pVCpu, pMixedCtx);
     7492            if (pMixedCtx->eflags.Bits.u1TF)
     7493                hmR0VmxSetPendingDebugXcptVmcs(pVCpu);
    74997494        }
    75007495        else if (pMixedCtx->eflags.Bits.u1TF)
    … …  
    89128907
    89138908    int rc;
    8914     if (!pVCpu->hm.s.fSingleInstruction && !DBGFIsStepping(pVCpu))
     8909    if (!pVCpu->hm.s.fSingleInstruction)
    89158910        rc = hmR0VmxRunGuestCodeNormal(pVM, pVCpu, pCtx);
    89168911    else
    … …  
    91819176     * See Intel spec. 32.2.1 "Debug Exceptions".
    91829177     */
    9183     hmR0VmxSetPendingDebugXcpt(pVCpu, pMixedCtx);
     9178    if (  !pVCpu->hm.s.fSingleInstruction
     9179        && pMixedCtx->eflags.Bits.u1TF)
     9180        hmR0VmxSetPendingDebugXcptVmcs(pVCpu);
    91849181
    91859182    return rc;
    … …  
    1014510142        rc     |= hmR0VmxSaveGuestSegmentRegs(pVCpu, pMixedCtx);    /* For long-mode checks in gimKvmHypercall(). */
    1014610143#endif
     10144        rc     |= hmR0VmxAdvanceGuestRip(pVCpu, pMixedCtx, pVmxTransient);
    1014710145        AssertRCReturn(rc, rc);
    1014810146
    1014910147        rc = GIMHypercall(pVCpu, pMixedCtx);
    10150         if (   rc == VINF_SUCCESS
    10151             || rc == VINF_GIM_R3_HYPERCALL)
    10152         {
    10153             /* If the hypercall changes anything other than guest general-purpose registers,
    10154                we would need to reload the guest changed bits here before VM-reentry. */
    10155             hmR0VmxAdvanceGuestRip(pVCpu, pMixedCtx, pVmxTransient);
    10156             return rc;
    10157         }
     10148        /* If the hypercall changes anything other than guest general-purpose registers,
     10149           we would need to reload the guest changed bits here before VM-entry. */
     10150        return rc;
    1015810151    }
    1015910152    else
     10153    {
    1016010154        Log4(("hmR0VmxExitVmcall: Hypercalls not enabled\n"));
    10161 
    10162     hmR0VmxSetPendingXcptUD(pVCpu, pMixedCtx);
     10155        hmR0VmxSetPendingXcptUD(pVCpu, pMixedCtx);
     10156    }
     10157
    1016310158    return VINF_SUCCESS;
    1016410159}
    … …  
    1091010905
    1091110906    /* Refer Intel spec. 27-5. "Exit Qualifications for I/O Instructions" for the format. */
    10912     uint32_t uIOPort   = VMX_EXIT_QUALIFICATION_IO_PORT(pVmxTransient->uExitQualification);
    10913     uint8_t  uIOWidth  = VMX_EXIT_QUALIFICATION_IO_WIDTH(pVmxTransient->uExitQualification);
    10914     bool     fIOWrite  = (   VMX_EXIT_QUALIFICATION_IO_DIRECTION(pVmxTransient->uExitQualification)
    10915                           == VMX_EXIT_QUALIFICATION_IO_DIRECTION_OUT);
    10916     bool     fIOString = VMX_EXIT_QUALIFICATION_IO_IS_STRING(pVmxTransient->uExitQualification);
    10917     bool     fStepping = RT_BOOL(pMixedCtx->eflags.Bits.u1TF);
     10907    uint32_t uIOPort      = VMX_EXIT_QUALIFICATION_IO_PORT(pVmxTransient->uExitQualification);
     10908    uint8_t  uIOWidth     = VMX_EXIT_QUALIFICATION_IO_WIDTH(pVmxTransient->uExitQualification);
     10909    bool     fIOWrite     = (   VMX_EXIT_QUALIFICATION_IO_DIRECTION(pVmxTransient->uExitQualification)
     10910                             == VMX_EXIT_QUALIFICATION_IO_DIRECTION_OUT);
     10911    bool     fIOString    = VMX_EXIT_QUALIFICATION_IO_IS_STRING(pVmxTransient->uExitQualification);
     10912    bool     fGstStepping = RT_BOOL(pMixedCtx->eflags.Bits.u1TF);
     10913    bool     fDbgStepping = pVCpu->hm.s.fSingleInstruction;
    1091810914    AssertReturn(uIOWidth <= 3 && uIOWidth != 2, VERR_VMX_IPE_1);
    1091910915
    … …  
    1104911045            HMCPU_CF_SET(pVCpu, HM_CHANGED_GUEST_RFLAGS);
    1105011046        }
    11051         else if (fStepping)
    11052             hmR0VmxSetPendingDebugXcpt(pVCpu, pMixedCtx);
     11047        else if (  !fDbgStepping
     11048                 && fGstStepping)
     11049        {
     11050            hmR0VmxSetPendingDebugXcptVmcs(pVCpu);
     11051        }
    1105311052
    1105411053        /*
    … …  
    1128311282
    1128411283    int rc = VERR_INTERNAL_ERROR_5;
    11285     if (   !DBGFIsStepping(pVCpu)
    11286         && !pVCpu->hm.s.fSingleInstruction
     11284    if (   !pVCpu->hm.s.fSingleInstruction
    1128711285        && !pVmxTransient->fWasHyperDebugStateActive)
    1128811286    {
     11287        Assert(!DBGFIsStepping(pVCpu));
     11288
    1128911289        /* Don't intercept MOV DRx and #DB any more. */
    1129011290        pVCpu->hm.s.vmx.u32ProcCtls &= ~VMX_VMCS_CTRL_PROC_EXEC_MOV_DR_EXIT;
    … …  
    1174011740    uint32_t cbOp     = 0;
    1174111741    PVM pVM           = pVCpu->CTX_SUFF(pVM);
     11742    bool fDbgStepping = pVCpu->hm.s.fSingleInstruction;
    1174211743    rc = EMInterpretDisasCurrent(pVM, pVCpu, pDis, &cbOp);
    1174311744    if (RT_SUCCESS(rc))
    … …  
    1175411755                pMixedCtx->rip += pDis->cbInstr;
    1175511756                HMCPU_CF_SET(pVCpu, HM_CHANGED_GUEST_RIP | HM_CHANGED_GUEST_RFLAGS);
    11756                 hmR0VmxSetPendingDebugXcpt(pVCpu, pMixedCtx);
     11757                if (   !fDbgStepping
     11758                    && pMixedCtx->eflags.Bits.u1TF)
     11759                    hmR0VmxSetPendingDebugXcptVmcs(pVCpu);
    1175711760                STAM_COUNTER_INC(&pVCpu->hm.s.StatExitCli);
    1175811761                break;
    … …  
    1177111774                }
    1177211775                HMCPU_CF_SET(pVCpu, HM_CHANGED_GUEST_RIP | HM_CHANGED_GUEST_RFLAGS);
    11773                 hmR0VmxSetPendingDebugXcpt(pVCpu, pMixedCtx);
     11776                if (   !fDbgStepping
     11777                    && pMixedCtx->eflags.Bits.u1TF)
     11778                    hmR0VmxSetPendingDebugXcptVmcs(pVCpu);
    1177411779                STAM_COUNTER_INC(&pVCpu->hm.s.StatExitSti);
    1177511780                break;
    … …  
    1179111796                uint32_t cbParm;
    1179211797                uint32_t uMask;
    11793                 bool     fStepping = RT_BOOL(pMixedCtx->eflags.Bits.u1TF);
     11798                bool     fGstStepping = RT_BOOL(pMixedCtx->eflags.Bits.u1TF);
    1179411799                if (pDis->fPrefix & DISPREFIX_OPSIZE)
    1179511800                {
    … …  
    1182911834                                    | HM_CHANGED_GUEST_RSP
    1183011835                                    | HM_CHANGED_GUEST_RFLAGS);
    11831                 /* Generate a pending-debug exception when stepping over POPF regardless of how POPF modifies EFLAGS.TF. */
    11832                 if (fStepping)
    11833                     hmR0VmxSetPendingDebugXcpt(pVCpu, pMixedCtx);
    11834 
     11836                /* Generate a pending-debug exception when the guest stepping over POPF regardless of how
     11837                   POPF restores EFLAGS.TF. */
     11838                if (  !fDbgStepping
     11839                    && fGstStepping)
     11840                    hmR0VmxSetPendingDebugXcptVmcs(pVCpu);
    1183511841                STAM_COUNTER_INC(&pVCpu->hm.s.StatExitPopf);
    1183611842                break;
    … …  
    1188111887                                    | HM_CHANGED_GUEST_RSP
    1188211888                                    | HM_CHANGED_GUEST_RFLAGS);
    11883                 hmR0VmxSetPendingDebugXcpt(pVCpu, pMixedCtx);
     11889                if (  !fDbgStepping
     11890                    && pMixedCtx->eflags.Bits.u1TF)
     11891                    hmR0VmxSetPendingDebugXcptVmcs(pVCpu);
    1188411892                STAM_COUNTER_INC(&pVCpu->hm.s.StatExitPushf);
    1188511893                break;
    … …  
    1189011898                /** @todo Handle 32-bit operand sizes and check stack limits. See Intel
    1189111899                 *        instruction reference. */
    11892                 RTGCPTR  GCPtrStack = 0;
    11893                 uint32_t uMask      = 0xffff;
    11894                 bool     fStepping  = RT_BOOL(pMixedCtx->eflags.Bits.u1TF);
     11900                RTGCPTR  GCPtrStack    = 0;
     11901                uint32_t uMask         = 0xffff;
     11902                bool     fGstStepping  = RT_BOOL(pMixedCtx->eflags.Bits.u1TF);
    1189511903                uint16_t aIretFrame[3];
    1189611904                if (pDis->fPrefix & (DISPREFIX_OPSIZE | DISPREFIX_ADDRSIZE))
    … …  
    1192511933                                    | HM_CHANGED_GUEST_RFLAGS);
    1192611934                /* Generate a pending-debug exception when stepping over IRET regardless of how IRET modifies EFLAGS.TF. */
    11927                 if (fStepping)
    11928                     hmR0VmxSetPendingDebugXcpt(pVCpu, pMixedCtx);
     11935                if (   !fDbgStepping
     11936                    && fGstStepping)
     11937                    hmR0VmxSetPendingDebugXcptVmcs(pVCpu);
    1192911938                Log4(("IRET %#RX32 to %04x:%04x\n", GCPtrStack, pMixedCtx->cs.Sel, pMixedCtx->ip));
    1193011939                STAM_COUNTER_INC(&pVCpu->hm.s.StatExitIret);
Note: See TracChangeset for help on using the changeset viewer.

© 2024 Oracle Support Privacy / Do Not Sell My Info Terms of Use Trademark Policy Automated Access Etiquette