minor x86-64 corrections

PATCH In review Filip Navara 2 participants 4 messages Jan 21, 2005

lore

Share & tools

lore

Filip Navara

author

2005-01-21 20:47 UTC #0 lore raw

PATCH 1/1 minor x86-64 corrections

Changelog:

2005-01-21 Filip Navara <[email protected]>

* Add support for CR8 register.
* Don't throw exception when assigning zero to SS register in 64-bit mode.

BTW, the callgate support for x86-64 in long mode is broken. See "AMD64 Architecture Programmer’s Manual Volume 3: General-Purpose and System Instructions", page 90. Anybody cares to fix it?

Regards,
Filip

Index: hw/apic.c
===================================================================
RCS file: /cvsroot/qemu/qemu/hw/apic.c,v
retrieving revision 1.1

apic.c +12 -0

diff -u -r1.1 apic.c
@@ -100,6 +100,18 @@
     return s->apicbase;
 }
 
+void cpu_set_apic_tpr(CPUState *env, uint8_t val)
+{
+    APICState *s = env->apic_state;
+    s->tpr = val;
+}
+
+uint8_t cpu_get_apic_tpr(CPUState *env)
+{
+    APICState *s = env->apic_state;
+    return s->tpr;
+}
+
 /* return -1 if no bit is set */
 static int get_highest_priority_int(uint32_t *tab)
 {

Index: target-i386/exec.h
===================================================================
RCS file: /cvsroot/qemu/qemu/target-i386/exec.h,v
retrieving revision 1.20

exec.h +2 -0

diff -u -r1.20 exec.h
@@ -167,6 +167,8 @@
 void cpu_x86_flush_tlb(CPUX86State *env, uint32_t addr);
 int cpu_x86_handle_mmu_fault(CPUX86State *env, target_ulong addr, 
                              int is_write, int is_user, int is_softmmu);
+void cpu_set_apic_tpr(CPUX86State *env, uint8_t val);
+uint8_t cpu_get_apic_tpr(CPUX86State *env);
 void tlb_fill(target_ulong addr, int is_write, int is_user, 
               void *retaddr);
 void __hidden cpu_lock(void);

Index: target-i386/helper.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-i386/helper.c,v retrieving revision 1.40

helper.c +11 -1

diff -u -r1.40 helper.c
@@ -1454,8 +1454,13 @@
     selector &= 0xffff;
     if ((selector & 0xfffc) == 0) {
         /* null selector case */
-        if (seg_reg == R_SS)
+        if (seg_reg == R_SS) {
+#ifdef TARGET_X86_64
+            if (env->hflags & HF_CS64_MASK)
+                return;
+#endif
             raise_exception_err(EXCP0D_GPF, 0);
+        }
         cpu_x86_load_seg_cache(env, seg_reg, selector, 0, 0, 0);
     } else {

@@ -2156,6 +2161,11 @@
     case 4:
         cpu_x86_update_cr4(env, T0);
         break;
+#ifdef TARGET_X86_64
+    case 8:
+        cpu_set_apic_tpr(env, (T0 & 0xf) << 4);
+        break;
+#endif
     default:
         env->cr[reg] = T0;
         break;

Index: target-i386/op.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-i386/op.c,v
retrieving revision 1.28

op.c +5 -0

diff -u -r1.28 op.c
@@ -1198,6 +1198,11 @@
     helper_movl_crN_T0(PARAM1);
 }
 
+void OPPROTO op_movtl_T0_cr7(void)
+{
+    T0 = (cpu_get_apic_tpr(env) & 0xf) >> 4;
+}
+
 /* DR registers access */
 void OPPROTO op_movl_drN_T0(void)
 {

Index: target-i386/translate.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-i386/translate.c,v retrieving revision 1.41

translate.c +13 -1

diff -u -r1.41 translate.c
@@ -5641,7 +5641,19 @@
                     gen_op_mov_reg_T0[ot][rm]();
                 }
                 break;
-                /* XXX: add CR8 for x86_64 */
+#ifdef TARGET_X86_64
+            case 8:
+                if (b & 2) {
+                    gen_op_mov_TN_reg[ot][0][rm]();
+                    gen_op_movl_crN_T0(reg);
+                    gen_jmp_im(s->pc - s->cs_base);
+                    gen_eob(s);
+                } else {
+                    gen_op_movtl_T0_cr7();
+                    gen_op_mov_reg_T0[ot][rm]();
+                }
+                break;
+#endif
             default:
                 goto illegal_op;
             }

