[Qemu-devel] ARM: BKPT instructions should raise prefetch aborts with IFSR type 00010

[Qemu-devel] ARM: BKPT instructions should raise prefetch aborts with IFSR type 00010

[Alex Zuepke]

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

Hi,

while digging through some problems with BKPT exceptions on ARM, I
discovered that QEMU does not update IFSR on prefetch aborts. This
should be done since ARMv6 according to ARM docs. Please include.

Best Regards,
Alex

-- 
Alexander Zuepke                                azuepke@sysgo.com
SYSGO AG ~ Am Pfaffenstein 14 ~ 55270 Klein-Winternheim ~ Germany

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

 target-arm: BKPT instructions should raise prefetch aborts with IFSR type 00010
 diff against qemu 0.14.0
 Signed-off-by: Alex Zuepke <azuepke@sysgo.com>
--- qemu-0.14.0.orig/target-arm/translate.c	2011-02-16 15:44:05.000000000 +0100
+++ qemu-0.14.0/target-arm/translate.c	2011-03-25 11:22:03.000000000 +0100
@@ -6389,6 +6389,7 @@
                 goto illegal_op;
             }
             /* bkpt */
+            env->cp15.c5_insn = 2;
             gen_exception_insn(s, 4, EXCP_BKPT);
             break;
         case 0x8: /* signed multiply */
@@ -8930,6 +8931,7 @@
             break;
 
         case 0xe: /* bkpt */
+            env->cp15.c5_insn = 2;
             gen_exception_insn(s, 2, EXCP_BKPT);
             break;
 

Re: [Qemu-devel] ARM: BKPT instructions should raise prefetch aborts with IFSR type 00010

[Peter Maydell]

On 25 March 2011 10:54, Alex Zuepke <azuepke@sysgo.com> wrote:
> while digging through some problems with BKPT exceptions on ARM, I
> discovered that QEMU does not update IFSR on prefetch aborts. This
> should be done since ARMv6 according to ARM docs. Please include.

This patch is the wrong approach to fixing this bug -- the
updating of the IFSR needs to be done when the exception
is taken, not when we translate the breakpoint instruction.

I'll put this on my todo list. If you happen to have a convenient
test case demonstrating the problem, that would make a fix happen
faster ;-)

-- PMM

Re: [Qemu-devel] ARM: BKPT instructions should raise prefetch aborts with IFSR type 00010

[Alex Zuepke]

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

Hi Peter,

Peter Maydell schrieb:
> On 25 March 2011 10:54, Alex Zuepke <azuepke@sysgo.com> wrote:
>> while digging through some problems with BKPT exceptions on ARM, I
>> discovered that QEMU does not update IFSR on prefetch aborts. This
>> should be done since ARMv6 according to ARM docs. Please include.
> 
> This patch is the wrong approach to fixing this bug -- the
> updating of the IFSR needs to be done when the exception
> is taken, not when we translate the breakpoint instruction.

--- qemu-0.14.0.orig/target-arm/helper.c	2011-02-16 15:44:05.000000000 +0100
+++ qemu-0.14.0/target-arm/helper.c	2011-03-25 14:00:31.000000000 +0100
@@ -808,6 +808,8 @@ void do_interrupt(CPUARMState *env)
                 return;
             }
         }
+        /* indicate debug exception in IFSR */
+        env->cp15.c5_insn = 2;
         /* Fall through to prefetch abort.  */
     case EXCP_PREFETCH_ABORT:
         new_mode = ARM_CPU_MODE_ABT;


Something like this? This neither looks good ...

> I'll put this on my todo list. If you happen to have a convenient
> test case demonstrating the problem, that would make a fix happen
> faster ;-)

Testcase is attached.

$ gunzip tc.elf.gz
$ qemu-system-arm.orig -nographic --cpu cortex-a8 -kernel tc.elf
testcase: IFSR undefined on QEMU
got prefetch abort, IFSR is 12345678
test: failed
HALT
Killed
$ qemu-system-arm.fixed -nographic --cpu cortex-a8 -kernel tc.elf
testcase: IFSR undefined on QEMU
got prefetch abort, IFSR is 00000002
test: OK
HALT
Killed

Best Regards,
Alex

-- 
Alexander Zuepke                                azuepke@sysgo.com
SYSGO AG ~ Am Pfaffenstein 14 ~ 55270 Klein-Winternheim ~ Germany

