x86: remove the use of vm86_mode()

Xen, being 64bit only, cannot run PV guests in vm86 mode.  HVM guests however
can be running in vm86 mode, and common codepaths need to be able to cope.

The definition of vm86_mode() in x86_64/regs.h is incorrect, as the predicate
is used by non-PV codepaths.

One buggy use is in hvm/emulate.c.  An HVM guest can be in vm86 mode, and
vm86_mode() sliently omits the check.  Luckily, due to the VEX prefix decoding
logic in x86_emulate(), there is no path to the erronious use with EFLAGS_VM
set.

Another potentially problematic use is in show_guest_stack().  In principle,
show_guest_stack() is common code called for both PV and HVM vcpus.  HVM vcpus
exit early (with no reasonable way of making the code generic), making this
part a PV-only codepath.

Open-code its use in emulate.c, matching the surrounding code.  This causes
all other uses to be in PV-only codepaths, making the code to be logically
dead.  Drop it completely, to avoid future misuse.

Part of resulting cleanup removes vm86attr from read_descriptor(), although
retaining one relevant piece of information; i.e. whether we are reading a
selector for an instruction fetch, or a data fetch.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index f5ab5bc8bbdced1e2ff6e67626f2761c3d35380c..cc0b84138beec3c7642c2af681c814e340868c7b 100644 (file)
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1559,7 +1559,7 @@ static int hvmemul_get_fpu(
         break;
     case X86EMUL_FPU_ymm:
         if ( !(curr->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE) ||
-             vm86_mode(ctxt->regs) ||
+             (ctxt->regs->eflags & X86_EFLAGS_VM) ||
              !(curr->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSXSAVE) ||
              !(curr->arch.xcr0 & XSTATE_SSE) ||
              !(curr->arch.xcr0 & XSTATE_YMM) )

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 6fbb1cf3dfe384cc8e81afbc18bd50e578c1f264..8125c53ffff438e725cfc6f8275520cd768da14b 100644 (file)
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -198,17 +198,8 @@ static void show_guest_stack(struct vcpu *v, const struct cpu_user_regs *regs)
         return;
     }
 
