[PATCH v1 1/1] target/microblaze: Add GDB XML files for Microblaze

[PATCH v1 1/1] target/microblaze: Add GDB XML files for Microblaze

[Joe Komlodi]

This adds the Microblaze core and stack protect XML files, and also
modifies the configuration to build those files.

Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
---
 configure                            |  1 +
 gdb-xml/microblaze-core.xml          | 62 ++++++++++++++++++++++++++++++++++++
 gdb-xml/microblaze-stack-protect.xml | 12 +++++++
 target/microblaze/cpu.c              |  1 +
 4 files changed, 76 insertions(+)
 create mode 100644 gdb-xml/microblaze-core.xml
 create mode 100644 gdb-xml/microblaze-stack-protect.xml

diff --git a/configure b/configure
index 2fc05c4..8aed2fd 100755
--- a/configure
+++ b/configure
@@ -7832,6 +7832,7 @@ case "$target_name" in
     TARGET_ARCH=microblaze
     TARGET_SYSTBL_ABI=common
     bflt="yes"
+    gdb_xml_files="microblaze-core.xml microblaze-stack-protect.xml"
     echo "TARGET_ABI32=y" >> $config_target_mak
   ;;
   mips|mipsel)
diff --git a/gdb-xml/microblaze-core.xml b/gdb-xml/microblaze-core.xml
new file mode 100644
index 0000000..d30e596
--- /dev/null
+++ b/gdb-xml/microblaze-core.xml
@@ -0,0 +1,62 @@
+<?xml version="1.0"?>
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.microblaze.core">
+  <reg name="r0" bitsize="32" regnum="0"/>
+  <reg name="r1" bitsize="32" type="data_ptr"/>
+  <reg name="r2" bitsize="32"/>
+  <reg name="r3" bitsize="32"/>
+  <reg name="r4" bitsize="32"/>
+  <reg name="r5" bitsize="32"/>
+  <reg name="r6" bitsize="32"/>
+  <reg name="r7" bitsize="32"/>
+  <reg name="r8" bitsize="32"/>
+  <reg name="r9" bitsize="32"/>
+  <reg name="r10" bitsize="32"/>
+  <reg name="r11" bitsize="32"/>
+  <reg name="r12" bitsize="32"/>
+  <reg name="r13" bitsize="32"/>
+  <reg name="r14" bitsize="32" type="code_ptr"/>
+  <reg name="r15" bitsize="32" type="code_ptr"/>
+  <reg name="r16" bitsize="32" type="code_ptr"/>
+  <reg name="r17" bitsize="32"/>
+  <reg name="r18" bitsize="32"/>
+  <reg name="r19" bitsize="32"/>
+  <reg name="r20" bitsize="32"/>
+  <reg name="r21" bitsize="32"/>
+  <reg name="r22" bitsize="32"/>
+  <reg name="r23" bitsize="32"/>
+  <reg name="r24" bitsize="32"/>
+  <reg name="r25" bitsize="32"/>
+  <reg name="r26" bitsize="32"/>
+  <reg name="r27" bitsize="32"/>
+  <reg name="r28" bitsize="32"/>
+  <reg name="r29" bitsize="32"/>
+  <reg name="r30" bitsize="32"/>
+  <reg name="r31" bitsize="32"/>
+  <reg name="rpc" bitsize="32" type="code_ptr"/>
+  <reg name="rmsr" bitsize="32"/>
+  <reg name="rear" bitsize="32"/>
+  <reg name="resr" bitsize="32"/>
+  <reg name="rfsr" bitsize="32"/>
+  <reg name="rbtr" bitsize="32"/>
+  <reg name="rpvr0" bitsize="32"/>
+  <reg name="rpvr1" bitsize="32"/>
+  <reg name="rpvr2" bitsize="32"/>
+  <reg name="rpvr3" bitsize="32"/>
+  <reg name="rpvr4" bitsize="32"/>
+  <reg name="rpvr5" bitsize="32"/>
+  <reg name="rpvr6" bitsize="32"/>
+  <reg name="rpvr7" bitsize="32"/>
+  <reg name="rpvr8" bitsize="32"/>
+  <reg name="rpvr9" bitsize="32"/>
+  <reg name="rpvr10" bitsize="32"/>
+  <reg name="rpvr11" bitsize="32"/>
+  <reg name="redr" bitsize="32"/>
+  <reg name="rpid" bitsize="32"/>
+  <reg name="rzpr" bitsize="32"/>
+  <reg name="rtlbx" bitsize="32"/>
+  <reg name="rtlbsx" bitsize="32"/>
+  <reg name="rtlblo" bitsize="32"/>
+  <reg name="rtlbhi" bitsize="32"/>
+</feature>
diff --git a/gdb-xml/microblaze-stack-protect.xml b/gdb-xml/microblaze-stack-protect.xml
new file mode 100644
index 0000000..1b16223
--- /dev/null
+++ b/gdb-xml/microblaze-stack-protect.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2014-2020 Free Software Foundation, Inc.
+
+     Copying and distribution of this file, with or without modification,
+     are permitted in any medium without royalty provided the copyright
+     notice and this notice are preserved.  -->
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.microblaze.stack-protect">
+  <reg name="rslr" bitsize="32"/>
+  <reg name="rshr" bitsize="32"/>
+</feature>
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 51e5c85..faa88e5 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -330,6 +330,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
     dc->vmsd = &vmstate_mb_cpu;
     device_class_set_props(dc, mb_properties);
     cc->gdb_num_core_regs = 32 + 27;
+    cc->gdb_core_xml_file = "microblaze-core.xml";
 
     cc->disas_set_info = mb_disas_set_info;
     cc->tcg_initialize = mb_tcg_init;