Filip Navara

author

2005-01-21 22:31 UTC #2 lore raw

Quoted 6 lines · Fabrice Bellard

Quoted 4 lines · Filip Navara
Changelog:
2005-01-21 Filip Navara <[email protected]>

Quoted 8 lines

Quoted 5 lines
* Add support for CR8 register.
* Don't throw exception when assigning zero to SS register in 64-bit mode.
BTW, the callgate support for x86-64 in long mode is broken. See "AMD64 Architecture Programmer’s Manual Volume 3: General-Purpose and System Instructions", page 90. Anybody cares to fix it?
No time yet. It would be interesting to find the problem for the Linux user code too.

Ok, I'll try to fix it myself. BTW the same issue is present also in the protected mode JMP code.

Quoted 17 lines

Quoted 4 lines

+++ target-i386/helper.c 21 Jan 2005 20:13:30 -0000
helper.c +0 -0
@@ -1454,8 +1454,13 @@
     selector &= 0xffff;
     if ((selector & 0xfffc) == 0) {

helper.c line 1464

Quoted 10 lines

helper.c +6 -1

         /* null selector case */
-        if (seg_reg == R_SS)
+        if (seg_reg == R_SS) {
+#ifdef TARGET_X86_64
+            if (env->hflags & HF_CS64_MASK)
+                return;
+#endif
             raise_exception_err(EXCP0D_GPF, 0);
+        }
         cpu_x86_load_seg_cache(env, seg_reg, selector, 0, 0, 0);

--- target-i386/helper.c 16 Jan 2005 23:35:43 -0000 1.40

Quoted 1 line

Maybe the SS segment cache should be set to zero ?

I'm not sure really, but I guess you're right.

MOV documentation:
... "It is possible to move a null segment selector value (0000–0003h) into the DS, ES, FS, or GS register. This action does not cause a general protection fault, but a subsequent reference to such a segment does cause a #GP exception. For more information about segment selectors, see “Segment Selectors and Registers” on page 84." ...

GPF is exception is thrown if ... "The SS register was loaded with a null segment selector in non-64-bit mode or while CPL = 3." ...

Quoted 6 lines

op.c line 1204
Quoted 4 lines
op.c +4 -0
+void OPPROTO op_movtl_T0_cr7(void)
+{
+    T0 = (cpu_get_apic_tpr(env) & 0xf) >> 4;
+}
Why do you call it cr7 ?

Actually that's a typo.

Filip Navara
[email protected]

Filip Navara

author

2005-01-21 23:05 UTC #3 lore raw

[snip]

Quoted 11 lines

Quoted 1 line
Maybe the SS segment cache should be set to zero ?
I'm not sure really, but I guess you're right.
MOV documentation:
... "It is possible to move a null segment selector value (0000–0003h) into the DS, ES, FS, or GS register. This action does not cause a general protection fault, but a subsequent reference to such a segment does cause a #GP exception. For more information about segment selectors, see “Segment Selectors and Registers” on page 84." ...
GPF is exception is thrown if ... "The SS register was loaded with a null segment selector in non-64-bit mode or while CPL = 3." ...

I looked at the disassembly and it's called after setting up GDT and IDT. Does the attached patch look better?

Regards,
Filip

Index: hw/apic.c
===================================================================
RCS file: /cvsroot/qemu/qemu/hw/apic.c,v
retrieving revision 1.1

apic.c +12 -0

diff -u -r1.1 apic.c
@@ -100,6 +100,18 @@
     return s->apicbase;
 }
 
