[Qemu-devel] Re: ARM page crossing inside insn? (Re: ARM ethernet fixes)

[Antti P Miettinen]

Antti P Miettinen <address@hidden> writes:
> but anyway - how would the ldm register update be made atomic? Or
> should the restart be able to continue in the middle? How are the
> atomicity issues handled in qemu?

I wonder how the ARM implementations handle it..

The below is a quick test how the atomicity could be achieved with
shadow registers. Is this a feasible approach? The diff is not a
proper/complete patch - I just wanted to see whether it makes a
different. It does - the firefox compile proceeds..

Hmm.. do people prefer inline diffs or attachments?

Index: target-arm/cpu.h
===================================================================
RCS file: /sources/qemu/qemu/target-arm/cpu.h,v
retrieving revision 1.13
diff -u -r1.13 cpu.h
--- target-arm/cpu.h    26 Nov 2005 10:46:39 -0000      1.13
+++ target-arm/cpu.h    19 Dec 2005 15:17:14 -0000
@@ -46,6 +46,8 @@
 typedef struct CPUARMState {
     /* Regs for current mode.  */
     uint32_t regs[16];
+    /* Shadow regs for atomic update.  */
+    uint32_t newregs[16];
     /* Frequently accessed CPSR bits are stored separately for efficiently.
        This contains all the other bits.  Use cpsr_{read,write} to accless
        the whole CPSR.  */
Index: target-arm/op.c
===================================================================
RCS file: /sources/qemu/qemu/target-arm/op.c,v
retrieving revision 1.17
diff -u -r1.17 op.c
--- target-arm/op.c     26 Nov 2005 10:46:39 -0000      1.17
+++ target-arm/op.c     19 Dec 2005 15:17:14 -0000
@@ -1175,3 +1175,20 @@
     }
     FORCE_RET();
 }
+
+void OPPROTO op_commit_newregs(void)
+{
+    int regs = PARAM1;
+    int i;
+    //cpu_lock();
+    for (i = 0; i < 15; ++i) {
+       if (regs & (1 << i))
+           env->regs[i] = env->newregs[i];
+    }
+    //cpu_unlock();
+}
+
+void OPPROTO op_movl_T0_newpc(void)
+{
+  T0 = env->newregs[15];
+}
Index: target-arm/op_template.h
===================================================================
RCS file: /sources/qemu/qemu/target-arm/op_template.h,v
retrieving revision 1.2
diff -u -r1.2 op_template.h
--- target-arm/op_template.h    31 Jan 2005 20:43:28 -0000      1.2
+++ target-arm/op_template.h    19 Dec 2005 15:17:14 -0000
@@ -48,6 +48,21 @@
     SET_REG (T1);
 }
 
+/* For storing to shadow regs to make ldm register update atomic */
+#define regs newregs
+
+void OPPROTO glue(glue(op_movl_, glue(new_, REGNAME)), _T0)(void)
+{
+    SET_REG (T0);
+}
+
+void OPPROTO glue(glue(op_movl_, glue(new_, REGNAME)), _T1)(void)
+{
+    SET_REG (T1);
+}
+
+#undef regs
+
 #undef REG
 #undef REGNAME
 #undef SET_REG
Index: target-arm/translate.c
===================================================================
RCS file: /sources/qemu/qemu/target-arm/translate.c,v
retrieving revision 1.35
diff -u -r1.35 translate.c
--- target-arm/translate.c      18 Dec 2005 16:55:25 -0000      1.35
+++ target-arm/translate.c      19 Dec 2005 15:17:15 -0000
@@ -267,6 +267,45 @@
     },
 };
 
+static GenOpFunc *gen_op_movl_newreg_TN[2][16] = {
+    {
+        gen_op_movl_new_r0_T0,
+        gen_op_movl_new_r1_T0,
+        gen_op_movl_new_r2_T0,
+        gen_op_movl_new_r3_T0,
+        gen_op_movl_new_r4_T0,
+        gen_op_movl_new_r5_T0,
+        gen_op_movl_new_r6_T0,
+        gen_op_movl_new_r7_T0,
+        gen_op_movl_new_r8_T0,
+        gen_op_movl_new_r9_T0,
+        gen_op_movl_new_r10_T0,
+        gen_op_movl_new_r11_T0,
+        gen_op_movl_new_r12_T0,
+        gen_op_movl_new_r13_T0,
+        gen_op_movl_new_r14_T0,
+        gen_op_movl_new_r15_T0,
+    },
+    {
+        gen_op_movl_new_r0_T1,
+        gen_op_movl_new_r1_T1,
+        gen_op_movl_new_r2_T1,
+        gen_op_movl_new_r3_T1,
+        gen_op_movl_new_r4_T1,
+        gen_op_movl_new_r5_T1,
+        gen_op_movl_new_r6_T1,
+        gen_op_movl_new_r7_T1,
+        gen_op_movl_new_r8_T1,
+        gen_op_movl_new_r9_T1,
+        gen_op_movl_new_r10_T1,
+        gen_op_movl_new_r11_T1,
+        gen_op_movl_new_r12_T1,
+        gen_op_movl_new_r13_T1,
+        gen_op_movl_new_r14_T1,
+        gen_op_movl_new_r15_T1,
+    },
+};
+
 static GenOpFunc1 *gen_op_movl_TN_im[3] = {
     gen_op_movl_T0_im,
     gen_op_movl_T1_im,
@@ -341,6 +380,16 @@
     gen_movl_reg_TN(s, reg, 0);
 }
 
+static inline void gen_movl_newreg_T0(int reg)
+{
+    gen_op_movl_newreg_TN[0][reg]();
+}
+
+static inline void gen_movl_newreg_T1(int reg)
+{
+    gen_op_movl_newreg_TN[1][reg]();
+}
+
 static inline void gen_movl_reg_T1(DisasContext *s, int reg)
 {
     gen_movl_reg_TN(s, reg, 1);
@@ -1665,12 +1714,13 @@
                         if (insn & (1 << 20)) {
                             /* load */
                             gen_ldst(ldl, s);
-                            if (i == 15) {
+                            if (0 && i == 15) {
                                 gen_bx(s);
                             } else if (user) {
                                 gen_op_movl_user_T0(i);
                             } else {
-                                gen_movl_reg_T0(s, i);
+                                gen_movl_newreg_T0(i);
+                               //gen_movl_reg_T0(s, i);
                             }
                         } else {
                             /* store */
@@ -1691,6 +1741,15 @@
                             gen_op_addl_T1_im(4);
                     }
                 }
+               if (/*0 &&*/ (insn & (1 << 20)) && !user) {
+                   /* commit the loaded registers */
+                   gen_op_commit_newregs(insn & 0xffff);
+                   if (insn & (1 << 15)) {
+                       /* PC updated */
+                       gen_op_movl_T0_newpc();
+                       gen_bx(s);
+                   }
+               }
                 if (insn & (1 << 21)) {
                     /* write back */
                     if (insn & (1 << 23)) {

-- 
http://www.iki.fi/~ananaza/