[Qemu-devel] ARM ethernet fixes

[Qemu-devel] ARM ethernet fixes

[Daniel Jacobowitz]

[-- Attachment #1: Type: text/plain, Size: 430 bytes --]

This is enough to let me use apt-get within qemu-system-arm :-)

What it doesn't have, today, is a hard drive.  I have some truly
hideous qemu patches, and a Linux block driver that Paul wrote, that
use ARM "semihosting" traps to simulate a block device using a host
file.  Ideally someone'll get around to emulating the Integrator/AP,
which has a PCI bus, and then we can do it normally.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

[-- Attachment #2: qemu-arm-ethernet.patch --]
[-- Type: text/plain, Size: 2478 bytes --]

Index: qemu/hw/smc91c111.c
===================================================================
--- qemu.orig/hw/smc91c111.c	2005-12-13 19:31:38.000000000 -0800
+++ qemu/hw/smc91c111.c	2005-12-13 19:32:29.000000000 -0800
@@ -35,8 +35,10 @@ typedef struct {
     int tx_fifo[NUM_PACKETS];
     int rx_fifo_len;
     int rx_fifo[NUM_PACKETS];
+    int tx_fifo_done_len;
+    int tx_fifo_done[NUM_PACKETS];
     /* Packet buffer memory.  */
-    uint8_t data[2048][NUM_PACKETS];
+    uint8_t data[NUM_PACKETS][2048];
     uint8_t int_level;
     uint8_t int_mask;
     uint8_t macaddr[6];
@@ -128,6 +130,18 @@ static void smc91c111_pop_rx_fifo(smc91c
     smc91c111_update(s);
 }
 
+/* Remove an item from the TX completion FIFO.  */
+static void smc91c111_pop_tx_fifo_done(smc91c111_state *s)
+{
+    int i;
+
+    if (s->tx_fifo_done_len == 0)
+        return;
+    s->tx_fifo_done_len--;
+    for (i = 0; i < s->tx_fifo_done_len; i++)
+        s->tx_fifo_done[i] = s->tx_fifo_done[i + 1];
+}
+
 /* Release the memory allocated to a packet.  */
 static void smc91c111_release_packet(smc91c111_state *s, int packet)
 {
@@ -184,12 +198,15 @@ static void smc91c111_do_tx(smc91c111_st
         add_crc = 0;
 #endif
         if (s->ctr & CTR_AUTO_RELEASE)
+            /* Race?  */
             smc91c111_release_packet(s, packetnum);
+        else if (s->tx_fifo_done_len < NUM_PACKETS)
+            s->tx_fifo_done[s->tx_fifo_done_len++] = packetnum;
         qemu_send_packet(s->vc, p, len);
     }
-    s->tx_fifo_len = 0;
     if ((s->ctr & CTR_AUTO_RELEASE) == 0)
         s->int_level |= INT_TX;
+    s->tx_fifo_len = 0;
     smc91c111_update(s);
 }
 
@@ -364,6 +381,8 @@ static void smc91c111_writeb(void *opaqu
             return;
         case 12: /* Interrupt ACK.  */
             s->int_level &= ~(value & 0xd6);
+            if (value & INT_TX)
+                smc91c111_pop_tx_fifo_done(s);
             smc91c111_update(s);
             return;
         case 13: /* Interrupt mask.  */
@@ -473,10 +492,10 @@ static uint32_t smc91c111_readb(void *op
         case 3: /* Allocation Result.  */
             return s->tx_alloc;
         case 4: /* TX FIFO */
-            if (s->tx_fifo_len == 0)
+            if (s->tx_fifo_done_len == 0)
                 return 0x80;
             else
-                return s->tx_fifo[0];
+                return s->tx_fifo_done[0];
         case 5: /* RX FIFO */
             if (s->rx_fifo_len == 0)
                 return 0x80;

Re: [Qemu-devel] ARM ethernet fixes

[Paul Brook]

On Wednesday 14 December 2005 22:17, Daniel Jacobowitz wrote:
> This is enough to let me use apt-get within qemu-system-arm :-)

I'd totally missed that there were _two_ TX FIFOs.

> @@ -364,6 +381,8 @@ static void smc91c111_writeb(void *opaqu
>              return;
>          case 12: /* Interrupt ACK.  */
>              s->int_level &= ~(value & 0xd6);
> +            if (value & INT_TX)
> +                smc91c111_pop_tx_fifo_done(s);
>              smc91c111_update(s);
>              return;

I'm fairly sure this is still wrong. We should only clear INT_TX if the tx 
completion fifo is empty. Maybe have smc91c111_update set the INT_TX bit if 
tx_fifo_done_len != 0.

You also need to make smc91c111_reset, smc91c111_writeb: bank 2 offset 0 cmd 2 
(Reset MMU) and smc91c111_writeb: bank 2 offset 0 cmd 7 (Reset TX FIFO) set 
s->tx_fifo_done_len = 0.

Paul

Re: [Qemu-devel] ARM ethernet fixes

[Paul Brook]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

On Thursday 15 December 2005 00:00, Paul Brook wrote:
> On Wednesday 14 December 2005 22:17, Daniel Jacobowitz wrote:
> > This is enough to let me use apt-get within qemu-system-arm :-)
>
> I'd totally missed that there were _two_ TX FIFOs.
>
> > @@ -364,6 +381,8 @@ static void smc91c111_writeb(void *opaqu
> >              return;
> >          case 12: /* Interrupt ACK.  */
> >              s->int_level &= ~(value & 0xd6);
> > +            if (value & INT_TX)
> > +                smc91c111_pop_tx_fifo_done(s);
> >              smc91c111_update(s);
> >              return;
>
> I'm fairly sure this is still wrong. We should only clear INT_TX if the tx
> completion fifo is empty. Maybe have smc91c111_update set the INT_TX bit if
> tx_fifo_done_len != 0.
>
> You also need to make smc91c111_reset, smc91c111_writeb: bank 2 offset 0
> cmd 2 (Reset MMU) and smc91c111_writeb: bank 2 offset 0 cmd 7 (Reset TX
> FIFO) set s->tx_fifo_done_len = 0.

Something like the attached patch.

Paul

[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 3115 bytes --]

=== hw/smc91c111.c
==================================================================
--- hw/smc91c111.c	(revision 1912)
+++ hw/smc91c111.c	(local)
@@ -35,8 +35,10 @@
     int tx_fifo[NUM_PACKETS];
     int rx_fifo_len;
     int rx_fifo[NUM_PACKETS];
+    int tx_fifo_done_len;
+    int tx_fifo_done[NUM_PACKETS];
     /* Packet buffer memory.  */
-    uint8_t data[2048][NUM_PACKETS];
+    uint8_t data[NUM_PACKETS][2048];
     uint8_t int_level;
     uint8_t int_mask;
     uint8_t macaddr[6];
@@ -81,6 +83,8 @@
 
     if (s->tx_fifo_len == 0)
         s->int_level |= INT_TX_EMPTY;
+    if (s->tx_fifo_done_len != 0)
+        s->int_level |= INT_TX;
     level = (s->int_level & s->int_mask) != 0;
     pic_set_irq_new(s->pic, s->irq, level);
 }
@@ -128,6 +132,18 @@
     smc91c111_update(s);
 }
 
+/* Remove an item from the TX completion FIFO.  */
+static void smc91c111_pop_tx_fifo_done(smc91c111_state *s)
+{
+    int i;
+
+    if (s->tx_fifo_done_len == 0)
+        return;
+    s->tx_fifo_done_len--;
+    for (i = 0; i < s->tx_fifo_done_len; i++)
+        s->tx_fifo_done[i] = s->tx_fifo_done[i + 1];
+}
+
 /* Release the memory allocated to a packet.  */
 static void smc91c111_release_packet(smc91c111_state *s, int packet)
 {
@@ -184,12 +200,13 @@
         add_crc = 0;
 #endif
         if (s->ctr & CTR_AUTO_RELEASE)
+            /* Race?  */
             smc91c111_release_packet(s, packetnum);
+        else if (s->tx_fifo_done_len < NUM_PACKETS)
+            s->tx_fifo_done[s->tx_fifo_done_len++] = packetnum;
         qemu_send_packet(s->vc, p, len);
     }
     s->tx_fifo_len = 0;
-    if ((s->ctr & CTR_AUTO_RELEASE) == 0)
-        s->int_level |= INT_TX;
     smc91c111_update(s);
 }
 
@@ -206,6 +223,7 @@
 {
     s->bank = 0;
     s->tx_fifo_len = 0;
+    s->tx_fifo_done_len = 0;
     s->rx_fifo_len = 0;
     s->allocated = 0;
     s->packet_num = 0;
@@ -306,6 +324,7 @@
             case 2: /* Reset MMU.  */
                 s->allocated = 0;
                 s->tx_fifo_len = 0;
+                s->tx_fifo_done_len = 0;
                 s->rx_fifo_len = 0;
                 s->tx_alloc = 0;
                 break;
@@ -326,6 +345,7 @@
                 break;
             case 7: /* Reset TX FIFO.  */
                 s->tx_fifo_len = 0;
+                s->tx_fifo_done_len = 0;
                 break;
             }
             return;
@@ -364,6 +384,8 @@
             return;
         case 12: /* Interrupt ACK.  */
             s->int_level &= ~(value & 0xd6);
+            if (value & INT_TX)
+                smc91c111_pop_tx_fifo_done(s);
             smc91c111_update(s);
             return;
         case 13: /* Interrupt mask.  */
@@ -473,10 +495,10 @@
         case 3: /* Allocation Result.  */
             return s->tx_alloc;
         case 4: /* TX FIFO */
-            if (s->tx_fifo_len == 0)
+            if (s->tx_fifo_done_len == 0)
                 return 0x80;
             else
-                return s->tx_fifo[0];
+                return s->tx_fifo_done[0];
         case 5: /* RX FIFO */
             if (s->rx_fifo_len == 0)
                 return 0x80;

Re: [Qemu-devel] ARM ethernet fixes

[Daniel Jacobowitz]

On Sun, Dec 18, 2005 at 04:51:02PM +0000, Paul Brook wrote:
> Something like the attached patch.

After getting myself, and probably Paul, completely confused about
array indexing, I agree that this version is right :-)  It also boots
and appears to work.  Network performance is not very good (averaging
about 10K/s - 30K/s most of the time, but occasionally spiking higher),
but there may be something we can do about that later.  Thanks.