-- 
2.7.4

[PATCH v1 0/1] target/microblaze: Add GDB XML files

[Joe Komlodi]

Hi all,

This adds GDB XML files for Microblaze CPUs.
For Microblaze, it's split up into core and stack protect XML files.

Thanks!
Joe

Joe Komlodi (1):
  target/microblaze: Add GDB XML files for Microblaze

 configure                            |  1 +
 gdb-xml/microblaze-core.xml          | 62 ++++++++++++++++++++++++++++++++++++
 gdb-xml/microblaze-stack-protect.xml | 12 +++++++
 target/microblaze/cpu.c              |  1 +
 4 files changed, 76 insertions(+)
 create mode 100644 gdb-xml/microblaze-core.xml
 create mode 100644 gdb-xml/microblaze-stack-protect.xml

-- 
2.7.4

Re: [PATCH v1 0/1] target/microblaze: Add GDB XML files

[Edgar E. Iglesias]

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

On Thu, May 21, 2020 at 09:02:14PM -0700, Joe Komlodi wrote:
> Hi all,
> 
> This adds GDB XML files for Microblaze CPUs.
> For Microblaze, it's split up into core and stack protect XML files.

Hi Joe,

Can you please add a license header to microblaze-core.xml?
I think you wrote this on your own but in case you took it from
GDB (or if they actually are identical) we can keep GDB's
permissive license?

We also need to register the stackprot xml with
gdb_register_coprocessor().

I think you were right about lowering gdb_num_core_regs back to:
    cc->gdb_num_core_regs = 32 + 25;

Things work even if we keep 32 + 27 but I think it's a little
confusing. I've tried a version that uses:

    cc->gdb_num_core_regs = 32 + 25;

And registers stack-prot:
    gdb_register_coprocessor(cs, mb_sp_gdb_get_reg, mb_sp_gdb_set_reg,
                             2, "microblaze-stack-protect.xml", 32 + 25);

That works with GDB from upstream and with Xilinx XSDB.
Vitis 2019.2 microblaze gdb thinks SLR/SHR are 64bit for some
reason and doesn't handle them well with any QEMU version.
We should probably talk to the Vitis folks about changing their
version to match upstream GDB and XSDB.

I'm attaching the diff I used as reference.

Thanks,
Edgar

[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 3315 bytes --]

diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index a31134b65c..faa48493f6 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -321,6 +321,8 @@ void mb_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
 hwaddr mb_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 int mb_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int mb_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
+int mb_sp_gdb_get_reg(CPUMBState *env, GByteArray *buf, int n);
+int mb_sp_gdb_set_reg(CPUMBState *env, uint8_t *buf, int n);
 
 void mb_tcg_init(void);
 /* you can call this signal handler from your SIGBUS and SIGSEGV
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index faa88e5865..c87257d928 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -28,6 +28,7 @@
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "exec/exec-all.h"
+#include "exec/gdbstub.h"
 #include "fpu/softfloat-helpers.h"
 
 static const struct {
@@ -226,6 +227,9 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
     env->pvr.regs[11] = (cpu->cfg.use_mmu ? PVR11_USE_MMU : 0) |
                         16 << 17;
 
+    gdb_register_coprocessor(cs, mb_sp_gdb_get_reg, mb_sp_gdb_set_reg,
+                             2, "microblaze-stack-protect.xml", 32 + 25);
+
     mcc->parent_realize(dev, errp);
 }
 
@@ -329,7 +333,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
 #endif
     dc->vmsd = &vmstate_mb_cpu;
     device_class_set_props(dc, mb_properties);
-    cc->gdb_num_core_regs = 32 + 27;
+    cc->gdb_num_core_regs = 32 + 25;
     cc->gdb_core_xml_file = "microblaze-core.xml";
 
     cc->disas_set_info = mb_disas_set_info;
diff --git a/target/microblaze/gdbstub.c b/target/microblaze/gdbstub.c
index 73e8973597..6eeccb9201 100644
--- a/target/microblaze/gdbstub.c
+++ b/target/microblaze/gdbstub.c
@@ -65,10 +65,12 @@ int mb_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
         /* Other SRegs aren't modeled, so report a value of 0 */
         case 19 ... 24:
             return gdb_get_reg32(mem_buf, 0);
+#if 0
         case 25:
             return gdb_get_reg32(mem_buf, env->slr);
         case 26:
             return gdb_get_reg32(mem_buf, env->shr);
+#endif
         default:
             return 0;
         }
@@ -129,13 +131,47 @@ int mb_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
         case 18:
             env->sregs[SR_EDR] = tmp;
             break;
+#if 0
         case 25:
             env->slr = tmp;
             break;
         case 26:
             env->shr = tmp;
             break;
+#endif
         }
     }
     return 4;
 }
+
+int mb_sp_gdb_get_reg(CPUMBState *env, GByteArray *buf, int n)
+{
+    printf("%s: n=%d\n", __func__, n);
+    switch (n) {
+    case 0:
+        return gdb_get_reg32(buf, env->slr);
+    case 1:
+        return gdb_get_reg32(buf, env->shr);
+    default:
+        return 0;
+    }
+    return 0;
+}
+
+int mb_sp_gdb_set_reg(CPUMBState *env, uint8_t *buf, int n)
+{
+    uint32_t tmp;
+
+    tmp = ldl_p(buf);
+
+    printf("%s: n=%d\n", __func__, n);
+    switch (n) {
+    case 25:
+        env->slr = tmp;
+        break;
+    case 26:
+        env->shr = tmp;
+        break;
+    }
+    return 4;
+}