+void cpu_set_apic_tpr(CPUState *env, uint8_t val)
+{
+    APICState *s = env->apic_state;
+    s->tpr = val;
+}
+
+uint8_t cpu_get_apic_tpr(CPUState *env)
+{
+    APICState *s = env->apic_state;
+    return s->tpr;
+}
+
 /* return -1 if no bit is set */
 static int get_highest_priority_int(uint32_t *tab)
 {

Index: target-i386/exec.h
===================================================================
RCS file: /cvsroot/qemu/qemu/target-i386/exec.h,v
retrieving revision 1.20

exec.h +2 -0

diff -u -r1.20 exec.h
@@ -167,6 +167,8 @@
 void cpu_x86_flush_tlb(CPUX86State *env, uint32_t addr);
 int cpu_x86_handle_mmu_fault(CPUX86State *env, target_ulong addr, 
                              int is_write, int is_user, int is_softmmu);
+void cpu_set_apic_tpr(CPUX86State *env, uint8_t val);
+uint8_t cpu_get_apic_tpr(CPUX86State *env);
 void tlb_fill(target_ulong addr, int is_write, int is_user, 
               void *retaddr);
 void __hidden cpu_lock(void);

Index: target-i386/helper.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-i386/helper.c,v retrieving revision 1.40

helper.c +11 -2

diff -u -r1.40 helper.c
@@ -1454,8 +1454,12 @@
     selector &= 0xffff;
     if ((selector & 0xfffc) == 0) {
         /* null selector case */
-        if (seg_reg == R_SS)
-            raise_exception_err(EXCP0D_GPF, 0);
+        if (seg_reg == R_SS) {
+#ifdef TARGET_X86_64
+            if (!env->hflags & HF_CS64_MASK)
+#endif
+                raise_exception_err(EXCP0D_GPF, 0);
+        }
         cpu_x86_load_seg_cache(env, seg_reg, selector, 0, 0, 0);
     } else {

@@ -2156,6 +2160,11 @@
     case 4:
         cpu_x86_update_cr4(env, T0);
         break;
+#ifdef TARGET_X86_64
+    case 8:
+        cpu_set_apic_tpr(env, (T0 & 0xf) << 4);
+        break;
+#endif
     default:
         env->cr[reg] = T0;
         break;

Index: target-i386/op.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-i386/op.c,v
retrieving revision 1.28

op.c +5 -0

diff -u -r1.28 op.c
@@ -1198,6 +1198,11 @@
     helper_movl_crN_T0(PARAM1);
 }
 
+void OPPROTO op_movtl_T0_cr8(void)
+{
+    T0 = (cpu_get_apic_tpr(env) & 0xf) >> 4;
+}
+
 /* DR registers access */
 void OPPROTO op_movl_drN_T0(void)
 {

Index: target-i386/translate.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-i386/translate.c,v retrieving revision 1.41

translate.c +13 -1

diff -u -r1.41 translate.c
@@ -5641,7 +5641,19 @@
                     gen_op_mov_reg_T0[ot][rm]();
                 }
                 break;
-                /* XXX: add CR8 for x86_64 */
+#ifdef TARGET_X86_64
+            case 8:
+                if (b & 2) {
+                    gen_op_mov_TN_reg[ot][0][rm]();
+                    gen_op_movl_crN_T0(reg);
+                    gen_jmp_im(s->pc - s->cs_base);
+                    gen_eob(s);
+                } else {
+                    gen_op_movtl_T0_cr8();
+                    gen_op_mov_reg_T0[ot][rm]();
+                }
+                break;
+#endif
             default:
                 goto illegal_op;
             }