Changeset 47247 in vbox

Timestamp:

Jul 19, 2013 10:01:36 AM (12 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

87376

Message:

HMVMX: preserve SS.DPL and CS.L/D/G when the 'unusable' bit is set. SS.DPL = CPL, so we absolutely must preserve that.

Location:

trunk

Files:

• include/iprt/x86.h (modified) (2 diffs)
• src/VBox/VMM/VMMR0/HMVMXR0.cpp (modified) (13 diffs)

trunk/include/iprt/x86.h

@@ -2230 +2230 @@
 #ifndef VBOX_FOR_DTRACE_LIB
 /**
- * Descriptor attributes.
+ * Descriptor attributes (as seen by VT-x).
  */
 typedef struct X86DESCATTRBITS
@@ -2254 +2254 @@
      * clear byte. */
     unsigned    u1Granularity : 1;
-    /** 10 - Unusable selector, special Intel (VT-x only?) bit. */
+    /** 10 - "Unusable" selector, special Intel (VT-x only?) bit. */
     unsigned    u1Unusable : 1;
 } X86DESCATTRBITS;
 #endif /* !VBOX_FOR_DTRACE_LIB */
+
+/** @name X86DESCATTR masks
+ * @{ */
+#define X86DESCATTR_TYPE            UINT32_C(0x0000000f)
+#define X86DESCATTR_DT              UINT32_C(0x00000010)
+#define X86DESCATTR_DPL             UINT32_C(0x00000060)
+#define X86DESCATTR_P               UINT32_C(0x00000800)
+#define X86DESCATTR_LIMIT_HIGH      UINT32_C(0x00000f00)
+#define X86DESCATTR_AVL             UINT32_C(0x00001000)
+#define X86DESCATTR_L               UINT32_C(0x00002000)
+#define X86DESCATTR_D               UINT32_C(0x00004000)
+#define X86DESCATTR_G               UINT32_C(0x00008000)
+#define X86DESCATTR_UNUSABLE        UINT32_C(0x00010000)
+/** @}  */
 
 #pragma pack(1)

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

@@ -60 +60 @@
 /** Use the function table. */
 #define HMVMX_USE_FUNCTION_TABLE
-
-/** This bit indicates the segment selector is unusable in VT-x. */
-#define HMVMX_SEL_UNUSABLE                       RT_BIT(16)
 
 /** Determine which tagged-TLB flush handler to use. */
@@ -3373 +3370 @@
                || (pCtx->cs.Attr.n.u1Granularity));
         /* CS cannot be loaded with NULL in protected mode. */
-        Assert(pCtx->cs.Attr.u && !(pCtx->cs.Attr.u & HMVMX_SEL_UNUSABLE)); /** @todo is this really true even for 64-bit CS?!? */
+        Assert(pCtx->cs.Attr.u && !(pCtx->cs.Attr.u & X86DESCATTR_UNUSABLE)); /** @todo is this really true even for 64-bit CS?!? */
         if (pCtx->cs.Attr.n.u4Type == 9 || pCtx->cs.Attr.n.u4Type == 11)
             Assert(pCtx->cs.Attr.n.u2Dpl == pCtx->ss.Attr.n.u2Dpl);
@@ -3389 +3386 @@
             Assert(!pCtx->ss.Attr.n.u2Dpl);
         }
-        if (pCtx->ss.Attr.u && !(pCtx->ss.Attr.u & HMVMX_SEL_UNUSABLE))
+        if (pCtx->ss.Attr.u && !(pCtx->ss.Attr.u & X86DESCATTR_UNUSABLE))
         {
             Assert((pCtx->ss.Sel & X86_SEL_RPL) == (pCtx->cs.Sel & X86_SEL_RPL));
@@ -3402 +3399 @@
         }
         /* DS, ES, FS, GS - only check for usable selectors, see hmR0VmxWriteSegmentReg(). */
-        if (pCtx->ds.Attr.u && !(pCtx->ds.Attr.u & HMVMX_SEL_UNUSABLE))
+        if (pCtx->ds.Attr.u && !(pCtx->ds.Attr.u & X86DESCATTR_UNUSABLE))
         {
             Assert(pCtx->ds.Attr.n.u4Type & X86_SEL_TYPE_ACCESSED);
@@ -3416 +3413 @@
                    || (pCtx->ds.Attr.n.u4Type & X86_SEL_TYPE_READ));
         }
-        if (pCtx->es.Attr.u && !(pCtx->es.Attr.u & HMVMX_SEL_UNUSABLE))
+        if (pCtx->es.Attr.u && !(pCtx->es.Attr.u & X86DESCATTR_UNUSABLE))
         {
             Assert(pCtx->es.Attr.n.u4Type & X86_SEL_TYPE_ACCESSED);
@@ -3430 +3427 @@
                    || (pCtx->es.Attr.n.u4Type & X86_SEL_TYPE_READ));
         }
-        if (pCtx->fs.Attr.u && !(pCtx->fs.Attr.u & HMVMX_SEL_UNUSABLE))
+        if (pCtx->fs.Attr.u && !(pCtx->fs.Attr.u & X86DESCATTR_UNUSABLE))
         {
             Assert(pCtx->fs.Attr.n.u4Type & X86_SEL_TYPE_ACCESSED);
@@ -3444 +3441 @@
                    || (pCtx->fs.Attr.n.u4Type & X86_SEL_TYPE_READ));
         }
