* [PATCH v1 0/4] target/riscv/kvm: QEMU support for KVM Guest Debug on RISC-V
@ 2024-05-27 2:19 Chao Du
2024-05-27 2:19 ` [PATCH v1 1/4] target/riscv/kvm: add software breakpoints support Chao Du
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Chao Du @ 2024-05-27 2:19 UTC (permalink / raw)
To: qemu-devel, qemu-riscv, pbonzini, alistair23, bin.meng, liweiwei,
dbarboza, zhiwei_liu, palmer, anup, duchao713
This series implements QEMU KVM Guest Debug on RISC-V, with which we
could debug RISC-V KVM guest from the host side, using software
breakpoints.
This series is based on riscv-to-apply.next branch (v9.0.0) and is also
available at:
https://github.com/Du-Chao/qemu-alistair23/tree/riscv-to-apply.next.0524
The corresponding KVM side patches have been merged already:
https://lore.kernel.org/kvm/20240402062628.5425-1-duchao@eswincomputing.com/
A TODO list which will be added later:
1. HW breakpoints support
2. A 'corner case' in which the debug exception is not inserted by the
debugger, need to be re-injected to the guest.
Changes from RFC->v1:
- Rebased on riscv-to-apply.next
- use configs/ definition to conditionalize debug support
RFC link:
https://lore.kernel.org/qemu-riscv/20231221094923.7349-1-duchao@eswincomputing.com/
Chao Du (4):
target/riscv/kvm: add software breakpoints support
target/riscv/kvm: implement kvm_arch_update_guest_debug()
target/riscv/kvm: handle the exit with debug reason
target/riscv/kvm: define TARGET_KVM_HAVE_GUEST_DEBUG
accel/kvm/kvm-all.c | 8 +--
configs/targets/riscv64-softmmu.mak | 1 +
include/sysemu/kvm.h | 6 +-
target/arm/kvm.c | 6 +-
target/i386/kvm/kvm.c | 6 +-
target/mips/kvm.c | 6 +-
target/ppc/kvm.c | 6 +-
target/riscv/kvm/kvm-cpu.c | 101 ++++++++++++++++++++++++++++
target/s390x/kvm/kvm.c | 6 +-
9 files changed, 130 insertions(+), 16 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 1/4] target/riscv/kvm: add software breakpoints support
2024-05-27 2:19 [PATCH v1 0/4] target/riscv/kvm: QEMU support for KVM Guest Debug on RISC-V Chao Du
@ 2024-05-27 2:19 ` Chao Du
2024-05-27 12:00 ` Daniel Henrique Barboza
2024-05-27 15:41 ` Andrew Jones
2024-05-27 2:19 ` [PATCH v1 2/4] target/riscv/kvm: implement kvm_arch_update_guest_debug() Chao Du
` (2 subsequent siblings)
3 siblings, 2 replies; 14+ messages in thread
From: Chao Du @ 2024-05-27 2:19 UTC (permalink / raw)
To: qemu-devel, qemu-riscv, pbonzini, alistair23, bin.meng, liweiwei,
dbarboza, zhiwei_liu, palmer, anup, duchao713
This patch implements insert/remove software breakpoint process:
Add an input parameter for kvm_arch_insert_sw_breakpoint() and
kvm_arch_remove_sw_breakpoint() to pass the length information,
which helps us to know whether it is a RVC instruction.
For some remove cases, we do not have the length info, so we need
to judge by ourselves.
For RISC-V, GDB treats single-step similarly to breakpoint: add a
breakpoint at the next step address, then continue. So this also
works for single-step debugging.
Add some stubs which are necessary for building, and will be
implemented later.
Signed-off-by: Chao Du <duchao@eswincomputing.com>
---
accel/kvm/kvm-all.c | 8 ++--
include/sysemu/kvm.h | 6 ++-
target/arm/kvm.c | 6 ++-
target/i386/kvm/kvm.c | 6 ++-
target/mips/kvm.c | 6 ++-
target/ppc/kvm.c | 6 ++-
target/riscv/kvm/kvm-cpu.c | 79 ++++++++++++++++++++++++++++++++++++++
target/s390x/kvm/kvm.c | 6 ++-
8 files changed, 107 insertions(+), 16 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index c0be9f5eed..d27e77dbb2 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3357,7 +3357,7 @@ int kvm_insert_breakpoint(CPUState *cpu, int type, vaddr addr, vaddr len)
bp = g_new(struct kvm_sw_breakpoint, 1);
bp->pc = addr;
bp->use_count = 1;
- err = kvm_arch_insert_sw_breakpoint(cpu, bp);
+ err = kvm_arch_insert_sw_breakpoint(cpu, bp, len);
if (err) {
g_free(bp);
return err;
@@ -3396,7 +3396,7 @@ int kvm_remove_breakpoint(CPUState *cpu, int type, vaddr addr, vaddr len)
return 0;
}
- err = kvm_arch_remove_sw_breakpoint(cpu, bp);
+ err = kvm_arch_remove_sw_breakpoint(cpu, bp, len);
if (err) {
return err;
}
@@ -3426,10 +3426,10 @@ void kvm_remove_all_breakpoints(CPUState *cpu)
CPUState *tmpcpu;
QTAILQ_FOREACH_SAFE(bp, &s->kvm_sw_breakpoints, entry, next) {
- if (kvm_arch_remove_sw_breakpoint(cpu, bp) != 0) {
+ if (kvm_arch_remove_sw_breakpoint(cpu, bp, 0) != 0) {
/* Try harder to find a CPU that currently sees the breakpoint. */
CPU_FOREACH(tmpcpu) {
- if (kvm_arch_remove_sw_breakpoint(tmpcpu, bp) == 0) {
+ if (kvm_arch_remove_sw_breakpoint(tmpcpu, bp, 0) == 0) {
break;
}
}
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index c31d9c7356..340e094ffb 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -391,9 +391,11 @@ struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *cpu,
int kvm_sw_breakpoints_active(CPUState *cpu);
int kvm_arch_insert_sw_breakpoint(CPUState *cpu,
- struct kvm_sw_breakpoint *bp);
+ struct kvm_sw_breakpoint *bp,
+ vaddr len);
int kvm_arch_remove_sw_breakpoint(CPUState *cpu,
- struct kvm_sw_breakpoint *bp);
+ struct kvm_sw_breakpoint *bp,
+ vaddr len);
int kvm_arch_insert_hw_breakpoint(vaddr addr, vaddr len, int type);
int kvm_arch_remove_hw_breakpoint(vaddr addr, vaddr len, int type);
void kvm_arch_remove_all_hw_breakpoints(void);
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 7cf5cf31de..84593db544 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -2402,7 +2402,8 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
/* C6.6.29 BRK instruction */
static const uint32_t brk_insn = 0xd4200000;
-int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
+ vaddr len)
{
if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) ||
cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&brk_insn, 4, 1)) {
@@ -2411,7 +2412,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
return 0;
}
-int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
+ vaddr len)
{
static uint32_t brk;
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index c5943605ee..6449f796d0 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -4992,7 +4992,8 @@ static int kvm_handle_tpr_access(X86CPU *cpu)
return 1;
}
-int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
+ vaddr len)
{
static const uint8_t int3 = 0xcc;
@@ -5003,7 +5004,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
return 0;
}
-int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
+ vaddr len)
{
uint8_t int3;
diff --git a/target/mips/kvm.c b/target/mips/kvm.c
index a631ab544f..129964cf6d 100644
--- a/target/mips/kvm.c
+++ b/target/mips/kvm.c
@@ -111,13 +111,15 @@ void kvm_mips_reset_vcpu(MIPSCPU *cpu)
DPRINTF("%s\n", __func__);
}
-int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
+ vaddr len)
{
DPRINTF("%s\n", __func__);
return 0;
}
-int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
+ vaddr len)
{
DPRINTF("%s\n", __func__);
return 0;
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 46fccff786..9b76bdaa05 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -1378,7 +1378,8 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env,
return 0;
}
-int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
+ vaddr len)
{
/* Mixed endian case is not handled */
uint32_t sc = debug_inst_opcode;
@@ -1392,7 +1393,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
return 0;
}
-int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
+ vaddr len)
{
uint32_t sc;
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 473416649f..cba55c552d 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1962,3 +1962,82 @@ static const TypeInfo riscv_kvm_cpu_type_infos[] = {
};
DEFINE_TYPES(riscv_kvm_cpu_type_infos)
+
+static const uint32_t ebreak_insn = 0x00100073;
+static const uint16_t c_ebreak_insn = 0x9002;
+
+int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
+ vaddr len)
+{
+ if (len != 4 && len != 2) {
+ return -EINVAL;
+ }
+
+ uint8_t * insn = (len == 4) ? (uint8_t *)&ebreak_insn :
+ (uint8_t *)&c_ebreak_insn;
+
+ if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, len, 0) ||
+ cpu_memory_rw_debug(cs, bp->pc, insn, len, 1)) {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
+ vaddr len)
+{
+ uint8_t length;
+
+ if (len == 4 || len == 2) {
+ length = (uint8_t)len;
+ } else if (len == 0) {
+ /* Need to decide the instruction length in this case. */
+ uint32_t read_4_bytes;
+ uint16_t read_2_bytes;
+
+ if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&read_4_bytes, 4, 0) ||
+ cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&read_2_bytes, 2, 0)) {
+ return -EINVAL;
+ }
+
+ if (read_4_bytes == ebreak_insn) {
+ length = 4;
+ } else if (read_2_bytes == c_ebreak_insn) {
+ length = 2;
+ } else {
+ return -EINVAL;
+ }
+ } else {
+ return -EINVAL;
+ }
+
+ if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn,
+ length, 1)) {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+int kvm_arch_insert_hw_breakpoint(vaddr addr, vaddr len, int type)
+{
+ /* TODO; To be implemented later. */
+ return -EINVAL;
+}
+
+int kvm_arch_remove_hw_breakpoint(vaddr addr, vaddr len, int type)
+{
+ /* TODO; To be implemented later. */
+ return -EINVAL;
+}
+
+void kvm_arch_remove_all_hw_breakpoints(void)
+{
+ /* TODO; To be implemented later. */
+}
+
+void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
+{
+ /* TODO; To be implemented later. */
+}
diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index 1b494ecc20..132952cc84 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -865,7 +865,8 @@ static void determine_sw_breakpoint_instr(void)
}
}
-int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
+ vaddr len)
{
determine_sw_breakpoint_instr();
@@ -877,7 +878,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
return 0;
}
-int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
+ vaddr len)
{
uint8_t t[MAX_ILEN];
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v1 2/4] target/riscv/kvm: implement kvm_arch_update_guest_debug()
2024-05-27 2:19 [PATCH v1 0/4] target/riscv/kvm: QEMU support for KVM Guest Debug on RISC-V Chao Du
2024-05-27 2:19 ` [PATCH v1 1/4] target/riscv/kvm: add software breakpoints support Chao Du
@ 2024-05-27 2:19 ` Chao Du
2024-05-27 12:00 ` Daniel Henrique Barboza
2024-05-27 15:42 ` Andrew Jones
2024-05-27 2:19 ` [PATCH v1 3/4] target/riscv/kvm: handle the exit with debug reason Chao Du
2024-05-27 2:19 ` [PATCH v1 4/4] target/riscv/kvm: define TARGET_KVM_HAVE_GUEST_DEBUG Chao Du
3 siblings, 2 replies; 14+ messages in thread
From: Chao Du @ 2024-05-27 2:19 UTC (permalink / raw)
To: qemu-devel, qemu-riscv, pbonzini, alistair23, bin.meng, liweiwei,
dbarboza, zhiwei_liu, palmer, anup, duchao713
Set the control flag when there are active breakpoints. This will
help KVM to know the status in the userspace.
Signed-off-by: Chao Du <duchao@eswincomputing.com>
---
target/riscv/kvm/kvm-cpu.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index cba55c552d..0bc3348b91 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -2039,5 +2039,7 @@ void kvm_arch_remove_all_hw_breakpoints(void)
void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
{
- /* TODO; To be implemented later. */
+ if (kvm_sw_breakpoints_active(cs)) {
+ dbg->control |= KVM_GUESTDBG_ENABLE;
+ }
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v1 3/4] target/riscv/kvm: handle the exit with debug reason
2024-05-27 2:19 [PATCH v1 0/4] target/riscv/kvm: QEMU support for KVM Guest Debug on RISC-V Chao Du
2024-05-27 2:19 ` [PATCH v1 1/4] target/riscv/kvm: add software breakpoints support Chao Du
2024-05-27 2:19 ` [PATCH v1 2/4] target/riscv/kvm: implement kvm_arch_update_guest_debug() Chao Du
@ 2024-05-27 2:19 ` Chao Du
2024-05-27 12:00 ` Daniel Henrique Barboza
2024-05-27 15:48 ` Andrew Jones
2024-05-27 2:19 ` [PATCH v1 4/4] target/riscv/kvm: define TARGET_KVM_HAVE_GUEST_DEBUG Chao Du
3 siblings, 2 replies; 14+ messages in thread
From: Chao Du @ 2024-05-27 2:19 UTC (permalink / raw)
To: qemu-devel, qemu-riscv, pbonzini, alistair23, bin.meng, liweiwei,
dbarboza, zhiwei_liu, palmer, anup, duchao713
If the breakpoint belongs to the userspace then set the ret value.
Signed-off-by: Chao Du <duchao@eswincomputing.com>
---
target/riscv/kvm/kvm-cpu.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 0bc3348b91..0c45e520b2 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1555,6 +1555,21 @@ static int kvm_riscv_handle_csr(CPUState *cs, struct kvm_run *run)
return ret;
}
+static bool kvm_riscv_handle_debug(CPUState *cs)
+{
+ RISCVCPU *cpu = RISCV_CPU(cs);
+ CPURISCVState *env = &cpu->env;
+
+ /* Ensure PC is synchronised */
+ kvm_cpu_synchronize_state(cs);
+
+ if (kvm_find_sw_breakpoint(cs, env->pc)) {
+ return true;
+ }
+
+ return false;
+}
+
int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
{
int ret = 0;
@@ -1565,6 +1580,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
case KVM_EXIT_RISCV_CSR:
ret = kvm_riscv_handle_csr(cs, run);
break;
+ case KVM_EXIT_DEBUG:
+ if (kvm_riscv_handle_debug(cs)) {
+ ret = EXCP_DEBUG;
+ }
+ break;
default:
qemu_log_mask(LOG_UNIMP, "%s: un-handled exit reason %d\n",
__func__, run->exit_reason);
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v1 4/4] target/riscv/kvm: define TARGET_KVM_HAVE_GUEST_DEBUG
2024-05-27 2:19 [PATCH v1 0/4] target/riscv/kvm: QEMU support for KVM Guest Debug on RISC-V Chao Du
` (2 preceding siblings ...)
2024-05-27 2:19 ` [PATCH v1 3/4] target/riscv/kvm: handle the exit with debug reason Chao Du
@ 2024-05-27 2:19 ` Chao Du
2024-05-27 12:00 ` Daniel Henrique Barboza
2024-05-27 15:50 ` Andrew Jones
3 siblings, 2 replies; 14+ messages in thread
From: Chao Du @ 2024-05-27 2:19 UTC (permalink / raw)
To: qemu-devel, qemu-riscv, pbonzini, alistair23, bin.meng, liweiwei,
dbarboza, zhiwei_liu, palmer, anup, duchao713
To enable the KVM GUEST DEBUG for RISC-V at QEMU side.
Signed-off-by: Chao Du <duchao@eswincomputing.com>
---
configs/targets/riscv64-softmmu.mak | 1 +
1 file changed, 1 insertion(+)
diff --git a/configs/targets/riscv64-softmmu.mak b/configs/targets/riscv64-softmmu.mak
index 7c0e7eeb42..f938cc1ee6 100644
--- a/configs/targets/riscv64-softmmu.mak
+++ b/configs/targets/riscv64-softmmu.mak
@@ -1,5 +1,6 @@
TARGET_ARCH=riscv64
TARGET_BASE_ARCH=riscv
TARGET_SUPPORTS_MTTCG=y
+TARGET_KVM_HAVE_GUEST_DEBUG=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
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/4] target/riscv/kvm: add software breakpoints support
2024-05-27 2:19 ` [PATCH v1 1/4] target/riscv/kvm: add software breakpoints support Chao Du
@ 2024-05-27 12:00 ` Daniel Henrique Barboza
2024-05-27 15:41 ` Andrew Jones
1 sibling, 0 replies; 14+ messages in thread
From: Daniel Henrique Barboza @ 2024-05-27 12:00 UTC (permalink / raw)
To: Chao Du, qemu-devel, qemu-riscv, pbonzini, alistair23, bin.meng,
liweiwei, zhiwei_liu, palmer, anup, duchao713
On 5/26/24 23:19, Chao Du wrote:
> This patch implements insert/remove software breakpoint process:
>
> Add an input parameter for kvm_arch_insert_sw_breakpoint() and
> kvm_arch_remove_sw_breakpoint() to pass the length information,
> which helps us to know whether it is a RVC instruction.
> For some remove cases, we do not have the length info, so we need
> to judge by ourselves.
>
> For RISC-V, GDB treats single-step similarly to breakpoint: add a
> breakpoint at the next step address, then continue. So this also
> works for single-step debugging.
>
> Add some stubs which are necessary for building, and will be
> implemented later.
>
> Signed-off-by: Chao Du <duchao@eswincomputing.com>
> ---
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> accel/kvm/kvm-all.c | 8 ++--
> include/sysemu/kvm.h | 6 ++-
> target/arm/kvm.c | 6 ++-
> target/i386/kvm/kvm.c | 6 ++-
> target/mips/kvm.c | 6 ++-
> target/ppc/kvm.c | 6 ++-
> target/riscv/kvm/kvm-cpu.c | 79 ++++++++++++++++++++++++++++++++++++++
> target/s390x/kvm/kvm.c | 6 ++-
> 8 files changed, 107 insertions(+), 16 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index c0be9f5eed..d27e77dbb2 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -3357,7 +3357,7 @@ int kvm_insert_breakpoint(CPUState *cpu, int type, vaddr addr, vaddr len)
> bp = g_new(struct kvm_sw_breakpoint, 1);
> bp->pc = addr;
> bp->use_count = 1;
> - err = kvm_arch_insert_sw_breakpoint(cpu, bp);
> + err = kvm_arch_insert_sw_breakpoint(cpu, bp, len);
> if (err) {
> g_free(bp);
> return err;
> @@ -3396,7 +3396,7 @@ int kvm_remove_breakpoint(CPUState *cpu, int type, vaddr addr, vaddr len)
> return 0;
> }
>
> - err = kvm_arch_remove_sw_breakpoint(cpu, bp);
> + err = kvm_arch_remove_sw_breakpoint(cpu, bp, len);
> if (err) {
> return err;
> }
> @@ -3426,10 +3426,10 @@ void kvm_remove_all_breakpoints(CPUState *cpu)
> CPUState *tmpcpu;
>
> QTAILQ_FOREACH_SAFE(bp, &s->kvm_sw_breakpoints, entry, next) {
> - if (kvm_arch_remove_sw_breakpoint(cpu, bp) != 0) {
> + if (kvm_arch_remove_sw_breakpoint(cpu, bp, 0) != 0) {
> /* Try harder to find a CPU that currently sees the breakpoint. */
> CPU_FOREACH(tmpcpu) {
> - if (kvm_arch_remove_sw_breakpoint(tmpcpu, bp) == 0) {
> + if (kvm_arch_remove_sw_breakpoint(tmpcpu, bp, 0) == 0) {
> break;
> }
> }
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index c31d9c7356..340e094ffb 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -391,9 +391,11 @@ struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *cpu,
> int kvm_sw_breakpoints_active(CPUState *cpu);
>
> int kvm_arch_insert_sw_breakpoint(CPUState *cpu,
> - struct kvm_sw_breakpoint *bp);
> + struct kvm_sw_breakpoint *bp,
> + vaddr len);
> int kvm_arch_remove_sw_breakpoint(CPUState *cpu,
> - struct kvm_sw_breakpoint *bp);
> + struct kvm_sw_breakpoint *bp,
> + vaddr len);
> int kvm_arch_insert_hw_breakpoint(vaddr addr, vaddr len, int type);
> int kvm_arch_remove_hw_breakpoint(vaddr addr, vaddr len, int type);
> void kvm_arch_remove_all_hw_breakpoints(void);
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 7cf5cf31de..84593db544 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -2402,7 +2402,8 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
> /* C6.6.29 BRK instruction */
> static const uint32_t brk_insn = 0xd4200000;
>
> -int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) ||
> cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&brk_insn, 4, 1)) {
> @@ -2411,7 +2412,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> return 0;
> }
>
> -int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> static uint32_t brk;
>
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index c5943605ee..6449f796d0 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -4992,7 +4992,8 @@ static int kvm_handle_tpr_access(X86CPU *cpu)
> return 1;
> }
>
> -int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> static const uint8_t int3 = 0xcc;
>
> @@ -5003,7 +5004,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> return 0;
> }
>
> -int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> uint8_t int3;
>
> diff --git a/target/mips/kvm.c b/target/mips/kvm.c
> index a631ab544f..129964cf6d 100644
> --- a/target/mips/kvm.c
> +++ b/target/mips/kvm.c
> @@ -111,13 +111,15 @@ void kvm_mips_reset_vcpu(MIPSCPU *cpu)
> DPRINTF("%s\n", __func__);
> }
>
> -int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> DPRINTF("%s\n", __func__);
> return 0;
> }
>
> -int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> DPRINTF("%s\n", __func__);
> return 0;
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 46fccff786..9b76bdaa05 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -1378,7 +1378,8 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env,
> return 0;
> }
>
> -int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> /* Mixed endian case is not handled */
> uint32_t sc = debug_inst_opcode;
> @@ -1392,7 +1393,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> return 0;
> }
>
> -int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> uint32_t sc;
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 473416649f..cba55c552d 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1962,3 +1962,82 @@ static const TypeInfo riscv_kvm_cpu_type_infos[] = {
> };
>
> DEFINE_TYPES(riscv_kvm_cpu_type_infos)
> +
> +static const uint32_t ebreak_insn = 0x00100073;
> +static const uint16_t c_ebreak_insn = 0x9002;
> +
> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> +{
> + if (len != 4 && len != 2) {
> + return -EINVAL;
> + }
> +
> + uint8_t * insn = (len == 4) ? (uint8_t *)&ebreak_insn :
> + (uint8_t *)&c_ebreak_insn;
> +
> + if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, len, 0) ||
> + cpu_memory_rw_debug(cs, bp->pc, insn, len, 1)) {
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> +{
> + uint8_t length;
> +
> + if (len == 4 || len == 2) {
> + length = (uint8_t)len;
> + } else if (len == 0) {
> + /* Need to decide the instruction length in this case. */
> + uint32_t read_4_bytes;
> + uint16_t read_2_bytes;
> +
> + if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&read_4_bytes, 4, 0) ||
> + cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&read_2_bytes, 2, 0)) {
> + return -EINVAL;
> + }
> +
> + if (read_4_bytes == ebreak_insn) {
> + length = 4;
> + } else if (read_2_bytes == c_ebreak_insn) {
> + length = 2;
> + } else {
> + return -EINVAL;
> + }
> + } else {
> + return -EINVAL;
> + }
> +
> + if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn,
> + length, 1)) {
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +int kvm_arch_insert_hw_breakpoint(vaddr addr, vaddr len, int type)
> +{
> + /* TODO; To be implemented later. */
> + return -EINVAL;
> +}
> +
> +int kvm_arch_remove_hw_breakpoint(vaddr addr, vaddr len, int type)
> +{
> + /* TODO; To be implemented later. */
> + return -EINVAL;
> +}
> +
> +void kvm_arch_remove_all_hw_breakpoints(void)
> +{
> + /* TODO; To be implemented later. */
> +}
> +
> +void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
> +{
> + /* TODO; To be implemented later. */
> +}
> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> index 1b494ecc20..132952cc84 100644
> --- a/target/s390x/kvm/kvm.c
> +++ b/target/s390x/kvm/kvm.c
> @@ -865,7 +865,8 @@ static void determine_sw_breakpoint_instr(void)
> }
> }
>
> -int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> determine_sw_breakpoint_instr();
>
> @@ -877,7 +878,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> return 0;
> }
>
> -int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> uint8_t t[MAX_ILEN];
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 2/4] target/riscv/kvm: implement kvm_arch_update_guest_debug()
2024-05-27 2:19 ` [PATCH v1 2/4] target/riscv/kvm: implement kvm_arch_update_guest_debug() Chao Du
@ 2024-05-27 12:00 ` Daniel Henrique Barboza
2024-05-27 15:42 ` Andrew Jones
1 sibling, 0 replies; 14+ messages in thread
From: Daniel Henrique Barboza @ 2024-05-27 12:00 UTC (permalink / raw)
To: Chao Du, qemu-devel, qemu-riscv, pbonzini, alistair23, bin.meng,
liweiwei, zhiwei_liu, palmer, anup, duchao713
On 5/26/24 23:19, Chao Du wrote:
> Set the control flag when there are active breakpoints. This will
> help KVM to know the status in the userspace.
>
> Signed-off-by: Chao Du <duchao@eswincomputing.com>
> ---
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> target/riscv/kvm/kvm-cpu.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index cba55c552d..0bc3348b91 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -2039,5 +2039,7 @@ void kvm_arch_remove_all_hw_breakpoints(void)
>
> void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
> {
> - /* TODO; To be implemented later. */
> + if (kvm_sw_breakpoints_active(cs)) {
> + dbg->control |= KVM_GUESTDBG_ENABLE;
> + }
> }
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 3/4] target/riscv/kvm: handle the exit with debug reason
2024-05-27 2:19 ` [PATCH v1 3/4] target/riscv/kvm: handle the exit with debug reason Chao Du
@ 2024-05-27 12:00 ` Daniel Henrique Barboza
2024-05-27 15:48 ` Andrew Jones
1 sibling, 0 replies; 14+ messages in thread
From: Daniel Henrique Barboza @ 2024-05-27 12:00 UTC (permalink / raw)
To: Chao Du, qemu-devel, qemu-riscv, pbonzini, alistair23, bin.meng,
liweiwei, zhiwei_liu, palmer, anup, duchao713
On 5/26/24 23:19, Chao Du wrote:
> If the breakpoint belongs to the userspace then set the ret value.
>
> Signed-off-by: Chao Du <duchao@eswincomputing.com>
> ---
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> target/riscv/kvm/kvm-cpu.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 0bc3348b91..0c45e520b2 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1555,6 +1555,21 @@ static int kvm_riscv_handle_csr(CPUState *cs, struct kvm_run *run)
> return ret;
> }
>
> +static bool kvm_riscv_handle_debug(CPUState *cs)
> +{
> + RISCVCPU *cpu = RISCV_CPU(cs);
> + CPURISCVState *env = &cpu->env;
> +
> + /* Ensure PC is synchronised */
> + kvm_cpu_synchronize_state(cs);
> +
> + if (kvm_find_sw_breakpoint(cs, env->pc)) {
> + return true;
> + }
> +
> + return false;
> +}
> +
> int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
> {
> int ret = 0;
> @@ -1565,6 +1580,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
> case KVM_EXIT_RISCV_CSR:
> ret = kvm_riscv_handle_csr(cs, run);
> break;
> + case KVM_EXIT_DEBUG:
> + if (kvm_riscv_handle_debug(cs)) {
> + ret = EXCP_DEBUG;
> + }
> + break;
> default:
> qemu_log_mask(LOG_UNIMP, "%s: un-handled exit reason %d\n",
> __func__, run->exit_reason);
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 4/4] target/riscv/kvm: define TARGET_KVM_HAVE_GUEST_DEBUG
2024-05-27 2:19 ` [PATCH v1 4/4] target/riscv/kvm: define TARGET_KVM_HAVE_GUEST_DEBUG Chao Du
@ 2024-05-27 12:00 ` Daniel Henrique Barboza
2024-05-27 15:50 ` Andrew Jones
1 sibling, 0 replies; 14+ messages in thread
From: Daniel Henrique Barboza @ 2024-05-27 12:00 UTC (permalink / raw)
To: Chao Du, qemu-devel, qemu-riscv, pbonzini, alistair23, bin.meng,
liweiwei, zhiwei_liu, palmer, anup, duchao713
On 5/26/24 23:19, Chao Du wrote:
> To enable the KVM GUEST DEBUG for RISC-V at QEMU side.
>
> Signed-off-by: Chao Du <duchao@eswincomputing.com>
> ---
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> configs/targets/riscv64-softmmu.mak | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/configs/targets/riscv64-softmmu.mak b/configs/targets/riscv64-softmmu.mak
> index 7c0e7eeb42..f938cc1ee6 100644
> --- a/configs/targets/riscv64-softmmu.mak
> +++ b/configs/targets/riscv64-softmmu.mak
> @@ -1,5 +1,6 @@
> TARGET_ARCH=riscv64
> TARGET_BASE_ARCH=riscv
> TARGET_SUPPORTS_MTTCG=y
> +TARGET_KVM_HAVE_GUEST_DEBUG=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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/4] target/riscv/kvm: add software breakpoints support
2024-05-27 2:19 ` [PATCH v1 1/4] target/riscv/kvm: add software breakpoints support Chao Du
2024-05-27 12:00 ` Daniel Henrique Barboza
@ 2024-05-27 15:41 ` Andrew Jones
2024-05-28 2:43 ` Chao Du
1 sibling, 1 reply; 14+ messages in thread
From: Andrew Jones @ 2024-05-27 15:41 UTC (permalink / raw)
To: Chao Du
Cc: qemu-devel, qemu-riscv, pbonzini, alistair23, bin.meng, liweiwei,
dbarboza, zhiwei_liu, palmer, anup, duchao713
On Mon, May 27, 2024 at 02:19:13AM GMT, Chao Du wrote:
> This patch implements insert/remove software breakpoint process:
>
> Add an input parameter for kvm_arch_insert_sw_breakpoint() and
> kvm_arch_remove_sw_breakpoint() to pass the length information,
> which helps us to know whether it is a RVC instruction.
> For some remove cases, we do not have the length info, so we need
> to judge by ourselves.
>
> For RISC-V, GDB treats single-step similarly to breakpoint: add a
> breakpoint at the next step address, then continue. So this also
> works for single-step debugging.
>
> Add some stubs which are necessary for building, and will be
> implemented later.
>
> Signed-off-by: Chao Du <duchao@eswincomputing.com>
> ---
> accel/kvm/kvm-all.c | 8 ++--
> include/sysemu/kvm.h | 6 ++-
> target/arm/kvm.c | 6 ++-
> target/i386/kvm/kvm.c | 6 ++-
> target/mips/kvm.c | 6 ++-
> target/ppc/kvm.c | 6 ++-
> target/riscv/kvm/kvm-cpu.c | 79 ++++++++++++++++++++++++++++++++++++++
> target/s390x/kvm/kvm.c | 6 ++-
> 8 files changed, 107 insertions(+), 16 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index c0be9f5eed..d27e77dbb2 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -3357,7 +3357,7 @@ int kvm_insert_breakpoint(CPUState *cpu, int type, vaddr addr, vaddr len)
> bp = g_new(struct kvm_sw_breakpoint, 1);
> bp->pc = addr;
> bp->use_count = 1;
> - err = kvm_arch_insert_sw_breakpoint(cpu, bp);
> + err = kvm_arch_insert_sw_breakpoint(cpu, bp, len);
> if (err) {
> g_free(bp);
> return err;
> @@ -3396,7 +3396,7 @@ int kvm_remove_breakpoint(CPUState *cpu, int type, vaddr addr, vaddr len)
> return 0;
> }
>
> - err = kvm_arch_remove_sw_breakpoint(cpu, bp);
> + err = kvm_arch_remove_sw_breakpoint(cpu, bp, len);
> if (err) {
> return err;
> }
> @@ -3426,10 +3426,10 @@ void kvm_remove_all_breakpoints(CPUState *cpu)
> CPUState *tmpcpu;
>
> QTAILQ_FOREACH_SAFE(bp, &s->kvm_sw_breakpoints, entry, next) {
> - if (kvm_arch_remove_sw_breakpoint(cpu, bp) != 0) {
> + if (kvm_arch_remove_sw_breakpoint(cpu, bp, 0) != 0) {
> /* Try harder to find a CPU that currently sees the breakpoint. */
> CPU_FOREACH(tmpcpu) {
> - if (kvm_arch_remove_sw_breakpoint(tmpcpu, bp) == 0) {
> + if (kvm_arch_remove_sw_breakpoint(tmpcpu, bp, 0) == 0) {
It's not nice to need to add 'len' to all arch insert/remove sw breakpoint
implementations, and the fact we have to pass zero sometimes implies it's
not the right approach.
> break;
> }
> }
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index c31d9c7356..340e094ffb 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -391,9 +391,11 @@ struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *cpu,
> int kvm_sw_breakpoints_active(CPUState *cpu);
>
> int kvm_arch_insert_sw_breakpoint(CPUState *cpu,
> - struct kvm_sw_breakpoint *bp);
> + struct kvm_sw_breakpoint *bp,
> + vaddr len);
> int kvm_arch_remove_sw_breakpoint(CPUState *cpu,
> - struct kvm_sw_breakpoint *bp);
> + struct kvm_sw_breakpoint *bp,
> + vaddr len);
> int kvm_arch_insert_hw_breakpoint(vaddr addr, vaddr len, int type);
> int kvm_arch_remove_hw_breakpoint(vaddr addr, vaddr len, int type);
> void kvm_arch_remove_all_hw_breakpoints(void);
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 7cf5cf31de..84593db544 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -2402,7 +2402,8 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
> /* C6.6.29 BRK instruction */
> static const uint32_t brk_insn = 0xd4200000;
>
> -int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) ||
> cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&brk_insn, 4, 1)) {
> @@ -2411,7 +2412,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> return 0;
> }
>
> -int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> static uint32_t brk;
>
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index c5943605ee..6449f796d0 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -4992,7 +4992,8 @@ static int kvm_handle_tpr_access(X86CPU *cpu)
> return 1;
> }
>
> -int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> static const uint8_t int3 = 0xcc;
>
> @@ -5003,7 +5004,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> return 0;
> }
>
> -int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> uint8_t int3;
>
> diff --git a/target/mips/kvm.c b/target/mips/kvm.c
> index a631ab544f..129964cf6d 100644
> --- a/target/mips/kvm.c
> +++ b/target/mips/kvm.c
> @@ -111,13 +111,15 @@ void kvm_mips_reset_vcpu(MIPSCPU *cpu)
> DPRINTF("%s\n", __func__);
> }
>
> -int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> DPRINTF("%s\n", __func__);
> return 0;
> }
>
> -int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> DPRINTF("%s\n", __func__);
> return 0;
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 46fccff786..9b76bdaa05 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -1378,7 +1378,8 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env,
> return 0;
> }
>
> -int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> /* Mixed endian case is not handled */
> uint32_t sc = debug_inst_opcode;
> @@ -1392,7 +1393,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> return 0;
> }
>
> -int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> uint32_t sc;
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 473416649f..cba55c552d 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1962,3 +1962,82 @@ static const TypeInfo riscv_kvm_cpu_type_infos[] = {
> };
>
> DEFINE_TYPES(riscv_kvm_cpu_type_infos)
> +
> +static const uint32_t ebreak_insn = 0x00100073;
> +static const uint16_t c_ebreak_insn = 0x9002;
> +
> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> +{
> + if (len != 4 && len != 2) {
> + return -EINVAL;
> + }
> +
> + uint8_t * insn = (len == 4) ? (uint8_t *)&ebreak_insn :
> + (uint8_t *)&c_ebreak_insn;
> +
> + if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, len, 0) ||
> + cpu_memory_rw_debug(cs, bp->pc, insn, len, 1)) {
> + return -EINVAL;
> + }
We can also read 2 bytes. So how about
if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 2, 0)) {
return -EINVAL;
}
if ((bp->saved_insn & 0x3) == 0x3) {
if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) ||
cpu_memory_rw_debug(cs, bp->pc, &ebreak_insn, 4, 1)) {
return -EINVAL;
}
} else {
if (cpu_memory_rw_debug(cs, bp->pc, &c_ebreak_insn, 2, 1)) {
return -EINVAL;
}
}
> +
> + return 0;
> +}
> +
> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> +{
> + uint8_t length;
> +
> + if (len == 4 || len == 2) {
> + length = (uint8_t)len;
> + } else if (len == 0) {
> + /* Need to decide the instruction length in this case. */
> + uint32_t read_4_bytes;
> + uint16_t read_2_bytes;
> +
> + if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&read_4_bytes, 4, 0) ||
> + cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&read_2_bytes, 2, 0)) {
> + return -EINVAL;
> + }
> +
> + if (read_4_bytes == ebreak_insn) {
> + length = 4;
> + } else if (read_2_bytes == c_ebreak_insn) {
> + length = 2;
> + } else {
> + return -EINVAL;
> + }
> + } else {
> + return -EINVAL;
> + }
> +
> + if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn,
> + length, 1)) {
> + return -EINVAL;
> + }
Also no need need for 'len'.
uint16_t ebreak;
if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&ebreak, 2, 0)) {
return -EINVAL;
}
if ((ebreak & 0x3) == 0x3) {
if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 1)) {
return -EINVAL;
}
} else {
if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 2, 1)) {
return -EINVAL;
}
}
Thanks,
drew
> +
> + return 0;
> +}
> +
> +int kvm_arch_insert_hw_breakpoint(vaddr addr, vaddr len, int type)
> +{
> + /* TODO; To be implemented later. */
> + return -EINVAL;
> +}
> +
> +int kvm_arch_remove_hw_breakpoint(vaddr addr, vaddr len, int type)
> +{
> + /* TODO; To be implemented later. */
> + return -EINVAL;
> +}
> +
> +void kvm_arch_remove_all_hw_breakpoints(void)
> +{
> + /* TODO; To be implemented later. */
> +}
> +
> +void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
> +{
> + /* TODO; To be implemented later. */
> +}
> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> index 1b494ecc20..132952cc84 100644
> --- a/target/s390x/kvm/kvm.c
> +++ b/target/s390x/kvm/kvm.c
> @@ -865,7 +865,8 @@ static void determine_sw_breakpoint_instr(void)
> }
> }
>
> -int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> determine_sw_breakpoint_instr();
>
> @@ -877,7 +878,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> return 0;
> }
>
> -int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> + vaddr len)
> {
> uint8_t t[MAX_ILEN];
>
> --
> 2.17.1
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 2/4] target/riscv/kvm: implement kvm_arch_update_guest_debug()
2024-05-27 2:19 ` [PATCH v1 2/4] target/riscv/kvm: implement kvm_arch_update_guest_debug() Chao Du
2024-05-27 12:00 ` Daniel Henrique Barboza
@ 2024-05-27 15:42 ` Andrew Jones
1 sibling, 0 replies; 14+ messages in thread
From: Andrew Jones @ 2024-05-27 15:42 UTC (permalink / raw)
To: Chao Du
Cc: qemu-devel, qemu-riscv, pbonzini, alistair23, bin.meng, liweiwei,
dbarboza, zhiwei_liu, palmer, anup, duchao713
On Mon, May 27, 2024 at 02:19:14AM GMT, Chao Du wrote:
> Set the control flag when there are active breakpoints. This will
> help KVM to know the status in the userspace.
>
> Signed-off-by: Chao Du <duchao@eswincomputing.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index cba55c552d..0bc3348b91 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -2039,5 +2039,7 @@ void kvm_arch_remove_all_hw_breakpoints(void)
>
> void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
> {
> - /* TODO; To be implemented later. */
> + if (kvm_sw_breakpoints_active(cs)) {
> + dbg->control |= KVM_GUESTDBG_ENABLE;
> + }
> }
> --
> 2.17.1
>
>
Should squash this into the previous patch.
Thanks,
drew
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 3/4] target/riscv/kvm: handle the exit with debug reason
2024-05-27 2:19 ` [PATCH v1 3/4] target/riscv/kvm: handle the exit with debug reason Chao Du
2024-05-27 12:00 ` Daniel Henrique Barboza
@ 2024-05-27 15:48 ` Andrew Jones
1 sibling, 0 replies; 14+ messages in thread
From: Andrew Jones @ 2024-05-27 15:48 UTC (permalink / raw)
To: Chao Du
Cc: qemu-devel, qemu-riscv, pbonzini, alistair23, bin.meng, liweiwei,
dbarboza, zhiwei_liu, palmer, anup, duchao713
On Mon, May 27, 2024 at 02:19:15AM GMT, Chao Du wrote:
> If the breakpoint belongs to the userspace then set the ret value.
>
> Signed-off-by: Chao Du <duchao@eswincomputing.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 0bc3348b91..0c45e520b2 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1555,6 +1555,21 @@ static int kvm_riscv_handle_csr(CPUState *cs, struct kvm_run *run)
> return ret;
> }
>
> +static bool kvm_riscv_handle_debug(CPUState *cs)
> +{
> + RISCVCPU *cpu = RISCV_CPU(cs);
> + CPURISCVState *env = &cpu->env;
> +
> + /* Ensure PC is synchronised */
> + kvm_cpu_synchronize_state(cs);
> +
> + if (kvm_find_sw_breakpoint(cs, env->pc)) {
> + return true;
> + }
> +
> + return false;
> +}
> +
> int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
> {
> int ret = 0;
> @@ -1565,6 +1580,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
> case KVM_EXIT_RISCV_CSR:
> ret = kvm_riscv_handle_csr(cs, run);
> break;
> + case KVM_EXIT_DEBUG:
> + if (kvm_riscv_handle_debug(cs)) {
> + ret = EXCP_DEBUG;
> + }
> + break;
> default:
> qemu_log_mask(LOG_UNIMP, "%s: un-handled exit reason %d\n",
> __func__, run->exit_reason);
> --
> 2.17.1
>
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 4/4] target/riscv/kvm: define TARGET_KVM_HAVE_GUEST_DEBUG
2024-05-27 2:19 ` [PATCH v1 4/4] target/riscv/kvm: define TARGET_KVM_HAVE_GUEST_DEBUG Chao Du
2024-05-27 12:00 ` Daniel Henrique Barboza
@ 2024-05-27 15:50 ` Andrew Jones
1 sibling, 0 replies; 14+ messages in thread
From: Andrew Jones @ 2024-05-27 15:50 UTC (permalink / raw)
To: Chao Du
Cc: qemu-devel, qemu-riscv, pbonzini, alistair23, bin.meng, liweiwei,
dbarboza, zhiwei_liu, palmer, anup, duchao713
On Mon, May 27, 2024 at 02:19:16AM GMT, Chao Du wrote:
> To enable the KVM GUEST DEBUG for RISC-V at QEMU side.
>
> Signed-off-by: Chao Du <duchao@eswincomputing.com>
> ---
> configs/targets/riscv64-softmmu.mak | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/configs/targets/riscv64-softmmu.mak b/configs/targets/riscv64-softmmu.mak
> index 7c0e7eeb42..f938cc1ee6 100644
> --- a/configs/targets/riscv64-softmmu.mak
> +++ b/configs/targets/riscv64-softmmu.mak
> @@ -1,5 +1,6 @@
> TARGET_ARCH=riscv64
> TARGET_BASE_ARCH=riscv
> TARGET_SUPPORTS_MTTCG=y
> +TARGET_KVM_HAVE_GUEST_DEBUG=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
> --
> 2.17.1
>
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/4] target/riscv/kvm: add software breakpoints support
2024-05-27 15:41 ` Andrew Jones
@ 2024-05-28 2:43 ` Chao Du
0 siblings, 0 replies; 14+ messages in thread
From: Chao Du @ 2024-05-28 2:43 UTC (permalink / raw)
To: Andrew Jones
Cc: qemu-devel, qemu-riscv, pbonzini, alistair23, bin.meng, liweiwei,
dbarboza, zhiwei_liu, palmer, anup, duchao713
On 2024-05-27 23:41, Andrew Jones <ajones@ventanamicro.com> wrote:
> On Mon, May 27, 2024 at 02:19:13AM GMT, Chao Du wrote:
> > This patch implements insert/remove software breakpoint process:
> >
> > Add an input parameter for kvm_arch_insert_sw_breakpoint() and
> > kvm_arch_remove_sw_breakpoint() to pass the length information,
> > which helps us to know whether it is a RVC instruction.
> > For some remove cases, we do not have the length info, so we need
> > to judge by ourselves.
> >
> > For RISC-V, GDB treats single-step similarly to breakpoint: add a
> > breakpoint at the next step address, then continue. So this also
> > works for single-step debugging.
> >
> > Add some stubs which are necessary for building, and will be
> > implemented later.
> >
> > Signed-off-by: Chao Du <duchao@eswincomputing.com>
> > ---
> > accel/kvm/kvm-all.c | 8 ++--
> > include/sysemu/kvm.h | 6 ++-
> > target/arm/kvm.c | 6 ++-
> > target/i386/kvm/kvm.c | 6 ++-
> > target/mips/kvm.c | 6 ++-
> > target/ppc/kvm.c | 6 ++-
> > target/riscv/kvm/kvm-cpu.c | 79 ++++++++++++++++++++++++++++++++++++++
> > target/s390x/kvm/kvm.c | 6 ++-
> > 8 files changed, 107 insertions(+), 16 deletions(-)
> >
> > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> > index c0be9f5eed..d27e77dbb2 100644
> > --- a/accel/kvm/kvm-all.c
> > +++ b/accel/kvm/kvm-all.c
> > @@ -3357,7 +3357,7 @@ int kvm_insert_breakpoint(CPUState *cpu, int type, vaddr addr, vaddr len)
> > bp = g_new(struct kvm_sw_breakpoint, 1);
> > bp->pc = addr;
> > bp->use_count = 1;
> > - err = kvm_arch_insert_sw_breakpoint(cpu, bp);
> > + err = kvm_arch_insert_sw_breakpoint(cpu, bp, len);
> > if (err) {
> > g_free(bp);
> > return err;
> > @@ -3396,7 +3396,7 @@ int kvm_remove_breakpoint(CPUState *cpu, int type, vaddr addr, vaddr len)
> > return 0;
> > }
> >
> > - err = kvm_arch_remove_sw_breakpoint(cpu, bp);
> > + err = kvm_arch_remove_sw_breakpoint(cpu, bp, len);
> > if (err) {
> > return err;
> > }
> > @@ -3426,10 +3426,10 @@ void kvm_remove_all_breakpoints(CPUState *cpu)
> > CPUState *tmpcpu;
> >
> > QTAILQ_FOREACH_SAFE(bp, &s->kvm_sw_breakpoints, entry, next) {
> > - if (kvm_arch_remove_sw_breakpoint(cpu, bp) != 0) {
> > + if (kvm_arch_remove_sw_breakpoint(cpu, bp, 0) != 0) {
> > /* Try harder to find a CPU that currently sees the breakpoint. */
> > CPU_FOREACH(tmpcpu) {
> > - if (kvm_arch_remove_sw_breakpoint(tmpcpu, bp) == 0) {
> > + if (kvm_arch_remove_sw_breakpoint(tmpcpu, bp, 0) == 0) {
>
> It's not nice to need to add 'len' to all arch insert/remove sw breakpoint
> implementations, and the fact we have to pass zero sometimes implies it's
> not the right approach.
Actually, I've considered checking the instruction length from the tail two
bits, as you suggested. But it may bring one additional memory read.
So I turned to use the length information from gdb. In most cases, we can
get the exact length and read/write accordingly. But the side effect is we
need to add an input parameter and hence all arch implementations need to
be adapted.
I chose the second way after a 'balance'.
Now I think the first one may be a 'cleaner' solution.
Will send the V2 series later.
Thanks.
Chao
>
> > break;
> > }
> > }
> > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> > index c31d9c7356..340e094ffb 100644
> > --- a/include/sysemu/kvm.h
> > +++ b/include/sysemu/kvm.h
> > @@ -391,9 +391,11 @@ struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *cpu,
> > int kvm_sw_breakpoints_active(CPUState *cpu);
> >
> > int kvm_arch_insert_sw_breakpoint(CPUState *cpu,
> > - struct kvm_sw_breakpoint *bp);
> > + struct kvm_sw_breakpoint *bp,
> > + vaddr len);
> > int kvm_arch_remove_sw_breakpoint(CPUState *cpu,
> > - struct kvm_sw_breakpoint *bp);
> > + struct kvm_sw_breakpoint *bp,
> > + vaddr len);
> > int kvm_arch_insert_hw_breakpoint(vaddr addr, vaddr len, int type);
> > int kvm_arch_remove_hw_breakpoint(vaddr addr, vaddr len, int type);
> > void kvm_arch_remove_all_hw_breakpoints(void);
> > diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> > index 7cf5cf31de..84593db544 100644
> > --- a/target/arm/kvm.c
> > +++ b/target/arm/kvm.c
> > @@ -2402,7 +2402,8 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
> > /* C6.6.29 BRK instruction */
> > static const uint32_t brk_insn = 0xd4200000;
> >
> > -int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> > +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> > + vaddr len)
> > {
> > if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) ||
> > cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&brk_insn, 4, 1)) {
> > @@ -2411,7 +2412,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> > return 0;
> > }
> >
> > -int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> > +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> > + vaddr len)
> > {
> > static uint32_t brk;
> >
> > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> > index c5943605ee..6449f796d0 100644
> > --- a/target/i386/kvm/kvm.c
> > +++ b/target/i386/kvm/kvm.c
> > @@ -4992,7 +4992,8 @@ static int kvm_handle_tpr_access(X86CPU *cpu)
> > return 1;
> > }
> >
> > -int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> > +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> > + vaddr len)
> > {
> > static const uint8_t int3 = 0xcc;
> >
> > @@ -5003,7 +5004,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> > return 0;
> > }
> >
> > -int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> > +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> > + vaddr len)
> > {
> > uint8_t int3;
> >
> > diff --git a/target/mips/kvm.c b/target/mips/kvm.c
> > index a631ab544f..129964cf6d 100644
> > --- a/target/mips/kvm.c
> > +++ b/target/mips/kvm.c
> > @@ -111,13 +111,15 @@ void kvm_mips_reset_vcpu(MIPSCPU *cpu)
> > DPRINTF("%s\n", __func__);
> > }
> >
> > -int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> > +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> > + vaddr len)
> > {
> > DPRINTF("%s\n", __func__);
> > return 0;
> > }
> >
> > -int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> > +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> > + vaddr len)
> > {
> > DPRINTF("%s\n", __func__);
> > return 0;
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index 46fccff786..9b76bdaa05 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -1378,7 +1378,8 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env,
> > return 0;
> > }
> >
> > -int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> > +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> > + vaddr len)
> > {
> > /* Mixed endian case is not handled */
> > uint32_t sc = debug_inst_opcode;
> > @@ -1392,7 +1393,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> > return 0;
> > }
> >
> > -int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> > +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> > + vaddr len)
> > {
> > uint32_t sc;
> >
> > diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> > index 473416649f..cba55c552d 100644
> > --- a/target/riscv/kvm/kvm-cpu.c
> > +++ b/target/riscv/kvm/kvm-cpu.c
> > @@ -1962,3 +1962,82 @@ static const TypeInfo riscv_kvm_cpu_type_infos[] = {
> > };
> >
> > DEFINE_TYPES(riscv_kvm_cpu_type_infos)
> > +
> > +static const uint32_t ebreak_insn = 0x00100073;
> > +static const uint16_t c_ebreak_insn = 0x9002;
> > +
> > +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> > + vaddr len)
> > +{
> > + if (len != 4 && len != 2) {
> > + return -EINVAL;
> > + }
> > +
> > + uint8_t * insn = (len == 4) ? (uint8_t *)&ebreak_insn :
> > + (uint8_t *)&c_ebreak_insn;
> > +
> > + if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, len, 0) ||
> > + cpu_memory_rw_debug(cs, bp->pc, insn, len, 1)) {
> > + return -EINVAL;
> > + }
>
> We can also read 2 bytes. So how about
>
> if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 2, 0)) {
> return -EINVAL;
> }
>
> if ((bp->saved_insn & 0x3) == 0x3) {
> if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) ||
> cpu_memory_rw_debug(cs, bp->pc, &ebreak_insn, 4, 1)) {
> return -EINVAL;
> }
> } else {
> if (cpu_memory_rw_debug(cs, bp->pc, &c_ebreak_insn, 2, 1)) {
> return -EINVAL;
> }
> }
>
> > +
> > + return 0;
> > +}
> > +
> > +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> > + vaddr len)
> > +{
> > + uint8_t length;
> > +
> > + if (len == 4 || len == 2) {
> > + length = (uint8_t)len;
> > + } else if (len == 0) {
> > + /* Need to decide the instruction length in this case. */
> > + uint32_t read_4_bytes;
> > + uint16_t read_2_bytes;
> > +
> > + if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&read_4_bytes, 4, 0) ||
> > + cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&read_2_bytes, 2, 0)) {
> > + return -EINVAL;
> > + }
> > +
> > + if (read_4_bytes == ebreak_insn) {
> > + length = 4;
> > + } else if (read_2_bytes == c_ebreak_insn) {
> > + length = 2;
> > + } else {
> > + return -EINVAL;
> > + }
> > + } else {
> > + return -EINVAL;
> > + }
> > +
> > + if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn,
> > + length, 1)) {
> > + return -EINVAL;
> > + }
>
> Also no need need for 'len'.
>
> uint16_t ebreak;
>
> if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&ebreak, 2, 0)) {
> return -EINVAL;
> }
>
> if ((ebreak & 0x3) == 0x3) {
> if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 1)) {
> return -EINVAL;
> }
> } else {
> if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 2, 1)) {
> return -EINVAL;
> }
> }
>
> Thanks,
> drew
>
> > +
> > + return 0;
> > +}
> > +
> > +int kvm_arch_insert_hw_breakpoint(vaddr addr, vaddr len, int type)
> > +{
> > + /* TODO; To be implemented later. */
> > + return -EINVAL;
> > +}
> > +
> > +int kvm_arch_remove_hw_breakpoint(vaddr addr, vaddr len, int type)
> > +{
> > + /* TODO; To be implemented later. */
> > + return -EINVAL;
> > +}
> > +
> > +void kvm_arch_remove_all_hw_breakpoints(void)
> > +{
> > + /* TODO; To be implemented later. */
> > +}
> > +
> > +void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
> > +{
> > + /* TODO; To be implemented later. */
> > +}
> > diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> > index 1b494ecc20..132952cc84 100644
> > --- a/target/s390x/kvm/kvm.c
> > +++ b/target/s390x/kvm/kvm.c
> > @@ -865,7 +865,8 @@ static void determine_sw_breakpoint_instr(void)
> > }
> > }
> >
> > -int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> > +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> > + vaddr len)
> > {
> > determine_sw_breakpoint_instr();
> >
> > @@ -877,7 +878,8 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> > return 0;
> > }
> >
> > -int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> > +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp,
> > + vaddr len)
> > {
> > uint8_t t[MAX_ILEN];
> >
> > --
> > 2.17.1
> >
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-05-28 2:47 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-27 2:19 [PATCH v1 0/4] target/riscv/kvm: QEMU support for KVM Guest Debug on RISC-V Chao Du
2024-05-27 2:19 ` [PATCH v1 1/4] target/riscv/kvm: add software breakpoints support Chao Du
2024-05-27 12:00 ` Daniel Henrique Barboza
2024-05-27 15:41 ` Andrew Jones
2024-05-28 2:43 ` Chao Du
2024-05-27 2:19 ` [PATCH v1 2/4] target/riscv/kvm: implement kvm_arch_update_guest_debug() Chao Du
2024-05-27 12:00 ` Daniel Henrique Barboza
2024-05-27 15:42 ` Andrew Jones
2024-05-27 2:19 ` [PATCH v1 3/4] target/riscv/kvm: handle the exit with debug reason Chao Du
2024-05-27 12:00 ` Daniel Henrique Barboza
2024-05-27 15:48 ` Andrew Jones
2024-05-27 2:19 ` [PATCH v1 4/4] target/riscv/kvm: define TARGET_KVM_HAVE_GUEST_DEBUG Chao Du
2024-05-27 12:00 ` Daniel Henrique Barboza
2024-05-27 15:50 ` Andrew Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).