[-- Attachment #2: tc.elf.gz --]
[-- Type: application/gzip, Size: 1713 bytes --]

Re: [Qemu-devel] ARM: BKPT instructions should raise prefetch aborts with IFSR type 00010

[Alex Zuepke]

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

Hi,

Peter Maydell schrieb:
> On 25 March 2011 10:54, Alex Zuepke <azuepke@sysgo.com> wrote:
>> while digging through some problems with BKPT exceptions on ARM, I
>> discovered that QEMU does not update IFSR on prefetch aborts. This
>> should be done since ARMv6 according to ARM docs. Please include.
> 
> This patch is the wrong approach to fixing this bug -- the
> updating of the IFSR needs to be done when the exception
> is taken, not when we translate the breakpoint instruction.
> 
> I'll put this on my todo list. If you happen to have a convenient
> test case demonstrating the problem, that would make a fix happen
> faster ;-)
> 
> -- PMM

I tried to fix it, new patch attached.
But I'm not sure if it is required for semihosting as well.

On ARMv7-M bkpt works differently, and debug registers aren't
implemented yet, so I didn't touch it.

Best Regards,
Alex

-- 
Alexander Zuepke                                azuepke@sysgo.com
SYSGO AG ~ Am Pfaffenstein 14 ~ 55270 Klein-Winternheim ~ Germany

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

 target-arm: BKPT instructions should raise prefetch aborts with IFSR type 00010
 diff against qemu 0.14.1
 Signed-off-by: Alex Zuepke <azuepke@sysgo.com>
diff --git a/target-arm/helper.c b/target-arm/helper.c
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -847,6 +849,7 @@ void do_interrupt(CPUARMState *env)
                 return;
             }
         }
+        env->cp15.c5_insn = 2;
         /* Fall through to prefetch abort.  */
     case EXCP_PREFETCH_ABORT:
         new_mode = ARM_CPU_MODE_ABT;

Re: [Qemu-devel] ARM: BKPT instructions should raise prefetch aborts with IFSR type 00010

[Peter Maydell]

On 18 May 2011 11:00, Alex Zuepke <azuepke@sysgo.com> wrote:
> Peter Maydell schrieb:
>> On 25 March 2011 10:54, Alex Zuepke <azuepke@sysgo.com> wrote:
>>> while digging through some problems with BKPT exceptions on ARM, I
>>> discovered that QEMU does not update IFSR on prefetch aborts. This
>>> should be done since ARMv6 according to ARM docs. Please include.

> I tried to fix it, new patch attached.

Thanks. I've looked at it and given it a quick test; I'm
happy with this version.

> But I'm not sure if it is required for semihosting as well.

I think the value of IFSR is not defined after a semihosting
request (different implementations might use an actual SVC handler
or might intercept the SVC before it becomes an actual CPU SVC
exception). For QEMU we handle the semihosting request and
return immediately rather than actually delivering a CPU exception,
so I think it makes sense not to set IFSR in that case.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] ARM: BKPT instructions should raise prefetch aborts with IFSR type 00010

[Aurelien Jarno]

On Wed, May 18, 2011 at 12:00:46PM +0200, Alex Zuepke wrote:
> Hi,
> 
> Peter Maydell schrieb:
> > On 25 March 2011 10:54, Alex Zuepke <azuepke@sysgo.com> wrote:
> >> while digging through some problems with BKPT exceptions on ARM, I
> >> discovered that QEMU does not update IFSR on prefetch aborts. This
> >> should be done since ARMv6 according to ARM docs. Please include.
> > 
> > This patch is the wrong approach to fixing this bug -- the
> > updating of the IFSR needs to be done when the exception
> > is taken, not when we translate the breakpoint instruction.
> > 
> > I'll put this on my todo list. If you happen to have a convenient
> > test case demonstrating the problem, that would make a fix happen
> > faster ;-)
> > 
> > -- PMM
> 
> I tried to fix it, new patch attached.
> But I'm not sure if it is required for semihosting as well.
> 
> On ARMv7-M bkpt works differently, and debug registers aren't
> implemented yet, so I didn't touch it.
> 

Thanks, applied. In the future, could you please send the patch inline,
or at least attach a patch that can be applied with git am?

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net