-        if (pCtx->gs.Attr.u && !(pCtx->gs.Attr.u & HMVMX_SEL_UNUSABLE))
+        if (pCtx->gs.Attr.u && !(pCtx->gs.Attr.u & X86DESCATTR_UNUSABLE))
         {
             Assert(pCtx->gs.Attr.n.u4Type & X86_SEL_TYPE_ACCESSED);
@@ -3560 +3557 @@
          */
         if (!u32Access)
-            u32Access = HMVMX_SEL_UNUSABLE;
+            u32Access = X86DESCATTR_UNUSABLE;
     }
 
     /* Validate segment access rights. Refer to Intel spec. "26.3.1.2 Checks on Guest Segment Registers". */
-    AssertMsg((u32Access & HMVMX_SEL_UNUSABLE) || (u32Access & X86_SEL_TYPE_ACCESSED),
+    AssertMsg((u32Access & X86DESCATTR_UNUSABLE) || (u32Access & X86_SEL_TYPE_ACCESSED),
               ("Access bit not set for usable segment. idx=%#x sel=%#x attr %#x\n", idxBase, pSelReg, pSelReg->Attr.u));
 
@@ -3698 +3695 @@
         AssertMsg(   (u32AccessRights & 0xf) == X86_SEL_TYPE_SYS_386_TSS_BUSY
                   || (u32AccessRights & 0xf) == X86_SEL_TYPE_SYS_286_TSS_BUSY, ("TSS is not busy!? %#x\n", u32AccessRights));
-        AssertMsg(!(u32AccessRights & HMVMX_SEL_UNUSABLE), ("TR unusable bit is not clear!? %#x\n", u32AccessRights));
+        AssertMsg(!(u32AccessRights & X86DESCATTR_UNUSABLE), ("TR unusable bit is not clear!? %#x\n", u32AccessRights));
         Assert(!(u32AccessRights & RT_BIT(4)));                 /* System MBZ.*/
         Assert(u32AccessRights & RT_BIT(7));                    /* Present MB1.*/
@@ -3738 +3735 @@
         uint32_t u32Access = 0;
         if (!pMixedCtx->ldtr.Attr.u)
-            u32Access = HMVMX_SEL_UNUSABLE;
+            u32Access = X86DESCATTR_UNUSABLE;
         else
             u32Access = pMixedCtx->ldtr.Attr.u;
@@ -3748 +3745 @@
 
         /* Validate. */
-        if (!(u32Access & HMVMX_SEL_UNUSABLE))
+        if (!(u32Access & X86DESCATTR_UNUSABLE))
         {
             Assert(!(pMixedCtx->ldtr.Sel & RT_BIT(2)));              /* TI MBZ. */
@@ -5438 +5435 @@
      * bird: This isn't quite as simple.  VT-x and VBox(!) requires the DPL for SS to be the the same as CPL.  In 64-bit mode it
      *       is possible (int/trap/xxx injects does this when switching rings) to load SS with a NULL selector and RPL=CPL.
-     *       The Attr.u = HMVMX_SEL_UNUSABLE works fine as long as nobody uses ring-1 or ring-2.  VT-x seems to set the DPL
+     *       The Attr.u = X86DESCATTR_UNUSABLE works fine as long as nobody uses ring-1 or ring-2.  VT-x seems to set the DPL
      *       correctly in the attributes even when the unusable bit is set, we need to preseve the DPL or we get invalid guest
      *       state trouble.  Try bs2-cpu-hidden-regs-1.
      */
-    if (pSelReg->Attr.u & HMVMX_SEL_UNUSABLE)
+    if (pSelReg->Attr.u & X86DESCATTR_UNUSABLE)
     {
         Assert(idxSel != VMX_VMCS16_GUEST_FIELD_TR);          /* TR is the only selector that can never be unusable. */
         Log(("idxSel=%#x attr=%#x\n", idxSel, pSelReg->Attr.u));
 
-#if 0 /** @todo Is there any code which will freak out if we do this for all?  Better track it down and fix. */
-        pSelReg->Attr.u &= HMVMX_SEL_UNUSABLE | (UINT32_C(3) << 5);
-#else
-        pSelReg->Attr.u = HMVMX_SEL_UNUSABLE;
-#endif
+        if (idxSel == VMX_VMCS16_GUEST_FIELD_SS)
+            pSelReg->Attr.u &= X86DESCATTR_UNUSABLE | X86DESCATTR_DPL;
+        else if (idxSel == VMX_VMCS16_GUEST_FIELD_CS)
+            pSelReg->Attr.u &= X86DESCATTR_UNUSABLE | X86DESCATTR_L | X86DESCATTR_D | X86DESCATTR_G;
+        else
+            pSelReg->Attr.u = X86DESCATTR_UNUSABLE;
     }
     return VINF_SUCCESS;
@@ -5458 +5456 @@
 
 #ifdef VMX_USE_CACHED_VMCS_ACCESSES
-#define VMXLOCAL_READ_SEG(Sel, CtxSel) \
+# define VMXLOCAL_READ_SEG(Sel, CtxSel) \
     hmR0VmxReadSegmentReg(pVCpu, VMX_VMCS16_GUEST_FIELD_##Sel, VMX_VMCS32_GUEST_##Sel##_LIMIT, \
                           VMX_VMCS_GUEST_##Sel##_BASE_CACHE_IDX, VMX_VMCS32_GUEST_##Sel##_ACCESS_RIGHTS, &pMixedCtx->CtxSel)
 #else
-#define VMXLOCAL_READ_SEG(Sel, CtxSel) \
+# define VMXLOCAL_READ_SEG(Sel, CtxSel) \
     hmR0VmxReadSegmentReg(pVCpu, VMX_VMCS16_GUEST_FIELD_##Sel, VMX_VMCS32_GUEST_##Sel##_LIMIT, \
                           VMX_VMCS_GUEST_##Sel##_BASE, VMX_VMCS32_GUEST_##Sel##_ACCESS_RIGHTS, &pMixedCtx->CtxSel)