arm: gdb-stub is broken by FEAT_HAFDBS
Hello, Richard,
We just noticed the gdb-stub is broken and probably caused by commit 4a3585568
("target/arm: Plumb debug into S1Translate").
(gdb) target remote :1234
Remote debugging using :1234
0x0e1716d0 in ?? ()
=> 0x0e1716d0: Cannot access memory at address 0xe1716d0
This issue can be workaround by below change.
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -2879,7 +2879,7 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs,
vaddr addr,
S1Translate ptw = {
.in_mmu_idx = arm_mmu_idx(env),
.in_secure = arm_is_secure(env),
-.in_debug = true,
+.in_debug = false,
};
Could you take a look at this? Thank you!
--
Cheers,
Changbin Du
Re: [PATCH 0/3] gdbstub: add support for switchable endianness
On Tue, Aug 24, 2021 at 10:11:14AM +0100, Peter Maydell wrote: > On Tue, 24 Aug 2021 at 00:05, Changbin Du wrote: > > > > On Mon, Aug 23, 2021 at 04:30:05PM +0100, Peter Maydell wrote: > > > changes to be more capable of handling dynamic target changes > > > (this would also help with eg debugging across 32<->64 bit switches); > > > as I understand it that gdb work would be pretty significant, > > > and at least for aarch64 pretty much nobody cares about > > > big-endian, so nobody's got round to doing it yet. > > > > > Mostly we do not care dynamic target changes because nearly all OS will > > setup > > endianness mode by its first instruction. And dynamic changes for gdb is > > hard > > since the byte order of debugging info in elf is fixed. And currently the > > GDB > > remote protocol does not support querying endianness info from remote. > > > > So usually we needn't change byte order during a debug session, but we just > > want > > the qemu gdbstub can send data in and handle data it received in right byte > > order. > > This patch does this work with the help of users via the option > > 'endianness='. > > I'm not a huge fan of putting in workarounds that deal with the > problem for specific cases and require users to tweak options settings, > rather than solving the problem in a more general way that would > let it all Just Work for all cases. > Probably we can add a new callback 'gdb_get_endianness' for CPUClass, and use this callback to determine if bswap is needed every time we read/write cpu registers. What's your thought? > -- PMM -- Cheers, Changbin Du
Re: [PATCH 0/3] gdbstub: add support for switchable endianness
On Mon, Aug 23, 2021 at 04:30:05PM +0100, Peter Maydell wrote: > On Mon, 23 Aug 2021 at 16:21, Philippe Mathieu-Daudé > wrote: > > > > On 8/23/21 4:20 PM, Changbin Du wrote: > > > To resolve the issue to debug switchable targets, this serias introduces > > > basic infrastructure for gdbstub and enable support for ARM and RISC-V > > > targets. > > > > > > For example, now there is no problem to debug an big-enadian aarch64 > > > target > > > on x86 host. > > > > > > $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ... > > > > I don't understand why you need all that. > > Maybe you aren't using gdb-multiarch? > > > > You can install it or start it via QEMU Debian Docker image: > > > > $ docker run -it --rm -v /tmp:/tmp -u $UID --network=host \ > > registry.gitlab.com/qemu-project/qemu/qemu/debian10 \ > > gdb-multiarch -q \ > > --ex 'set architecture aarch64' \ > > --ex 'set endian big' > > The target architecture is assumed to be aarch64 > > The target is assumed to be big endian > > (gdb) target remote 172.17.0.1:1234 > > I don't think that will help, because an AArch64 CPU (at least > in the boards we model) will always start up in little-endian, > and our gdbstub will always transfer register data etc in > little-endian order, because gdb cannot cope with a target that > isn't always the same endianness. Fixing this requires gdb Yes, that's the problem. > changes to be more capable of handling dynamic target changes > (this would also help with eg debugging across 32<->64 bit switches); > as I understand it that gdb work would be pretty significant, > and at least for aarch64 pretty much nobody cares about > big-endian, so nobody's got round to doing it yet. > Mostly we do not care dynamic target changes because nearly all OS will setup endianness mode by its first instruction. And dynamic changes for gdb is hard since the byte order of debugging info in elf is fixed. And currently the GDB remote protocol does not support querying endianness info from remote. So usually we needn't change byte order during a debug session, but we just want the qemu gdbstub can send data in and handle data it received in right byte order. This patch does this work with the help of users via the option 'endianness='. > Our target/ppc/gdbstub.c code takes a different tack: it > always sends register data in the same order the CPU is > currently in, which has a different set of cases when it > goes wrong. > Yes, I tried to do this before. But as I said above GDB unable to handle dynamic target changing. Maybe we can take this way as 'endianness=default'? Anyway, this requires each target provides a interface to determine the current byte order. > thanks > -- PMM -- Cheers, Changbin Du
Re: [PATCH 0/3] gdbstub: add support for switchable endianness
On Mon, Aug 23, 2021 at 05:21:07PM +0200, Philippe Mathieu-Daudé wrote: > On 8/23/21 4:20 PM, Changbin Du wrote: > > To resolve the issue to debug switchable targets, this serias introduces > > basic infrastructure for gdbstub and enable support for ARM and RISC-V > > targets. > > > > For example, now there is no problem to debug an big-enadian aarch64 target > > on x86 host. > > > > $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ... > > I don't understand why you need all that. > Maybe you aren't using gdb-multiarch? > Nope, my gdb support all architectures. > You can install it or start it via QEMU Debian Docker image: > > $ docker run -it --rm -v /tmp:/tmp -u $UID --network=host \ > registry.gitlab.com/qemu-project/qemu/qemu/debian10 \ > gdb-multiarch -q \ > --ex 'set architecture aarch64' \ > --ex 'set endian big' > The target architecture is assumed to be aarch64 > The target is assumed to be big endian > (gdb) target remote 172.17.0.1:1234 > (gdb) > The gdb has no problem to read endianness and arch info from elf. The problem is how qemu gdbstub handles the byte order it received. Now let's try to debug a big-enadian aarch64 linux kernel. 1) start qemu with '-gdb tcp::1234' $ gdb-multiarch vmlinux (gdb) target remote :1234 Remote debugging using :1234 0x0040 in ?? () => 0x0040: Cannot access memory at address 0x40 (gdb) ni Cannot access memory at address 0x40 (gdb) show architecture The target architecture is set to "auto" (currently "aarch64"). (gdb) show endian The target endianness is set automatically (currently big endian). You see it an't work, not to mention adding breakpoints. 2) start qemu with '-gdb tcp::1234,endianness=big' $ gdb-multiarch vmlinux (gdb) target remote :1234 Remote debugging using :1234 0x4000 in ?? () => 0x4000: c0 00 00 58 ldr x0, 0x4018 (gdb) ni 0x4004 in ?? () => 0x4004: e1 03 1f aa mov x1, xzr (gdb) b start_kernel Breakpoint 1 at 0x800011130ee8 (2 locations) (gdb) c Continuing. Thread 1 hit Breakpoint 1, 0x800011130ee8 in start_kernel () => 0x800011130ee8 : 5f 24 03 d5 bti c (gdb) bt #0 0x800011130ee8 in start_kernel () #1 0x8000111303c8 in __primary_switched () at arch/arm64/kernel/head.S:467 Backtrace stopped: previous frame identical to this frame (corrupt stack?) okay, now it works fine. -- Cheers, Changbin Du
[PATCH 2/3] arm: gdbstub: add support for switchable endianness
Apply new gdbstub interfaces we added previously to support both little
and big endian guest debugging for ARM. And enable the
TARGET_SWICHABLE_ENDIANNESS option.
Signed-off-by: Changbin Du
---
configs/targets/aarch64-softmmu.mak | 1 +
configs/targets/arm-softmmu.mak | 1 +
target/arm/gdbstub.c| 2 +-
target/arm/gdbstub64.c | 2 +-
4 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/configs/targets/aarch64-softmmu.mak
b/configs/targets/aarch64-softmmu.mak
index 7703127674..14e7f166a7 100644
--- a/configs/targets/aarch64-softmmu.mak
+++ b/configs/targets/aarch64-softmmu.mak
@@ -3,3 +3,4 @@ TARGET_BASE_ARCH=arm
TARGET_SUPPORTS_MTTCG=y
TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml
gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml
gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
TARGET_NEED_FDT=y
+TARGET_SWICHABLE_ENDIANNESS=y
diff --git a/configs/targets/arm-softmmu.mak b/configs/targets/arm-softmmu.mak
index 84a98f4818..5f40e858f9 100644
--- a/configs/targets/arm-softmmu.mak
+++ b/configs/targets/arm-softmmu.mak
@@ -2,3 +2,4 @@ TARGET_ARCH=arm
TARGET_SUPPORTS_MTTCG=y
TARGET_XML_FILES= gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml
gdb-xml/arm-vfp3.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
TARGET_NEED_FDT=y
+TARGET_SWICHABLE_ENDIANNESS=y
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 826601b341..188e82d938 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -74,7 +74,7 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t
*mem_buf, int n)
CPUARMState *env = >env;
uint32_t tmp;
-tmp = ldl_p(mem_buf);
+tmp = gdb_read_reg32(mem_buf);
/* Mask out low bit of PC to workaround gdb bugs. This will probably
cause problems if we ever implement the Jazelle DBX extensions. */
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 251539ef79..5358ad31b4 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -47,7 +47,7 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t
*mem_buf, int n)
CPUARMState *env = >env;
uint64_t tmp;
-tmp = ldq_p(mem_buf);
+tmp = gdb_read_reg64(mem_buf);
if (n < 31) {
/* Core integer register. */
--
2.32.0
[PATCH 1/3] gdbstub: add basic infrastructure to support switchable endianness
Some architectures (e.g. ARM versions 3 and above, RISC-V, PowerPC, Alpha,
MIPS, IA-64...) allow switchable endianness. Now our emulation code can
handle both endianness well, but the gdbstub can only handle one of them.
For example, it is problematic to debug a ARM big endian guest on x86 host.
This because the GDB remote protocol transfers values in target byte order
but qemu always take it as little endian on x86 host.
To support switchable endianness targets, this patch introduces:
- a new sub-option 'endianness' for '-gdb'.
- common interfaces to swap byte order according to host and target
byte order.
- a new configuration option TARGET_SWICHABLE_ENDIANNESS.
For example, to debug a arm64 big endian target, you could start qemu as
below:
$ qemu-system-aarch64 -gdb tcp::1234,endianness=big ...
Latter we will add switchable endianness support for ARM and RISC-V
targets. For other switchable targets them can be supported in future.
Signed-off-by: Changbin Du
---
gdbstub.c | 11 +++
include/exec/gdbstub.h | 72 +++---
qemu-options.hx| 7 ++--
softmmu/vl.c | 50 -
4 files changed, 119 insertions(+), 21 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 52bde5bdc9..ec67d6a299 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -62,6 +62,17 @@
static int phy_memory_mode;
#endif
+#ifdef HOST_WORDS_BIGENDIAN
+const bool gdb_host_bigendian = true;
+#else
+const bool gdb_host_bigendian = false;
+#endif
+#ifdef TARGET_WORDS_BIGENDIAN
+bool gdb_target_bigendian = true;
+#else
+bool gdb_target_bigendian = false;
+#endif
+
static inline int target_memory_rw_debug(CPUState *cpu, target_ulong addr,
uint8_t *buf, int len, bool is_write)
{
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index a024a0350d..2c6f90fc28 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -84,9 +84,17 @@ void gdb_register_coprocessor(CPUState *cpu,
gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg,
int num_regs, const char *xml, int g_pos);
+extern const bool gdb_host_bigendian;
+extern bool gdb_target_bigendian;
+
+/* The GDB remote protocol transfers values in target byte order. */
+static inline bool gdb_bswap_needed(void)
+{
+return gdb_host_bigendian != gdb_target_bigendian;
+}
+
/*
- * The GDB remote protocol transfers values in target byte order. As
- * the gdbstub may be batching up several register values we always
+ * As the gdbstub may be batching up several register values we always
* append to the array.
*/
@@ -98,21 +106,21 @@ static inline int gdb_get_reg8(GByteArray *buf, uint8_t
val)
static inline int gdb_get_reg16(GByteArray *buf, uint16_t val)
{
-uint16_t to_word = tswap16(val);
+uint16_t to_word = gdb_bswap_needed() ? bswap16(val) : val;
g_byte_array_append(buf, (uint8_t *) _word, 2);
return 2;
}
static inline int gdb_get_reg32(GByteArray *buf, uint32_t val)
{
-uint32_t to_long = tswap32(val);
+uint32_t to_long = gdb_bswap_needed() ? bswap32(val) : val;
g_byte_array_append(buf, (uint8_t *) _long, 4);
return 4;
}
static inline int gdb_get_reg64(GByteArray *buf, uint64_t val)
{
-uint64_t to_quad = tswap64(val);
+uint64_t to_quad = gdb_bswap_needed() ? bswap64(val) : val;
g_byte_array_append(buf, (uint8_t *) _quad, 8);
return 8;
}
@@ -121,17 +129,20 @@ static inline int gdb_get_reg128(GByteArray *buf,
uint64_t val_hi,
uint64_t val_lo)
{
uint64_t to_quad;
-#ifdef TARGET_WORDS_BIGENDIAN
-to_quad = tswap64(val_hi);
-g_byte_array_append(buf, (uint8_t *) _quad, 8);
-to_quad = tswap64(val_lo);
-g_byte_array_append(buf, (uint8_t *) _quad, 8);
-#else
-to_quad = tswap64(val_lo);
-g_byte_array_append(buf, (uint8_t *) _quad, 8);
-to_quad = tswap64(val_hi);
-g_byte_array_append(buf, (uint8_t *) _quad, 8);
-#endif
+
+if (gdb_bswap_needed()) {
+if (gdb_target_bigendian) {
+to_quad = bswap64(val_hi);
+g_byte_array_append(buf, (uint8_t *) _quad, 8);
+to_quad = bswap64(val_lo);
+g_byte_array_append(buf, (uint8_t *) _quad, 8);
+} else {
+to_quad = bswap64(val_lo);
+g_byte_array_append(buf, (uint8_t *) _quad, 8);
+to_quad = bswap64(val_hi);
+g_byte_array_append(buf, (uint8_t *) _quad, 8);
+}
+}
return 16;
}
@@ -157,13 +168,38 @@ static inline uint8_t * gdb_get_reg_ptr(GByteArray *buf,
int len)
return buf->data + buf->len - len;
}
+static inline uint8_t gdb_read_reg8(uint8_t *mem_buf)
+{
+return *mem_buf;
+}
+
+static inline uint16_t gdb_read_reg16(uint8_t *mem_buf)
+{
+uint16_t val = lduw_p(mem_buf);
+return gdb_bswap_needed() ? bswap16(val) : val;
+}
+
+static inline uint32_t gdb_read
[PATCH 3/3] riscv: gdbstub: add support for switchable endianness
Apply new gdbstub interfaces we added previously to support both little
and big endian guest debugging for RISC-V. And enable the
TARGET_SWICHABLE_ENDIANNESS option.
Signed-off-by: Changbin Du
---
configs/targets/riscv32-softmmu.mak | 1 +
configs/targets/riscv64-softmmu.mak | 1 +
target/riscv/gdbstub.c | 12 ++--
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/configs/targets/riscv32-softmmu.mak
b/configs/targets/riscv32-softmmu.mak
index d8b71cddcd..7f02e67c72 100644
--- a/configs/targets/riscv32-softmmu.mak
+++ b/configs/targets/riscv32-softmmu.mak
@@ -3,3 +3,4 @@ TARGET_BASE_ARCH=riscv
TARGET_SUPPORTS_MTTCG=y
TARGET_XML_FILES= gdb-xml/riscv-32bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml
gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-32bit-virtual.xml
TARGET_NEED_FDT=y
+TARGET_SWICHABLE_ENDIANNESS=y
diff --git a/configs/targets/riscv64-softmmu.mak
b/configs/targets/riscv64-softmmu.mak
index 7c0e7eeb42..c3e812495c 100644
--- a/configs/targets/riscv64-softmmu.mak
+++ b/configs/targets/riscv64-softmmu.mak
@@ -3,3 +3,4 @@ TARGET_BASE_ARCH=riscv
TARGET_SUPPORTS_MTTCG=y
TARGET_XML_FILES= gdb-xml/riscv-64bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml
gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-64bit-virtual.xml
TARGET_NEED_FDT=y
+TARGET_SWICHABLE_ENDIANNESS=y
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index a7a9c0b1fe..d639cea859 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -42,10 +42,10 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t
*mem_buf, int n)
/* discard writes to x0 */
return sizeof(target_ulong);
} else if (n < 32) {
-env->gpr[n] = ldtul_p(mem_buf);
+env->gpr[n] = gdb_read_regl(mem_buf);
return sizeof(target_ulong);
} else if (n == 32) {
-env->pc = ldtul_p(mem_buf);
+env->pc = gdb_read_regl(mem_buf);
return sizeof(target_ulong);
}
return 0;
@@ -81,11 +81,11 @@ static int riscv_gdb_get_fpu(CPURISCVState *env, GByteArray
*buf, int n)
static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t *mem_buf, int n)
{
if (n < 32) {
-env->fpr[n] = ldq_p(mem_buf); /* always 64-bit */
+env->fpr[n] = gdb_read_reg64(mem_buf); /* always 64-bit */
return sizeof(uint64_t);
/* there is hole between ft11 and fflags in fpu.xml */
} else if (n < 36 && n > 32) {
-target_ulong val = ldtul_p(mem_buf);
+target_ulong val = gdb_read_regl(mem_buf);
int result;
/*
* CSR_FFLAGS is at index 1 in csr_register, and gdb says it is FP
@@ -118,7 +118,7 @@ static int riscv_gdb_get_csr(CPURISCVState *env, GByteArray
*buf, int n)
static int riscv_gdb_set_csr(CPURISCVState *env, uint8_t *mem_buf, int n)
{
if (n < CSR_TABLE_SIZE) {
-target_ulong val = ldtul_p(mem_buf);
+target_ulong val = gdb_read_regl(mem_buf);
int result;
result = riscv_csrrw_debug(env, n, NULL, val, -1);
@@ -145,7 +145,7 @@ static int riscv_gdb_set_virtual(CPURISCVState *cs, uint8_t
*mem_buf, int n)
{
if (n == 0) {
#ifndef CONFIG_USER_ONLY
-cs->priv = ldtul_p(mem_buf) & 0x3;
+cs->priv = gdb_read_regl(mem_buf) & 0x3;
if (cs->priv == PRV_H) {
cs->priv = PRV_S;
}
--
2.32.0
[PATCH 0/3] gdbstub: add support for switchable endianness
To resolve the issue to debug switchable targets, this serias introduces basic infrastructure for gdbstub and enable support for ARM and RISC-V targets. For example, now there is no problem to debug an big-enadian aarch64 target on x86 host. $ qemu-system-aarch64 -gdb tcp::1234,endianness=big ... Changbin Du (3): gdbstub: add basic infrastructure to support switchable endianness arm: gdbstub: add support for switchable endianness riscv: gdbstub: add support for switchable endianness configs/targets/aarch64-softmmu.mak | 1 + configs/targets/arm-softmmu.mak | 1 + configs/targets/riscv32-softmmu.mak | 1 + configs/targets/riscv64-softmmu.mak | 1 + gdbstub.c | 11 + include/exec/gdbstub.h | 72 + qemu-options.hx | 7 ++- softmmu/vl.c| 50 +++- target/arm/gdbstub.c| 2 +- target/arm/gdbstub64.c | 2 +- target/riscv/gdbstub.c | 12 ++--- 11 files changed, 131 insertions(+), 29 deletions(-) -- 2.32.0
[PATCH v2] target/riscv: Dump CSR mscratch/sscratch/satp
This dumps the CSR mscratch/sscratch/satp and meanwhile aligns
the output of CSR mtval/stval.
Signed-off-by: Changbin Du
Reviewed-by: Alistair Francis
Reviewed-by: Bin Meng
---
v2: Rebase to latest mainline.
---
target/riscv/cpu.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 3191fd0082..c4132d9845 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -286,12 +286,15 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f,
int flags)
if (riscv_has_ext(env, RVH)) {
qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "vscause ", env->vscause);
}
-qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mtval ", env->mtval);
-qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "stval ", env->stval);
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mtval ", env->mtval);
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "stval ", env->stval);
if (riscv_has_ext(env, RVH)) {
qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "htval ", env->htval);
qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mtval2 ", env->mtval2);
}
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mscratch", env->mscratch);
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "sscratch", env->sscratch);
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "satp", env->satp);
#endif
for (i = 0; i < 32; i++) {
--
2.30.2
[PATCH] target/riscv: Dump CSR mscratch/sscratch/satp
This dumps the CSR mscratch/sscratch/satp and meanwhile aligns
the output of CSR mtval/stval.
Signed-off-by: Changbin Du
---
target/riscv/cpu.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 7d6ed80f6b67..73af6f5445ba 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -281,12 +281,15 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f,
int flags)
if (riscv_has_ext(env, RVH)) {
qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "vscause ", env->vscause);
}
-qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mtval ", env->mtval);
-qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "stval ", env->sbadaddr);
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mtval ", env->mtval);
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "stval ", env->sbadaddr);
if (riscv_has_ext(env, RVH)) {
qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "htval ", env->htval);
qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mtval2 ", env->mtval2);
}
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mscratch", env->mscratch);
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "sscratch", env->sscratch);
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "satp", env->satp);
#endif
for (i = 0; i < 32; i++) {
--
2.27.0
[PATCH v3] arm/monitor: Add support for 'info tlb' command
This adds hmp 'info tlb' command support for the arm platform.
The limitation is that this only implements a page walker for
ARMv8-A AArch64 Long Descriptor format, 32bit addressing is
not supported yet.
To reuse existing code, this patch also extracts some APIs from
helper.c, including regime_translation_disabled(),
pt_start_level_stage1(), regime_ttbr().
(qemu) info tlb
vaddrpaddrsize attr
--
8000 4000 1000 RWAF PXN
UXN
80001000 40001000 1000 RWAF PXN
UXN
80002000 40002000 1000 RWAF PXN
UXN
80003000 40003000 1000 RWAF PXN
UXN
80004000 40004000 1000 RWAF PXN
UXN
80005000 40005000 1000 RWAF PXN
UXN
80006000 40006000 1000 RWAF PXN
UXN
Signed-off-by: Changbin Du
---
v3: rebase to latest mainline.
v2:
o fix coding style
o extract common code pt_start_level_stage1()
---
hmp-commands-info.hx | 3 +-
target/arm/helper.c| 30 +--
target/arm/internals.h | 33
target/arm/monitor.c | 183 +
4 files changed, 220 insertions(+), 29 deletions(-)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 117ba25f91..1b5b3f2616 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -222,7 +222,8 @@ SRST
ERST
#if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC) || \
-defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K)
+defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K) || \
+defined(TARGET_ARM)
{
.name = "tlb",
.args_type = "",
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 11b0803df7..e7f0f27c8e 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9974,8 +9974,7 @@ static inline uint64_t regime_sctlr(CPUARMState *env,
ARMMMUIdx mmu_idx)
#ifndef CONFIG_USER_ONLY
/* Return true if the specified stage of address translation is disabled */
-static inline bool regime_translation_disabled(CPUARMState *env,
- ARMMMUIdx mmu_idx)
+bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx)
{
if (arm_feature(env, ARM_FEATURE_M)) {
switch (env->v7m.mpu_ctrl[regime_is_secure(env, mmu_idx)] &
@@ -10021,20 +10020,6 @@ static inline bool
regime_translation_big_endian(CPUARMState *env,
return (regime_sctlr(env, mmu_idx) & SCTLR_EE) != 0;
}
-/* Return the TTBR associated with this translation regime */
-static inline uint64_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
- int ttbrn)
-{
-if (mmu_idx == ARMMMUIdx_Stage2) {
-return env->cp15.vttbr_el2;
-}
-if (ttbrn == 0) {
-return env->cp15.ttbr0_el[regime_el(env, mmu_idx)];
-} else {
-return env->cp15.ttbr1_el[regime_el(env, mmu_idx)];
-}
-}
-
#endif /* !CONFIG_USER_ONLY */
/* Convert a possible stage1+2 MMU index into the appropriate
@@ -11077,18 +11062,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
target_ulong address,
}
if (mmu_idx != ARMMMUIdx_Stage2) {
-/* The starting level depends on the virtual address size (which can
- * be up to 48 bits) and the translation granule size. It indicates
- * the number of strides (stride bits at a time) needed to
- * consume the bits of the input address. In the pseudocode this is:
- * level = 4 - RoundUp((inputsize - grainsize) / stride)
- * where their 'inputsize' is our 'inputsize', 'grainsize' is
- * our 'stride + 3' and 'stride' is our 'stride'.
- * Applying the usual "rounded up m/n is (m+n-1)/n" and simplifying:
- * = 4 - (inputsize - stride - 3 + stride - 1) / stride
- * = 4 - (inputsize - 4) / stride;
- */
-level = 4 - (inputsize - 4) / stride;
+level = pt_start_level_stage1(inputsize, stride);
} else {
/* For stage 2 translations the starting level is specified by the
* VTCR_EL2.SL0 field (whose interpretation depends on the page size)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 5460678756..69c21be774 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -949,6 +949,8 @@ static inline uint32_t regime_el(CPUARMState *env,
ARMMMUIdx mmu_idx)
}
}
+bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx);
+
/* Return the TCR controlling this translation regime */
static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
{
@@ -958,6 +960,20 @@ static
[PATCH v2] arm/monitor: Add support for 'info tlb' command
This adds hmp 'info tlb' command support for the arm platform.
The limitation is that this only implements a page walker for
ARMv8-A AArch64 Long Descriptor format, 32bit addressing is
not supported yet.
To reuse existing code, this patch also extracts some APIs from
helper.c, including regime_translation_disabled(),
pt_start_level_stage1(), regime_ttbr().
Signed-off-by: Changbin Du
---
v2:
o fix coding style
o extract common code pt_start_level_stage1()
Signed-off-by: Changbin Du
---
hmp-commands-info.hx | 3 +-
target/arm/helper.c| 30 +--
target/arm/internals.h | 32 +++
target/arm/monitor.c | 183 +
4 files changed, 219 insertions(+), 29 deletions(-)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 117ba25f91..1b5b3f2616 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -222,7 +222,8 @@ SRST
ERST
#if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC) || \
-defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K)
+defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K) || \
+defined(TARGET_ARM)
{
.name = "tlb",
.args_type = "",
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 11b0803df7..e7f0f27c8e 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9974,8 +9974,7 @@ static inline uint64_t regime_sctlr(CPUARMState *env,
ARMMMUIdx mmu_idx)
#ifndef CONFIG_USER_ONLY
/* Return true if the specified stage of address translation is disabled */
-static inline bool regime_translation_disabled(CPUARMState *env,
- ARMMMUIdx mmu_idx)
+bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx)
{
if (arm_feature(env, ARM_FEATURE_M)) {
switch (env->v7m.mpu_ctrl[regime_is_secure(env, mmu_idx)] &
@@ -10021,20 +10020,6 @@ static inline bool
regime_translation_big_endian(CPUARMState *env,
return (regime_sctlr(env, mmu_idx) & SCTLR_EE) != 0;
}
-/* Return the TTBR associated with this translation regime */
-static inline uint64_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
- int ttbrn)
-{
-if (mmu_idx == ARMMMUIdx_Stage2) {
-return env->cp15.vttbr_el2;
-}
-if (ttbrn == 0) {
-return env->cp15.ttbr0_el[regime_el(env, mmu_idx)];
-} else {
-return env->cp15.ttbr1_el[regime_el(env, mmu_idx)];
-}
-}
-
#endif /* !CONFIG_USER_ONLY */
/* Convert a possible stage1+2 MMU index into the appropriate
@@ -11077,18 +11062,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
target_ulong address,
}
if (mmu_idx != ARMMMUIdx_Stage2) {
-/* The starting level depends on the virtual address size (which can
- * be up to 48 bits) and the translation granule size. It indicates
- * the number of strides (stride bits at a time) needed to
- * consume the bits of the input address. In the pseudocode this is:
- * level = 4 - RoundUp((inputsize - grainsize) / stride)
- * where their 'inputsize' is our 'inputsize', 'grainsize' is
- * our 'stride + 3' and 'stride' is our 'stride'.
- * Applying the usual "rounded up m/n is (m+n-1)/n" and simplifying:
- * = 4 - (inputsize - stride - 3 + stride - 1) / stride
- * = 4 - (inputsize - 4) / stride;
- */
-level = 4 - (inputsize - 4) / stride;
+level = pt_start_level_stage1(inputsize, stride);
} else {
/* For stage 2 translations the starting level is specified by the
* VTCR_EL2.SL0 field (whose interpretation depends on the page size)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 5460678756..0f3a1d0188 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -949,6 +949,8 @@ static inline uint32_t regime_el(CPUARMState *env,
ARMMMUIdx mmu_idx)
}
}
+bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx);
+
/* Return the TCR controlling this translation regime */
static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
{
@@ -958,6 +960,20 @@ static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx
mmu_idx)
return >cp15.tcr_el[regime_el(env, mmu_idx)];
}
+/* Return the TTBR associated with this translation regime */
+static inline uint64_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
+ int ttbrn)
+{
+if (mmu_idx == ARMMMUIdx_Stage2) {
+return env->cp15.vttbr_el2;
+}
+if (ttbrn == 0) {
+return env->cp15.ttbr0_el[regime_el(env, mmu_idx)];
+} else {
+return env->cp15.ttbr1_el[regime_el(env, mmu_idx)];
+}
+}
+
/* Return the FSR value for a debug exception (watchpoint, hardware
* breakpoint or BKPT insn) targeting the specified exception level.
*/
@@ -1291,6 +1307,22 @@ t
Re: [PATCH] arm/monitor: Add support for 'info tlb' command
On Fri, Nov 13, 2020 at 01:13:42PM +, Dr. David Alan Gilbert wrote: > * Peter Maydell (peter.mayd...@linaro.org) wrote: > > On Fri, 13 Nov 2020 at 09:59, Changbin Du wrote: > > > > > > This adds hmp 'info tlb' command support for the arm platform. > > > The limitation is that this only implements a page walker for > > > ARMv8-A AArch64 Long Descriptor format, 32bit addressing is > > > not supported yet. > > > > "info tlb" needs its own entirely independent implementation > > of a page-table walk? I see this is how x86 has done it ,but > > it seems like a recipe for the info command being perpetually > > behind what we actually implement... > > I think the challenge is you want 'info tlb' to be quite easy > to read from a debuggers point of view and robust at pointing out > things that are odd; I'd hope at least you can use most of the macros > to extract fields etc > Actually I refered to function get_phys_addr_lpae() and try to use definitions what I found. But most of the bit definitions are hardcode. I have fixed the code style issues in V2. Thanks for your kind review! > Dave > > > thanks > > -- PMM > > > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > -- Cheers, Changbin Du
[PATCH] arm/monitor: Add support for 'info tlb' command
This adds hmp 'info tlb' command support for the arm platform.
The limitation is that this only implements a page walker for
ARMv8-A AArch64 Long Descriptor format, 32bit addressing is
not supported yet.
Signed-off-by: Changbin Du
---
hmp-commands-info.hx | 3 +-
target/arm/helper.c| 17 +---
target/arm/internals.h | 16
target/arm/monitor.c | 187 +
4 files changed, 206 insertions(+), 17 deletions(-)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 117ba25f91..1b5b3f2616 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -222,7 +222,8 @@ SRST
ERST
#if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC) || \
-defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K)
+defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K) || \
+defined(TARGET_ARM)
{
.name = "tlb",
.args_type = "",
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 11b0803df7..c73a08943b 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9974,8 +9974,7 @@ static inline uint64_t regime_sctlr(CPUARMState *env,
ARMMMUIdx mmu_idx)
#ifndef CONFIG_USER_ONLY
/* Return true if the specified stage of address translation is disabled */
-static inline bool regime_translation_disabled(CPUARMState *env,
- ARMMMUIdx mmu_idx)
+bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx)
{
if (arm_feature(env, ARM_FEATURE_M)) {
switch (env->v7m.mpu_ctrl[regime_is_secure(env, mmu_idx)] &
@@ -10021,20 +10020,6 @@ static inline bool
regime_translation_big_endian(CPUARMState *env,
return (regime_sctlr(env, mmu_idx) & SCTLR_EE) != 0;
}
-/* Return the TTBR associated with this translation regime */
-static inline uint64_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
- int ttbrn)
-{
-if (mmu_idx == ARMMMUIdx_Stage2) {
-return env->cp15.vttbr_el2;
-}
-if (ttbrn == 0) {
-return env->cp15.ttbr0_el[regime_el(env, mmu_idx)];
-} else {
-return env->cp15.ttbr1_el[regime_el(env, mmu_idx)];
-}
-}
-
#endif /* !CONFIG_USER_ONLY */
/* Convert a possible stage1+2 MMU index into the appropriate
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 5460678756..12f883c636 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -949,6 +949,8 @@ static inline uint32_t regime_el(CPUARMState *env,
ARMMMUIdx mmu_idx)
}
}
+bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx);
+
/* Return the TCR controlling this translation regime */
static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
{
@@ -958,6 +960,20 @@ static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx
mmu_idx)
return >cp15.tcr_el[regime_el(env, mmu_idx)];
}
+/* Return the TTBR associated with this translation regime */
+static inline uint64_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
+ int ttbrn)
+{
+if (mmu_idx == ARMMMUIdx_Stage2) {
+return env->cp15.vttbr_el2;
+}
+if (ttbrn == 0) {
+return env->cp15.ttbr0_el[regime_el(env, mmu_idx)];
+} else {
+return env->cp15.ttbr1_el[regime_el(env, mmu_idx)];
+}
+}
+
/* Return the FSR value for a debug exception (watchpoint, hardware
* breakpoint or BKPT insn) targeting the specified exception level.
*/
diff --git a/target/arm/monitor.c b/target/arm/monitor.c
index 169d8a64b6..d6b64ea3b6 100644
--- a/target/arm/monitor.c
+++ b/target/arm/monitor.c
@@ -31,6 +31,9 @@
#include "qapi/qmp/qerror.h"
#include "qapi/qmp/qdict.h"
#include "qom/qom-qobject.h"
+#include "monitor/monitor.h"
+#include "monitor/hmp-target.h"
+#include "internals.h"
static GICCapability *gic_cap_new(int version)
{
@@ -236,3 +239,187 @@ CpuModelExpansionInfo
*qmp_query_cpu_model_expansion(CpuModelExpansionType type,
return expansion_info;
}
+
+/* Perform linear address sign extension */
+static target_ulong addr_canonical(int va_bits, target_ulong addr)
+{
+#ifdef TARGET_AARCH64
+if (addr & (1UL << (va_bits - 1))) {
+addr |= (hwaddr)-(1L << va_bits);
+}
+#endif
+
+return addr;
+}
+
+#define PTE_HEADER_FIELDS "vaddrpaddr"\
+"size attr\n"
+#define PTE_HEADER_DELIMITER" "\
+"
--\n"
+
+static void print_pte_header(Monitor *mon)
+{
+monitor_printf(mon, PTE_HEADER_FIELDS);
+monitor_printf(mon, PTE_HEADER_DELIMITER);
+}
+
+static void
+print_pte_lpae(Monitor *mon, uint32_t tableattrs, int
Re: [PATCH v2] ui/sdl2: fix segment fault caused by null pointer dereference
hello, is this ready to merge now?
On Mon, Apr 27, 2020 at 09:24:12PM +0800, Changbin Du wrote:
> I found SDL_GetWindowFromID() sometimes return NULL when I start qemu via
> ssh forwarding even the window has been crated already. I am not sure
> whether this is a bug of SDL, but we'd better check it carefully.
>
> Signed-off-by: Changbin Du
>
> ---
> v2: fix typo.
> ---
> ui/sdl2.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index 3c9424eb42..61c7956da3 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -332,6 +332,10 @@ static void handle_keydown(SDL_Event *ev)
> int gui_key_modifier_pressed = get_mod_state();
> int gui_keysym = 0;
>
> +if (!scon) {
> +return;
> +}
> +
> if (!scon->ignore_hotkeys && gui_key_modifier_pressed &&
> !ev->key.repeat) {
> switch (ev->key.keysym.scancode) {
> case SDL_SCANCODE_2:
> @@ -412,6 +416,10 @@ static void handle_keyup(SDL_Event *ev)
> {
> struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
>
> +if (!scon) {
> +return;
> +}
> +
> scon->ignore_hotkeys = false;
> sdl2_process_key(scon, >key);
> }
> @@ -421,6 +429,10 @@ static void handle_textinput(SDL_Event *ev)
> struct sdl2_console *scon = get_scon_from_window(ev->text.windowID);
> QemuConsole *con = scon ? scon->dcl.con : NULL;
>
> +if (!con) {
> +return;
> +}
> +
> if (qemu_console_is_graphic(con)) {
> return;
> }
> --
> 2.25.1
>
--
Cheers,
Changbin Du
[PATCH v2] ui/sdl2: fix segment fault caused by null pointer dereference
I found SDL_GetWindowFromID() sometimes return NULL when I start qemu via
ssh forwarding even the window has been crated already. I am not sure
whether this is a bug of SDL, but we'd better check it carefully.
Signed-off-by: Changbin Du
---
v2: fix typo.
---
ui/sdl2.c | 12
1 file changed, 12 insertions(+)
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 3c9424eb42..61c7956da3 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -332,6 +332,10 @@ static void handle_keydown(SDL_Event *ev)
int gui_key_modifier_pressed = get_mod_state();
int gui_keysym = 0;
+if (!scon) {
+return;
+}
+
if (!scon->ignore_hotkeys && gui_key_modifier_pressed && !ev->key.repeat) {
switch (ev->key.keysym.scancode) {
case SDL_SCANCODE_2:
@@ -412,6 +416,10 @@ static void handle_keyup(SDL_Event *ev)
{
struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
+if (!scon) {
+return;
+}
+
scon->ignore_hotkeys = false;
sdl2_process_key(scon, >key);
}
@@ -421,6 +429,10 @@ static void handle_textinput(SDL_Event *ev)
struct sdl2_console *scon = get_scon_from_window(ev->text.windowID);
QemuConsole *con = scon ? scon->dcl.con : NULL;
+if (!con) {
+return;
+}
+
if (qemu_console_is_graphic(con)) {
return;
}
--
2.25.1
Re: [PATCH] ui/sdl2: fix segment fault caused by null pointer dereference
On Mon, Apr 27, 2020 at 02:11:59PM +0100, Peter Maydell wrote:
> On Mon, 27 Apr 2020 at 13:19, Changbin Du wrote:
> >
> > I found SDL_GetWindowFromID() sometimes return NULL when I start qemu via
> > ssh forwarding even the window has been crated already. I am not sure
> > whether this is a bug of SDL, but we'd better check it carefully.
> >
> > Signed-off-by: Changbin Du
> > ---
> > ui/sdl2.c | 9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/ui/sdl2.c b/ui/sdl2.c
> > index 3c9424eb42..7c9c93b951 100644
> > --- a/ui/sdl2.c
> > +++ b/ui/sdl2.c
> > @@ -332,6 +332,9 @@ static void handle_keydown(SDL_Event *ev)
> > int gui_key_modifier_pressed = get_mod_state();
> > int gui_keysym = 0;
> >
> > +if (!scon)
> > +return;
> > +
> > if (!scon->ignore_hotkeys && gui_key_modifier_pressed &&
> > !ev->key.repeat) {
> > switch (ev->key.keysym.scancode) {
> > case SDL_SCANCODE_2:
> > @@ -412,6 +415,9 @@ static void handle_keyup(SDL_Event *ev)
> > {
> > struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
> >
> > +if (!sconf)
> > +return;
>
> It's generally a good idea to make sure your patch at least compiles
> before sending it :-)
>
sorry for this. I don't know why my make didn't recompile it after
changing.
> QEMU coding style demands {} on all if statements.
>
sure.
> thanks
> -- PMM
--
Cheers,
Changbin Du
[PATCH] ui/sdl2: fix segment fault caused by null pointer dereference
I found SDL_GetWindowFromID() sometimes return NULL when I start qemu via
ssh forwarding even the window has been crated already. I am not sure
whether this is a bug of SDL, but we'd better check it carefully.
Signed-off-by: Changbin Du
---
ui/sdl2.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 3c9424eb42..7c9c93b951 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -332,6 +332,9 @@ static void handle_keydown(SDL_Event *ev)
int gui_key_modifier_pressed = get_mod_state();
int gui_keysym = 0;
+if (!scon)
+return;
+
if (!scon->ignore_hotkeys && gui_key_modifier_pressed && !ev->key.repeat) {
switch (ev->key.keysym.scancode) {
case SDL_SCANCODE_2:
@@ -412,6 +415,9 @@ static void handle_keyup(SDL_Event *ev)
{
struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
+if (!sconf)
+return;
+
scon->ignore_hotkeys = false;
sdl2_process_key(scon, >key);
}
@@ -421,6 +427,9 @@ static void handle_textinput(SDL_Event *ev)
struct sdl2_console *scon = get_scon_from_window(ev->text.windowID);
QemuConsole *con = scon ? scon->dcl.con : NULL;
+if (!con)
+return;
+
if (qemu_console_is_graphic(con)) {
return;
}
--
2.20.1
Re: [PATCH] gdbstub: Fix segment fault for i386 and m68k
On Sun, Apr 12, 2020 at 02:58:56PM +0200, Laurent Vivier wrote: > CC: Alex Bennée > > The one for m68k is already queued by Alex. > Great. I will send x86 fix only. Thanks! > Thanks, > Laurent > -- Cheers, Changbin Du
[PATCH v2] gdbstub: Fix segment fault for i386 target
With GByteArray, we should pass the object itself but not to plus an offset.
gdb log:
Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
__memmove_avx_unaligned_erms () at
../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:384
384 ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S: No such file
or directory.
Fixes: a010bdbe71 ("gdbstub: extend GByteArray to read register helpers")
Signed-off-by: Changbin Du
---
v2: remove m68k fix since it's already queued.
---
target/i386/gdbstub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
index f3d23b614e..b98a99500a 100644
--- a/target/i386/gdbstub.c
+++ b/target/i386/gdbstub.c
@@ -106,7 +106,7 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray
*mem_buf, int n)
} else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
floatx80 *fp = (floatx80 *) >fpregs[n - IDX_FP_REGS];
int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low));
-len += gdb_get_reg16(mem_buf + len, cpu_to_le16(fp->high));
+len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high));
return len;
} else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
n -= IDX_XMM_REGS;
--
2.25.1
[PATCH] gdbstub: Fix segment fault for i386 and m68k
With GByteArray, we should pass the object itself but not to plus an offset.
gdb log:
Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
__memmove_avx_unaligned_erms () at
../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:384
384 ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S: No such file
or directory.
Fixes: a010bdbe71 ("gdbstub: extend GByteArray to read register helpers")
Signed-off-by: Changbin Du
---
target/i386/gdbstub.c | 2 +-
target/m68k/helper.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
index f3d23b614e..b98a99500a 100644
--- a/target/i386/gdbstub.c
+++ b/target/i386/gdbstub.c
@@ -106,7 +106,7 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray
*mem_buf, int n)
} else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
floatx80 *fp = (floatx80 *) >fpregs[n - IDX_FP_REGS];
int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low));
-len += gdb_get_reg16(mem_buf + len, cpu_to_le16(fp->high));
+len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high));
return len;
} else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
n -= IDX_XMM_REGS;
diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 014657c637..cad4083895 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -109,8 +109,8 @@ static int m68k_fpu_gdb_get_reg(CPUM68KState *env,
GByteArray *mem_buf, int n)
{
if (n < 8) {
int len = gdb_get_reg16(mem_buf, env->fregs[n].l.upper);
-len += gdb_get_reg16(mem_buf + len, 0);
-len += gdb_get_reg64(mem_buf + len, env->fregs[n].l.lower);
+len += gdb_get_reg16(mem_buf, 0);
+len += gdb_get_reg64(mem_buf, env->fregs[n].l.lower);
return len;
}
switch (n) {
--
2.25.1
Re: [PATCH] target/arm: fix incorrect current EL bug in aarch32 exception emulation
Hi, Peter,
Could you take this fix as high priority? This bug has made qemu-system-arm
broken.
On Sat, Mar 28, 2020 at 10:02:32PM +0800, Changbin Du wrote:
> The arm_current_el() should be invoked after mode switching. Otherwise, we
> get a wrong current EL value, since current EL is also determined by
> current mode.
>
> Fixes: 4a2696c0d4 ("target/arm: Set PAN bit as required on exception entry")
> Signed-off-by: Changbin Du
> ---
> target/arm/helper.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index b7b6887241..163c91a1cc 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -9172,7 +9172,6 @@ static void take_aarch32_exception(CPUARMState *env,
> int new_mode,
>
> /* Change the CPU state so as to actually take the exception. */
> switch_mode(env, new_mode);
> -new_el = arm_current_el(env);
>
> /*
> * For exceptions taken to AArch32 we must clear the SS bit in both
> @@ -9184,6 +9183,10 @@ static void take_aarch32_exception(CPUARMState *env,
> int new_mode,
> env->condexec_bits = 0;
> /* Switch to the new mode, and to the correct instruction set. */
> env->uncached_cpsr = (env->uncached_cpsr & ~CPSR_M) | new_mode;
> +
> +/* This must be after mode switching. */
> +new_el = arm_current_el(env);
> +
> /* Set new mode endianness */
> env->uncached_cpsr &= ~CPSR_E;
> if (env->cp15.sctlr_el[new_el] & SCTLR_EE) {
> --
> 2.25.1
>
--
Cheers,
Changbin Du
[PATCH] target/arm: fix incorrect current EL bug in aarch32 exception emulation
The arm_current_el() should be invoked after mode switching. Otherwise, we
get a wrong current EL value, since current EL is also determined by
current mode.
Fixes: 4a2696c0d4 ("target/arm: Set PAN bit as required on exception entry")
Signed-off-by: Changbin Du
---
target/arm/helper.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index b7b6887241..163c91a1cc 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9172,7 +9172,6 @@ static void take_aarch32_exception(CPUARMState *env, int
new_mode,
/* Change the CPU state so as to actually take the exception. */
switch_mode(env, new_mode);
-new_el = arm_current_el(env);
/*
* For exceptions taken to AArch32 we must clear the SS bit in both
@@ -9184,6 +9183,10 @@ static void take_aarch32_exception(CPUARMState *env, int
new_mode,
env->condexec_bits = 0;
/* Switch to the new mode, and to the correct instruction set. */
env->uncached_cpsr = (env->uncached_cpsr & ~CPSR_M) | new_mode;
+
+/* This must be after mode switching. */
+new_el = arm_current_el(env);
+
/* Set new mode endianness */
env->uncached_cpsr &= ~CPSR_E;
if (env->cp15.sctlr_el[new_el] & SCTLR_EE) {
--
2.25.1
Re: [PATCH v2] gdbstub: Fix single-step issue by confirming 'vContSupported+' feature to gdb
hello, is this patch ready to merge now? Thanks! On Fri, Feb 21, 2020 at 08:25:59AM +0800, Changbin Du wrote: > Recently when debugging an arm32 system on qemu, I found sometimes the > single-step command (stepi) is not working. This can be reproduced by > below steps: > 1) start qemu-system-arm -s -S .. and wait for gdb connection. > 2) start gdb and connect to qemu. In my case, gdb gets a wrong value > (0x60) for PC, which is an another bug. > 3) After connected, type 'stepi' and expect it will stop at next ins. > > But, it has never stopped. This because: > 1) We doesn't report ‘vContSupported’ feature to gdb explicitly and gdb > think we do not support it. In this case, gdb use a software breakpoint > to emulate single-step. > 2) Since gdb gets a wrong initial value of PC, then gdb inserts a > breakpoint to wrong place (PC+4). > > Not only for the arm target, Philippe has also encountered this on MIPS. > Probably gdb has different assumption for different architectures. > > Since we do support ‘vContSupported’ query command, so let's tell gdb that > we support it. > > Before this change, gdb send below 'Z0' packet to implement single-step: > gdb_handle_packet: Z0,4,4 > > After this change, gdb send "vCont;s.." which is expected: > gdb_handle_packet: vCont? > put_packet: vCont;c;C;s;S > gdb_handle_packet: vCont;s:p1.1;c:p1.-1 > > Signed-off-by: Changbin Du > Tested-by: Philippe Mathieu-Daudé > > --- > v2: polish commit message. > --- > gdbstub.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gdbstub.c b/gdbstub.c > index ce304ff482..adccd938e2 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -2111,7 +2111,7 @@ static void handle_query_supported(GdbCmdContext > *gdb_ctx, void *user_ctx) > gdb_ctx->s->multiprocess = true; > } > > -pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";multiprocess+"); > +pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), > ";vContSupported+;multiprocess+"); > put_packet(gdb_ctx->s, gdb_ctx->str_buf); > } > > -- > 2.25.0 > -- Cheers, Changbin Du
Re: [PATCH] tcg: gdbstub: Fix single-step issue on arm target
On Thu, Feb 20, 2020 at 10:24:37PM +0100, Luc Michel wrote: > I'm curious, I never experienced this behaviour from GDB. What GDB and > QEMU versions are you using? > > On my side (GDB 9.1), even without 'vContSupported+' in the 'qSupported' > answer, GDB sends a 'vCont?' packet on the first stepi: > > 0x in ?? () > (gdb) si > Sending packet: $m0,4#fd...Ack > Packet received: > Sending packet: $vCont?#49...Ack > Packet received: vCont;c;C;s;S > Packet vCont (verbose-resume) is supported > Sending packet: $vCont;s:p1.1;c:p1.-1#f7...Ack > Packet received: T05thread:p01.01; > > Your second issue (wrong PC value) should be investigated though. Does > it happen on QEMU vanilla? Do you have a way to reproduce this bug? > Just confirmed this issue. This is an endianness problem for gdb. I was debugging an big-endian elf and my host cpu is little-endian. QEMU gdbstub always uses host cpu endian but gdb client treats it as big-endian by inspecting elf info. I can mannually set it to little-endian but it is painful. The gdb complains abount invalid opcode error in debuginfo. I also noticed that someoneelse has already tried to resolve this issue. https://patchwork.kernel.org/patch/9528947/ > Anyway after re-reading the GDB remote protocol documentation, I think > your patch is right, the feature should be advertised. > > However I think your commit message needs some modifications. This fix > is not specific to ARM or TCG, but to the gdbstub itself. You also > mention this bug you have with PC, which is not related to the bug you > are fixing here. Could you rewrite it in a more generic way? You simply > need to emphasis the effect of advertising the 'vContSupported+' feature > on GDB. > > Thanks. > > -- > Luc -- Cheers, Changbin Du
[PATCH v2] gdbstub: Fix single-step issue by confirming 'vContSupported+' feature to gdb
Recently when debugging an arm32 system on qemu, I found sometimes the single-step command (stepi) is not working. This can be reproduced by below steps: 1) start qemu-system-arm -s -S .. and wait for gdb connection. 2) start gdb and connect to qemu. In my case, gdb gets a wrong value (0x60) for PC, which is an another bug. 3) After connected, type 'stepi' and expect it will stop at next ins. But, it has never stopped. This because: 1) We doesn't report ‘vContSupported’ feature to gdb explicitly and gdb think we do not support it. In this case, gdb use a software breakpoint to emulate single-step. 2) Since gdb gets a wrong initial value of PC, then gdb inserts a breakpoint to wrong place (PC+4). Not only for the arm target, Philippe has also encountered this on MIPS. Probably gdb has different assumption for different architectures. Since we do support ‘vContSupported’ query command, so let's tell gdb that we support it. Before this change, gdb send below 'Z0' packet to implement single-step: gdb_handle_packet: Z0,4,4 After this change, gdb send "vCont;s.." which is expected: gdb_handle_packet: vCont? put_packet: vCont;c;C;s;S gdb_handle_packet: vCont;s:p1.1;c:p1.-1 Signed-off-by: Changbin Du Tested-by: Philippe Mathieu-Daudé --- v2: polish commit message. --- gdbstub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdbstub.c b/gdbstub.c index ce304ff482..adccd938e2 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -2111,7 +2111,7 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx) gdb_ctx->s->multiprocess = true; } -pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";multiprocess+"); +pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";vContSupported+;multiprocess+"); put_packet(gdb_ctx->s, gdb_ctx->str_buf); } -- 2.25.0
Re: [PATCH] tcg: gdbstub: Fix single-step issue on arm target
On Thu, Feb 20, 2020 at 06:47:26PM +0100, Philippe Mathieu-Daudé wrote: > On 2/20/20 4:58 PM, Changbin Du wrote: > > Recently when debugging an arm32 system on qemu, I found sometimes the > > single-step command (stepi) is not working. This can be reproduced by > > below steps: > > 1) start qemu-system-arm -s -S .. and wait for gdb connection. > > 2) start gdb and connect to qemu. In my case, gdb gets a wrong value > > (0x60) for PC. > > 3) After connected, type 'stepi' and expect it will stop at next ins. > > > > But, it has never stopped. This because: > > 1) We doesn't report ‘vContSupported’ feature to gdb explicitly and gdb > > think we do not support it. In this case, gdb use a software breakpoint > > to emulate single-step. > > 2) Since gdb gets a wrong initial value of PC, then gdb inserts a > > breakpoint to wrong place (PC+4). > > > > Since we do support ‘vContSupported’ query command, so let's tell gdb that > > we support it. > > > > Before this change, gdb send below 'Z0' packet to implement single-step: > > gdb_handle_packet: Z0,4,4 > > > > After this change, gdb send "vCont;s.." which is expected: > > gdb_handle_packet: vCont? > > put_packet: vCont;c;C;s;S > > gdb_handle_packet: vCont;s:p1.1;c:p1.-1 > > You actually fixed this for all architectures :) > > This has been annoying me on MIPS since more than a year... > > I haven't checked the GDB protocol spec, but so far: > Tested-by: Philippe Mathieu-Daudé > Thanks for your feedback. :) -- Cheers, Changbin Du
Re: [PATCH] tcg: gdbstub: Fix single-step issue on arm target
On Thu, Feb 20, 2020 at 10:24:37PM +0100, Luc Michel wrote: > Hi, > > On 2/20/20 4:58 PM, Changbin Du wrote: > > Recently when debugging an arm32 system on qemu, I found sometimes the > > single-step command (stepi) is not working. This can be reproduced by > > below steps: > > 1) start qemu-system-arm -s -S .. and wait for gdb connection. > > 2) start gdb and connect to qemu. In my case, gdb gets a wrong value > > (0x60) for PC. > > 3) After connected, type 'stepi' and expect it will stop at next ins. > > > > But, it has never stopped. This because: > > 1) We doesn't report ‘vContSupported’ feature to gdb explicitly and gdb > > think we do not support it. In this case, gdb use a software breakpoint > > to emulate single-step. > > 2) Since gdb gets a wrong initial value of PC, then gdb inserts a > > breakpoint to wrong place (PC+4). > > > > Since we do support ‘vContSupported’ query command, so let's tell gdb that > > we support it. > > > > Before this change, gdb send below 'Z0' packet to implement single-step: > > gdb_handle_packet: Z0,4,4 > > > > After this change, gdb send "vCont;s.." which is expected: > > gdb_handle_packet: vCont? > > put_packet: vCont;c;C;s;S > > gdb_handle_packet: vCont;s:p1.1;c:p1.-1 > I'm curious, I never experienced this behaviour from GDB. What GDB and > QEMU versions are you using? > For QEMU, it's built from mainline. For GDB, I have tried 8.1 and latest 9.1. > On my side (GDB 9.1), even without 'vContSupported+' in the 'qSupported' > answer, GDB sends a 'vCont?' packet on the first stepi: > > 0x in ?? () > (gdb) si > Sending packet: $m0,4#fd...Ack > Packet received: > Sending packet: $vCont?#49...Ack > Packet received: vCont;c;C;s;S > Packet vCont (verbose-resume) is supported > Sending packet: $vCont;s:p1.1;c:p1.-1#f7...Ack > Packet received: T05thread:p01.01; > hmm, On my side, this is 100% reproducable on arm32, but aarch64 doesn't. I think the GDB has different assumptions for different arch. > Your second issue (wrong PC value) should be investigated though. Does > it happen on QEMU vanilla? Do you have a way to reproduce this bug? > This is also 100% reproducable for my tested elf guest. But so sorry that I can't share it. Probablly I will check this issue some days later. > Anyway after re-reading the GDB remote protocol documentation, I think > your patch is right, the feature should be advertised. > > However I think your commit message needs some modifications. This fix > is not specific to ARM or TCG, but to the gdbstub itself. You also > mention this bug you have with PC, which is not related to the bug you > are fixing here. Could you rewrite it in a more generic way? You simply > need to emphasis the effect of advertising the 'vContSupported+' feature > on GDB. > sure. > Thanks. > > -- > Luc -- Cheers, Changbin Du
[PATCH] tcg: gdbstub: Fix single-step issue on arm target
Recently when debugging an arm32 system on qemu, I found sometimes the single-step command (stepi) is not working. This can be reproduced by below steps: 1) start qemu-system-arm -s -S .. and wait for gdb connection. 2) start gdb and connect to qemu. In my case, gdb gets a wrong value (0x60) for PC. 3) After connected, type 'stepi' and expect it will stop at next ins. But, it has never stopped. This because: 1) We doesn't report ‘vContSupported’ feature to gdb explicitly and gdb think we do not support it. In this case, gdb use a software breakpoint to emulate single-step. 2) Since gdb gets a wrong initial value of PC, then gdb inserts a breakpoint to wrong place (PC+4). Since we do support ‘vContSupported’ query command, so let's tell gdb that we support it. Before this change, gdb send below 'Z0' packet to implement single-step: gdb_handle_packet: Z0,4,4 After this change, gdb send "vCont;s.." which is expected: gdb_handle_packet: vCont? put_packet: vCont;c;C;s;S gdb_handle_packet: vCont;s:p1.1;c:p1.-1 Signed-off-by: Changbin Du --- gdbstub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdbstub.c b/gdbstub.c index ce304ff482..adccd938e2 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -2111,7 +2111,7 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx) gdb_ctx->s->multiprocess = true; } -pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";multiprocess+"); +pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";vContSupported+;multiprocess+"); put_packet(gdb_ctx->s, gdb_ctx->str_buf); } -- 2.25.0
Re: [PATCH] tcg: gdbstub: Fix missing breakpoint issue
On Wed, Feb 05, 2020 at 11:03:23AM +, Richard Henderson wrote: > On 1/24/20 2:17 AM, Changbin Du wrote: > > When inserting breakpoints, we need to invalidate related TBs to apply > > helper call. This is done by breakpoint_invalidate(). But many users > > found the BPs sometimes never hit. > > > > In system mode emulation, the BPs are global in guest but not particular > > address space. The issue is that the current implementation only trys to > > invalidate TB of paddr corresponding to the target vaddr in current MMU > > context. Then some cached TBs continue running without BPs applied. > > > > To fix this issue, we can just invalidate all TBs as what step mode does. > > (For old version users, issuing a step command can workaround this problem.) > > > > Signed-off-by: Changbin Du > > --- > > exec.c | 29 + > > 1 file changed, 1 insertion(+), 28 deletions(-) > > > > diff --git a/exec.c b/exec.c > > index 67e520d18e..9d9fd48519 100644 > > --- a/exec.c > > +++ b/exec.c > > @@ -997,36 +997,9 @@ static void breakpoint_invalidate(CPUState *cpu, > > target_ulong pc) > > tb_invalidate_phys_addr(pc); > > } > > #else > > -void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, MemTxAttrs > > attrs) > > You can't remove this function yet. > You should have seen that xtensa-softmmu no longer builds. > > I've cc'd you into Max Filippov's thread that addresses the same problem, and > I'm going to apply his fix for now. > > > r~ > Got it, just go ahead with that one. thanks~ -- Cheers, Changbin Du
Re: [PATCH] tcg: gdbstub: Fix missing breakpoint issue
Hello,
Any review?
Thanks!
On Fri, Jan 24, 2020 at 10:17:28AM +0800, Changbin Du wrote:
> When inserting breakpoints, we need to invalidate related TBs to apply
> helper call. This is done by breakpoint_invalidate(). But many users
> found the BPs sometimes never hit.
>
> In system mode emulation, the BPs are global in guest but not particular
> address space. The issue is that the current implementation only trys to
> invalidate TB of paddr corresponding to the target vaddr in current MMU
> context. Then some cached TBs continue running without BPs applied.
>
> To fix this issue, we can just invalidate all TBs as what step mode does.
> (For old version users, issuing a step command can workaround this problem.)
>
> Signed-off-by: Changbin Du
> ---
> exec.c | 29 +
> 1 file changed, 1 insertion(+), 28 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 67e520d18e..9d9fd48519 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -997,36 +997,9 @@ static void breakpoint_invalidate(CPUState *cpu,
> target_ulong pc)
> tb_invalidate_phys_addr(pc);
> }
> #else
> -void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, MemTxAttrs attrs)
> -{
> -ram_addr_t ram_addr;
> -MemoryRegion *mr;
> -hwaddr l = 1;
> -
> -if (!tcg_enabled()) {
> -return;
> -}
> -
> -RCU_READ_LOCK_GUARD();
> -mr = address_space_translate(as, addr, , , false, attrs);
> -if (!(memory_region_is_ram(mr)
> - || memory_region_is_romd(mr))) {
> -return;
> -}
> -ram_addr = memory_region_get_ram_addr(mr) + addr;
> -tb_invalidate_phys_page_range(ram_addr, ram_addr + 1);
> -}
> -
> static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
> {
> -MemTxAttrs attrs;
> -hwaddr phys = cpu_get_phys_page_attrs_debug(cpu, pc, );
> -int asidx = cpu_asidx_from_attrs(cpu, attrs);
> -if (phys != -1) {
> -/* Locks grabbed by tb_invalidate_phys_addr */
> -tb_invalidate_phys_addr(cpu->cpu_ases[asidx].as,
> -phys | (pc & ~TARGET_PAGE_MASK), attrs);
> -}
> +tb_flush(cpu);
> }
> #endif
>
> --
> 2.24.0
>
--
Cheers,
Changbin Du
[PATCH] tcg: gdbstub: Fix missing breakpoint issue
When inserting breakpoints, we need to invalidate related TBs to apply
helper call. This is done by breakpoint_invalidate(). But many users
found the BPs sometimes never hit.
In system mode emulation, the BPs are global in guest but not particular
address space. The issue is that the current implementation only trys to
invalidate TB of paddr corresponding to the target vaddr in current MMU
context. Then some cached TBs continue running without BPs applied.
To fix this issue, we can just invalidate all TBs as what step mode does.
(For old version users, issuing a step command can workaround this problem.)
Signed-off-by: Changbin Du
---
exec.c | 29 +
1 file changed, 1 insertion(+), 28 deletions(-)
diff --git a/exec.c b/exec.c
index 67e520d18e..9d9fd48519 100644
--- a/exec.c
+++ b/exec.c
@@ -997,36 +997,9 @@ static void breakpoint_invalidate(CPUState *cpu,
target_ulong pc)
tb_invalidate_phys_addr(pc);
}
#else
-void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, MemTxAttrs attrs)
-{
-ram_addr_t ram_addr;
-MemoryRegion *mr;
-hwaddr l = 1;
-
-if (!tcg_enabled()) {
-return;
-}
-
-RCU_READ_LOCK_GUARD();
-mr = address_space_translate(as, addr, , , false, attrs);
-if (!(memory_region_is_ram(mr)
- || memory_region_is_romd(mr))) {
-return;
-}
-ram_addr = memory_region_get_ram_addr(mr) + addr;
-tb_invalidate_phys_page_range(ram_addr, ram_addr + 1);
-}
-
static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
{
-MemTxAttrs attrs;
-hwaddr phys = cpu_get_phys_page_attrs_debug(cpu, pc, );
-int asidx = cpu_asidx_from_attrs(cpu, attrs);
-if (phys != -1) {
-/* Locks grabbed by tb_invalidate_phys_addr */
-tb_invalidate_phys_addr(cpu->cpu_ases[asidx].as,
-phys | (pc & ~TARGET_PAGE_MASK), attrs);
-}
+tb_flush(cpu);
}
#endif
--
2.24.0
Re: [Qemu-devel] [PATCH 00/39] Windbg supporting
as srv*c:\tmp*http://msdl.microsoft.com/download/symbols. > > How it works: > The WinDbg debugger has the possibility of connecting to a remote debug server > (Kdsrv.exe) in the Windows kernel. Therefore, it is possible to connect > to the guest system running in the QEMU emulator. Kernel debugging is possible > only with the enabled debugging mode, may change at the same time. > Our module of WinDbg debugger for QEMU is an alternative of the remote > debugging > service in the kernel. Thus, the debugger connects to the debugging module, > not to the kernel of the operating system. The module obtains all the > necessary > information answering debugger requests from the QEMU emulator. At the same > time > for debugging there is no need to enable debugging mode in the kernel. > This leads to hidden debugging. Our module supports all features of WinDbg > regarding remote debugging, besides interception of events and exceptions. > Supports i386 and x86_64 architectures. > > Tested-by: Ladi Prosek > --- > > Mikhail Abakumov (39): > windbg: add empty windbgstub files > windbg: add windbg's KD header file > windbg: add -windbg option > windbg: add helper features > windbg: add WindbgState > windbg: add chardev > windbg: hook to wrmsr operation > windbg: implement windbg_on_load > windbg: implement find_KPCR > windbg: implement find_kdVersion > windbg: add windbg_search_vmaddr > windbg: implement find_kdDebuggerDataBlock > windbg: parsing data stream > windbg: send data and control packets > windbg: handler of parsing context > windbg: init DBGKD_ANY_WAIT_STATE_CHANGE > windbg: generate ExceptionStateChange and LoadSymbolsStateChange > windbg: implement windbg_process_control_packet > windbg: implement windbg_process_data_packet > windbg: implement windbg_process_manipulate_packet > windbg: implement kd_api_read_virtual_memory and > kd_api_write_virtual_memory > windbg: some kernel structures > windbg: add helper functions > windbg: [de]serialization cpu context > windbg: [de]serialization cpu spec registers > windbg: implement kd_api_get_context and kd_api_set_context > windbg: implement kd_api_get_context_ex and kd_api_set_context_ex > windbg: implement kd_api_read_control_space and > kd_api_write_control_space > windbg: implement kd_api_write_breakpoint and kd_api_restore_breakpoint > windbg: debug exception subscribing > windbg: implement kd_api_continue > windbg: implement kd_api_read_io_space and kd_api_write_io_space > windbg: implement kd_api_read_physical_memory and > kd_api_write_physical_memory > windbg: implement kd_api_get_version > windbg: implement kd_api_read_msr and kd_api_write_msr > windbg: implement kd_api_search_memory > windbg: implement kd_api_fill_memory > windbg: implement kd_api_query_memory > windbg: maintainers > > > MAINTAINERS | 12 > Makefile.target |3 > cpus.c | 19 + > default-configs/i386-softmmu.mak |1 > gdbstub.c|4 > include/exec/windbgkd.h | 928 ++ > include/exec/windbgstub-utils.h | 104 +++ > include/exec/windbgstub.h| 25 + > include/sysemu/sysemu.h |2 > qemu-options.hx |8 > stubs/Makefile.objs |1 > stubs/windbgstub.c | 22 + > target/i386/Makefile.objs|1 > target/i386/cpu.h|5 > target/i386/misc_helper.c| 38 + > target/i386/windbgstub.c | 1368 > ++ > vl.c |8 > windbgstub-utils.c | 508 ++ > windbgstub.c | 545 +++ > 19 files changed, 3592 insertions(+), 10 deletions(-) > create mode 100644 include/exec/windbgkd.h > create mode 100644 include/exec/windbgstub-utils.h > create mode 100644 include/exec/windbgstub.h > create mode 100644 stubs/windbgstub.c > create mode 100644 target/i386/windbgstub.c > create mode 100644 windbgstub-utils.c > create mode 100644 windbgstub.c > > -- > Mikhail Abakumov > -- Cheers, Changbin Du