-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [Qemu-devel] ARM ethernet fixes

[M. Warner Losh]

In message: <20051218172558.GA446@nevyn.them.org>
            Daniel Jacobowitz <drow@false.org> writes:
: On Sun, Dec 18, 2005 at 04:51:02PM +0000, Paul Brook wrote:
: > Something like the attached patch.
: 
: After getting myself, and probably Paul, completely confused about
: array indexing, I agree that this version is right :-)  It also boots
: and appears to work.  Network performance is not very good (averaging
: about 10K/s - 30K/s most of the time, but occasionally spiking higher),
: but there may be something we can do about that later.  Thanks.

Given all these improvements in arm support, what's the status of
system level support for arm, and what system is emulated?  You don't
need device emulation to do userland stuf...

Warner

Re: [Qemu-devel] ARM ethernet fixes

[Paul Brook]

> Given all these improvements in arm support, what's the status of
> system level support for arm, and what system is emulated?  

Short answer is it should work. A default linux kernel config doesn't quite 
work because it hangs probing for a non-existant VGA card. Disable the VGA 
console driver and it's fine.

It emulates most of an Arm Integrator/CP board with an arm1026E cpu.
Available peripherals are two UARTs, and a NIC. More than enough to boot 
linux :-)

> You don't need device emulation to do userland stuf...

