diff mbox

[1/3] target-i386: fix segment flags for SMM, user-mode emulation and VM86 mode

Message ID 1400270365-12316-2-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini May 16, 2014, 7:59 p.m. UTC
With the next patch, these need to be correct or VM86 tasks
have the wrong CPL.  The flags are basically what the Intel VMX
documentation say is mandatory for entry into a VM86 guest.

The CPL also needs to be set before the user-mode segments
in linux-user and bsd-user.

For consistency, SMM ought to have the same flags except with
CPL=0.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 bsd-user/main.c          |  2 +-
 linux-user/main.c        |  2 +-
 target-i386/gdbstub.c    |  4 +++-
 target-i386/seg_helper.c | 11 ++++++++---
 target-i386/smm_helper.c | 24 ++++++++++++++++++------
 5 files changed, 31 insertions(+), 12 deletions(-)

Comments

Bandan Das May 27, 2014, 8:33 p.m. UTC | #1
Paolo Bonzini <pbonzini@redhat.com> writes:

> With the next patch, these need to be correct or VM86 tasks
> have the wrong CPL.  The flags are basically what the Intel VMX
> documentation say is mandatory for entry into a VM86 guest.

Are you referring to 26.3.1.2 ? Just a nit, I guess it helps 
to add a pointer in the comments for someone (like me :)) going
through the code.

