[PATCH] Fix aarch64 debug register names.

[Chris Howard]

From 5de17d5aacb9cf21de4c9736b227b0498c607709 Mon Sep 17 00:00:00 2001
From: CHRIS HOWARD 
Date: Thu, 12 May 2022 11:35:17 +0200
Subject: [PATCH] Fix aarch64 debug register names.

Signed-off-by: CHRIS HOWARD 
---
 target/arm/helper.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 432bd81919..0dd97cb905 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6543,14 +6543,16 @@ static void define_debug_regs(ARMCPU *cpu)
 }
 
 for (i = 0; i < brps; i++) {
+char *dbgbvr_el1_name = g_strdup_printf("DBGBVR%d_EL1", i);
+char *dbgbcr_el1_name = g_strdup_printf("DBGBCR%d_EL1", i);
 ARMCPRegInfo dbgregs[] = {
-{ .name = "DBGBVR", .state = ARM_CP_STATE_BOTH,
+{ .name = dbgbvr_el1_name, .state = ARM_CP_STATE_BOTH,
   .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 4,
   .access = PL1_RW, .accessfn = access_tda,
   .fieldoffset = offsetof(CPUARMState, cp15.dbgbvr[i]),
   .writefn = dbgbvr_write, .raw_writefn = raw_write
 },
-{ .name = "DBGBCR", .state = ARM_CP_STATE_BOTH,
+{ .name = dbgbcr_el1_name, .state = ARM_CP_STATE_BOTH,
   .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 5,
   .access = PL1_RW, .accessfn = access_tda,
   .fieldoffset = offsetof(CPUARMState, cp15.dbgbcr[i]),
@@ -6558,17 +6560,21 @@ static void define_debug_regs(ARMCPU *cpu)
 },
 };
 define_arm_cp_regs(cpu, dbgregs);
+g_free(dbgbvr_el1_name);
+g_free(dbgbcr_el1_name);
 }
 
 for (i = 0; i < wrps; i++) {
+char *dbgwvr_el1_name = g_strdup_printf("DBGWVR%d_EL1", i);
+char *dbgwcr_el1_name = g_strdup_printf("DBGWCR%d_EL1", i);
 ARMCPRegInfo dbgregs[] = {
-{ .name = "DBGWVR", .state = ARM_CP_STATE_BOTH,
+{ .name = dbgwvr_el1_name, .state = ARM_CP_STATE_BOTH,
   .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 6,
   .access = PL1_RW, .accessfn = access_tda,
   .fieldoffset = offsetof(CPUARMState, cp15.dbgwvr[i]),
   .writefn = dbgwvr_write, .raw_writefn = raw_write
 },
-{ .name = "DBGWCR", .state = ARM_CP_STATE_BOTH,
+{ .name = dbgwcr_el1_name, .state = ARM_CP_STATE_BOTH,
   .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 7,
   .access = PL1_RW, .accessfn = access_tda,
   .fieldoffset = offsetof(CPUARMState, cp15.dbgwcr[i]),
@@ -6576,6 +6582,8 @@ static void define_debug_regs(ARMCPU *cpu)
 },
 };
 define_arm_cp_regs(cpu, dbgregs);
+g_free(dbgwvr_el1_name);
+g_free(dbgwcr_el1_name);
 }
 }
 
-- 
2.20.1 (Apple Git-117)

Re: Possible bug in Aarch64 single-stepping

[Chris Howard]

On 8. May 2022, at 14:08, Peter Maydell  wrote:
> 
> On Sat, 7 May 2022 at 14:44, Chris Howard  wrote:
>> 
>> Hi, I’m writing a simple debugger in assembly code for the Raspberry Pi 3B 
>> (in aarch64).
>> 
>> I’m using QEMU 7.0.0. Everything is running in EL1. (I have MDE and KDE set 
>> in MDSCR_EL1).
>> 
>> I’m coming across Unexpected Behaviour when playing with single-stepping:
>> 
>> It appears that single-stepping is enabled (ie. an exception is generated 
>> after every instruction) when the SS bit (bit-0) is set in MDSCR_EL1 and 
>> debug events are enabled in the CPSR (“D” bit clear) *** irrespective of 
>> whether the SS bit (bit-21) is set in CPSR or not ***.
>> 
>> I thought the SS bit (bit-21) needs to be set in CPSR for single-stepping to 
>> occur (and that it gets cleared whenever an exception is taken and needs to 
>> be reset if one wants to single-step again).
>> 
>> Have I misunderstood / misconfigured something, or is this a bug?
> 
> I think you've misunderstood how the architectural single
> step works. This is described in section D2.12 of the Arm ARM
> (DDI0487H.a), but briefly, there is a state machine with three
> states: Inactive, Active-not-pending, and Active-pending.
> 
> * Inactive is when MDSCR_EL1.SS is 0 or debug exceptions are
> disabled from the current EL or security state.
> 
> * Active-not-pending is when we're not Inactive (ie MDSCR_EL1.SS is 1
> and so on) and PSTATE.SS is 1. This is the state for "we're currently
> pointing at the instruction we would like to step". The CPU
> does the step by doing "execute this one instruction, and then
> clear PSTATE.SS". It does *not* take a "single step completed"
> exception. (I ignore for the moment the possibility that the
> insn resulted in some other exception.)
> 
> * Active-pending is when we're not Inactive and PSTATE.SS is 0.
> This state means "Software step is active, and a software step
> is pending on the current instruction".
> The usual way we get here is that we were in Active-not-pending
> and then we executed the instruction and cleared PSTATE.SS.
> But you can also get here in other ways (as your test case does).
> In Active-pending state, the CPU does "take a software step
> exception immediately, without doing anything else" -- which is
> what you see.
> 
> In other words, the design effectively separates out the
> "execute one instruction" part from the "take the exception
> that says we completed a step" part. (This separation is
> irrelevant for the 'normal case' where the stepped instruction
> doesn't cause any exceptions and no interrupts arrive either,
> but it can matter if there is some interrupt to be taken. For
> instance, suppose that we do a step, and while the insn is
> executing an interrupt comes in that is routed to EL3. We
> want to take that interrupt first, before taking the
> 'single step complete' exception. Because the distinction
> between 'active-not-pending' and 'active-pending' is stored
> in PSTATE.SS, this works automatically -- we go to EL3, do
> whatever the interrupt handler does, and then on the eret
> to where we started, PSTATE.SS is restored to 0: so we then
> correctly take the 'single step complete' exception without
> executing another instruction.)
> 
> So if you don't want to see single-step exceptions you need
> to make sure you stay in the Inactive state.
> 
> thanks
> — PMM


I had indeed misunderstood :-) even though I had looked at that
section (and the state machine diagram)  :-/