No, but it's a diskless board. You either run everything off an initrd, or you 
need network.

Paul

Re: [Qemu-devel] ARM ethernet fixes

[M. Warner Losh]

In message: <200512181753.40316.paul@codesourcery.com>
            Paul Brook <paul@codesourcery.com> writes:
: > Given all these improvements in arm support, what's the status of
: > system level support for arm, and what system is emulated?  
: 
: Short answer is it should work. A default linux kernel config doesn't quite 
: work because it hangs probing for a non-existant VGA card. Disable the VGA 
: console driver and it's fine.
: 
: It emulates most of an Arm Integrator/CP board with an arm1026E cpu.
: Available peripherals are two UARTs, and a NIC. More than enough to boot 
: linux :-)

Well, I'm hoping to boot FreeBSD/arm on the thing.  two uarts and a
nic is more than enough to boot FreeBSD as well :-)

: > You don't need device emulation to do userland stuf...
: 
: No, but it's a diskless board. You either run everything off an initrd, or you 
: need network.

That's good to know.

Warner

Re: [Qemu-devel] ARM ethernet fixes

[Daniel Jacobowitz]

On Sun, Dec 18, 2005 at 10:42:16AM -0700, M. Warner Losh wrote:
> In message: <20051218172558.GA446@nevyn.them.org>
>             Daniel Jacobowitz <drow@false.org> writes:
> : On Sun, Dec 18, 2005 at 04:51:02PM +0000, Paul Brook wrote:
> : > Something like the attached patch.
> : 
> : After getting myself, and probably Paul, completely confused about
> : array indexing, I agree that this version is right :-)  It also boots
> : and appears to work.  Network performance is not very good (averaging
> : about 10K/s - 30K/s most of the time, but occasionally spiking higher),
> : but there may be something we can do about that later.  Thanks.
> 
> Given all these improvements in arm support, what's the status of
> system level support for arm, and what system is emulated?  You don't
> need device emulation to do userland stuf...

