[Qemu-devel] Qemu arm emulation

[Qemu-devel] Qemu arm emulation

[Charlie Baylis]

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


Hi Fabrice

I have had a look at some faults in the ARM port of Qemu. I have discovered a
couple of bugs. To avoid difficulties with my employer I can't distribute my
patch which contains the fixes, but they are fairly trivial so I am detailing
the changes required here.

1. The RRX operand shift on data processing instructions is incorrectly decoded
as a rotate right (ROR) of 0 bits.
RRX should have the effect of an extended rotate right of 1 bit where the carry
flag is shifted into the top bit of the result. If the S bit is set, then the
carry flag is set to the bottom bit of the source value.

2. Shifter carry out for immediates
When an immediate value is generated the shifter carry out is set to bit31 of
the resulting immediate if the shift value is non zero. If the shift value
is zero, then the shifter carry out has the value of the C flag.

Therefore, the following instructions should alter the carry flag when used
with an immediate which has a non-zero shift. 
ANDS BICS EORS MOVS MVNS ORRS TEQS and TSTS
(The remaining data processing instructions generate the C flag from the
calculation performed by the instruction)

Test case is attached as a C file and as assembler file. The assembler function
f will return if these bugs are fixed and loops forever otherwise. I can email
you a statically linked binary if you don't have an ARM toolchain.


Apologies for not being able to just send a patch.

Regards
Charlie