-    if ( vm86_mode(regs) )
-    {
-        stack = (unsigned long *)((regs->ss << 4) + (regs->esp & 0xffff));
-        printk("Guest stack trace from ss:sp = %04x:%04x (VM86)\n  ",
-               regs->ss, (uint16_t)(regs->esp & 0xffff));
-    }
-    else
-    {
-        stack = (unsigned long *)regs->esp;
-        printk("Guest stack trace from "__OP"sp=%p:\n  ", stack);
-    }
+    stack = (unsigned long *)regs->esp;
+    printk("Guest stack trace from "__OP"sp=%p:\n  ", stack);
 
     if ( !access_ok(stack, sizeof(*stack)) )
     {
@@ -1683,28 +1674,20 @@ static int read_descriptor(unsigned int sel,
                            unsigned long *base,
                            unsigned long *limit,
                            unsigned int *ar,
-                           unsigned int vm86attr)
+                           bool_t insn_fetch)
 {
     struct desc_struct desc;
 
-    if ( !vm86_mode(regs) )
-    {
-        if ( sel < 4)
-            desc.b = desc.a = 0;
-        else if ( __get_user(desc,
-                        (const struct desc_struct *)(!(sel & 4)
-                                                     ? GDT_VIRT_START(v)
-                                                     : LDT_VIRT_START(v))
-                        + (sel >> 3)) )
-            return 0;
-        if ( !(vm86attr & _SEGMENT_CODE) )
-            desc.b &= ~_SEGMENT_L;
-    }
-    else
-    {
-        desc.a = (sel << 20) | 0xffff;
-        desc.b = vm86attr | (sel >> 12);
-    }
+    if ( sel < 4)
+        desc.b = desc.a = 0;
+    else if ( __get_user(desc,
+                         (const struct desc_struct *)(!(sel & 4)
+                                                      ? GDT_VIRT_START(v)
+                                                      : LDT_VIRT_START(v))
+                         + (sel >> 3)) )
+        return 0;
+    if ( !insn_fetch )
+        desc.b &= ~_SEGMENT_L;
 
     *ar = desc.b & 0x00f0ff00;
     if ( !(desc.b & _SEGMENT_L) )
@@ -1715,7 +1698,7 @@ static int read_descriptor(unsigned int sel,
         if ( desc.b & _SEGMENT_G )
             *limit = ((*limit + 1) << 12) - 1;
 #ifndef NDEBUG
-        if ( !vm86_mode(regs) && (sel > 3) )
+        if ( sel > 3 )
         {
             unsigned int a, l;
             unsigned char valid;
@@ -1805,8 +1788,7 @@ static int guest_io_okay(
     int user_mode = !(v->arch.flags & TF_kernel_mode);
 #define TOGGLE_MODE() if ( user_mode ) toggle_guest_mode(v)
 
-    if ( !vm86_mode(regs) &&
-         (v->arch.pv_vcpu.iopl >= (guest_kernel_mode(v, regs) ? 1 : 3)) )
+    if ( v->arch.pv_vcpu.iopl >= (guest_kernel_mode(v, regs) ? 1 : 3) )
         return 1;
 
     if ( v->arch.pv_vcpu.iobmp_limit > (port + bytes) )
@@ -2094,8 +2076,7 @@ static int emulate_privileged_op(struct cpu_user_regs *regs)
     bool_t vpmu_msr;
 
     if ( !read_descriptor(regs->cs, v, regs,
-                          &code_base, &code_limit, &ar,
-                          _SEGMENT_CODE|_SEGMENT_S|_SEGMENT_DPL|_SEGMENT_P) )
+                          &code_base, &code_limit, &ar, 1) )
         goto fail;
     op_default = op_bytes = (ar & (_SEGMENT_L|_SEGMENT_DB)) ? 4 : 2;
     ad_default = ad_bytes = (ar & _SEGMENT_L) ? 8 : op_default;
@@ -2187,9 +2168,7 @@ static int emulate_privileged_op(struct cpu_user_regs *regs)
         if ( !(ar & _SEGMENT_L) )
         {
             if ( !read_descriptor(data_sel, v, regs,
-                                  &data_base, &data_limit, &ar,
-                                  _SEGMENT_WR|_SEGMENT_S|_SEGMENT_DPL|
-                                  _SEGMENT_P) )
+                                  &data_base, &data_limit, &ar, 0) )
                 goto fail;
             if ( !(ar & _SEGMENT_S) ||
                  !(ar & _SEGMENT_P) ||

diff --git a/xen/include/asm-x86/regs.h b/xen/include/asm-x86/regs.h
index a6338f0ad169a952a55109e13f083e497051820b..22efc42bed987bfdee643e315d84f61aebf4d94e 100644 (file)
--- a/xen/include/asm-x86/regs.h
+++ b/xen/include/asm-x86/regs.h
@@ -10,7 +10,7 @@
     /* Frame pointer must point into current CPU stack. */                    \
     ASSERT(diff < STACK_SIZE);                                                \
     /* If not a guest frame, it must be a hypervisor frame. */                \
-    ASSERT((diff == 0) || (!vm86_mode(r) && (r->cs == __HYPERVISOR_CS)));     \
+    ASSERT((diff == 0) || (r->cs == __HYPERVISOR_CS));                        \
     /* Return TRUE if it's a guest frame. */                                  \
     (diff == 0);                                                              \
 })

diff --git a/xen/include/asm-x86/x86_64/regs.h b/xen/include/asm-x86/x86_64/regs.h
index 3cdc702ef5488473f1a7826fb0579d44ba85d5f8..171cf9a2e2175865556ef589091d448bdd977254 100644 (file)
--- a/xen/include/asm-x86/x86_64/regs.h
+++ b/xen/include/asm-x86/x86_64/regs.h
@@ -4,7 +4,6 @@
 #include <xen/types.h>
 #include <public/xen.h>
 
-#define vm86_mode(r) (0) /* No VM86 support in long mode. */
 #define ring_0(r)    (((r)->cs & 3) == 0)
 #define ring_1(r)    (((r)->cs & 3) == 1)
 #define ring_2(r)    (((r)->cs & 3) == 2)