* [PATCH V2 1/4] target/microblaze: gdb: Add dynamic GDB XML register support
@ 2020-05-13 18:08 Joe Komlodi
2020-05-13 18:08 ` [PATCH V2 2/4] target/microblaze: gdb: Extend the number of registers presented to GDB Joe Komlodi
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Joe Komlodi @ 2020-05-13 18:08 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias
Add dynamic GDB register XML for Microblaze, and modify the config file to
use XML when building for Microblaze.
For the dynamic XML to be read, there still needs to be a core XML file.
Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
---
configure | 1 +
gdb-xml/microblaze-core.xml | 64 +++++++++++++++++++++++
target/microblaze/cpu.c | 4 ++
target/microblaze/cpu.h | 9 ++++
target/microblaze/gdbstub.c | 123 ++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 201 insertions(+)
create mode 100644 gdb-xml/microblaze-core.xml
diff --git a/configure b/configure
index 0d69c36..5a099b6 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"
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..13e2c08
--- /dev/null
+++ b/gdb-xml/microblaze-core.xml
@@ -0,0 +1,64 @@
+<?xml version="1.0"?>
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.microblaze.core">
+ <reg name="r0" bitsize="32"/>
+ <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"/>
+ <reg name="rslr" bitsize="32"/>
+ <reg name="rshr" bitsize="32"/>
+</feature>
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index aa99830..41cac1b 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -226,6 +226,8 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
env->pvr.regs[11] = (cpu->cfg.use_mmu ? PVR11_USE_MMU : 0) |
16 << 17;
+ mb_gen_dynamic_xml(cpu);
+
mcc->parent_realize(dev, errp);
}
@@ -330,6 +332,8 @@ 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 + 5;
+ cc->gdb_get_dynamic_xml = mb_gdb_get_dynamic_xml;
+ cc->gdb_core_xml_file = "microblaze-core.xml";
cc->disas_set_info = mb_disas_set_info;
cc->tcg_initialize = mb_tcg_init;
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index a31134b..074a18e 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -25,6 +25,8 @@
#include "fpu/softfloat-types.h"
typedef struct CPUMBState CPUMBState;
+typedef struct DynamicMBGDBXMLInfo DynamicMBGDBXMLInfo;
+
#if !defined(CONFIG_USER_ONLY)
#include "mmu.h"
#endif
@@ -272,6 +274,10 @@ struct CPUMBState {
} pvr;
};
+struct DynamicMBGDBXMLInfo {
+ char *xml;
+};
+
/**
* MicroBlazeCPU:
* @env: #CPUMBState
@@ -286,6 +292,7 @@ struct MicroBlazeCPU {
CPUNegativeOffsetState neg;
CPUMBState env;
+ DynamicMBGDBXMLInfo dyn_xml;
/* Microblaze Configuration Settings */
struct {
@@ -321,6 +328,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);
+void mb_gen_dynamic_xml(MicroBlazeCPU *cpu);
+const char *mb_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname);
void mb_tcg_init(void);
/* you can call this signal handler from your SIGBUS and SIGSEGV
diff --git a/target/microblaze/gdbstub.c b/target/microblaze/gdbstub.c
index f41ebf1..cdca434 100644
--- a/target/microblaze/gdbstub.c
+++ b/target/microblaze/gdbstub.c
@@ -54,3 +54,126 @@ int mb_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
}
return 4;
}
+
+static void mb_gen_xml_reg_tag(const MicroBlazeCPU *cpu, GString *s,
+ const char *name, uint8_t bitsize,
+ const char *type)
+{
+ g_string_append_printf(s, "<reg name=\"%s\" bitsize=\"%d\"",
+ name, bitsize);
+ if (type) {
+ g_string_append_printf(s, " type=\"%s\"", type);
+ }
+ g_string_append_printf(s, "/>\n");
+}
+
+static uint8_t mb_cpu_sreg_size(const MicroBlazeCPU *cpu, uint8_t index)
+{
+ /*
+ * NOTE: mb-gdb will refuse to connect if we say registers are
+ * larger then 32-bits.
+ * For now, say none of our registers are dynamically sized, and are
+ * therefore only 32-bits.
+ */
+
+ return 32;
+}
+
+static void mb_gen_xml_reg_tags(const MicroBlazeCPU *cpu, GString *s)
+{
+ uint8_t i;
+ const char *type;
+ char reg_name[4];
+ bool has_hw_exception = cpu->cfg.dopb_bus_exception ||
+ cpu->cfg.iopb_bus_exception ||
+ cpu->cfg.illegal_opcode_exception ||
+ cpu->cfg.opcode_0_illegal ||
+ cpu->cfg.div_zero_exception ||
+ cpu->cfg.unaligned_exceptions;
+
+ static const char *reg_types[32] = {
+ [1] = "data_ptr",
+ [14] = "code_ptr",
+ [15] = "code_ptr",
+ [16] = "code_ptr",
+ [17] = "code_ptr"
+ };
+
+ for (i = 0; i < 32; ++i) {
+ type = reg_types[i];
+ /* r17 only has a code_ptr tag if we have HW exceptions */
+ if (i == 17 && !has_hw_exception) {
+ type = NULL;
+ }
+
+ sprintf(reg_name, "r%d", i);
+ mb_gen_xml_reg_tag(cpu, s, reg_name, 32, type);
+ }
+}
+
+static void mb_gen_xml_sreg_tags(const MicroBlazeCPU *cpu, GString *s)
+{
+ uint8_t i;
+
+ static const char *sreg_names[] = {
+ "rpc",
+ "rmsr",
+ "rear",
+ "resr",
+ "rfsr",
+ "rbtr",
+ "rpvr0",
+ "rpvr1",
+ "rpvr2",
+ "rpvr3",
+ "rpvr4",
+ "rpvr5",
+ "rpvr6",
+ "rpvr7",
+ "rpvr8",
+ "rpvr9",
+ "rpvr10",
+ "rpvr11",
+ "redr",
+ "rpid",
+ "rzpr",
+ "rtlblo",
+ "rtlbhi",
+ "rtlbx",
+ "rtlbsx",
+ "rslr",
+ "rshr"
+ };
+
+ static const char *sreg_types[ARRAY_SIZE(sreg_names)] = {
+ [SR_PC] = "code_ptr"
+ };
+
+ for (i = 0; i < ARRAY_SIZE(sreg_names); ++i) {
+ mb_gen_xml_reg_tag(cpu, s, sreg_names[i], mb_cpu_sreg_size(cpu, i),
+ sreg_types[i]);
+ }
+}
+
+void mb_gen_dynamic_xml(MicroBlazeCPU *cpu)
+{
+ GString *s = g_string_new(NULL);
+
+ g_string_printf(s, "<?xml version=\"1.0\"?>\n"
+ "<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">\n"
+ "<feature name=\"org.gnu.gdb.microblaze.core\">\n");
+
+ mb_gen_xml_reg_tags(cpu, s);
+ mb_gen_xml_sreg_tags(cpu, s);
+
+ g_string_append_printf(s, "</feature>");
+
+ cpu->dyn_xml.xml = g_string_free(s, false);
+}
+
+const char *mb_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
+{
+ MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
+
+ return cpu->dyn_xml.xml;
+}
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V2 2/4] target/microblaze: gdb: Extend the number of registers presented to GDB
2020-05-13 18:08 [PATCH V2 1/4] target/microblaze: gdb: Add dynamic GDB XML register support Joe Komlodi
@ 2020-05-13 18:08 ` Joe Komlodi
2020-05-14 13:49 ` Edgar E. Iglesias
2020-05-13 18:08 ` [PATCH V2 3/4] target/microblaze: gdb: Fix incorrect SReg reporting Joe Komlodi
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Joe Komlodi @ 2020-05-13 18:08 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias
Increase the number of Microblaze registers QEMU will report when
talking to GDB.
Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
---
target/microblaze/cpu.c | 2 +-
target/microblaze/gdbstub.c | 52 ++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 41cac1b..5b6ad5b 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -331,7 +331,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 + 5;
+ cc->gdb_num_core_regs = 32 + 27;
cc->gdb_get_dynamic_xml = mb_gdb_get_dynamic_xml;
cc->gdb_core_xml_file = "microblaze-core.xml";
diff --git a/target/microblaze/gdbstub.c b/target/microblaze/gdbstub.c
index cdca434..af29f00 100644
--- a/target/microblaze/gdbstub.c
+++ b/target/microblaze/gdbstub.c
@@ -26,12 +26,37 @@ int mb_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
CPUMBState *env = &cpu->env;
+ /*
+ * GDB expects registers to be reported in this order:
+ * R0-R31
+ * PC-BTR
+ * PVR0-PVR11
+ * EDR-TLBHI
+ * SLR-SHR
+ */
if (n < 32) {
return gdb_get_reg32(mem_buf, env->regs[n]);
} else {
- return gdb_get_reg32(mem_buf, env->sregs[n - 32]);
+ n -= 32;
+ switch (n) {
+ case 0 ... 5:
+ return gdb_get_reg32(mem_buf, env->sregs[n]);
+ /* PVR12 is intentionally skipped */
+ case 6 ... 17:
+ n -= 6;
+ return gdb_get_reg32(mem_buf, env->pvr.regs[n]);
+ case 18 ... 24:
+ /* Add an offset of 6 to resume where we left off with SRegs */
+ n = n - 18 + 6;
+ return gdb_get_reg32(mem_buf, env->sregs[n]);
+ case 25:
+ return gdb_get_reg32(mem_buf, env->slr);
+ case 26:
+ return gdb_get_reg32(mem_buf, env->shr);
+ default:
+ return 0;
+ }
}
- return 0;
}
int mb_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
@@ -50,7 +75,28 @@ int mb_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
if (n < 32) {
env->regs[n] = tmp;
} else {
- env->sregs[n - 32] = tmp;
+ n -= 32;
+ switch (n) {
+ case 0 ... 5:
+ env->sregs[n] = tmp;
+ break;
+ /* PVR12 is intentionally skipped */
+ case 6 ... 17:
+ n -= 6;
+ env->pvr.regs[n] = tmp;
+ break;
+ case 18 ... 24:
+ /* Add an offset of 6 to resume where we left off with SRegs */
+ n = n - 18 + 6;
+ env->sregs[n] = tmp;
+ break;
+ case 25:
+ env->slr = tmp;
+ break;
+ case 26:
+ env->shr = tmp;
+ break;
+ }
}
return 4;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V2 3/4] target/microblaze: gdb: Fix incorrect SReg reporting
2020-05-13 18:08 [PATCH V2 1/4] target/microblaze: gdb: Add dynamic GDB XML register support Joe Komlodi
2020-05-13 18:08 ` [PATCH V2 2/4] target/microblaze: gdb: Extend the number of registers presented to GDB Joe Komlodi
@ 2020-05-13 18:08 ` Joe Komlodi
2020-05-14 13:50 ` Edgar E. Iglesias
2020-05-13 18:08 ` [PATCH V2 4/4] target/microblaze: monitor: Increase the number of registers reported Joe Komlodi
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Joe Komlodi @ 2020-05-13 18:08 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias
SRegs used to be reported to GDB by iterating over the SRegs array,
however we do not store them in an order that allows them to be
reported to GDB in that way.
To fix this, a simple map is used to map the register GDB wants to its
location in the SRegs array.
Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
---
target/microblaze/gdbstub.c | 59 +++++++++++++++++++++++++++++++++++++--------
1 file changed, 49 insertions(+), 10 deletions(-)
diff --git a/target/microblaze/gdbstub.c b/target/microblaze/gdbstub.c
index af29f00..485b717 100644
--- a/target/microblaze/gdbstub.c
+++ b/target/microblaze/gdbstub.c
@@ -25,6 +25,21 @@ int mb_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
{
MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
CPUMBState *env = &cpu->env;
+ /*
+ * GDB expects SREGs in the following order:
+ * PC, MSR, EAR, ESR, FSR, BTR, EDR, PID, ZPR, TLBX, TLBSX, TLBLO, TLBHI.
+ * They aren't stored in this order, so make a map.
+ * PID, ZPR, TLBx, TLBsx, TLBLO, and TLBHI aren't modeled, so we don't
+ * map them to anything and return a value of 0 instead.
+ */
+ static const uint8_t sreg_map[6] = {
+ SR_PC,
+ SR_MSR,
+ SR_EAR,
+ SR_ESR,
+ SR_FSR,
+ SR_BTR
+ };
/*
* GDB expects registers to be reported in this order:
@@ -40,15 +55,16 @@ int mb_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
n -= 32;
switch (n) {
case 0 ... 5:
- return gdb_get_reg32(mem_buf, env->sregs[n]);
+ return gdb_get_reg32(mem_buf, env->sregs[sreg_map[n]]);
/* PVR12 is intentionally skipped */
case 6 ... 17:
n -= 6;
return gdb_get_reg32(mem_buf, env->pvr.regs[n]);
- case 18 ... 24:
- /* Add an offset of 6 to resume where we left off with SRegs */
- n = n - 18 + 6;
- return gdb_get_reg32(mem_buf, env->sregs[n]);
+ case 18:
+ return gdb_get_reg32(mem_buf, env->sregs[SR_EDR]);
+ /* Other SRegs aren't modeled, so report a value of 0 */
+ case 19 ... 24:
+ return gdb_get_reg32(mem_buf, 0);
case 25:
return gdb_get_reg32(mem_buf, env->slr);
case 26:
@@ -66,29 +82,52 @@ int mb_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
CPUMBState *env = &cpu->env;
uint32_t tmp;
+ /*
+ * GDB expects SREGs in the following order:
+ * PC, MSR, EAR, ESR, FSR, BTR, EDR, PID, ZPR, TLBX, TLBSX, TLBLO, TLBHI.
+ * They aren't stored in this order, so make a map.
+ * PID, ZPR, TLBx, TLBsx, TLBLO, and TLBHI aren't modeled, so we don't
+ * map them to anything.
+ */
+ static const uint8_t sreg_map[6] = {
+ SR_PC,
+ SR_MSR,
+ SR_EAR,
+ SR_ESR,
+ SR_FSR,
+ SR_BTR
+ };
+
if (n > cc->gdb_num_core_regs) {
return 0;
}
tmp = ldl_p(mem_buf);
+ /*
+ * GDB expects registers to be reported in this order:
+ * R0-R31
+ * PC-BTR
+ * PVR0-PVR11
+ * EDR-TLBHI
+ * SLR-SHR
+ */
if (n < 32) {
env->regs[n] = tmp;
} else {
n -= 32;
switch (n) {
case 0 ... 5:
- env->sregs[n] = tmp;
+ env->sregs[sreg_map[n]] = tmp;
break;
/* PVR12 is intentionally skipped */
case 6 ... 17:
n -= 6;
env->pvr.regs[n] = tmp;
break;
- case 18 ... 24:
- /* Add an offset of 6 to resume where we left off with SRegs */
- n = n - 18 + 6;
- env->sregs[n] = tmp;
+ /* Only EDR is modeled in these indeces, so ignore the rest */
+ case 18:
+ env->sregs[SR_EDR] = tmp;
break;
case 25:
env->slr = tmp;
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V2 4/4] target/microblaze: monitor: Increase the number of registers reported
2020-05-13 18:08 [PATCH V2 1/4] target/microblaze: gdb: Add dynamic GDB XML register support Joe Komlodi
2020-05-13 18:08 ` [PATCH V2 2/4] target/microblaze: gdb: Extend the number of registers presented to GDB Joe Komlodi
2020-05-13 18:08 ` [PATCH V2 3/4] target/microblaze: gdb: Fix incorrect SReg reporting Joe Komlodi
@ 2020-05-13 18:08 ` Joe Komlodi
2020-05-14 13:50 ` Edgar E. Iglesias
2020-05-13 18:08 ` [PATCH V2 0/4] target/microblaze: Add GDB XML and correct SReg reporting Joe Komlodi
2020-05-14 13:41 ` [PATCH V2 1/4] target/microblaze: gdb: Add dynamic GDB XML register support Edgar E. Iglesias
4 siblings, 1 reply; 11+ messages in thread
From: Joe Komlodi @ 2020-05-13 18:08 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias
Increase the number of registers reported to match GDB.
Registers that aren't modeled are reported as 0.
Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
---
target/microblaze/translate.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 20b7427..4e7f903a 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1788,9 +1788,11 @@ void mb_cpu_dump_state(CPUState *cs, FILE *f, int flags)
qemu_fprintf(f, "IN: PC=%" PRIx64 " %s\n",
env->sregs[SR_PC], lookup_symbol(env->sregs[SR_PC]));
qemu_fprintf(f, "rmsr=%" PRIx64 " resr=%" PRIx64 " rear=%" PRIx64 " "
- "debug=%x imm=%x iflags=%x fsr=%" PRIx64 "\n",
+ "debug=%x imm=%x iflags=%x fsr=%" PRIx64 " "
+ "rbtr=%" PRIx64 "\n",
env->sregs[SR_MSR], env->sregs[SR_ESR], env->sregs[SR_EAR],
- env->debug, env->imm, env->iflags, env->sregs[SR_FSR]);
+ env->debug, env->imm, env->iflags, env->sregs[SR_FSR],
+ env->sregs[SR_BTR]);
qemu_fprintf(f, "btaken=%d btarget=%" PRIx64 " mode=%s(saved=%s) "
"eip=%d ie=%d\n",
env->btaken, env->btarget,
@@ -1798,7 +1800,17 @@ void mb_cpu_dump_state(CPUState *cs, FILE *f, int flags)
(env->sregs[SR_MSR] & MSR_UMS) ? "user" : "kernel",
(bool)(env->sregs[SR_MSR] & MSR_EIP),
(bool)(env->sregs[SR_MSR] & MSR_IE));
+ for (i = 0; i < 12; i++) {
+ qemu_fprintf(f, "rpvr%2.2d=%8.8x ", i, env->pvr.regs[i]);
+ if ((i + 1) % 4 == 0) {
+ qemu_fprintf(f, "\n");
+ }
+ }
+ /* Registers that aren't modeled are reported as 0 */
+ qemu_fprintf(f, "redr=%" PRIx64 " rpid=0 rzpr=0 rtlbx=0 rtlbsx=0 "
+ "rtlblo=0 rtlbhi=0\n", env->sregs[SR_EDR]);
+ qemu_fprintf(f, "slr=%x shr=%x\n", env->slr, env->shr);
for (i = 0; i < 32; i++) {
qemu_fprintf(f, "r%2.2d=%8.8x ", i, env->regs[i]);
if ((i + 1) % 4 == 0)
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V2 0/4] target/microblaze: Add GDB XML and correct SReg reporting
2020-05-13 18:08 [PATCH V2 1/4] target/microblaze: gdb: Add dynamic GDB XML register support Joe Komlodi
` (2 preceding siblings ...)
2020-05-13 18:08 ` [PATCH V2 4/4] target/microblaze: monitor: Increase the number of registers reported Joe Komlodi
@ 2020-05-13 18:08 ` Joe Komlodi
2020-05-14 13:52 ` Edgar E. Iglesias
2020-05-14 13:41 ` [PATCH V2 1/4] target/microblaze: gdb: Add dynamic GDB XML register support Edgar E. Iglesias
4 siblings, 1 reply; 11+ messages in thread
From: Joe Komlodi @ 2020-05-13 18:08 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias
Hi all,
This series adds dynamic GDB XML support for Micraoblaze CPUs, and
fixes an issue when reporting Microblaze SRegs through GDB.
The SRegs used to be printed out by iterating over the SReg array, but
the SReg array isn't laid out in memory in the same order that GDB expects them.
When reporting register to GDB, note that even though 32-bit
Microblaze supports having certain registers wider than 32-bits, we're
repoting all of them as being 32-bits wide right now to maintain compatibility with GDB.
Thanks!
Joe
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Changelog:
v1 -> v2
- 1/4: Added missing core XML file
Joe Komlodi (4):
target/microblaze: gdb: Add dynamic GDB XML register support
target/microblaze: gdb: Extend the number of registers presented to
GDB
target/microblaze: gdb: Fix incorrect SReg reporting
target/microblaze: monitor: Increase the number of registers reported
configure | 1 +
gdb-xml/microblaze-core.xml | 64 +++++++++++++
target/microblaze/cpu.c | 6 +-
target/microblaze/cpu.h | 9 ++
target/microblaze/gdbstub.c | 214 +++++++++++++++++++++++++++++++++++++++++-
target/microblaze/translate.c | 16 +++-
6 files changed, 304 insertions(+), 6 deletions(-)
create mode 100644 gdb-xml/microblaze-core.xml
--
2.7.4
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 1/4] target/microblaze: gdb: Add dynamic GDB XML register support
2020-05-13 18:08 [PATCH V2 1/4] target/microblaze: gdb: Add dynamic GDB XML register support Joe Komlodi
` (3 preceding siblings ...)
2020-05-13 18:08 ` [PATCH V2 0/4] target/microblaze: Add GDB XML and correct SReg reporting Joe Komlodi
@ 2020-05-14 13:41 ` Edgar E. Iglesias
2020-05-14 17:05 ` Joe Komlodi
4 siblings, 1 reply; 11+ messages in thread
From: Edgar E. Iglesias @ 2020-05-14 13:41 UTC (permalink / raw)
To: Joe Komlodi; +Cc: qemu-devel
On Wed, May 13, 2020 at 11:08:45AM -0700, Joe Komlodi wrote:
> Add dynamic GDB register XML for Microblaze, and modify the config file to
> use XML when building for Microblaze.
> For the dynamic XML to be read, there still needs to be a core XML file.
Hi Joe,
I was looking a little closer at this and got a bit confused with
this approach.
So we're adding microblaze-core.xml but we're actually at runtime
dynamically generating and providing the contents for it. So the static
builtin file does not get used.
We should do either (not both):
1. Keep the dynamic generation of the XML file and remove the addintion
of gdb_xml_files= and microblaze-core.xml.
or
2. Keep the addition of static microblaze-core.xml and remove the dynamic
generation of it.
Since we're not yet using the dynamic aspects for anything relevant (only
r17 code_ptr) my preference would be to use the static files for now.
Also, it's probably a good idea to move this patch to after the patches
that fix the register ordering.
A few more comments inline.
>
> Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
> ---
> configure | 1 +
> gdb-xml/microblaze-core.xml | 64 +++++++++++++++++++++++
> target/microblaze/cpu.c | 4 ++
> target/microblaze/cpu.h | 9 ++++
> target/microblaze/gdbstub.c | 123 ++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 201 insertions(+)
> create mode 100644 gdb-xml/microblaze-core.xml
>
> diff --git a/configure b/configure
> index 0d69c36..5a099b6 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"
> 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..13e2c08
> --- /dev/null
> +++ b/gdb-xml/microblaze-core.xml
> @@ -0,0 +1,64 @@
> +<?xml version="1.0"?>
> +
> +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> +<feature name="org.gnu.gdb.microblaze.core">
> + <reg name="r0" bitsize="32"/>
> + <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"/>
> + <reg name="rslr" bitsize="32"/>
> + <reg name="rshr" bitsize="32"/>
This last part doesn't look right.
slr and shr are optional and should only be presented when the core
supports stack protection.
I think it would be easier if we simply copied both these files
from GDB:
https://github.com/bminor/binutils-gdb/blob/master/gdb/features/microblaze-core.xml
https://github.com/bminor/binutils-gdb/blob/master/gdb/features/microblaze-stack-protect.xml
Add both to gdb_xml_files= and register the stack protect XML file with
gdb_register_coprocessor() if stack protection is enabled.
> +</feature>
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index aa99830..41cac1b 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -226,6 +226,8 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
> env->pvr.regs[11] = (cpu->cfg.use_mmu ? PVR11_USE_MMU : 0) |
> 16 << 17;
>
> + mb_gen_dynamic_xml(cpu);
> +
> mcc->parent_realize(dev, errp);
> }
>
> @@ -330,6 +332,8 @@ 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 + 5;
> + cc->gdb_get_dynamic_xml = mb_gdb_get_dynamic_xml;
> + cc->gdb_core_xml_file = "microblaze-core.xml";
>
> cc->disas_set_info = mb_disas_set_info;
> cc->tcg_initialize = mb_tcg_init;
> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> index a31134b..074a18e 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -25,6 +25,8 @@
> #include "fpu/softfloat-types.h"
>
> typedef struct CPUMBState CPUMBState;
> +typedef struct DynamicMBGDBXMLInfo DynamicMBGDBXMLInfo;
> +
> #if !defined(CONFIG_USER_ONLY)
> #include "mmu.h"
> #endif
> @@ -272,6 +274,10 @@ struct CPUMBState {
> } pvr;
> };
>
> +struct DynamicMBGDBXMLInfo {
> + char *xml;
> +};
> +
> /**
> * MicroBlazeCPU:
> * @env: #CPUMBState
> @@ -286,6 +292,7 @@ struct MicroBlazeCPU {
>
> CPUNegativeOffsetState neg;
> CPUMBState env;
> + DynamicMBGDBXMLInfo dyn_xml;
>
> /* Microblaze Configuration Settings */
> struct {
> @@ -321,6 +328,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);
> +void mb_gen_dynamic_xml(MicroBlazeCPU *cpu);
> +const char *mb_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname);
>
> void mb_tcg_init(void);
> /* you can call this signal handler from your SIGBUS and SIGSEGV
> diff --git a/target/microblaze/gdbstub.c b/target/microblaze/gdbstub.c
> index f41ebf1..cdca434 100644
> --- a/target/microblaze/gdbstub.c
> +++ b/target/microblaze/gdbstub.c
> @@ -54,3 +54,126 @@ int mb_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> }
> return 4;
> }
> +
> +static void mb_gen_xml_reg_tag(const MicroBlazeCPU *cpu, GString *s,
> + const char *name, uint8_t bitsize,
> + const char *type)
> +{
> + g_string_append_printf(s, "<reg name=\"%s\" bitsize=\"%d\"",
> + name, bitsize);
> + if (type) {
> + g_string_append_printf(s, " type=\"%s\"", type);
> + }
> + g_string_append_printf(s, "/>\n");
> +}
> +
> +static uint8_t mb_cpu_sreg_size(const MicroBlazeCPU *cpu, uint8_t index)
> +{
> + /*
> + * NOTE: mb-gdb will refuse to connect if we say registers are
> + * larger then 32-bits.
> + * For now, say none of our registers are dynamically sized, and are
> + * therefore only 32-bits.
> + */
> +
> + return 32;
> +}
> +
> +static void mb_gen_xml_reg_tags(const MicroBlazeCPU *cpu, GString *s)
> +{
> + uint8_t i;
> + const char *type;
> + char reg_name[4];
> + bool has_hw_exception = cpu->cfg.dopb_bus_exception ||
> + cpu->cfg.iopb_bus_exception ||
> + cpu->cfg.illegal_opcode_exception ||
> + cpu->cfg.opcode_0_illegal ||
> + cpu->cfg.div_zero_exception ||
> + cpu->cfg.unaligned_exceptions;
> +
> + static const char *reg_types[32] = {
> + [1] = "data_ptr",
> + [14] = "code_ptr",
> + [15] = "code_ptr",
> + [16] = "code_ptr",
> + [17] = "code_ptr"
> + };
> +
> + for (i = 0; i < 32; ++i) {
> + type = reg_types[i];
> + /* r17 only has a code_ptr tag if we have HW exceptions */
> + if (i == 17 && !has_hw_exception) {
> + type = NULL;
> + }
> +
> + sprintf(reg_name, "r%d", i);
> + mb_gen_xml_reg_tag(cpu, s, reg_name, 32, type);
> + }
> +}
> +
> +static void mb_gen_xml_sreg_tags(const MicroBlazeCPU *cpu, GString *s)
> +{
> + uint8_t i;
> +
> + static const char *sreg_names[] = {
> + "rpc",
> + "rmsr",
> + "rear",
> + "resr",
> + "rfsr",
> + "rbtr",
> + "rpvr0",
> + "rpvr1",
> + "rpvr2",
> + "rpvr3",
> + "rpvr4",
> + "rpvr5",
> + "rpvr6",
> + "rpvr7",
> + "rpvr8",
> + "rpvr9",
> + "rpvr10",
> + "rpvr11",
> + "redr",
> + "rpid",
> + "rzpr",
> + "rtlblo",
> + "rtlbhi",
> + "rtlbx",
> + "rtlbsx",
In case we decide to keep this dynamic approach, tlbx and tlbsx should be
before tlblo and tlbhi.
> + "rslr",
> + "rshr"
These need to be optional and in a separate XML description with
org.gnu.gdb.microblaze.stack-protect.
> + };
> +
> + static const char *sreg_types[ARRAY_SIZE(sreg_names)] = {
> + [SR_PC] = "code_ptr"
> + };
> +
> + for (i = 0; i < ARRAY_SIZE(sreg_names); ++i) {
> + mb_gen_xml_reg_tag(cpu, s, sreg_names[i], mb_cpu_sreg_size(cpu, i),
> + sreg_types[i]);
> + }
> +}
> +
> +void mb_gen_dynamic_xml(MicroBlazeCPU *cpu)
> +{
> + GString *s = g_string_new(NULL);
> +
> + g_string_printf(s, "<?xml version=\"1.0\"?>\n"
> + "<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">\n"
> + "<feature name=\"org.gnu.gdb.microblaze.core\">\n");
> +
> + mb_gen_xml_reg_tags(cpu, s);
> + mb_gen_xml_sreg_tags(cpu, s);
> +
> + g_string_append_printf(s, "</feature>");
> +
> + cpu->dyn_xml.xml = g_string_free(s, false);
> +}
> +
> +const char *mb_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
> +{
> + MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
> +
> + return cpu->dyn_xml.xml;
> +}
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 2/4] target/microblaze: gdb: Extend the number of registers presented to GDB
2020-05-13 18:08 ` [PATCH V2 2/4] target/microblaze: gdb: Extend the number of registers presented to GDB Joe Komlodi
@ 2020-05-14 13:49 ` Edgar E. Iglesias
0 siblings, 0 replies; 11+ messages in thread
From: Edgar E. Iglesias @ 2020-05-14 13:49 UTC (permalink / raw)
To: Joe Komlodi; +Cc: qemu-devel
On Wed, May 13, 2020 at 11:08:46AM -0700, Joe Komlodi wrote:
> Increase the number of Microblaze registers QEMU will report when
> talking to GDB.
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
> ---
> target/microblaze/cpu.c | 2 +-
> target/microblaze/gdbstub.c | 52 ++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index 41cac1b..5b6ad5b 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -331,7 +331,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 + 5;
> + cc->gdb_num_core_regs = 32 + 27;
> cc->gdb_get_dynamic_xml = mb_gdb_get_dynamic_xml;
> cc->gdb_core_xml_file = "microblaze-core.xml";
>
> diff --git a/target/microblaze/gdbstub.c b/target/microblaze/gdbstub.c
> index cdca434..af29f00 100644
> --- a/target/microblaze/gdbstub.c
> +++ b/target/microblaze/gdbstub.c
> @@ -26,12 +26,37 @@ int mb_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
> MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
> CPUMBState *env = &cpu->env;
>
> + /*
> + * GDB expects registers to be reported in this order:
> + * R0-R31
> + * PC-BTR
> + * PVR0-PVR11
> + * EDR-TLBHI
> + * SLR-SHR
> + */
> if (n < 32) {
> return gdb_get_reg32(mem_buf, env->regs[n]);
> } else {
> - return gdb_get_reg32(mem_buf, env->sregs[n - 32]);
> + n -= 32;
> + switch (n) {
> + case 0 ... 5:
> + return gdb_get_reg32(mem_buf, env->sregs[n]);
> + /* PVR12 is intentionally skipped */
> + case 6 ... 17:
> + n -= 6;
> + return gdb_get_reg32(mem_buf, env->pvr.regs[n]);
> + case 18 ... 24:
> + /* Add an offset of 6 to resume where we left off with SRegs */
> + n = n - 18 + 6;
> + return gdb_get_reg32(mem_buf, env->sregs[n]);
> + case 25:
> + return gdb_get_reg32(mem_buf, env->slr);
> + case 26:
> + return gdb_get_reg32(mem_buf, env->shr);
> + default:
> + return 0;
> + }
> }
> - return 0;
> }
>
> int mb_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> @@ -50,7 +75,28 @@ int mb_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> if (n < 32) {
> env->regs[n] = tmp;
> } else {
> - env->sregs[n - 32] = tmp;
> + n -= 32;
> + switch (n) {
> + case 0 ... 5:
> + env->sregs[n] = tmp;
> + break;
> + /* PVR12 is intentionally skipped */
> + case 6 ... 17:
> + n -= 6;
> + env->pvr.regs[n] = tmp;
> + break;
> + case 18 ... 24:
> + /* Add an offset of 6 to resume where we left off with SRegs */
> + n = n - 18 + 6;
> + env->sregs[n] = tmp;
> + break;
> + case 25:
> + env->slr = tmp;
> + break;
> + case 26:
> + env->shr = tmp;
> + break;
> + }
> }
> return 4;
> }
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 3/4] target/microblaze: gdb: Fix incorrect SReg reporting
2020-05-13 18:08 ` [PATCH V2 3/4] target/microblaze: gdb: Fix incorrect SReg reporting Joe Komlodi
@ 2020-05-14 13:50 ` Edgar E. Iglesias
0 siblings, 0 replies; 11+ messages in thread
From: Edgar E. Iglesias @ 2020-05-14 13:50 UTC (permalink / raw)
To: Joe Komlodi; +Cc: qemu-devel
On Wed, May 13, 2020 at 11:08:47AM -0700, Joe Komlodi wrote:
> SRegs used to be reported to GDB by iterating over the SRegs array,
> however we do not store them in an order that allows them to be
> reported to GDB in that way.
>
> To fix this, a simple map is used to map the register GDB wants to its
> location in the SRegs array.
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
> ---
> target/microblaze/gdbstub.c | 59 +++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 49 insertions(+), 10 deletions(-)
>
> diff --git a/target/microblaze/gdbstub.c b/target/microblaze/gdbstub.c
> index af29f00..485b717 100644
> --- a/target/microblaze/gdbstub.c
> +++ b/target/microblaze/gdbstub.c
> @@ -25,6 +25,21 @@ int mb_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
> {
> MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
> CPUMBState *env = &cpu->env;
> + /*
> + * GDB expects SREGs in the following order:
> + * PC, MSR, EAR, ESR, FSR, BTR, EDR, PID, ZPR, TLBX, TLBSX, TLBLO, TLBHI.
> + * They aren't stored in this order, so make a map.
> + * PID, ZPR, TLBx, TLBsx, TLBLO, and TLBHI aren't modeled, so we don't
> + * map them to anything and return a value of 0 instead.
> + */
> + static const uint8_t sreg_map[6] = {
> + SR_PC,
> + SR_MSR,
> + SR_EAR,
> + SR_ESR,
> + SR_FSR,
> + SR_BTR
> + };
>
> /*
> * GDB expects registers to be reported in this order:
> @@ -40,15 +55,16 @@ int mb_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
> n -= 32;
> switch (n) {
> case 0 ... 5:
> - return gdb_get_reg32(mem_buf, env->sregs[n]);
> + return gdb_get_reg32(mem_buf, env->sregs[sreg_map[n]]);
> /* PVR12 is intentionally skipped */
> case 6 ... 17:
> n -= 6;
> return gdb_get_reg32(mem_buf, env->pvr.regs[n]);
> - case 18 ... 24:
> - /* Add an offset of 6 to resume where we left off with SRegs */
> - n = n - 18 + 6;
> - return gdb_get_reg32(mem_buf, env->sregs[n]);
> + case 18:
> + return gdb_get_reg32(mem_buf, env->sregs[SR_EDR]);
> + /* Other SRegs aren't modeled, so report a value of 0 */
> + case 19 ... 24:
> + return gdb_get_reg32(mem_buf, 0);
> case 25:
> return gdb_get_reg32(mem_buf, env->slr);
> case 26:
> @@ -66,29 +82,52 @@ int mb_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> CPUMBState *env = &cpu->env;
> uint32_t tmp;
>
> + /*
> + * GDB expects SREGs in the following order:
> + * PC, MSR, EAR, ESR, FSR, BTR, EDR, PID, ZPR, TLBX, TLBSX, TLBLO, TLBHI.
> + * They aren't stored in this order, so make a map.
> + * PID, ZPR, TLBx, TLBsx, TLBLO, and TLBHI aren't modeled, so we don't
> + * map them to anything.
> + */
> + static const uint8_t sreg_map[6] = {
> + SR_PC,
> + SR_MSR,
> + SR_EAR,
> + SR_ESR,
> + SR_FSR,
> + SR_BTR
> + };
> +
> if (n > cc->gdb_num_core_regs) {
> return 0;
> }
>
> tmp = ldl_p(mem_buf);
>
> + /*
> + * GDB expects registers to be reported in this order:
> + * R0-R31
> + * PC-BTR
> + * PVR0-PVR11
> + * EDR-TLBHI
> + * SLR-SHR
> + */
> if (n < 32) {
> env->regs[n] = tmp;
> } else {
> n -= 32;
> switch (n) {
> case 0 ... 5:
> - env->sregs[n] = tmp;
> + env->sregs[sreg_map[n]] = tmp;
> break;
> /* PVR12 is intentionally skipped */
> case 6 ... 17:
> n -= 6;
> env->pvr.regs[n] = tmp;
> break;
> - case 18 ... 24:
> - /* Add an offset of 6 to resume where we left off with SRegs */
> - n = n - 18 + 6;
> - env->sregs[n] = tmp;
> + /* Only EDR is modeled in these indeces, so ignore the rest */
> + case 18:
> + env->sregs[SR_EDR] = tmp;
> break;
> case 25:
> env->slr = tmp;
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 4/4] target/microblaze: monitor: Increase the number of registers reported
2020-05-13 18:08 ` [PATCH V2 4/4] target/microblaze: monitor: Increase the number of registers reported Joe Komlodi
@ 2020-05-14 13:50 ` Edgar E. Iglesias
0 siblings, 0 replies; 11+ messages in thread
From: Edgar E. Iglesias @ 2020-05-14 13:50 UTC (permalink / raw)
To: Joe Komlodi; +Cc: qemu-devel
On Wed, May 13, 2020 at 11:08:48AM -0700, Joe Komlodi wrote:
> Increase the number of registers reported to match GDB.
>
> Registers that aren't modeled are reported as 0.
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
> ---
> target/microblaze/translate.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
> index 20b7427..4e7f903a 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -1788,9 +1788,11 @@ void mb_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> qemu_fprintf(f, "IN: PC=%" PRIx64 " %s\n",
> env->sregs[SR_PC], lookup_symbol(env->sregs[SR_PC]));
> qemu_fprintf(f, "rmsr=%" PRIx64 " resr=%" PRIx64 " rear=%" PRIx64 " "
> - "debug=%x imm=%x iflags=%x fsr=%" PRIx64 "\n",
> + "debug=%x imm=%x iflags=%x fsr=%" PRIx64 " "
> + "rbtr=%" PRIx64 "\n",
> env->sregs[SR_MSR], env->sregs[SR_ESR], env->sregs[SR_EAR],
> - env->debug, env->imm, env->iflags, env->sregs[SR_FSR]);
> + env->debug, env->imm, env->iflags, env->sregs[SR_FSR],
> + env->sregs[SR_BTR]);
> qemu_fprintf(f, "btaken=%d btarget=%" PRIx64 " mode=%s(saved=%s) "
> "eip=%d ie=%d\n",
> env->btaken, env->btarget,
> @@ -1798,7 +1800,17 @@ void mb_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> (env->sregs[SR_MSR] & MSR_UMS) ? "user" : "kernel",
> (bool)(env->sregs[SR_MSR] & MSR_EIP),
> (bool)(env->sregs[SR_MSR] & MSR_IE));
> + for (i = 0; i < 12; i++) {
> + qemu_fprintf(f, "rpvr%2.2d=%8.8x ", i, env->pvr.regs[i]);
> + if ((i + 1) % 4 == 0) {
> + qemu_fprintf(f, "\n");
> + }
> + }
>
> + /* Registers that aren't modeled are reported as 0 */
> + qemu_fprintf(f, "redr=%" PRIx64 " rpid=0 rzpr=0 rtlbx=0 rtlbsx=0 "
> + "rtlblo=0 rtlbhi=0\n", env->sregs[SR_EDR]);
> + qemu_fprintf(f, "slr=%x shr=%x\n", env->slr, env->shr);
> for (i = 0; i < 32; i++) {
> qemu_fprintf(f, "r%2.2d=%8.8x ", i, env->regs[i]);
> if ((i + 1) % 4 == 0)
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 0/4] target/microblaze: Add GDB XML and correct SReg reporting
2020-05-13 18:08 ` [PATCH V2 0/4] target/microblaze: Add GDB XML and correct SReg reporting Joe Komlodi
@ 2020-05-14 13:52 ` Edgar E. Iglesias
0 siblings, 0 replies; 11+ messages in thread
From: Edgar E. Iglesias @ 2020-05-14 13:52 UTC (permalink / raw)
To: Joe Komlodi; +Cc: qemu-devel
On Wed, May 13, 2020 at 11:08:49AM -0700, Joe Komlodi wrote:
> Hi all,
>
> This series adds dynamic GDB XML support for Micraoblaze CPUs, and
> fixes an issue when reporting Microblaze SRegs through GDB.
>
> The SRegs used to be printed out by iterating over the SReg array, but
> the SReg array isn't laid out in memory in the same order that GDB expects them.
>
> When reporting register to GDB, note that even though 32-bit
> Microblaze supports having certain registers wider than 32-bits, we're
> repoting all of them as being 32-bits wide right now to maintain compatibility with GDB.
Hi Joe,
I've taken patches #2 - 4 into my queue.
Patch #1 needs some more work.
Thanks,
Edgar
>
> Thanks!
> Joe
>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>
> Changelog:
> v1 -> v2
> - 1/4: Added missing core XML file
>
>
> Joe Komlodi (4):
> target/microblaze: gdb: Add dynamic GDB XML register support
> target/microblaze: gdb: Extend the number of registers presented to
> GDB
> target/microblaze: gdb: Fix incorrect SReg reporting
> target/microblaze: monitor: Increase the number of registers reported
>
> configure | 1 +
> gdb-xml/microblaze-core.xml | 64 +++++++++++++
> target/microblaze/cpu.c | 6 +-
> target/microblaze/cpu.h | 9 ++
> target/microblaze/gdbstub.c | 214 +++++++++++++++++++++++++++++++++++++++++-
> target/microblaze/translate.c | 16 +++-
> 6 files changed, 304 insertions(+), 6 deletions(-)
> create mode 100644 gdb-xml/microblaze-core.xml
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH V2 1/4] target/microblaze: gdb: Add dynamic GDB XML register support
2020-05-14 13:41 ` [PATCH V2 1/4] target/microblaze: gdb: Add dynamic GDB XML register support Edgar E. Iglesias
@ 2020-05-14 17:05 ` Joe Komlodi
0 siblings, 0 replies; 11+ messages in thread
From: Joe Komlodi @ 2020-05-14 17:05 UTC (permalink / raw)
To: Edgar Iglesias; +Cc: qemu-devel
Hi Edgar,
Comments marked with [Joe]
-----Original Message-----
From: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Sent: Thursday, May 14, 2020 6:41 AM
To: Joe Komlodi <komlodi@xilinx.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [PATCH V2 1/4] target/microblaze: gdb: Add dynamic GDB XML register support
On Wed, May 13, 2020 at 11:08:45AM -0700, Joe Komlodi wrote:
> Add dynamic GDB register XML for Microblaze, and modify the config
> file to use XML when building for Microblaze.
> For the dynamic XML to be read, there still needs to be a core XML file.
Hi Joe,
I was looking a little closer at this and got a bit confused with this approach.
So we're adding microblaze-core.xml but we're actually at runtime dynamically generating and providing the contents for it. So the static builtin file does not get used.
[Joe] If I recall correctly, the GDB stub wouldn't use any dynamic XML files without a static XML file present. This might have changed since then, since this was written on an older version of QEMU.
We should do either (not both):
1. Keep the dynamic generation of the XML file and remove the addintion
of gdb_xml_files= and microblaze-core.xml.
or
2. Keep the addition of static microblaze-core.xml and remove the dynamic
generation of it.
Since we're not yet using the dynamic aspects for anything relevant (only
r17 code_ptr) my preference would be to use the static files for now.
[Joe] That sounds good to me.
Also, it's probably a good idea to move this patch to after the patches that fix the register ordering.
A few more comments inline.
>
> Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
> ---
> configure | 1 +
> gdb-xml/microblaze-core.xml | 64 +++++++++++++++++++++++
> target/microblaze/cpu.c | 4 ++
> target/microblaze/cpu.h | 9 ++++
> target/microblaze/gdbstub.c | 123
> ++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 201 insertions(+)
> create mode 100644 gdb-xml/microblaze-core.xml
>
> diff --git a/configure b/configure
> index 0d69c36..5a099b6 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"
> 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..13e2c08
> --- /dev/null
> +++ b/gdb-xml/microblaze-core.xml
> @@ -0,0 +1,64 @@
> +<?xml version="1.0"?>
> +
> +<!DOCTYPE feature SYSTEM "gdb-target.dtd"> <feature
> +name="org.gnu.gdb.microblaze.core">
> + <reg name="r0" bitsize="32"/>
> + <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"/>
> + <reg name="rslr" bitsize="32"/>
> + <reg name="rshr" bitsize="32"/>
This last part doesn't look right.
slr and shr are optional and should only be presented when the core supports stack protection.
I think it would be easier if we simply copied both these files from GDB:
https://github.com/bminor/binutils-gdb/blob/master/gdb/features/microblaze-core.xml
https://github.com/bminor/binutils-gdb/blob/master/gdb/features/microblaze-stack-protect.xml
Add both to gdb_xml_files= and register the stack protect XML file with
gdb_register_coprocessor() if stack protection is enabled.
[Joe] Agreed.
> +</feature>
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c index
> aa99830..41cac1b 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -226,6 +226,8 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
> env->pvr.regs[11] = (cpu->cfg.use_mmu ? PVR11_USE_MMU : 0) |
> 16 << 17;
>
> + mb_gen_dynamic_xml(cpu);
> +
> mcc->parent_realize(dev, errp);
> }
>
> @@ -330,6 +332,8 @@ 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 + 5;
> + cc->gdb_get_dynamic_xml = mb_gdb_get_dynamic_xml;
> + cc->gdb_core_xml_file = "microblaze-core.xml";
>
> cc->disas_set_info = mb_disas_set_info;
> cc->tcg_initialize = mb_tcg_init; diff --git
> a/target/microblaze/cpu.h b/target/microblaze/cpu.h index
> a31134b..074a18e 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -25,6 +25,8 @@
> #include "fpu/softfloat-types.h"
>
> typedef struct CPUMBState CPUMBState;
> +typedef struct DynamicMBGDBXMLInfo DynamicMBGDBXMLInfo;
> +
> #if !defined(CONFIG_USER_ONLY)
> #include "mmu.h"
> #endif
> @@ -272,6 +274,10 @@ struct CPUMBState {
> } pvr;
> };
>
> +struct DynamicMBGDBXMLInfo {
> + char *xml;
> +};
> +
> /**
> * MicroBlazeCPU:
> * @env: #CPUMBState
> @@ -286,6 +292,7 @@ struct MicroBlazeCPU {
>
> CPUNegativeOffsetState neg;
> CPUMBState env;
> + DynamicMBGDBXMLInfo dyn_xml;
>
> /* Microblaze Configuration Settings */
> struct {
> @@ -321,6 +328,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);
> +void mb_gen_dynamic_xml(MicroBlazeCPU *cpu); const char
> +*mb_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname);
>
> void mb_tcg_init(void);
> /* you can call this signal handler from your SIGBUS and SIGSEGV diff
> --git a/target/microblaze/gdbstub.c b/target/microblaze/gdbstub.c
> index f41ebf1..cdca434 100644
> --- a/target/microblaze/gdbstub.c
> +++ b/target/microblaze/gdbstub.c
> @@ -54,3 +54,126 @@ int mb_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> }
> return 4;
> }
> +
> +static void mb_gen_xml_reg_tag(const MicroBlazeCPU *cpu, GString *s,
> + const char *name, uint8_t bitsize,
> + const char *type) {
> + g_string_append_printf(s, "<reg name=\"%s\" bitsize=\"%d\"",
> + name, bitsize);
> + if (type) {
> + g_string_append_printf(s, " type=\"%s\"", type);
> + }
> + g_string_append_printf(s, "/>\n"); }
> +
> +static uint8_t mb_cpu_sreg_size(const MicroBlazeCPU *cpu, uint8_t
> +index) {
> + /*
> + * NOTE: mb-gdb will refuse to connect if we say registers are
> + * larger then 32-bits.
> + * For now, say none of our registers are dynamically sized, and are
> + * therefore only 32-bits.
> + */
> +
> + return 32;
> +}
> +
> +static void mb_gen_xml_reg_tags(const MicroBlazeCPU *cpu, GString *s)
> +{
> + uint8_t i;
> + const char *type;
> + char reg_name[4];
> + bool has_hw_exception = cpu->cfg.dopb_bus_exception ||
> + cpu->cfg.iopb_bus_exception ||
> + cpu->cfg.illegal_opcode_exception ||
> + cpu->cfg.opcode_0_illegal ||
> + cpu->cfg.div_zero_exception ||
> + cpu->cfg.unaligned_exceptions;
> +
> + static const char *reg_types[32] = {
> + [1] = "data_ptr",
> + [14] = "code_ptr",
> + [15] = "code_ptr",
> + [16] = "code_ptr",
> + [17] = "code_ptr"
> + };
> +
> + for (i = 0; i < 32; ++i) {
> + type = reg_types[i];
> + /* r17 only has a code_ptr tag if we have HW exceptions */
> + if (i == 17 && !has_hw_exception) {
> + type = NULL;
> + }
> +
> + sprintf(reg_name, "r%d", i);
> + mb_gen_xml_reg_tag(cpu, s, reg_name, 32, type);
> + }
> +}
> +
> +static void mb_gen_xml_sreg_tags(const MicroBlazeCPU *cpu, GString
> +*s) {
> + uint8_t i;
> +
> + static const char *sreg_names[] = {
> + "rpc",
> + "rmsr",
> + "rear",
> + "resr",
> + "rfsr",
> + "rbtr",
> + "rpvr0",
> + "rpvr1",
> + "rpvr2",
> + "rpvr3",
> + "rpvr4",
> + "rpvr5",
> + "rpvr6",
> + "rpvr7",
> + "rpvr8",
> + "rpvr9",
> + "rpvr10",
> + "rpvr11",
> + "redr",
> + "rpid",
> + "rzpr",
> + "rtlblo",
> + "rtlbhi",
> + "rtlbx",
> + "rtlbsx",
In case we decide to keep this dynamic approach, tlbx and tlbsx should be before tlblo and tlbhi.
> + "rslr",
> + "rshr"
These need to be optional and in a separate XML description with org.gnu.gdb.microblaze.stack-protect.
> + };
> +
> + static const char *sreg_types[ARRAY_SIZE(sreg_names)] = {
> + [SR_PC] = "code_ptr"
> + };
> +
> + for (i = 0; i < ARRAY_SIZE(sreg_names); ++i) {
> + mb_gen_xml_reg_tag(cpu, s, sreg_names[i], mb_cpu_sreg_size(cpu, i),
> + sreg_types[i]);
> + }
> +}
> +
> +void mb_gen_dynamic_xml(MicroBlazeCPU *cpu) {
> + GString *s = g_string_new(NULL);
> +
> + g_string_printf(s, "<?xml version=\"1.0\"?>\n"
> + "<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">\n"
> + "<feature
> + name=\"org.gnu.gdb.microblaze.core\">\n");
> +
> + mb_gen_xml_reg_tags(cpu, s);
> + mb_gen_xml_sreg_tags(cpu, s);
> +
> + g_string_append_printf(s, "</feature>");
> +
> + cpu->dyn_xml.xml = g_string_free(s, false); }
> +
> +const char *mb_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
> +{
> + MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
> +
> + return cpu->dyn_xml.xml;
> +}
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-05-14 17:06 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-13 18:08 [PATCH V2 1/4] target/microblaze: gdb: Add dynamic GDB XML register support Joe Komlodi
2020-05-13 18:08 ` [PATCH V2 2/4] target/microblaze: gdb: Extend the number of registers presented to GDB Joe Komlodi
2020-05-14 13:49 ` Edgar E. Iglesias
2020-05-13 18:08 ` [PATCH V2 3/4] target/microblaze: gdb: Fix incorrect SReg reporting Joe Komlodi
2020-05-14 13:50 ` Edgar E. Iglesias
2020-05-13 18:08 ` [PATCH V2 4/4] target/microblaze: monitor: Increase the number of registers reported Joe Komlodi
2020-05-14 13:50 ` Edgar E. Iglesias
2020-05-13 18:08 ` [PATCH V2 0/4] target/microblaze: Add GDB XML and correct SReg reporting Joe Komlodi
2020-05-14 13:52 ` Edgar E. Iglesias
2020-05-14 13:41 ` [PATCH V2 1/4] target/microblaze: gdb: Add dynamic GDB XML register support Edgar E. Iglesias
2020-05-14 17:05 ` Joe Komlodi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.