* [PATCH v3 0/3] RISC-V: KVM: Guest Debug Support - Software Breakpoint Part
@ 2024-03-27 7:55 Chao Du
2024-03-27 7:55 ` [PATCH v3 1/3] RISC-V: KVM: Implement kvm_arch_vcpu_ioctl_set_guest_debug() Chao Du
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Chao Du @ 2024-03-27 7:55 UTC (permalink / raw)
To: kvm, kvm-riscv, anup, atishp, pbonzini, shuah, dbarboza,
paul.walmsley, palmer, aou, haibo1.xu, duchao713
This series implements the "KVM Guset Debug" feature on RISC-V. This is
an existing feature which is already supported by some other arches.
It allows us to debug a RISC-V KVM guest from GDB in host side.
As the first stage, the software breakpoints (ebreak instruction) is
implemented. HW breakpoints support will come later after a synthetically
consideration with the SBI debug trigger extension.
A selftest case was added in this series. Manual test was done on QEMU
RISC-V hypervisor emulator. (add '-s' to enable the gdbserver in QEMU)
This series is based on Linux 6.9-rc1 and also available at:
https://github.com/Du-Chao/kvm-riscv/tree/guest_debug_sw_v3_6.9-rc1
The matched QEMU is available at:
https://github.com/Du-Chao/qemu/tree/riscv_gd_sw
Changes from v2->v3:
- Rebased on Linux 6.9-rc1.
- Use BIT() in the macro definition.
- set/clear the bit EXC_BREAKPOINT explicitly.
- change the testcase name to ebreak_test.
- test the scenario without GUEST_DEBUG. vm_install_exception_handler() is used
thanks to Haibo's patch.
Changes from v1->v2:
- Rebased on Linux 6.8-rc6.
- Maintain a hedeleg in "struct kvm_vcpu_config" for each VCPU.
- Update the HEDELEG csr in kvm_arch_vcpu_load().
Changes from RFC->v1:
- Rebased on Linux 6.8-rc2.
- Merge PATCH1 and PATCH2 into one patch.
- kselftest case added.
v2 link:
https://lore.kernel.org/kvm/20240301013545.10403-1-duchao@eswincomputing.com
v1 link:
https://lore.kernel.org/kvm/20240206074931.22930-1-duchao@eswincomputing.com
RFC link:
https://lore.kernel.org/kvm/20231221095002.7404-1-duchao@eswincomputing.com
Chao Du (3):
RISC-V: KVM: Implement kvm_arch_vcpu_ioctl_set_guest_debug()
RISC-V: KVM: Handle breakpoint exits for VCPU
RISC-V: KVM: selftests: Add ebreak test support
arch/riscv/include/asm/kvm_host.h | 12 +++
arch/riscv/kvm/main.c | 18 +---
arch/riscv/kvm/vcpu.c | 16 +++-
arch/riscv/kvm/vcpu_exit.c | 4 +
arch/riscv/kvm/vm.c | 1 +
tools/testing/selftests/kvm/Makefile | 1 +
.../testing/selftests/kvm/riscv/ebreak_test.c | 84 +++++++++++++++++++
7 files changed, 118 insertions(+), 18 deletions(-)
create mode 100644 tools/testing/selftests/kvm/riscv/ebreak_test.c
--
2.17.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/3] RISC-V: KVM: Implement kvm_arch_vcpu_ioctl_set_guest_debug()
2024-03-27 7:55 [PATCH v3 0/3] RISC-V: KVM: Guest Debug Support - Software Breakpoint Part Chao Du
@ 2024-03-27 7:55 ` Chao Du
2024-04-02 3:55 ` Anup Patel
2024-03-27 7:55 ` [PATCH v3 2/3] RISC-V: KVM: Handle breakpoint exits for VCPU Chao Du
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Chao Du @ 2024-03-27 7:55 UTC (permalink / raw)
To: kvm, kvm-riscv, anup, atishp, pbonzini, shuah, dbarboza,
paul.walmsley, palmer, aou, haibo1.xu, duchao713
kvm_vm_ioctl_check_extension(): Return 1 if KVM_CAP_SET_GUEST_DEBUG is
been checked.
kvm_arch_vcpu_ioctl_set_guest_debug(): Update the guest_debug flags
from userspace accordingly. Route the breakpoint exceptions to HS mode
if the VCPU is being debugged by userspace, by clearing the
corresponding bit in hedeleg.
Initialize the hedeleg configuration in kvm_riscv_vcpu_setup_config().
Write the actual CSR in kvm_arch_vcpu_load().
Signed-off-by: Chao Du <duchao@eswincomputing.com>
---
arch/riscv/include/asm/kvm_host.h | 12 ++++++++++++
arch/riscv/kvm/main.c | 18 ++----------------
arch/riscv/kvm/vcpu.c | 16 ++++++++++++++--
arch/riscv/kvm/vm.c | 1 +
4 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
index 484d04a92fa6..da4ab7e175ff 100644
--- a/arch/riscv/include/asm/kvm_host.h
+++ b/arch/riscv/include/asm/kvm_host.h
@@ -43,6 +43,17 @@
KVM_ARCH_REQ_FLAGS(5, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
#define KVM_REQ_STEAL_UPDATE KVM_ARCH_REQ(6)
+#define KVM_HEDELEG_DEFAULT (BIT(EXC_INST_MISALIGNED) | \
+ BIT(EXC_BREAKPOINT) | \
+ BIT(EXC_SYSCALL) | \
+ BIT(EXC_INST_PAGE_FAULT) | \
+ BIT(EXC_LOAD_PAGE_FAULT) | \
+ BIT(EXC_STORE_PAGE_FAULT))
+
+#define KVM_HIDELEG_DEFAULT (BIT(IRQ_VS_SOFT) | \
+ BIT(IRQ_VS_TIMER) | \
+ BIT(IRQ_VS_EXT))
+
enum kvm_riscv_hfence_type {
KVM_RISCV_HFENCE_UNKNOWN = 0,
KVM_RISCV_HFENCE_GVMA_VMID_GPA,
@@ -169,6 +180,7 @@ struct kvm_vcpu_csr {
struct kvm_vcpu_config {
u64 henvcfg;
u64 hstateen0;
+ unsigned long hedeleg;
};
struct kvm_vcpu_smstateen_csr {
diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
index 225a435d9c9a..bab2ec34cd87 100644
--- a/arch/riscv/kvm/main.c
+++ b/arch/riscv/kvm/main.c
@@ -22,22 +22,8 @@ long kvm_arch_dev_ioctl(struct file *filp,
int kvm_arch_hardware_enable(void)
{
- unsigned long hideleg, hedeleg;
-
- hedeleg = 0;
- hedeleg |= (1UL << EXC_INST_MISALIGNED);
- hedeleg |= (1UL << EXC_BREAKPOINT);
- hedeleg |= (1UL << EXC_SYSCALL);
- hedeleg |= (1UL << EXC_INST_PAGE_FAULT);
- hedeleg |= (1UL << EXC_LOAD_PAGE_FAULT);
- hedeleg |= (1UL << EXC_STORE_PAGE_FAULT);
- csr_write(CSR_HEDELEG, hedeleg);
-
- hideleg = 0;
- hideleg |= (1UL << IRQ_VS_SOFT);
- hideleg |= (1UL << IRQ_VS_TIMER);
- hideleg |= (1UL << IRQ_VS_EXT);
- csr_write(CSR_HIDELEG, hideleg);
+ csr_write(CSR_HEDELEG, KVM_HEDELEG_DEFAULT);
+ csr_write(CSR_HIDELEG, KVM_HIDELEG_DEFAULT);
/* VS should access only the time counter directly. Everything else should trap */
csr_write(CSR_HCOUNTEREN, 0x02);
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index b5ca9f2e98ac..f3c87f0c93ba 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -475,8 +475,15 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
{
- /* TODO; To be implemented later. */
- return -EINVAL;
+ if (dbg->control & KVM_GUESTDBG_ENABLE) {
+ vcpu->guest_debug = dbg->control;
+ vcpu->arch.cfg.hedeleg &= ~BIT(EXC_BREAKPOINT);
+ } else {
+ vcpu->guest_debug = 0;
+ vcpu->arch.cfg.hedeleg |= BIT(EXC_BREAKPOINT);
+ }
+
+ return 0;
}
static void kvm_riscv_vcpu_setup_config(struct kvm_vcpu *vcpu)
@@ -505,6 +512,10 @@ static void kvm_riscv_vcpu_setup_config(struct kvm_vcpu *vcpu)
if (riscv_isa_extension_available(isa, SMSTATEEN))
cfg->hstateen0 |= SMSTATEEN0_SSTATEEN0;
}
+
+ cfg->hedeleg = KVM_HEDELEG_DEFAULT;
+ if (vcpu->guest_debug)
+ cfg->hedeleg &= ~BIT(EXC_BREAKPOINT);
}
void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
@@ -519,6 +530,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
csr_write(CSR_VSEPC, csr->vsepc);
csr_write(CSR_VSCAUSE, csr->vscause);
csr_write(CSR_VSTVAL, csr->vstval);
+ csr_write(CSR_HEDELEG, cfg->hedeleg);
csr_write(CSR_HVIP, csr->hvip);
csr_write(CSR_VSATP, csr->vsatp);
csr_write(CSR_HENVCFG, cfg->henvcfg);
diff --git a/arch/riscv/kvm/vm.c b/arch/riscv/kvm/vm.c
index ce58bc48e5b8..7396b8654f45 100644
--- a/arch/riscv/kvm/vm.c
+++ b/arch/riscv/kvm/vm.c
@@ -186,6 +186,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_READONLY_MEM:
case KVM_CAP_MP_STATE:
case KVM_CAP_IMMEDIATE_EXIT:
+ case KVM_CAP_SET_GUEST_DEBUG:
r = 1;
break;
case KVM_CAP_NR_VCPUS:
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 2/3] RISC-V: KVM: Handle breakpoint exits for VCPU
2024-03-27 7:55 [PATCH v3 0/3] RISC-V: KVM: Guest Debug Support - Software Breakpoint Part Chao Du
2024-03-27 7:55 ` [PATCH v3 1/3] RISC-V: KVM: Implement kvm_arch_vcpu_ioctl_set_guest_debug() Chao Du
@ 2024-03-27 7:55 ` Chao Du
2024-04-02 3:57 ` Anup Patel
2024-03-27 7:55 ` [PATCH v3 3/3] RISC-V: KVM: selftests: Add ebreak test support Chao Du
2024-04-02 3:58 ` [PATCH v3 0/3] RISC-V: KVM: Guest Debug Support - Software Breakpoint Part Anup Patel
3 siblings, 1 reply; 9+ messages in thread
From: Chao Du @ 2024-03-27 7:55 UTC (permalink / raw)
To: kvm, kvm-riscv, anup, atishp, pbonzini, shuah, dbarboza,
paul.walmsley, palmer, aou, haibo1.xu, duchao713
Exit to userspace for breakpoint traps. Set the exit_reason as
KVM_EXIT_DEBUG before exit.
Signed-off-by: Chao Du <duchao@eswincomputing.com>
---
arch/riscv/kvm/vcpu_exit.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c
index 2415722c01b8..5761f95abb60 100644
--- a/arch/riscv/kvm/vcpu_exit.c
+++ b/arch/riscv/kvm/vcpu_exit.c
@@ -204,6 +204,10 @@ int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV)
ret = kvm_riscv_vcpu_sbi_ecall(vcpu, run);
break;
+ case EXC_BREAKPOINT:
+ run->exit_reason = KVM_EXIT_DEBUG;
+ ret = 0;
+ break;
default:
break;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 3/3] RISC-V: KVM: selftests: Add ebreak test support
2024-03-27 7:55 [PATCH v3 0/3] RISC-V: KVM: Guest Debug Support - Software Breakpoint Part Chao Du
2024-03-27 7:55 ` [PATCH v3 1/3] RISC-V: KVM: Implement kvm_arch_vcpu_ioctl_set_guest_debug() Chao Du
2024-03-27 7:55 ` [PATCH v3 2/3] RISC-V: KVM: Handle breakpoint exits for VCPU Chao Du
@ 2024-03-27 7:55 ` Chao Du
2024-03-27 9:00 ` Andrew Jones
2024-04-02 3:58 ` [PATCH v3 0/3] RISC-V: KVM: Guest Debug Support - Software Breakpoint Part Anup Patel
3 siblings, 1 reply; 9+ messages in thread
From: Chao Du @ 2024-03-27 7:55 UTC (permalink / raw)
To: kvm, kvm-riscv, anup, atishp, pbonzini, shuah, dbarboza,
paul.walmsley, palmer, aou, haibo1.xu, duchao713
Initial support for RISC-V KVM ebreak test. Check the exit reason and
the PC when guest debug is enabled. Also to make sure the guest could
handle the ebreak exception without exiting to the VMM when guest debug
is not enabled.
Signed-off-by: Chao Du <duchao@eswincomputing.com>
---
tools/testing/selftests/kvm/Makefile | 1 +
.../testing/selftests/kvm/riscv/ebreak_test.c | 84 +++++++++++++++++++
2 files changed, 85 insertions(+)
create mode 100644 tools/testing/selftests/kvm/riscv/ebreak_test.c
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 741c7dc16afc..7f4430242c9e 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -189,6 +189,7 @@ TEST_GEN_PROGS_s390x += rseq_test
TEST_GEN_PROGS_s390x += set_memory_region_test
TEST_GEN_PROGS_s390x += kvm_binary_stats_test
+TEST_GEN_PROGS_riscv += riscv/ebreak_test
TEST_GEN_PROGS_riscv += arch_timer
TEST_GEN_PROGS_riscv += demand_paging_test
TEST_GEN_PROGS_riscv += dirty_log_test
diff --git a/tools/testing/selftests/kvm/riscv/ebreak_test.c b/tools/testing/selftests/kvm/riscv/ebreak_test.c
new file mode 100644
index 000000000000..4c79c778e026
--- /dev/null
+++ b/tools/testing/selftests/kvm/riscv/ebreak_test.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RISC-V KVM ebreak test.
+ *
+ * Copyright 2024 Beijing ESWIN Computing Technology Co., Ltd.
+ *
+ */
+#include "kvm_util.h"
+
+#define PC(v) ((uint64_t)&(v))
+
+extern unsigned char sw_bp_1, sw_bp_2;
+static volatile uint64_t sw_bp_addr;
+
+static void guest_code(void)
+{
+ /*
+ * nops are inserted to make sure that the "pc += 4" operation is
+ * compatible with the compressed instructions.
+ */
+ asm volatile("sw_bp_1: ebreak\n"
+ "nop\n"
+ "sw_bp_2: ebreak\n"
+ "nop\n");
+ GUEST_ASSERT_EQ(sw_bp_addr, PC(sw_bp_2));
+
+ GUEST_DONE();
+}
+
+static void guest_breakpoint_handler(struct ex_regs *regs)
+{
+ sw_bp_addr = regs->epc;
+ regs->epc += 4;
+}
+
+int main(void)
+{
+ struct kvm_vm *vm;
+ struct kvm_vcpu *vcpu;
+ uint64_t pc;
+ struct kvm_guest_debug debug = {
+ .control = KVM_GUESTDBG_ENABLE,
+ };
+ struct ucall uc;
+
+ TEST_REQUIRE(kvm_has_cap(KVM_CAP_SET_GUEST_DEBUG));
+
+ vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+
+ vm_init_vector_tables(vm);
+ vcpu_init_vector_tables(vcpu);
+ vm_install_exception_handler(vm, EXC_BREAKPOINT,
+ guest_breakpoint_handler);
+
+ /*
+ * Enable the guest debug.
+ * ebreak should exit to the VMM with KVM_EXIT_DEBUG reason.
+ */
+ vcpu_guest_debug_set(vcpu, &debug);
+ vcpu_run(vcpu);
+
+ TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_DEBUG);
+
+ vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.pc), &pc);
+ TEST_ASSERT_EQ(pc, PC(sw_bp_1));
+
+ /* skip sw_bp_1 */
+ vcpu_set_reg(vcpu, RISCV_CORE_REG(regs.pc), pc + 4);
+
+ /*
+ * Disable all debug controls.
+ * Guest should handle the ebreak without exiting to the VMM.
+ */
+ memset(&debug, 0, sizeof(debug));
+ vcpu_guest_debug_set(vcpu, &debug);
+
+ vcpu_run(vcpu);
+
+ TEST_ASSERT_EQ(get_ucall(vcpu, &uc), UCALL_DONE);
+
+ kvm_vm_free(vm);
+
+ return 0;
+}
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] RISC-V: KVM: selftests: Add ebreak test support
2024-03-27 7:55 ` [PATCH v3 3/3] RISC-V: KVM: selftests: Add ebreak test support Chao Du
@ 2024-03-27 9:00 ` Andrew Jones
2024-03-27 10:17 ` Chao Du
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Jones @ 2024-03-27 9:00 UTC (permalink / raw)
To: Chao Du
Cc: kvm, kvm-riscv, anup, atishp, pbonzini, shuah, dbarboza,
paul.walmsley, palmer, aou, haibo1.xu, duchao713
On Wed, Mar 27, 2024 at 07:55:26AM +0000, Chao Du wrote:
> Initial support for RISC-V KVM ebreak test. Check the exit reason and
> the PC when guest debug is enabled. Also to make sure the guest could
> handle the ebreak exception without exiting to the VMM when guest debug
> is not enabled.
>
> Signed-off-by: Chao Du <duchao@eswincomputing.com>
> ---
> tools/testing/selftests/kvm/Makefile | 1 +
> .../testing/selftests/kvm/riscv/ebreak_test.c | 84 +++++++++++++++++++
> 2 files changed, 85 insertions(+)
> create mode 100644 tools/testing/selftests/kvm/riscv/ebreak_test.c
>
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 741c7dc16afc..7f4430242c9e 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -189,6 +189,7 @@ TEST_GEN_PROGS_s390x += rseq_test
> TEST_GEN_PROGS_s390x += set_memory_region_test
> TEST_GEN_PROGS_s390x += kvm_binary_stats_test
>
> +TEST_GEN_PROGS_riscv += riscv/ebreak_test
> TEST_GEN_PROGS_riscv += arch_timer
> TEST_GEN_PROGS_riscv += demand_paging_test
> TEST_GEN_PROGS_riscv += dirty_log_test
> diff --git a/tools/testing/selftests/kvm/riscv/ebreak_test.c b/tools/testing/selftests/kvm/riscv/ebreak_test.c
> new file mode 100644
> index 000000000000..4c79c778e026
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/riscv/ebreak_test.c
> @@ -0,0 +1,84 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RISC-V KVM ebreak test.
> + *
> + * Copyright 2024 Beijing ESWIN Computing Technology Co., Ltd.
> + *
> + */
> +#include "kvm_util.h"
> +
> +#define PC(v) ((uint64_t)&(v))
PC() isn't a good name for the function this is doing. It's getting
the address of a label. LABEL_ADDRESS() would be more appropriate.
> +
> +extern unsigned char sw_bp_1, sw_bp_2;
> +static volatile uint64_t sw_bp_addr;
Drop volatile here and use READ/WRITE_ONCE on sw_bp_addr when reading and
writing it.
> +
> +static void guest_code(void)
> +{
> + /*
> + * nops are inserted to make sure that the "pc += 4" operation is
> + * compatible with the compressed instructions.
> + */
> + asm volatile("sw_bp_1: ebreak\n"
> + "nop\n"
> + "sw_bp_2: ebreak\n"
> + "nop\n");
The nops are fine, but options should work too, something like
asm volatile(
".option push\n"
".option norvc\n"
"sw_bp_1: ebreak\n"
"sw_bp_2: ebreak\n"
".option pop\n"
);
> + GUEST_ASSERT_EQ(sw_bp_addr, PC(sw_bp_2));
> +
> + GUEST_DONE();
> +}
> +
> +static void guest_breakpoint_handler(struct ex_regs *regs)
> +{
> + sw_bp_addr = regs->epc;
> + regs->epc += 4;
> +}
> +
> +int main(void)
> +{
> + struct kvm_vm *vm;
> + struct kvm_vcpu *vcpu;
> + uint64_t pc;
> + struct kvm_guest_debug debug = {
> + .control = KVM_GUESTDBG_ENABLE,
> + };
> + struct ucall uc;
You don't use 'uc', so you can drop it and...
> +
> + TEST_REQUIRE(kvm_has_cap(KVM_CAP_SET_GUEST_DEBUG));
> +
> + vm = vm_create_with_one_vcpu(&vcpu, guest_code);
> +
> + vm_init_vector_tables(vm);
> + vcpu_init_vector_tables(vcpu);
> + vm_install_exception_handler(vm, EXC_BREAKPOINT,
> + guest_breakpoint_handler);
> +
> + /*
> + * Enable the guest debug.
> + * ebreak should exit to the VMM with KVM_EXIT_DEBUG reason.
> + */
> + vcpu_guest_debug_set(vcpu, &debug);
> + vcpu_run(vcpu);
> +
> + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_DEBUG);
> +
> + vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.pc), &pc);
> + TEST_ASSERT_EQ(pc, PC(sw_bp_1));
> +
> + /* skip sw_bp_1 */
> + vcpu_set_reg(vcpu, RISCV_CORE_REG(regs.pc), pc + 4);
> +
> + /*
> + * Disable all debug controls.
> + * Guest should handle the ebreak without exiting to the VMM.
> + */
> + memset(&debug, 0, sizeof(debug));
> + vcpu_guest_debug_set(vcpu, &debug);
> +
> + vcpu_run(vcpu);
> +
> + TEST_ASSERT_EQ(get_ucall(vcpu, &uc), UCALL_DONE);
...call get_ucall() with NULL for the second parameter.
> +
> + kvm_vm_free(vm);
> +
> + return 0;
> +}
> --
> 2.17.1
>
Thanks,
drew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] RISC-V: KVM: selftests: Add ebreak test support
2024-03-27 9:00 ` Andrew Jones
@ 2024-03-27 10:17 ` Chao Du
0 siblings, 0 replies; 9+ messages in thread
From: Chao Du @ 2024-03-27 10:17 UTC (permalink / raw)
To: Andrew Jones
Cc: kvm, kvm-riscv, anup, atishp, pbonzini, shuah, dbarboza,
paul.walmsley, palmer, aou, haibo1.xu, duchao713
Thanks Andrew for all the good suggestions.
Will take them in next revision.
Regards,
Chao
On 2024-03-27 17:00, Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Wed, Mar 27, 2024 at 07:55:26AM +0000, Chao Du wrote:
> > Initial support for RISC-V KVM ebreak test. Check the exit reason and
> > the PC when guest debug is enabled. Also to make sure the guest could
> > handle the ebreak exception without exiting to the VMM when guest debug
> > is not enabled.
> >
> > Signed-off-by: Chao Du <duchao@eswincomputing.com>
> > ---
> > tools/testing/selftests/kvm/Makefile | 1 +
> > .../testing/selftests/kvm/riscv/ebreak_test.c | 84 +++++++++++++++++++
> > 2 files changed, 85 insertions(+)
> > create mode 100644 tools/testing/selftests/kvm/riscv/ebreak_test.c
> >
> > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> > index 741c7dc16afc..7f4430242c9e 100644
> > --- a/tools/testing/selftests/kvm/Makefile
> > +++ b/tools/testing/selftests/kvm/Makefile
> > @@ -189,6 +189,7 @@ TEST_GEN_PROGS_s390x += rseq_test
> > TEST_GEN_PROGS_s390x += set_memory_region_test
> > TEST_GEN_PROGS_s390x += kvm_binary_stats_test
> >
> > +TEST_GEN_PROGS_riscv += riscv/ebreak_test
> > TEST_GEN_PROGS_riscv += arch_timer
> > TEST_GEN_PROGS_riscv += demand_paging_test
> > TEST_GEN_PROGS_riscv += dirty_log_test
> > diff --git a/tools/testing/selftests/kvm/riscv/ebreak_test.c b/tools/testing/selftests/kvm/riscv/ebreak_test.c
> > new file mode 100644
> > index 000000000000..4c79c778e026
> > --- /dev/null
> > +++ b/tools/testing/selftests/kvm/riscv/ebreak_test.c
> > @@ -0,0 +1,84 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * RISC-V KVM ebreak test.
> > + *
> > + * Copyright 2024 Beijing ESWIN Computing Technology Co., Ltd.
> > + *
> > + */
> > +#include "kvm_util.h"
> > +
> > +#define PC(v) ((uint64_t)&(v))
>
> PC() isn't a good name for the function this is doing. It's getting
> the address of a label. LABEL_ADDRESS() would be more appropriate.
>
> > +
> > +extern unsigned char sw_bp_1, sw_bp_2;
> > +static volatile uint64_t sw_bp_addr;
>
> Drop volatile here and use READ/WRITE_ONCE on sw_bp_addr when reading and
> writing it.
>
> > +
> > +static void guest_code(void)
> > +{
> > + /*
> > + * nops are inserted to make sure that the "pc += 4" operation is
> > + * compatible with the compressed instructions.
> > + */
> > + asm volatile("sw_bp_1: ebreak\n"
> > + "nop\n"
> > + "sw_bp_2: ebreak\n"
> > + "nop\n");
>
> The nops are fine, but options should work too, something like
>
> asm volatile(
> ".option push\n"
> ".option norvc\n"
> "sw_bp_1: ebreak\n"
> "sw_bp_2: ebreak\n"
> ".option pop\n"
> );
>
> > + GUEST_ASSERT_EQ(sw_bp_addr, PC(sw_bp_2));
> > +
> > + GUEST_DONE();
> > +}
> > +
> > +static void guest_breakpoint_handler(struct ex_regs *regs)
> > +{
> > + sw_bp_addr = regs->epc;
> > + regs->epc += 4;
> > +}
> > +
> > +int main(void)
> > +{
> > + struct kvm_vm *vm;
> > + struct kvm_vcpu *vcpu;
> > + uint64_t pc;
> > + struct kvm_guest_debug debug = {
> > + .control = KVM_GUESTDBG_ENABLE,
> > + };
> > + struct ucall uc;
>
> You don't use 'uc', so you can drop it and...
>
> > +
> > + TEST_REQUIRE(kvm_has_cap(KVM_CAP_SET_GUEST_DEBUG));
> > +
> > + vm = vm_create_with_one_vcpu(&vcpu, guest_code);
> > +
> > + vm_init_vector_tables(vm);
> > + vcpu_init_vector_tables(vcpu);
> > + vm_install_exception_handler(vm, EXC_BREAKPOINT,
> > + guest_breakpoint_handler);
> > +
> > + /*
> > + * Enable the guest debug.
> > + * ebreak should exit to the VMM with KVM_EXIT_DEBUG reason.
> > + */
> > + vcpu_guest_debug_set(vcpu, &debug);
> > + vcpu_run(vcpu);
> > +
> > + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_DEBUG);
> > +
> > + vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.pc), &pc);
> > + TEST_ASSERT_EQ(pc, PC(sw_bp_1));
> > +
> > + /* skip sw_bp_1 */
> > + vcpu_set_reg(vcpu, RISCV_CORE_REG(regs.pc), pc + 4);
> > +
> > + /*
> > + * Disable all debug controls.
> > + * Guest should handle the ebreak without exiting to the VMM.
> > + */
> > + memset(&debug, 0, sizeof(debug));
> > + vcpu_guest_debug_set(vcpu, &debug);
> > +
> > + vcpu_run(vcpu);
> > +
> > + TEST_ASSERT_EQ(get_ucall(vcpu, &uc), UCALL_DONE);
>
> ...call get_ucall() with NULL for the second parameter.
>
> > +
> > + kvm_vm_free(vm);
> > +
> > + return 0;
> > +}
> > --
> > 2.17.1
> >
>
> Thanks,
> drew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] RISC-V: KVM: Implement kvm_arch_vcpu_ioctl_set_guest_debug()
2024-03-27 7:55 ` [PATCH v3 1/3] RISC-V: KVM: Implement kvm_arch_vcpu_ioctl_set_guest_debug() Chao Du
@ 2024-04-02 3:55 ` Anup Patel
0 siblings, 0 replies; 9+ messages in thread
From: Anup Patel @ 2024-04-02 3:55 UTC (permalink / raw)
To: Chao Du
Cc: kvm, kvm-riscv, atishp, pbonzini, shuah, dbarboza, paul.walmsley,
palmer, aou, haibo1.xu, duchao713
On Wed, Mar 27, 2024 at 1:29 PM Chao Du <duchao@eswincomputing.com> wrote:
>
> kvm_vm_ioctl_check_extension(): Return 1 if KVM_CAP_SET_GUEST_DEBUG is
> been checked.
>
> kvm_arch_vcpu_ioctl_set_guest_debug(): Update the guest_debug flags
> from userspace accordingly. Route the breakpoint exceptions to HS mode
> if the VCPU is being debugged by userspace, by clearing the
> corresponding bit in hedeleg.
>
> Initialize the hedeleg configuration in kvm_riscv_vcpu_setup_config().
> Write the actual CSR in kvm_arch_vcpu_load().
>
> Signed-off-by: Chao Du <duchao@eswincomputing.com>
LGTM.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> arch/riscv/include/asm/kvm_host.h | 12 ++++++++++++
> arch/riscv/kvm/main.c | 18 ++----------------
> arch/riscv/kvm/vcpu.c | 16 ++++++++++++++--
> arch/riscv/kvm/vm.c | 1 +
> 4 files changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
> index 484d04a92fa6..da4ab7e175ff 100644
> --- a/arch/riscv/include/asm/kvm_host.h
> +++ b/arch/riscv/include/asm/kvm_host.h
> @@ -43,6 +43,17 @@
> KVM_ARCH_REQ_FLAGS(5, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
> #define KVM_REQ_STEAL_UPDATE KVM_ARCH_REQ(6)
>
> +#define KVM_HEDELEG_DEFAULT (BIT(EXC_INST_MISALIGNED) | \
> + BIT(EXC_BREAKPOINT) | \
> + BIT(EXC_SYSCALL) | \
> + BIT(EXC_INST_PAGE_FAULT) | \
> + BIT(EXC_LOAD_PAGE_FAULT) | \
> + BIT(EXC_STORE_PAGE_FAULT))
> +
> +#define KVM_HIDELEG_DEFAULT (BIT(IRQ_VS_SOFT) | \
> + BIT(IRQ_VS_TIMER) | \
> + BIT(IRQ_VS_EXT))
> +
> enum kvm_riscv_hfence_type {
> KVM_RISCV_HFENCE_UNKNOWN = 0,
> KVM_RISCV_HFENCE_GVMA_VMID_GPA,
> @@ -169,6 +180,7 @@ struct kvm_vcpu_csr {
> struct kvm_vcpu_config {
> u64 henvcfg;
> u64 hstateen0;
> + unsigned long hedeleg;
> };
>
> struct kvm_vcpu_smstateen_csr {
> diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
> index 225a435d9c9a..bab2ec34cd87 100644
> --- a/arch/riscv/kvm/main.c
> +++ b/arch/riscv/kvm/main.c
> @@ -22,22 +22,8 @@ long kvm_arch_dev_ioctl(struct file *filp,
>
> int kvm_arch_hardware_enable(void)
> {
> - unsigned long hideleg, hedeleg;
> -
> - hedeleg = 0;
> - hedeleg |= (1UL << EXC_INST_MISALIGNED);
> - hedeleg |= (1UL << EXC_BREAKPOINT);
> - hedeleg |= (1UL << EXC_SYSCALL);
> - hedeleg |= (1UL << EXC_INST_PAGE_FAULT);
> - hedeleg |= (1UL << EXC_LOAD_PAGE_FAULT);
> - hedeleg |= (1UL << EXC_STORE_PAGE_FAULT);
> - csr_write(CSR_HEDELEG, hedeleg);
> -
> - hideleg = 0;
> - hideleg |= (1UL << IRQ_VS_SOFT);
> - hideleg |= (1UL << IRQ_VS_TIMER);
> - hideleg |= (1UL << IRQ_VS_EXT);
> - csr_write(CSR_HIDELEG, hideleg);
> + csr_write(CSR_HEDELEG, KVM_HEDELEG_DEFAULT);
> + csr_write(CSR_HIDELEG, KVM_HIDELEG_DEFAULT);
>
> /* VS should access only the time counter directly. Everything else should trap */
> csr_write(CSR_HCOUNTEREN, 0x02);
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index b5ca9f2e98ac..f3c87f0c93ba 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -475,8 +475,15 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> struct kvm_guest_debug *dbg)
> {
> - /* TODO; To be implemented later. */
> - return -EINVAL;
> + if (dbg->control & KVM_GUESTDBG_ENABLE) {
> + vcpu->guest_debug = dbg->control;
> + vcpu->arch.cfg.hedeleg &= ~BIT(EXC_BREAKPOINT);
> + } else {
> + vcpu->guest_debug = 0;
> + vcpu->arch.cfg.hedeleg |= BIT(EXC_BREAKPOINT);
> + }
> +
> + return 0;
> }
>
> static void kvm_riscv_vcpu_setup_config(struct kvm_vcpu *vcpu)
> @@ -505,6 +512,10 @@ static void kvm_riscv_vcpu_setup_config(struct kvm_vcpu *vcpu)
> if (riscv_isa_extension_available(isa, SMSTATEEN))
> cfg->hstateen0 |= SMSTATEEN0_SSTATEEN0;
> }
> +
> + cfg->hedeleg = KVM_HEDELEG_DEFAULT;
> + if (vcpu->guest_debug)
> + cfg->hedeleg &= ~BIT(EXC_BREAKPOINT);
> }
>
> void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> @@ -519,6 +530,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> csr_write(CSR_VSEPC, csr->vsepc);
> csr_write(CSR_VSCAUSE, csr->vscause);
> csr_write(CSR_VSTVAL, csr->vstval);
> + csr_write(CSR_HEDELEG, cfg->hedeleg);
> csr_write(CSR_HVIP, csr->hvip);
> csr_write(CSR_VSATP, csr->vsatp);
> csr_write(CSR_HENVCFG, cfg->henvcfg);
> diff --git a/arch/riscv/kvm/vm.c b/arch/riscv/kvm/vm.c
> index ce58bc48e5b8..7396b8654f45 100644
> --- a/arch/riscv/kvm/vm.c
> +++ b/arch/riscv/kvm/vm.c
> @@ -186,6 +186,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> case KVM_CAP_READONLY_MEM:
> case KVM_CAP_MP_STATE:
> case KVM_CAP_IMMEDIATE_EXIT:
> + case KVM_CAP_SET_GUEST_DEBUG:
> r = 1;
> break;
> case KVM_CAP_NR_VCPUS:
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/3] RISC-V: KVM: Handle breakpoint exits for VCPU
2024-03-27 7:55 ` [PATCH v3 2/3] RISC-V: KVM: Handle breakpoint exits for VCPU Chao Du
@ 2024-04-02 3:57 ` Anup Patel
0 siblings, 0 replies; 9+ messages in thread
From: Anup Patel @ 2024-04-02 3:57 UTC (permalink / raw)
To: Chao Du
Cc: kvm, kvm-riscv, atishp, pbonzini, shuah, dbarboza, paul.walmsley,
palmer, aou, haibo1.xu, duchao713
On Wed, Mar 27, 2024 at 1:29 PM Chao Du <duchao@eswincomputing.com> wrote:
>
> Exit to userspace for breakpoint traps. Set the exit_reason as
> KVM_EXIT_DEBUG before exit.
>
> Signed-off-by: Chao Du <duchao@eswincomputing.com>
I had already given Reviewed-by for this patch. In future, please include
tags obtained in previous patch revision over here otherwise reviewers
end-up wasting time reviewing already reviewed patches.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> arch/riscv/kvm/vcpu_exit.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c
> index 2415722c01b8..5761f95abb60 100644
> --- a/arch/riscv/kvm/vcpu_exit.c
> +++ b/arch/riscv/kvm/vcpu_exit.c
> @@ -204,6 +204,10 @@ int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
> if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV)
> ret = kvm_riscv_vcpu_sbi_ecall(vcpu, run);
> break;
> + case EXC_BREAKPOINT:
> + run->exit_reason = KVM_EXIT_DEBUG;
> + ret = 0;
> + break;
> default:
> break;
> }
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/3] RISC-V: KVM: Guest Debug Support - Software Breakpoint Part
2024-03-27 7:55 [PATCH v3 0/3] RISC-V: KVM: Guest Debug Support - Software Breakpoint Part Chao Du
` (2 preceding siblings ...)
2024-03-27 7:55 ` [PATCH v3 3/3] RISC-V: KVM: selftests: Add ebreak test support Chao Du
@ 2024-04-02 3:58 ` Anup Patel
3 siblings, 0 replies; 9+ messages in thread
From: Anup Patel @ 2024-04-02 3:58 UTC (permalink / raw)
To: Chao Du
Cc: kvm, kvm-riscv, atishp, pbonzini, shuah, dbarboza, paul.walmsley,
palmer, aou, haibo1.xu, duchao713
On Wed, Mar 27, 2024 at 1:29 PM Chao Du <duchao@eswincomputing.com> wrote:
>
> This series implements the "KVM Guset Debug" feature on RISC-V. This is
> an existing feature which is already supported by some other arches.
> It allows us to debug a RISC-V KVM guest from GDB in host side.
>
> As the first stage, the software breakpoints (ebreak instruction) is
> implemented. HW breakpoints support will come later after a synthetically
> consideration with the SBI debug trigger extension.
>
> A selftest case was added in this series. Manual test was done on QEMU
> RISC-V hypervisor emulator. (add '-s' to enable the gdbserver in QEMU)
>
> This series is based on Linux 6.9-rc1 and also available at:
> https://github.com/Du-Chao/kvm-riscv/tree/guest_debug_sw_v3_6.9-rc1
>
> The matched QEMU is available at:
> https://github.com/Du-Chao/qemu/tree/riscv_gd_sw
>
>
> Changes from v2->v3:
> - Rebased on Linux 6.9-rc1.
> - Use BIT() in the macro definition.
> - set/clear the bit EXC_BREAKPOINT explicitly.
> - change the testcase name to ebreak_test.
> - test the scenario without GUEST_DEBUG. vm_install_exception_handler() is used
> thanks to Haibo's patch.
>
> Changes from v1->v2:
> - Rebased on Linux 6.8-rc6.
> - Maintain a hedeleg in "struct kvm_vcpu_config" for each VCPU.
> - Update the HEDELEG csr in kvm_arch_vcpu_load().
>
> Changes from RFC->v1:
> - Rebased on Linux 6.8-rc2.
> - Merge PATCH1 and PATCH2 into one patch.
> - kselftest case added.
>
> v2 link:
> https://lore.kernel.org/kvm/20240301013545.10403-1-duchao@eswincomputing.com
> v1 link:
> https://lore.kernel.org/kvm/20240206074931.22930-1-duchao@eswincomputing.com
> RFC link:
> https://lore.kernel.org/kvm/20231221095002.7404-1-duchao@eswincomputing.com
>
> Chao Du (3):
> RISC-V: KVM: Implement kvm_arch_vcpu_ioctl_set_guest_debug()
> RISC-V: KVM: Handle breakpoint exits for VCPU
> RISC-V: KVM: selftests: Add ebreak test support
Please address Drew's comments on PATCH3 and send v4 so that I
can test this series at my end.
Regards,
Anup
>
> arch/riscv/include/asm/kvm_host.h | 12 +++
> arch/riscv/kvm/main.c | 18 +---
> arch/riscv/kvm/vcpu.c | 16 +++-
> arch/riscv/kvm/vcpu_exit.c | 4 +
> arch/riscv/kvm/vm.c | 1 +
> tools/testing/selftests/kvm/Makefile | 1 +
> .../testing/selftests/kvm/riscv/ebreak_test.c | 84 +++++++++++++++++++
> 7 files changed, 118 insertions(+), 18 deletions(-)
> create mode 100644 tools/testing/selftests/kvm/riscv/ebreak_test.c
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-04-02 3:58 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-27 7:55 [PATCH v3 0/3] RISC-V: KVM: Guest Debug Support - Software Breakpoint Part Chao Du
2024-03-27 7:55 ` [PATCH v3 1/3] RISC-V: KVM: Implement kvm_arch_vcpu_ioctl_set_guest_debug() Chao Du
2024-04-02 3:55 ` Anup Patel
2024-03-27 7:55 ` [PATCH v3 2/3] RISC-V: KVM: Handle breakpoint exits for VCPU Chao Du
2024-04-02 3:57 ` Anup Patel
2024-03-27 7:55 ` [PATCH v3 3/3] RISC-V: KVM: selftests: Add ebreak test support Chao Du
2024-03-27 9:00 ` Andrew Jones
2024-03-27 10:17 ` Chao Du
2024-04-02 3:58 ` [PATCH v3 0/3] RISC-V: KVM: Guest Debug Support - Software Breakpoint Part Anup Patel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.