Thanks for the EXCELLENT explanation! Very much appreciated.

—chris

Re: Possible bug in Aarch64 single-stepping [PATCH]

[Chris Howard]

On 8. May 2022, at 14:18, Peter Maydell  wrote:

> On Sat, 7 May 2022 at 15:18, Chris Howard  wrote:
>> PS. In plain gdb (ie. no nice user interface) a large number (but not all) 
>> of the system registers gets displayed after each step. It would be nice if 
>> these were sorted in some way. At the moment they’re completely jumbled — 
>> not alphabetic, not grouped by EL, nor by “meaning”  (DBGWVR0_EL1 isn’t 
>> necessarily next to DBGWCR0_EL1).
>> 
>> Also, there are multiple (identical?) instances of “DBGBVR” and “DBGBCR” 
>> (and  “DBGWVR” and “DBGWCR”) rather than the expected “DBGWVR0_EL1”, 
>> “DBGWVR1_EL1” etc.
>> 
>> Would this be a QEMU or a GDB issue? Or isn’t it an issue at all? :-)
> 
> My gdb doesn't do that. Basically QEMU provides gdb with some XML
> telling it that the sysregs are present, but it's up to gdb at
> what points it chooses to display what registers and how it does that.
> 
> The system register read access via the gdbstub is "best-effort"
> on QEMU's part -- we implement it to the extent that it wasn't too
> difficult to do, but there are some sharp edges, like the
> register names not always being quite right, and also the way
> that if you try to read a register that isn't supposed to be
> accessible by the current EL you might find it's not correct.
> Trying to read SP_EL2 while at EL2 is an example of that.
> 
> The reason register names are sometimes funny is that the
> infrastructure for system registers within QEMU was originally
> written with the assumption that the name strings were merely
> for convenience when debugging QEMU itself, so it's sometimes
> a bit careless about them. We only added the "tell GDB about
> these" part later.
> 
> That said, adding the numbers into the watchpoint and breakpoint
> registers would be pretty easy, so we should do that. That is,
> in this code:
> https://gitlab.com/qemu-project/qemu/-/blob/master/target/arm/helper.c#L6567
> we should use g_strdup_printf() to create unique per-register
> names, the same way we do for the PMU registers already here:
> https://gitlab.com/qemu-project/qemu/-/blob/master/target/arm/helper.c#L6632
> 
> thanks
> -- PMM