It's an Integrator/CP.  It has a network card and a serial port, and
that's about it - NFS root works, now that the network card's been
fixed.  I use some "fake hard drive" patches inspired by Paul, and I'll
post them if anyone really, really wants to see them, but they're
hideous.  Emulation of something with PCI and an IDE controller is pretty
high up on my list :-)

It appears to work fairly well.  I was able to run debian-installer in
it with only a few glitches (d-i does not like systems without hard
drives!).

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [Qemu-devel] ARM ethernet fixes

[Dave Feustel]

On Sunday 18 December 2005 12:54, Daniel Jacobowitz wrote:
>  Emulation of something with PCI and an IDE controller is pretty
> high up on my list :

Does qemu do StrongARM emulations? There is a nice
StrongARM PCI motherboard from CATS with IDE controllers, etc.
-- 
Lose, v., experience a loss, get rid of, "lose the weight"
Loose, adj., not tight, let go, free, "loose clothing"

[Qemu-devel] Re: ARM ethernet fixes

[Antti P Miettinen]

Daniel Jacobowitz <drow@false.org> writes:
> It appears to work fairly well.

I can confirm that. I'm currently debugging a segmentation fault in
compiling one source file of firefox. Something that the current ARM
sysemu can do is e.g.
- boot debian/sid NFS root
- compile gcc far enough to produce a cc1plus with debug info
- run gdb inside emacs to debug cc1plus compiling a ff source file
- run cc1plus under gdb with a breakpoint with ignorecount
- be debugged with arm-linux-gdb

IMHO, thats quite impressive :-)

Might as well describe the thing I'm trying to debug. Dunno what the
exact problem is but it is repeatable. The complete test case is a bit
large, but the segfaulting code qestion looks like:

(gdb) disas $pc-16 $pc+16
Dump of assembler code from 0x31bdac to 0x31bdcc:
0x0031bdac <global_alloc+1016>:	ldr	r1, [r2, #28]
0x0031bdb0 <global_alloc+1020>:	add	r3, r3, r1
0x0031bdb4 <global_alloc+1024>:	str	r3, [r4, #8]
0x0031bdb8 <global_alloc+1028>:	add	r1, r4, #12	; 0xc
0x0031bdbc <global_alloc+1032>:	ldmia	r1, {r1, lr}
0x0031bdc0 <global_alloc+1036>:	ldr	r3, [r0, #16]
0x0031bdc4 <global_alloc+1040>:	ldr	r2, [r3, #32]
0x0031bdc8 <global_alloc+1044>:	add	r1, r1, r2
End of assembler dump.

And the registers before SIGSEGV look like:

(gdb) info reg
r0             0x58ff48	5832520
r1             0x4205effc	1107685372
r2             0x41fc7320	1107063584
r3             0x0	0
r4             0x4205eff0	1107685360
r5             0x29e	670
r6             0x42059008	1107660808
r7             0x5d0ab0	6097584
r8             0x3ba8ea	3909866
r9             0x43d9e8	4446696
r10            0x1	1
r11            0x3ecd20	4115744
r12            0xc	12
sp             0xbef3b234	-1091325388
lr             0x2	2
pc             0x31bdbc	3259836
fps            0x1001010	16781328
cpsr           0x10	16

So it looks like we are about to load a couple of longs from
0x4205effc. But when I step:

(gdb) si

Program received signal SIGSEGV, Segmentation fault.
0x0031bdbc in global_alloc (file=0x0) at ../../src/gcc/global.c:490

Looks like the kernel somehow ends up thinking that we are loading
something from zero. Hmm.. the value being loaded to r1 is zero. So
could this be related to crossing a page boundary in the middle of an
instruction? But anyway - I'll try to debug this further..

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

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

[Antti P Miettinen]

Antti P Miettinen <ananaza@iki.fi> writes:
> Looks like the kernel somehow ends up thinking that we are loading
> something from zero. Hmm.. the value being loaded to r1 is zero. So
> could this be related to crossing a page boundary in the middle of an
> instruction? But anyway - I'll try to debug this further..

Looks like it might be the page crossing. The below little program is
a  bit smaller test case. Without args, no segfault, with arg it
segfaults (inside ARM sysemu - at least for me):

#include <stdio.h>
#include <string.h>

long buf[8192/4];

int
main(int ac, char **av)
{
    unsigned long base = (unsigned long)(void *)(buf + 4096/4);
    unsigned long dummy;

    if (ac > 1)
    {
	base &= ~0xfff;
	base -= 4;
    }
    printf("buf: %p, base: 0x%08lx, dummy: 0x%08lx\n", buf, base, dummy);
    __asm__ __volatile__ ("ldmia %0,{%1,%2}"
			  : "=r" (base) : "0" (base), "r" (dummy));
    printf("base: 0x%08lx, dummy: 0x%08lx\n", base, dummy);
    return 0;
}

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

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

[Antti P Miettinen]

Antti P Miettinen <ananaza@iki.fi> writes:
>     __asm__ __volatile__ ("ldmia %0,{%1,%2}"
> 			  : "=r" (base) : "0" (base), "r" (dummy));

Hmm.. thats probably wrong.. is this closer:

    __asm__ __volatile__ ("ldmia %0,{%0,%1}"
			  : "+r" (base), "=r" (dummy));

or

    __asm__ __volatile__ ("ldmia %2,{%0,%1}"
			  : "=r" (base), "=r" (dummy)
                          : "0" (base));

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?

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

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

[Antti P Miettinen]

Antti P Miettinen <ananaza@iki.fi> 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/

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

[Daniel Jacobowitz]

On Mon, Dec 19, 2005 at 05:24:48PM +0200, Antti P Miettinen wrote:
> Antti P Miettinen <ananaza@iki.fi> 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..

You don't need anything near this complicated: if the register being
set is the base register, copy it into T2 instead of overwriting the
base register.  Then at the end, if the base register was loaded, copy
T2 into the base register.  That's all the atomicity you need; the
instruction is being restarted after the base register was clobbered.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

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

[Antti P Miettinen]

Daniel Jacobowitz <drow@false.org> writes:
> You don't need anything near this complicated: if the register being
> set is the base register, copy it into T2 instead of overwriting the
> base register.  Then at the end, if the base register was loaded, copy
> T2 into the base register.  That's all the atomicity you need; the
> instruction is being restarted after the base register was clobbered.

Eee.. I knew that. I was just.. practicing my fingers (ahem.. is there
a saying in english about how the whole body suffers from a stupid
head? :-)

Something like this?

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 19:14:45 -0000
@@ -106,6 +106,11 @@
     T0 = T1;
 }
 
+void OPPROTO op_movl_T2_T0(void)
+{
+    T2 = T0;
+}
+
 void OPPROTO op_movl_T1_im(void)
 {
     T1 = PARAM1;
@@ -1175,3 +1180,18 @@
     }
     FORCE_RET();
 }
+
+void OPPROTO op_movl_user_T2(void)
+{
+    int regno = PARAM1;
+    if (regno == 13) {
+        env->banked_r13[0] = T2;
+    } else if (regno == 14) {
+        env->banked_r14[0] = T2;
+    } else if ((env->uncached_cpsr & 0x1f) == ARM_CPU_MODE_FIQ) {
+        env->usr_regs[regno - 8] = T2;
+    } else {
+        env->regs[regno] = T2;
+    }
+    FORCE_RET();
+}
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 19:14:45 -0000
@@ -48,6 +48,11 @@
     SET_REG (T1);
 }
 
+void OPPROTO glue(glue(op_movl_, REGNAME), _T2)(void)
+{
+    SET_REG (T2);
+}
+
 #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 19:14:46 -0000
@@ -228,7 +228,7 @@
     },
 };
 