> The CPL also needs to be set before the user-mode segments
> in linux-user and bsd-user.
>
> For consistency, SMM ought to have the same flags except with
> CPL=0.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  bsd-user/main.c          |  2 +-
>  linux-user/main.c        |  2 +-
>  target-i386/gdbstub.c    |  4 +++-
>  target-i386/seg_helper.c | 11 ++++++++---
>  target-i386/smm_helper.c | 24 ++++++++++++++++++------
>  5 files changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 4ba61da..0e8c26c 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -1004,7 +1004,7 @@ int main(int argc, char **argv)
>  
>  #if defined(TARGET_I386)
>      env->cr[0] = CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK;
> -    env->hflags |= HF_PE_MASK;
> +    env->hflags |= HF_PE_MASK | HF_CPL_MASK;
>      if (env->features[FEAT_1_EDX] & CPUID_SSE) {
>          env->cr[4] |= CR4_OSFXSR_MASK;
>          env->hflags |= HF_OSFXSR_MASK;
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 882186e..3e21024 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -4052,7 +4052,7 @@ int main(int argc, char **argv, char **envp)
>  
>  #if defined(TARGET_I386)
>      env->cr[0] = CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK;
> -    env->hflags |= HF_PE_MASK;
> +    env->hflags |= HF_PE_MASK | HF_CPL_MASK;
>      if (env->features[FEAT_1_EDX] & CPUID_SSE) {
>          env->cr[4] |= CR4_OSFXSR_MASK;
>          env->hflags |= HF_OSFXSR_MASK;
> diff --git a/target-i386/gdbstub.c b/target-i386/gdbstub.c
> index d34e535..19fe9ad 100644
> --- a/target-i386/gdbstub.c
> +++ b/target-i386/gdbstub.c
> @@ -127,9 +127,11 @@ static int x86_cpu_gdb_load_seg(X86CPU *cpu, int sreg, uint8_t *mem_buf)
>          target_ulong base;
>  
>          if (!(env->cr[0] & CR0_PE_MASK) || (env->eflags & VM_MASK)) {
> +            int dpl = (env->eflags & VM_MASK) ? 3 : 0;
>              base = selector << 4;
>              limit = 0xffff;
> -            flags = 0;
> +            flags = DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
> +                    DESC_A_MASK | (dpl << DESC_DPL_SHIFT);
>          } else {
>              if (!cpu_x86_get_descr_debug(env, selector, &base, &limit,
>                                           &flags)) {
> diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
> index 6c0142a..dd2c8b4 100644
> --- a/target-i386/seg_helper.c
> +++ b/target-i386/seg_helper.c
> @@ -88,8 +88,10 @@ static inline void load_seg_cache_raw_dt(SegmentCache *sc, uint32_t e1,
>  static inline void load_seg_vm(CPUX86State *env, int seg, int selector)
>  {
>      selector &= 0xffff;
> -    cpu_x86_load_seg_cache(env, seg, selector,
> -                           (selector << 4), 0xffff, 0);
> +
> +    cpu_x86_load_seg_cache(env, seg, selector, (selector << 4), 0xffff,
> +                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
> +                           DESC_A_MASK | (3 << DESC_DPL_SHIFT));
>  }
>  
>  static inline void get_ss_esp_from_tss(CPUX86State *env, uint32_t *ss_ptr,
> @@ -2462,9 +2464,12 @@ void helper_verw(CPUX86State *env, target_ulong selector1)
>  void cpu_x86_load_seg(CPUX86State *env, int seg_reg, int selector)
>  {
>      if (!(env->cr[0] & CR0_PE_MASK) || (env->eflags & VM_MASK)) {
> +        int dpl = (env->eflags & VM_MASK) ? 3 : 0;
>          selector &= 0xffff;
>          cpu_x86_load_seg_cache(env, seg_reg, selector,
> -                               (selector << 4), 0xffff, 0);
> +                               (selector << 4), 0xffff,
> +                               DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
> +                               DESC_A_MASK | (dpl << DESC_DPL_SHIFT));
>      } else {
>          helper_load_seg(env, seg_reg, selector);
>      }
> diff --git a/target-i386/smm_helper.c b/target-i386/smm_helper.c
> index 2f99493..1e5f5ce 100644
> --- a/target-i386/smm_helper.c
> +++ b/target-i386/smm_helper.c
> @@ -170,12 +170,24 @@ void do_smm_enter(X86CPU *cpu)
>      env->dr[7] = 0x00000400;
>  
>      cpu_x86_load_seg_cache(env, R_CS, (env->smbase >> 4) & 0xffff, env->smbase,
> -                           0xffffffff, 0);
> -    cpu_x86_load_seg_cache(env, R_DS, 0, 0, 0xffffffff, 0);
> -    cpu_x86_load_seg_cache(env, R_ES, 0, 0, 0xffffffff, 0);
> -    cpu_x86_load_seg_cache(env, R_SS, 0, 0, 0xffffffff, 0);
> -    cpu_x86_load_seg_cache(env, R_FS, 0, 0, 0xffffffff, 0);
> -    cpu_x86_load_seg_cache(env, R_GS, 0, 0, 0xffffffff, 0);
> +                           0xffffffff,
> +                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
> +                           DESC_A_MASK);
> +    cpu_x86_load_seg_cache(env, R_DS, 0, 0, 0xffffffff,
> +                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
> +                           DESC_A_MASK);
> +    cpu_x86_load_seg_cache(env, R_ES, 0, 0, 0xffffffff,
> +                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
> +                           DESC_A_MASK);
> +    cpu_x86_load_seg_cache(env, R_SS, 0, 0, 0xffffffff,
> +                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
> +                           DESC_A_MASK);
> +    cpu_x86_load_seg_cache(env, R_FS, 0, 0, 0xffffffff,
> +                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
> +                           DESC_A_MASK);
> +    cpu_x86_load_seg_cache(env, R_GS, 0, 0, 0xffffffff,
> +                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
> +                           DESC_A_MASK);
>  }
>  
>  void helper_rsm(CPUX86State *env)
Paolo Bonzini May 28, 2014, 11:34 a.m. UTC | #2
Il 27/05/2014 22:33, Bandan Das ha scritto:
> > With the next patch, these need to be correct or VM86 tasks
> > have the wrong CPL.  The flags are basically what the Intel VMX
> > documentation say is mandatory for entry into a VM86 guest.
>
> Are you referring to 26.3.1.2 ?

Yes:

CS, SS, DS, ES, FS, GS.
If the guest will be virtual-8086, the field must be 000000F3H. This 
implies the following:
- Bits 3:0 (Type) must be 3, indicating an expand-up read/write accessed 
data segment.
- Bit 4 (S) must be 1.
- Bits 6:5 (DPL) must be 3.
- Bit 7 (P) must be 1.
- Bits 11:8 (reserved), bit 12 (software available), bit 13 
(reserved/L), bit 14 (D/B), bit 15 (G), bit 16 (unusable), and bits 
31:17 (reserved) must all be 0.

Paolo

> Just a nit, I guess it helps
> to add a pointer in the comments for someone (like me :)) going
> through the code.
diff mbox

Patch

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 4ba61da..0e8c26c 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -1004,7 +1004,7 @@  int main(int argc, char **argv)
 
 #if defined(TARGET_I386)
     env->cr[0] = CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK;
-    env->hflags |= HF_PE_MASK;
+    env->hflags |= HF_PE_MASK | HF_CPL_MASK;
     if (env->features[FEAT_1_EDX] & CPUID_SSE) {
         env->cr[4] |= CR4_OSFXSR_MASK;
         env->hflags |= HF_OSFXSR_MASK;
diff --git a/linux-user/main.c b/linux-user/main.c
index 882186e..3e21024 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -4052,7 +4052,7 @@  int main(int argc, char **argv, char **envp)
 
 #if defined(TARGET_I386)
     env->cr[0] = CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK;
-    env->hflags |= HF_PE_MASK;
+    env->hflags |= HF_PE_MASK | HF_CPL_MASK;
     if (env->features[FEAT_1_EDX] & CPUID_SSE) {
         env->cr[4] |= CR4_OSFXSR_MASK;
         env->hflags |= HF_OSFXSR_MASK;
diff --git a/target-i386/gdbstub.c b/target-i386/gdbstub.c
index d34e535..19fe9ad 100644
--- a/target-i386/gdbstub.c
+++ b/target-i386/gdbstub.c
@@ -127,9 +127,11 @@  static int x86_cpu_gdb_load_seg(X86CPU *cpu, int sreg, uint8_t *mem_buf)
         target_ulong base;
 
         if (!(env->cr[0] & CR0_PE_MASK) || (env->eflags & VM_MASK)) {
+            int dpl = (env->eflags & VM_MASK) ? 3 : 0;
             base = selector << 4;
             limit = 0xffff;
-            flags = 0;
+            flags = DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
+                    DESC_A_MASK | (dpl << DESC_DPL_SHIFT);
         } else {
             if (!cpu_x86_get_descr_debug(env, selector, &base, &limit,
                                          &flags)) {
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index 6c0142a..dd2c8b4 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -88,8 +88,10 @@  static inline void load_seg_cache_raw_dt(SegmentCache *sc, uint32_t e1,
 static inline void load_seg_vm(CPUX86State *env, int seg, int selector)
 {
     selector &= 0xffff;
-    cpu_x86_load_seg_cache(env, seg, selector,
-                           (selector << 4), 0xffff, 0);
+
+    cpu_x86_load_seg_cache(env, seg, selector, (selector << 4), 0xffff,
+                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
+                           DESC_A_MASK | (3 << DESC_DPL_SHIFT));
 }
 
 static inline void get_ss_esp_from_tss(CPUX86State *env, uint32_t *ss_ptr,
@@ -2462,9 +2464,12 @@  void helper_verw(CPUX86State *env, target_ulong selector1)
 void cpu_x86_load_seg(CPUX86State *env, int seg_reg, int selector)
 {
     if (!(env->cr[0] & CR0_PE_MASK) || (env->eflags & VM_MASK)) {
+        int dpl = (env->eflags & VM_MASK) ? 3 : 0;
         selector &= 0xffff;
         cpu_x86_load_seg_cache(env, seg_reg, selector,
-                               (selector << 4), 0xffff, 0);
+                               (selector << 4), 0xffff,
+                               DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
+                               DESC_A_MASK | (dpl << DESC_DPL_SHIFT));
     } else {
         helper_load_seg(env, seg_reg, selector);
     }
diff --git a/target-i386/smm_helper.c b/target-i386/smm_helper.c
index 2f99493..1e5f5ce 100644
--- a/target-i386/smm_helper.c
+++ b/target-i386/smm_helper.c
@@ -170,12 +170,24 @@  void do_smm_enter(X86CPU *cpu)
     env->dr[7] = 0x00000400;
 
     cpu_x86_load_seg_cache(env, R_CS, (env->smbase >> 4) & 0xffff, env->smbase,
-                           0xffffffff, 0);
-    cpu_x86_load_seg_cache(env, R_DS, 0, 0, 0xffffffff, 0);
-    cpu_x86_load_seg_cache(env, R_ES, 0, 0, 0xffffffff, 0);
-    cpu_x86_load_seg_cache(env, R_SS, 0, 0, 0xffffffff, 0);
-    cpu_x86_load_seg_cache(env, R_FS, 0, 0, 0xffffffff, 0);
-    cpu_x86_load_seg_cache(env, R_GS, 0, 0, 0xffffffff, 0);
+                           0xffffffff,
+                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
+                           DESC_A_MASK);
+    cpu_x86_load_seg_cache(env, R_DS, 0, 0, 0xffffffff,
+                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
+                           DESC_A_MASK);
+    cpu_x86_load_seg_cache(env, R_ES, 0, 0, 0xffffffff,
+                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
+                           DESC_A_MASK);
+    cpu_x86_load_seg_cache(env, R_SS, 0, 0, 0xffffffff,
+                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
+                           DESC_A_MASK);
+    cpu_x86_load_seg_cache(env, R_FS, 0, 0, 0xffffffff,
+                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
+                           DESC_A_MASK);
+    cpu_x86_load_seg_cache(env, R_GS, 0, 0, 0xffffffff,
+                           DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
+                           DESC_A_MASK);
 }
 
 void helper_rsm(CPUX86State *env)