Thanks for the explanation. What with this being “pretty easy” I’m attempting 
my first ever patch!  :-)

BE WARNED!

This is a context diff with respect to the cloned git repository (Version 
7.0.50)

$ git clone https://gitlab.com/qemu-project/qemu.git

created with

$ diff -c qemu/target/arm/helper.c qemu-patch/target/arm/helper.c > 
aarch-dbg-regnames.patch

to be applied (in the target/arm directory) with

$ patch -p3 <../../../aarch-dbg-regnames.patch


— chris


 
*** qemu/target/arm/helper.c2022-05-08 20:41:48.0 +0200
--- qemu-patch/target/arm/helper.c  2022-05-08 20:55:25.0 +0200
***
*** 6551,6559 
  define_one_arm_cp_reg(cpu, &dbgdidr);
  }
  
! /* Note that all these register fields hold "number of Xs minus 1". */
! brps = arm_num_brps(cpu);
! wrps = arm_num_wrps(cpu);
  ctx_cmps = arm_num_ctx_cmps(cpu);
  
  assert(ctx_cmps <= brps);
--- 6551,6559 
  define_one_arm_cp_reg(cpu, &dbgdidr);
  }
  
! /* Note that all these reg fields (in ID_AA64DFR0_EL1) hold "number of Xs 
minus 1". */
! brps = arm_num_brps(cpu); /* returns actual number of 
breakpoints */
! wrps = arm_num_wrps(cpu); /* returns actual number of 
watchpoints */
  ctx_cmps = arm_num_ctx_cmps(cpu);
  
  assert(ctx_cmps <= brps);