-static GenOpFunc *gen_op_movl_reg_TN[2][16] = {
+static GenOpFunc *gen_op_movl_reg_TN[3][16] = {
     {
         gen_op_movl_r0_T0,
         gen_op_movl_r1_T0,
@@ -265,6 +265,24 @@
         gen_op_movl_r14_T1,
         gen_op_movl_r15_T1,
     },
+    {
+        gen_op_movl_r0_T2,
+        gen_op_movl_r1_T2,
+        gen_op_movl_r2_T2,
+        gen_op_movl_r3_T2,
+        gen_op_movl_r4_T2,
+        gen_op_movl_r5_T2,
+        gen_op_movl_r6_T2,
+        gen_op_movl_r7_T2,
+        gen_op_movl_r8_T2,
+        gen_op_movl_r9_T2,
+        gen_op_movl_r10_T2,
+        gen_op_movl_r11_T2,
+        gen_op_movl_r12_T2,
+        gen_op_movl_r13_T2,
+        gen_op_movl_r14_T2,
+        gen_op_movl_r15_T2,
+    },
 };
 
 static GenOpFunc1 *gen_op_movl_TN_im[3] = {
@@ -1665,8 +1683,11 @@
                         if (insn & (1 << 20)) {
                             /* load */
                             gen_ldst(ldl, s);
-                            if (i == 15) {
-                                gen_bx(s);
+			    if (i == 15) {
+				/* later */
+			    } else if (i == rn) {
+				/* base register */
+				gen_op_movl_T2_T0();
                             } else if (user) {
                                 gen_op_movl_user_T0(i);
                             } else {
@@ -1691,6 +1712,19 @@
                             gen_op_addl_T1_im(4);
                     }
                 }
+		if (insn & (1 << 20)) {
+		    /* check if base register and pc were loaded */
+		    if (insn & (1 << rn)) {
+			if (user)
+			    gen_op_movl_user_T2(rn);
+			else
+			    gen_op_movl_reg_TN[2][rn]();
+		    }
+		    if (insn & (1 << 15)) {
+			/* r15 is loaded last - value should be in T0 */
+			gen_bx(s);
+		    }
+		}
                 if (insn & (1 << 21)) {
                     /* write back */
                     if (insn & (1 << 23)) {


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

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

[Antti P Miettinen]

Antti P Miettinen <ananaza@iki.fi> writes:
> Something like this?

Hmm.. I suppose the branch should be the last op to generate if PC is
loaded by the ldm. So the possible gen_bx should be after the
writeback? But what about the S bit? Or is this all highly academic?
Does ldm ever load pc in practice?

Anyway - I managed to rebuild firefox with debug info and found where
it segfaulted. And with the below change it runs :-)

diff -ur firefox-1.4.99+1.5rc3.dfsg/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_arm.cpp /usr/local/arm/sid/home/apm/src/firefox-1.4.99+1.5rc3.dfsg/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_arm.cpp
--- firefox-1.4.99+1.5rc3.dfsg/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_arm.cpp	2005-10-14 00:33:10.000000000 +0300
+++ /usr/local/arm/sid/home/apm/src/firefox-1.4.99+1.5rc3.dfsg/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_arm.cpp	2005-12-25 15:15:46.000000000 +0200
@@ -212,7 +212,7 @@
     "add	sp, sp, r4	\n\t"	/* restore stack pointer		*/
     "mov	%0, r0		\n\t"	/* the result...			*/
     : "=r" (result)
-    : "r" (&my_params)
+    : "r" (&my_params), "m" (my_params)
     : "r0", "r1", "r2", "r3", "r4", "ip", "lr", "sp"
     );
     
-- 
http://www.iki.fi/~ananaza/