[-- Attachment #2: p.c --]
[-- Type: text/x-csrc, Size: 107 bytes --]


#include <unistd.h>

extern unsigned f();

int main(unsigned int argc, char**argv)
{
  f();
  return 0;
}

[-- Attachment #3: ps.s --]
[-- Type: text/plain, Size: 273 bytes --]

	.file	"ps.s"
	.section	.rodata
	.align	2
	.global f

f:
        cmp r0, r0 @ set carry flag
        mov r0, r0, rrx
        cmp r0, #0x80000000
loop1:  bne loop1
        cmp r0, r0 @ set carry flag
        tst r0, #0x04000000
loop2:  bcs loop2
        mov pc, lr
        

Re: [Qemu-devel] Qemu arm emulation

[Paul Brook]

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

On Friday 03 December 2004 21:07, Charlie Baylis wrote:
> Hi Fabrice
>
> I have had a look at some faults in the ARM port of Qemu. I have discovered
> a couple of bugs. To avoid difficulties with my employer I can't distribute
> my patch which contains the fixes, but they are fairly trivial so I am
> detailing the changes required here.
>
> 1. The RRX operand shift on data processing instructions is incorrectly
> decoded as a rotate right (ROR) of 0 bits.
> RRX should have the effect of an extended rotate right of 1 bit where the
> carry flag is shifted into the top bit of the result. If the S bit is set,
> then the carry flag is set to the bottom bit of the source value.

The attached patch fixes this.

I posted it a while back, but it never got into CVS and I never chased it.
Paul

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

Index: target-arm/op.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-arm/op.c,v
retrieving revision 1.3
diff -u -p -r1.3 op.c
--- target-arm/op.c	30 Nov 2003 19:40:08 -0000	1.3
+++ target-arm/op.c	1 Aug 2004 21:43:22 -0000
@@ -485,6 +502,11 @@ void OPPROTO op_rorl_T1_im(void)
     T1 = ((uint32_t)T1 >> shift) | (T1 << (32 - shift));
 }
 
+void OPPROTO op_rrxl_T1(void)
+{
+    T1 = ((uint32_t)T1 >> 1) | ((uint32_t)env->CF << 31);
+}
+
 /* T1 based, set C flag */
 void OPPROTO op_shll_T1_im_cc(void)
 {
@@ -512,6 +534,14 @@ void OPPROTO op_rorl_T1_im_cc(void)
     T1 = ((uint32_t)T1 >> shift) | (T1 << (32 - shift));
 }
 
+void OPPROTO op_rrxl_T1_cc(void)
+{
+    uint32_t c;
+    c = T1 & 1;
+    T1 = ((uint32_t)T1 >> 1) | ((uint32_t)env->CF << 31);
+    env->CF = c;
+}
+
 /* T2 based */
 void OPPROTO op_shll_T2_im(void)
 {
Index: target-arm/translate.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-arm/translate.c,v
retrieving revision 1.10
diff -u -p -r1.10 translate.c
--- target-arm/translate.c	22 Jun 2004 10:55:49 -0000	1.10
+++ target-arm/translate.c	1 Aug 2004 21:43:22 -0000
@@ -365,6 +484,11 @@ static void disas_arm_insn(DisasContext 
                     } else {
                         gen_shift_T1_im[shiftop](shift);
                     }
+                } else if (shiftop == 3) {
+                    if (logic_cc)
+                        gen_op_rrxl_T1_cc();
+                    else
+                        gen_op_rrxl_T1();
                 }
             } else {
                 rs = (insn >> 8) & 0xf;

Re: [Qemu-devel] Qemu arm emulation

[Fabrice Bellard]

Thank you for the patch ! From the spec, I think there are still errors 
for 0 shifts when shiftop != 3. Have you made a patch for that too ?

Fabrice.

Paul Brook wrote:
> On Friday 03 December 2004 21:07, Charlie Baylis wrote:
> 
>>Hi Fabrice
>>
>>I have had a look at some faults in the ARM port of Qemu. I have discovered
>>a couple of bugs. To avoid difficulties with my employer I can't distribute
>>my patch which contains the fixes, but they are fairly trivial so I am
>>detailing the changes required here.
>>
>>1. The RRX operand shift on data processing instructions is incorrectly
>>decoded as a rotate right (ROR) of 0 bits.
>>RRX should have the effect of an extended rotate right of 1 bit where the
>>carry flag is shifted into the top bit of the result. If the S bit is set,
>>then the carry flag is set to the bottom bit of the source value.
> 
> 
> The attached patch fixes this.
> 
> I posted it a while back, but it never got into CVS and I never chased it.
> Paul
> 
> 
> ------------------------------------------------------------------------
> 
> Index: target-arm/op.c
> ===================================================================
> RCS file: /cvsroot/qemu/qemu/target-arm/op.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 op.c
> --- target-arm/op.c	30 Nov 2003 19:40:08 -0000	1.3
> +++ target-arm/op.c	1 Aug 2004 21:43:22 -0000
> @@ -485,6 +502,11 @@ void OPPROTO op_rorl_T1_im(void)
>      T1 = ((uint32_t)T1 >> shift) | (T1 << (32 - shift));
>  }
>  
> +void OPPROTO op_rrxl_T1(void)
> +{
> +    T1 = ((uint32_t)T1 >> 1) | ((uint32_t)env->CF << 31);
> +}
> +
>  /* T1 based, set C flag */
>  void OPPROTO op_shll_T1_im_cc(void)
>  {
> @@ -512,6 +534,14 @@ void OPPROTO op_rorl_T1_im_cc(void)
>      T1 = ((uint32_t)T1 >> shift) | (T1 << (32 - shift));
>  }
>  
> +void OPPROTO op_rrxl_T1_cc(void)
> +{
> +    uint32_t c;
> +    c = T1 & 1;
> +    T1 = ((uint32_t)T1 >> 1) | ((uint32_t)env->CF << 31);
> +    env->CF = c;
> +}
> +
>  /* T2 based */
>  void OPPROTO op_shll_T2_im(void)
>  {
> Index: target-arm/translate.c
> ===================================================================
> RCS file: /cvsroot/qemu/qemu/target-arm/translate.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 translate.c
> --- target-arm/translate.c	22 Jun 2004 10:55:49 -0000	1.10
> +++ target-arm/translate.c	1 Aug 2004 21:43:22 -0000
> @@ -365,6 +484,11 @@ static void disas_arm_insn(DisasContext 
>                      } else {
>                          gen_shift_T1_im[shiftop](shift);
>                      }
> +                } else if (shiftop == 3) {
> +                    if (logic_cc)
> +                        gen_op_rrxl_T1_cc();
> +                    else
> +                        gen_op_rrxl_T1();
>                  }
>              } else {
>                  rs = (insn >> 8) & 0xf;
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel

Re: [Qemu-devel] Qemu arm emulation

[Paul Brook]

On Wednesday 08 December 2004 22:33, Fabrice Bellard wrote:
> Thank you for the patch ! From the spec, I think there are still errors
> for 0 shifts when shiftop != 3. Have you made a patch for that too ?

No, and I probably won't have chance to look at it in the next week or so.

Paul

Re: [Qemu-devel] Qemu arm emulation

[Ulrich Hecht]

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

Hi!

On Friday 03 December 2004 22:07, Charlie Baylis wrote:
> 2. Shifter carry out for immediates
> When an immediate value is generated the shifter carry out is set to
> bit31 of the resulting immediate if the shift value is non zero. If
> the shift value is zero, then the shifter carry out has the value of
> the C flag.
>
> Therefore, the following instructions should alter the carry flag when
> used with an immediate which has a non-zero shift.
> ANDS BICS EORS MOVS MVNS ORRS TEQS and TSTS
> (The remaining data processing instructions generate the C flag from
> the calculation performed by the instruction)

Here's a patch that fixes the testcase. I made it for 0.6.1, but it still 
applies and works for CVS.

CU
Uli

[-- Attachment #2: qemu-0.6.1-shifter_carry.patch --]
[-- Type: text/x-diff, Size: 714 bytes --]

--- target-arm/op.c
+++ target-arm/op.c
@@ -98,6 +98,12 @@
     T1 = PARAM1;
 }
 
+void OPPROTO op_movl_T1_im_cc(void)
+{
+    T1 = PARAM1;
+    env->CF = PARAM1 >> 31;
+}
+
 void OPPROTO op_movl_T2_im(void)
 {
     T2 = PARAM1;
--- target-arm/translate.c
+++ target-arm/translate.c
@@ -350,7 +350,11 @@
             shift = ((insn >> 8) & 0xf) * 2;
             if (shift)
                 val = (val >> shift) | (val << (32 - shift));
-            gen_op_movl_T1_im(val);
+            if (logic_cc && shift)
+              gen_op_movl_T1_im_cc(val);
+            else
+              gen_op_movl_T1_im(val);
+                
             /* XXX: is CF modified ? */
         } else {
             /* register */

Re: [Qemu-devel] Qemu arm emulation

[Paul Brook]

On Friday 04 February 2005 12:30, Ulrich Hecht wrote:
> Hi!
>
> On Friday 03 December 2004 22:07, Charlie Baylis wrote:
> > 2. Shifter carry out for immediates
> > When an immediate value is generated the shifter carry out is set to
> > bit31 of the resulting immediate if the shift value is non zero. If
> > the shift value is zero, then the shifter carry out has the value of
> > the C flag.
> >
> > Therefore, the following instructions should alter the carry flag when
> > used with an immediate which has a non-zero shift.
> > ANDS BICS EORS MOVS MVNS ORRS TEQS and TSTS
> > (The remaining data processing instructions generate the C flag from
> > the calculation performed by the instruction)
>
> Here's a patch that fixes the testcase. I made it for 0.6.1, but it still
> applies and works for CVS.

> +void OPPROTO op_movl_T1_im_cc(void)
> +{
> +    T1 = PARAM1;
> +    env->CF = PARAM1 >> 31;
> +}

This should be "((uint32_t) PARAM1) >> 31".
The comments say CF is 0 or 1. PARAM1 is signed, so your code will result in 0 
or -1 on 32-bit hosts.

>              /* XXX: is CF modified ? */

Probably want to remove this comment now.

Paul

Re: [Qemu-devel] Qemu arm emulation

[Ulrich Hecht]

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

Hi!

On Friday 04 February 2005 15:19, Paul Brook wrote:
> On Friday 04 February 2005 12:30, Ulrich Hecht wrote:
> > +void OPPROTO op_movl_T1_im_cc(void)
> > +{
> > +    T1 = PARAM1;
> > +    env->CF = PARAM1 >> 31;
> > +}
>
> This should be "((uint32_t) PARAM1) >> 31".
> The comments say CF is 0 or 1. PARAM1 is signed, so your code will
> result in 0 or -1 on 32-bit hosts.
>
> >              /* XXX: is CF modified ? */
>
> Probably want to remove this comment now.

OK.

CU
Uli

[-- Attachment #2: qemu-shifter_carry.patch --]
[-- Type: text/x-diff, Size: 1241 bytes --]

Index: target-arm/op.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-arm/op.c,v
retrieving revision 1.7
diff -u -r1.7 op.c
--- target-arm/op.c	2 Feb 2005 20:43:01 -0000	1.7
+++ target-arm/op.c	4 Feb 2005 14:24:39 -0000
@@ -105,6 +105,12 @@
     T1 = PARAM1;
 }
 
+void OPPROTO op_movl_T1_im_cc(void)
+{
+    T1 = PARAM1;
+    env->CF = ((uint32_t)PARAM1) >> 31;
+}
+
 void OPPROTO op_movl_T2_im(void)
 {
     T2 = PARAM1;
Index: target-arm/translate.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-arm/translate.c,v
retrieving revision 1.15
diff -u -r1.15 translate.c
--- target-arm/translate.c	31 Jan 2005 20:43:28 -0000	1.15
+++ target-arm/translate.c	4 Feb 2005 14:24:39 -0000
@@ -535,8 +535,10 @@
             shift = ((insn >> 8) & 0xf) * 2;
             if (shift)
                 val = (val >> shift) | (val << (32 - shift));
-            gen_op_movl_T1_im(val);
-            /* XXX: is CF modified ? */
+            if (logic_cc && shift)
+              gen_op_movl_T1_im_cc(val);
+            else
+              gen_op_movl_T1_im(val);
         } else {
             /* register */
             rm = (insn) & 0xf;

[Qemu-devel] qemu & arm emulation

[Philippe BEAU]

Hello all,

At first, I'm new to this mailing-list. I subscribe it because i got a few
problem with using qemu & arm emulation.

I would like to know if it's possible to install a debian running arm
processor with qemu ?

Is anyone did it ? if yes, can he contact me about this ?

Best regards

Philippe,
Email / philippe@beau.nom.fr