***
*** 6565,6578 
  }
  
  for (i = 0; i < brps; i++) {
  ARMCPRegInfo dbgregs[] = {
! { .name = "DBGBVR", .state = ARM_CP_STATE_BOTH,
.cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 4,
.access = PL1_RW, .accessfn = access_tda,
.fieldoffset = offsetof(CPUARMState, cp15.dbgbvr[i]),
.writefn = dbgbvr_write, .raw_writefn = raw_write
  },
! { .name = "DBGBCR", .state = ARM_CP_STATE_BOTH,
.cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 5,
.access = PL1_RW, .accessfn = access_tda,
.fieldoffset = offsetof(CPUARMState, cp15.dbgbcr[i]),
--- 6565,6580 
  }
  
  for (i = 0; i < brps; i++) {
+   char *dbgbvr_el1_name = g_strdup_printf("DBGBVR%d_EL1", i);
+   char *dbgbcr_el1_name = g_strdup_printf("DBGBCR%d_EL1", i);
  ARMCPRegInfo dbgregs[] = {
! { .name = dbgbvr_el1_name, .state = ARM_CP_STATE_BOTH,
.cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 4,
.access = PL1_RW

Re: Possible bug in Aarch64 single-stepping

[Chris Howard]

On 7. May 2022, at 15:42, Chris Howard  wrote:
> 
> Hi, I’m writing a simple debugger in assembly code for the Raspberry Pi 3B 
> (in aarch64).
> 
> I’m using QEMU 7.0.0. Everything is running in EL1. (I have MDE and KDE set 
> in MDSCR_EL1).
> 
> I’m coming across Unexpected Behaviour when playing with single-stepping:
> 
> It appears that single-stepping is enabled (ie. an exception is generated 
> after every instruction) when the SS bit (bit-0) is set in MDSCR_EL1 and 
> debug events are enabled in the CPSR (“D” bit clear) *** irrespective of 
> whether the SS bit (bit-21) is set in CPSR or not ***.
> 
> I thought the SS bit (bit-21) needs to be set in CPSR for single-stepping to 
> occur (and that it gets cleared whenever an exception is taken and needs to 
> be reset if one wants to single-step again).
> 
> Have I misunderstood / misconfigured something, or is this a bug?
> 
> Attached is a minimal(ish) example:

Oh, and the exception occurs immediately (after the ERET), rather than after 
the instruction has been executed. It appears to be acting like a hardware 
breakpoint.


PS. In plain gdb (ie. no nice user interface) a large number (but not all) of 
the system registers gets displayed after each step. It would be nice if these 
were sorted in some way. At the moment they’re completely jumbled — not 
alphabetic, not grouped by EL, nor by “meaning”  (DBGWVR0_EL1 isn’t necessarily 
next to DBGWCR0_EL1).

Also, there are multiple (identical?) instances of “DBGBVR” and “DBGBCR” (and  
“DBGWVR” and “DBGWCR”) rather than the expected “DBGWVR0_EL1”, “DBGWVR1_EL1” 
etc.

Would this be a QEMU or a GDB issue? Or isn’t it an issue at all? :-)

Possible bug in Aarch64 single-stepping

[Chris Howard]

Hi, I’m writing a simple debugger in assembly code for the Raspberry Pi 3B (in 
aarch64).

I’m using QEMU 7.0.0. Everything is running in EL1. (I have MDE and KDE set in 
MDSCR_EL1).

I’m coming across Unexpected Behaviour when playing with single-stepping:

It appears that single-stepping is enabled (ie. an exception is generated after 
every instruction) when the SS bit (bit-0) is set in MDSCR_EL1 and debug events 
are enabled in the CPSR (“D” bit clear) *** irrespective of whether the SS bit 
(bit-21) is set in CPSR or not ***.

I thought the SS bit (bit-21) needs to be set in CPSR for single-stepping to 
occur (and that it gets cleared whenever an exception is taken and needs to be 
reset if one wants to single-step again).

Have I misunderstood / misconfigured something, or is this a bug?

Attached is a minimal(ish) example:


Regards,

chris


/* 
**

 built with 

aarch64-elf-as -march=armv8-a+crc+crypto -mcpu=cortex-a53 --gstabs minimal.s -o 
minimal.o
aarch64-elf-ld minimal.o -T memmap -o minimal.elf
aarch64-elf-objcopy -O binary minimal.elf minimal.img
aarch64-elf-objdump -d -j .text minimal.elf > minimal.list

 memmap is 

MEMORY
{
ram : ORIGIN = 0x8, LENGTH = 0x8000
}

SECTIONS
{
.init : { *(.init*) } > ram
.text : { *(.text*) } > ram
.data : { *(.data*) } > ram
.bss  : { *(.bss*)  } > ram
}

 qemu cmd 

qemu-7.0.0/build/qemu-system-aarch64 -M raspi3b -kernel minimal.img -serial 
null -serial stdio -s -S

 gdb cmd 

/opt/local/gnuaarch64/bin/aarch64-none-elf-gdb -x gdb.cmds minimal.elf

 gdb.cmds is 

target remote :1234
set $pc = 0x8

**
 */

.section .text

.global _start
_start:

mrs x0, mpidr_el1   // x0 = .8000.
and x0, x0, 0b11
cbz x0, 2f  // branch fwd if core == 0 ...
1:
wfe // ...  else wait for exception (ie 
sleep)
b   1b
2:
//  mrs x0, scr_el3 // Can't access this from EL2. But gdb 
says it's 0x0501
// .0101|.0001  10:RW=1  
8:HCE=1  0:NS=1

//  mrs x0, hcr_el2 // x0 -> 1<<31 (34:E2H=0  31:RW=1  
27:TGE=0  2:C=0  1:M=0)

//  mrs x0, sctlr_el1   // x0 -> 00c5.0838 = 
.|1100.0101|.1000|0011.1000
mov x0, 0x0800  // -SA -SA0, -CP15BEN -nTWE -nTWI 
+20:RES1 +28:RES1 +29:RES1
movkx0, 0x30d0, lsl 16  // x0 -> 30d0.0800 = 
0011.|1101.|.1000|.
msr sctlr_el1, x0

adr x0, vectors
msr vbar_el1, x0

adr x0, _start  // 0x0008. = 512k
msr sp_el1, x0  // set EL1's SP

mov x0, 0x3c5   // NOTE: bit 21:SS is NOT set
msr spsr_el2, x0// DAIF exceptions masked. A64 state. 
EL1. SP = SP_ELx

adr x0, init
msr elr_el2, x0

eret// We're in EL2. Pretend we're 
"returning" to EL1 init:


init:
msr OSLAR_EL1, xzr  // unlock OS lock

mov x0, 0b101<<13   // 15:MDE (enable debugging) and 13:KDE 
(and kernel debug events)
orr x0, x0, 0b1<<0  // enable SS debug exceptions
msr MDSCR_EL1, x0

mov w0, 0   // these all execute ok
mov w0, 1
b   1f
mov w0, -1
1:

msr daifclr, 0b // enable all interrupts and debugging 
exceptions

mov w0, 3   // this causes an exception ESR_EL1 = 
0xce22 (see ARMv8ARM D1-1801)
mov w0, 4   // 0xce = 0b1100.1110|- >> 26 = 
0b0011.0011 = 0x33 = exception class
// ie. SS from same EL. ** even though 
CPSR was 0x0005 **
svc 124

mov w0, 5
b   2f
mov w0, -2
2:
mov w0, 6


loop:
b   loop


// 
**

exc_svc:
and w0, w0, 0x
cmp w0, 123
beq svc_123
cmp w0, 124
beq svc_124
b   exception_return// undefined SVCs ignored


svc_123:
mrs x0, SPSR_EL1
orr x0, x0, 0b1<<21 // (re)enable SS in CPSR
msr SPSR_EL1, x0
b   exception_return


svc_124:
mrs x0, SPSR_EL1
bic x0, x0, 0b1<<21 // disable SS in CPSR (not really 
necessary since SS gets
msr SPSR_EL1, x0// cleared when an exception (eg. SVC) 
gets taken anyway)
b   exception_return


exc_dbg:
mrs x0, elr_el1

mrs x0, SPSR_EL1
bic x0, x0, 0b1<<21 // (re)enable SS in CPSR

Re: Possible bug when setting aarch64 watchpoints

[Chris Howard]

Sorry, I need to correct my previous post:



If I set

DBGWVR0_EL1 = 1<<23 // ie. 0x0080

and

DBGWCR0_EL1 = 0x17<<24 | 0xFF<<5 | 0b11<<3 | 0b11<<1 | 0b1<<0   // ie. 
MASK = 23 = 0b10111

and then access  memory [0x0080007F]  I get a watchpoint exception. (ie. 
watchpoints ARE working/enabled)

But if I access [0x00800080] I *don’t* get an exception.

**If the MASK field gets set to 0b0111 instead of 0b10111 then only the bottom 
7 bits of the address get masked (instead of 23) and the masked address isn’t 
0x0080, and the exception won’t be triggered.**

(if I *attempt* to set the MASK to 0b1, but it actually gets set to 
0b0, then I get the behaviour quoted below).


> On 24. Apr 2022, at 13:40, Chris Howard  wrote:
> 
> Hi, I’m new to qemu (and even bug-reporting) so apologies in advance…
> 
> The MASK field in DBGWCRx_EL1 is **5** bits wide [28:24].
> 
> In target/arm/kvm64.c I found the line:
> 
> wp.wcr = deposit32(wp.wcr, 24, 4, bits);  // ie **4** bits 
> instead of **5**
> 
> 
> If it’s not copying (or calculating?) the number of bits correctly this would 
> explain the behaviour I’m seeing:
> 
> If I set
> 
> DBGWVR0_EL1 = 0x0080
> 
> and
> 
> DBGWCR0_EL1 = 0x1F<<24 | 0xFF<<5 | 0b11<<3 | 0b11<<1 | 0b1<<0
> 
> and then access  memory [0x00807FFF]  I get a watchpoint exception. (ie. 
> watchpoints ARE working/enabled)
> 
> But if I access [0x00808] I *don’t* get an exception.
> 
> **If the MASK field gets set to 0b instead of 0b1 then only the 
> bottom 15 bits of the address get masked (instead of 31) and the masked 
> address isn’t 0x0080, and the exception won’t be triggered.**
> 
> 
> Unfortunately, changing the 4 to a 5 and recompiling had no effect :-(
> 
> I may well have misunderstood something. :-/
> 
> —Chris

Possible bug when setting aarch64 watchpoints

[Chris Howard]

Hi, I’m new to qemu (and even bug-reporting) so apologies in advance…

The MASK field in DBGWCRx_EL1 is **5** bits wide [28:24].

In target/arm/kvm64.c I found the line:

 wp.wcr = deposit32(wp.wcr, 24, 4, bits);   // ie **4** bits 
instead of **5**


If it’s not copying (or calculating?) the number of bits correctly this would 
explain the behaviour I’m seeing:

If I set

DBGWVR0_EL1 = 0x0080

and

DBGWCR0_EL1 = 0x1F<<24 | 0xFF<<5 | 0b11<<3 | 0b11<<1 | 0b1<<0

and then access  memory [0x00807FFF]  I get a watchpoint exception. (ie. 
watchpoints ARE working/enabled)

But if I access [0x00808] I *don’t* get an exception.

**If the MASK field gets set to 0b instead of 0b1 then only the bottom 
15 bits of the address get masked (instead of 31) and the masked address isn’t 
0x0080, and the exception won’t be triggered.**


Unfortunately, changing the 4 to a 5 and recompiling had no effect :-(

I may well have misunderstood something. :-/

—Chris