kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] KVM: arm64: Fix bugs of single-step execution enabled by userspace
@ 2022-09-17  1:05 Reiji Watanabe
  2022-09-17  1:05 ` [PATCH v2 1/4] KVM: arm64: Preserve PSTATE.SS for the guest while single-step is enabled Reiji Watanabe
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Reiji Watanabe @ 2022-09-17  1:05 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

This series fixes two bugs of single-step execution enabled by
userspace, and add a test case for KVM_GUESTDBG_SINGLESTEP to
the debug-exception test to verify the single-step behavior.

Patch 1 fixes a bug that KVM might unintentionally change PSTATE.SS
for the guest when single-step execution is enabled for the vCPU by
userspace.

Patch 2 fixes a bug that KVM could erroneously perform an extra
single step (without returning to userspace) due to setting PSTATE.SS
to 1 on every guest entry, when single-step execution is enabled for
the vCPU by userspace.

Patch 3-4 adds a test for KVM_GUESTDBG_SINGLESTEP to the
debug-exception test to verify the single-step behavior.

The series is based on 6.0-rc5.

v2:
 - Change kvm_handle_guest_debug() to use switch/case statement [Marc]
 - Clear PSTATE.SS on guest entry if the Software step state at the
   last guest exit was "Active-pending" to make DBG_SS_ACTIVE_PENDING
   and PSTATE.SS consistent [Marc]
 - Add a fix to preserve PSTATE.SS for the guest.

v1: https://lore.kernel.org/all/20220909044636.1997755-1-reijiw@google.com/

Reiji Watanabe (4):
  KVM: arm64: Preserve PSTATE.SS for the guest while single-step is
    enabled
  KVM: arm64: Clear PSTATE.SS when the Software Step state was
    Active-pending
  KVM: arm64: selftests: Refactor debug-exceptions to make it amenable
    to new test cases
  KVM: arm64: selftests: Add a test case for KVM_GUESTDBG_SINGLESTEP

 arch/arm64/include/asm/kvm_host.h             |   4 +
 arch/arm64/kvm/debug.c                        |  34 +++-
 arch/arm64/kvm/guest.c                        |   1 +
 arch/arm64/kvm/handle_exit.c                  |   8 +-
 .../selftests/kvm/aarch64/debug-exceptions.c  | 149 +++++++++++++++++-
 5 files changed, 190 insertions(+), 6 deletions(-)


base-commit: 80e78fcce86de0288793a0ef0f6acf37656ee4cf
-- 
2.37.3.968.ga6b4b080e4-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/4] KVM: arm64: Preserve PSTATE.SS for the guest while single-step is enabled
  2022-09-17  1:05 [PATCH v2 0/4] KVM: arm64: Fix bugs of single-step execution enabled by userspace Reiji Watanabe
@ 2022-09-17  1:05 ` Reiji Watanabe
  2022-09-17  1:05 ` [PATCH v2 2/4] KVM: arm64: Clear PSTATE.SS when the Software Step state was Active-pending Reiji Watanabe
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Reiji Watanabe @ 2022-09-17  1:05 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Preserve the PSTATE.SS value for the guest while userspace enables
single-step (i.e. while KVM manipulates the PSTATE.SS) for the vCPU.

Currently, while userspace enables single-step for the vCPU
(with KVM_GUESTDBG_SINGLESTEP), KVM sets PSTATE.SS to 1 on every
guest entry, not saving its original value.
When userspace disables single-step, KVM doesn't restore the original
value for the subsequent guest entry (use the current value instead).
Exception return instructions copy PSTATE.SS from SPSR_ELx.SS
only in certain cases when single-step is enabled (and set it to 0
in other cases). So, the value matters only when the guest enables
single-step (and when the guest's Software step state isn't affected
by single-step enabled by userspace, practically), though.

Fix this by preserving the original PSTATE.SS value while userspace
enables single-step, and restoring the value once it is disabled.

This fix modifies the behavior of GET_ONE_REG/SET_ONE_REG for the
PSTATE.SS while single-step is enabled by userspace.
Presently, GET_ONE_REG/SET_ONE_REG gets/sets the current PSTATE.SS
value, which KVM will override on the next guest entry (i.e. the
value userspace gets/sets is not used for the next guest entry).
With this patch, GET_ONE_REG/SET_ONE_REG will get/set the guest's
preserved value, which KVM will preserve and try to restore after
single-step is disabled.

Fixes: 337b99bf7edf ("KVM: arm64: guest debug, add support for single-step")
Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/include/asm/kvm_host.h |  1 +
 arch/arm64/kvm/debug.c            | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index e9c9388ccc02..ccf8a144f009 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -393,6 +393,7 @@ struct kvm_vcpu_arch {
 	 */
 	struct {
 		u32	mdscr_el1;
+		bool	pstate_ss;
 	} guest_debug_preserved;
 
 	/* vcpu power state */
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index 0b28d7db7c76..1bd2a1aee11c 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -32,6 +32,10 @@ static DEFINE_PER_CPU(u64, mdcr_el2);
  *
  * Guest access to MDSCR_EL1 is trapped by the hypervisor and handled
  * after we have restored the preserved value to the main context.
+ *
+ * When single-step is enabled by userspace, we tweak PSTATE.SS on every
+ * guest entry. Preserve PSTATE.SS so we can restore the original value
+ * for the vcpu after the single-step is disabled.
  */
 static void save_guest_debug_regs(struct kvm_vcpu *vcpu)
 {
@@ -41,6 +45,9 @@ static void save_guest_debug_regs(struct kvm_vcpu *vcpu)
 
 	trace_kvm_arm_set_dreg32("Saved MDSCR_EL1",
 				vcpu->arch.guest_debug_preserved.mdscr_el1);
+
+	vcpu->arch.guest_debug_preserved.pstate_ss =
+					(*vcpu_cpsr(vcpu) & DBG_SPSR_SS);
 }
 
 static void restore_guest_debug_regs(struct kvm_vcpu *vcpu)
@@ -51,6 +58,11 @@ static void restore_guest_debug_regs(struct kvm_vcpu *vcpu)
 
 	trace_kvm_arm_set_dreg32("Restored MDSCR_EL1",
 				vcpu_read_sys_reg(vcpu, MDSCR_EL1));
+
+	if (vcpu->arch.guest_debug_preserved.pstate_ss)
+		*vcpu_cpsr(vcpu) |= DBG_SPSR_SS;
+	else
+		*vcpu_cpsr(vcpu) &= ~DBG_SPSR_SS;
 }
 
 /**
-- 
2.37.3.968.ga6b4b080e4-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/4] KVM: arm64: Clear PSTATE.SS when the Software Step state was Active-pending
  2022-09-17  1:05 [PATCH v2 0/4] KVM: arm64: Fix bugs of single-step execution enabled by userspace Reiji Watanabe
  2022-09-17  1:05 ` [PATCH v2 1/4] KVM: arm64: Preserve PSTATE.SS for the guest while single-step is enabled Reiji Watanabe
@ 2022-09-17  1:05 ` Reiji Watanabe
  2022-09-17  1:05 ` [PATCH v2 3/4] KVM: arm64: selftests: Refactor debug-exceptions to make it amenable to new test cases Reiji Watanabe
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Reiji Watanabe @ 2022-09-17  1:05 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

While userspace enables single-step, if the Software Step state at the
last guest exit was "Active-pending", clear PSTATE.SS on guest entry
to restore the state.

Currently, KVM sets PSTATE.SS to 1 on every guest entry while userspace
enables single-step for the vCPU (with KVM_GUESTDBG_SINGLESTEP).
It means KVM always makes the vCPU's Software Step state
"Active-not-pending" on the guest entry, which lets the VCPU perform
single-step (then Software Step exception is taken). This could cause
extra single-step (without returning to userspace) if the Software Step
state at the last guest exit was "Active-pending" (i.e. the last
exit was triggered by an asynchronous exception after the single-step
is performed, but before the Software Step exception is taken.
See "Figure D2-3 Software step state machine" and "D2.12.7 Behavior
in the active-pending state" in ARM DDI 0487I.a for more info about
this behavior).

Fix this by clearing PSTATE.SS on guest entry if the Software Step state
at the last exit was "Active-pending" so that KVM restore the state (and
the exception is taken before further single-step is performed).

Fixes: 337b99bf7edf ("KVM: arm64: guest debug, add support for single-step")
Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/include/asm/kvm_host.h |  3 +++
 arch/arm64/kvm/debug.c            | 22 +++++++++++++++++++++-
 arch/arm64/kvm/guest.c            |  1 +
 arch/arm64/kvm/handle_exit.c      |  8 +++++++-
 4 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index ccf8a144f009..45e2136322ba 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -536,6 +536,9 @@ struct kvm_vcpu_arch {
 #define IN_WFIT			__vcpu_single_flag(sflags, BIT(3))
 /* vcpu system registers loaded on physical CPU */
 #define SYSREGS_ON_CPU		__vcpu_single_flag(sflags, BIT(4))
+/* Software step state is Active-pending */
+#define DBG_SS_ACTIVE_PENDING	__vcpu_single_flag(sflags, BIT(5))
+
 
 /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
 #define vcpu_sve_pffr(vcpu) (kern_hyp_va((vcpu)->arch.sve_state) +	\
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index 1bd2a1aee11c..56361e512b8a 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -200,7 +200,18 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
 		 * debugging the system.
 		 */
 		if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
-			*vcpu_cpsr(vcpu) |=  DBG_SPSR_SS;
+			/*
+			 * If the software step state at the last guest exit
+			 * was Active-pending, we don't set DBG_SPSR_SS so
+			 * that the state is maintained (to not run another
+			 * single-step until the pending Software Step
+			 * exception is taken).
+			 */
+			if (!vcpu_get_flag(vcpu, DBG_SS_ACTIVE_PENDING))
+				*vcpu_cpsr(vcpu) |= DBG_SPSR_SS;
+			else
+				*vcpu_cpsr(vcpu) &= ~DBG_SPSR_SS;
+
 			mdscr = vcpu_read_sys_reg(vcpu, MDSCR_EL1);
 			mdscr |= DBG_MDSCR_SS;
 			vcpu_write_sys_reg(vcpu, mdscr, MDSCR_EL1);
@@ -274,6 +285,15 @@ void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
 	 * Restore the guest's debug registers if we were using them.
 	 */
 	if (vcpu->guest_debug || kvm_vcpu_os_lock_enabled(vcpu)) {
+		if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
+			if (!(*vcpu_cpsr(vcpu) & DBG_SPSR_SS))
+				/*
+				 * Mark the vcpu as ACTIVE_PENDING
+				 * until Software Step exception is taken.
+				 */
+				vcpu_set_flag(vcpu, DBG_SS_ACTIVE_PENDING);
+		}
+
 		restore_guest_debug_regs(vcpu);
 
 		/*
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index f802a3b3f8db..2ff13a3f8479 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -937,6 +937,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
 	} else {
 		/* If not enabled clear all flags */
 		vcpu->guest_debug = 0;
+		vcpu_clear_flag(vcpu, DBG_SS_ACTIVE_PENDING);
 	}
 
 out:
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index bbe5b393d689..e778eefcf214 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -152,8 +152,14 @@ static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu)
 	run->debug.arch.hsr_high = upper_32_bits(esr);
 	run->flags = KVM_DEBUG_ARCH_HSR_HIGH_VALID;
 
-	if (ESR_ELx_EC(esr) == ESR_ELx_EC_WATCHPT_LOW)
+	switch (ESR_ELx_EC(esr)) {
+	case ESR_ELx_EC_WATCHPT_LOW:
 		run->debug.arch.far = vcpu->arch.fault.far_el2;
+		break;
+	case ESR_ELx_EC_SOFTSTP_LOW:
+		vcpu_clear_flag(vcpu, DBG_SS_ACTIVE_PENDING);
+		break;
+	}
 
 	return 0;
 }
-- 
2.37.3.968.ga6b4b080e4-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/4] KVM: arm64: selftests: Refactor debug-exceptions to make it amenable to new test cases
  2022-09-17  1:05 [PATCH v2 0/4] KVM: arm64: Fix bugs of single-step execution enabled by userspace Reiji Watanabe
  2022-09-17  1:05 ` [PATCH v2 1/4] KVM: arm64: Preserve PSTATE.SS for the guest while single-step is enabled Reiji Watanabe
  2022-09-17  1:05 ` [PATCH v2 2/4] KVM: arm64: Clear PSTATE.SS when the Software Step state was Active-pending Reiji Watanabe
@ 2022-09-17  1:05 ` Reiji Watanabe
  2022-09-17  1:06 ` [PATCH v2 4/4] KVM: arm64: selftests: Add a test case for KVM_GUESTDBG_SINGLESTEP Reiji Watanabe
  2022-09-19 10:04 ` [PATCH v2 0/4] KVM: arm64: Fix bugs of single-step execution enabled by userspace Marc Zyngier
  4 siblings, 0 replies; 8+ messages in thread
From: Reiji Watanabe @ 2022-09-17  1:05 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Split up the current test into a helper, but leave the debug version
checking in main(), to make it convenient to add a new debug exception
test case in a subsequent patch.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 .../selftests/kvm/aarch64/debug-exceptions.c   | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
index 2ee35cf9801e..e6e83b895fd5 100644
--- a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
+++ b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
@@ -246,7 +246,7 @@ static int debug_version(struct kvm_vcpu *vcpu)
 	return id_aa64dfr0 & 0xf;
 }
 
-int main(int argc, char *argv[])
+static void test_guest_debug_exceptions(void)
 {
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
@@ -259,9 +259,6 @@ int main(int argc, char *argv[])
 	vm_init_descriptor_tables(vm);
 	vcpu_init_descriptor_tables(vcpu);
 
-	__TEST_REQUIRE(debug_version(vcpu) >= 6,
-		       "Armv8 debug architecture not supported.");
-
 	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
 				ESR_EC_BRK_INS, guest_sw_bp_handler);
 	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
@@ -294,5 +291,18 @@ int main(int argc, char *argv[])
 
 done:
 	kvm_vm_free(vm);
+}
+
+int main(int argc, char *argv[])
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+
+	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+	__TEST_REQUIRE(debug_version(vcpu) >= 6,
+		       "Armv8 debug architecture not supported.");
+	kvm_vm_free(vm);
+	test_guest_debug_exceptions();
+
 	return 0;
 }
-- 
2.37.3.968.ga6b4b080e4-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 4/4] KVM: arm64: selftests: Add a test case for KVM_GUESTDBG_SINGLESTEP
  2022-09-17  1:05 [PATCH v2 0/4] KVM: arm64: Fix bugs of single-step execution enabled by userspace Reiji Watanabe
                   ` (2 preceding siblings ...)
  2022-09-17  1:05 ` [PATCH v2 3/4] KVM: arm64: selftests: Refactor debug-exceptions to make it amenable to new test cases Reiji Watanabe
@ 2022-09-17  1:06 ` Reiji Watanabe
  2022-09-19  9:36   ` Marc Zyngier
  2022-09-19 10:04 ` [PATCH v2 0/4] KVM: arm64: Fix bugs of single-step execution enabled by userspace Marc Zyngier
  4 siblings, 1 reply; 8+ messages in thread
From: Reiji Watanabe @ 2022-09-17  1:06 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Add a test case for KVM_GUESTDBG_SINGLESTEP to the debug-exceptions test.
The test enables single-step execution from userspace, and check if the
exit to userspace occurs for each instruction that is stepped.
Set the default number of the test iterations to a number of iterations
sufficient to always reproduce the problem that the previous patch fixes
on an Ampere Altra machine.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 .../selftests/kvm/aarch64/debug-exceptions.c  | 131 ++++++++++++++++++
 1 file changed, 131 insertions(+)

diff --git a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
index e6e83b895fd5..947bd201435c 100644
--- a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
+++ b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
@@ -22,6 +22,7 @@
 #define SPSR_SS		(1 << 21)
 
 extern unsigned char sw_bp, sw_bp2, hw_bp, hw_bp2, bp_svc, bp_brk, hw_wp, ss_start;
+extern unsigned char iter_ss_begin, iter_ss_end;
 static volatile uint64_t sw_bp_addr, hw_bp_addr;
 static volatile uint64_t wp_addr, wp_data_addr;
 static volatile uint64_t svc_addr;
@@ -238,6 +239,46 @@ static void guest_svc_handler(struct ex_regs *regs)
 	svc_addr = regs->pc;
 }
 
+enum single_step_op {
+	SINGLE_STEP_ENABLE = 0,
+	SINGLE_STEP_DISABLE = 1,
+};
+
+static void guest_code_ss(int test_cnt)
+{
+	uint64_t i;
+	uint64_t bvr, wvr, w_bvr, w_wvr;
+
+	for (i = 0; i < test_cnt; i++) {
+		/* Bits [1:0] of dbg{b,w}vr are RES0 */
+		w_bvr = i << 2;
+		w_wvr = i << 2;
+
+		/* Enable Single Step execution */
+		GUEST_SYNC(SINGLE_STEP_ENABLE);
+
+		/*
+		 * The userspace will veriry that the pc is as expected during
+		 * single step execution between iter_ss_begin and iter_ss_end.
+		 */
+		asm volatile("iter_ss_begin:nop\n");
+
+		write_sysreg(w_bvr, dbgbvr0_el1);
+		write_sysreg(w_wvr, dbgwvr0_el1);
+		bvr = read_sysreg(dbgbvr0_el1);
+		wvr = read_sysreg(dbgwvr0_el1);
+
+		asm volatile("iter_ss_end:\n");
+
+		/* Disable Single Step execution */
+		GUEST_SYNC(SINGLE_STEP_DISABLE);
+
+		GUEST_ASSERT(bvr == w_bvr);
+		GUEST_ASSERT(wvr == w_wvr);
+	}
+	GUEST_DONE();
+}
+
 static int debug_version(struct kvm_vcpu *vcpu)
 {
 	uint64_t id_aa64dfr0;
@@ -293,16 +334,106 @@ static void test_guest_debug_exceptions(void)
 	kvm_vm_free(vm);
 }
 
+void test_single_step_from_userspace(int test_cnt)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	struct ucall uc;
+	struct kvm_run *run;
+	uint64_t pc, cmd;
+	uint64_t test_pc = 0;
+	bool ss_enable = false;
+	struct kvm_guest_debug debug = {};
+
+	vm = vm_create_with_one_vcpu(&vcpu, guest_code_ss);
+	ucall_init(vm, NULL);
+	run = vcpu->run;
+	vcpu_args_set(vcpu, 1, test_cnt);
+
+	while (1) {
+		vcpu_run(vcpu);
+		if (run->exit_reason != KVM_EXIT_DEBUG) {
+			cmd = get_ucall(vcpu, &uc);
+			if (cmd == UCALL_ABORT) {
+				REPORT_GUEST_ASSERT(uc);
+				/* NOT REACHED */
+			} else if (cmd == UCALL_DONE) {
+				break;
+			}
+
+			TEST_ASSERT(cmd == UCALL_SYNC,
+				    "Unexpected ucall cmd 0x%lx", cmd);
+
+			if (uc.args[1] == SINGLE_STEP_ENABLE) {
+				debug.control = KVM_GUESTDBG_ENABLE |
+						KVM_GUESTDBG_SINGLESTEP;
+				ss_enable = true;
+			} else {
+				debug.control = SINGLE_STEP_DISABLE;
+				ss_enable = false;
+			}
+
+			vcpu_guest_debug_set(vcpu, &debug);
+			continue;
+		}
+
+		TEST_ASSERT(ss_enable, "Unexpected KVM_EXIT_DEBUG");
+
+		/* Check if the current pc is expected. */
+		vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pc), &pc);
+		TEST_ASSERT(!test_pc || pc == test_pc,
+			    "Unexpected pc 0x%lx (expected 0x%lx)",
+			    pc, test_pc);
+
+		/*
+		 * If the current pc is between iter_ss_bgin and
+		 * iter_ss_end, the pc for the next KVM_EXIT_DEBUG should
+		 * be the current pc + 4.
+		 */
+		if ((pc >= (uint64_t)&iter_ss_begin) &&
+		    (pc < (uint64_t)&iter_ss_end))
+			test_pc = pc + 4;
+		else
+			test_pc = 0;
+	}
+
+	kvm_vm_free(vm);
+}
+
+static void help(char *name)
+{
+	puts("");
+	printf("Usage: %s [-h] [-i iterations of the single step test]\n", name);
+	puts("");
+	exit(0);
+}
+
 int main(int argc, char *argv[])
 {
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
+	int opt;
+	int ss_iteration = 10000;
 
 	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
 	__TEST_REQUIRE(debug_version(vcpu) >= 6,
 		       "Armv8 debug architecture not supported.");
 	kvm_vm_free(vm);
+
+	while ((opt = getopt(argc, argv, "i:")) != -1) {
+		switch (opt) {
+		case 'i':
+			ss_iteration = atoi(optarg);
+			break;
+		case 'h':
+		default:
+			help(argv[0]);
+			break;
+		}
+	}
+
 	test_guest_debug_exceptions();
+	test_single_step_from_userspace(ss_iteration);
 
 	return 0;
 }
-- 
2.37.3.968.ga6b4b080e4-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 4/4] KVM: arm64: selftests: Add a test case for KVM_GUESTDBG_SINGLESTEP
  2022-09-17  1:06 ` [PATCH v2 4/4] KVM: arm64: selftests: Add a test case for KVM_GUESTDBG_SINGLESTEP Reiji Watanabe
@ 2022-09-19  9:36   ` Marc Zyngier
  2022-09-20  2:13     ` Reiji Watanabe
  0 siblings, 1 reply; 8+ messages in thread
From: Marc Zyngier @ 2022-09-19  9:36 UTC (permalink / raw)
  To: Reiji Watanabe
  Cc: kvmarm, kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata

On Sat, 17 Sep 2022 02:06:00 +0100,
Reiji Watanabe <reijiw@google.com> wrote:
> 
> Add a test case for KVM_GUESTDBG_SINGLESTEP to the debug-exceptions test.
> The test enables single-step execution from userspace, and check if the
> exit to userspace occurs for each instruction that is stepped.
> Set the default number of the test iterations to a number of iterations
> sufficient to always reproduce the problem that the previous patch fixes
> on an Ampere Altra machine.

A possibly more aggressive version of this test would be to force a
(short lived) timer to fire on the same CPU, forcing an exit. This
should hopefully result in a more predictable way to trigger the
issue. But that's a reasonable test as a start.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/4] KVM: arm64: Fix bugs of single-step execution enabled by userspace
  2022-09-17  1:05 [PATCH v2 0/4] KVM: arm64: Fix bugs of single-step execution enabled by userspace Reiji Watanabe
                   ` (3 preceding siblings ...)
  2022-09-17  1:06 ` [PATCH v2 4/4] KVM: arm64: selftests: Add a test case for KVM_GUESTDBG_SINGLESTEP Reiji Watanabe
@ 2022-09-19 10:04 ` Marc Zyngier
  4 siblings, 0 replies; 8+ messages in thread
From: Marc Zyngier @ 2022-09-19 10:04 UTC (permalink / raw)
  To: kvmarm, Reiji Watanabe
  Cc: linux-arm-kernel, Ricardo Koller, Jing Zhang, Alexandru Elisei,
	Oliver Upton, Suzuki K Poulose, Paolo Bonzini, James Morse,
	Raghavendra Rao Anata, kvm

On Fri, 16 Sep 2022 18:05:56 -0700, Reiji Watanabe wrote:
> This series fixes two bugs of single-step execution enabled by
> userspace, and add a test case for KVM_GUESTDBG_SINGLESTEP to
> the debug-exception test to verify the single-step behavior.
> 
> Patch 1 fixes a bug that KVM might unintentionally change PSTATE.SS
> for the guest when single-step execution is enabled for the vCPU by
> userspace.
> 
> [...]

Applied to next, thanks!

[1/4] KVM: arm64: Preserve PSTATE.SS for the guest while single-step is enabled
      commit: 34fbdee086cfcc20fe889d2b83afddfbe2ac3096
[2/4] KVM: arm64: Clear PSTATE.SS when the Software Step state was Active-pending
      commit: 370531d1e95be57c62fdf065fb04fd8db7ade8f9
[3/4] KVM: arm64: selftests: Refactor debug-exceptions to make it amenable to new test cases
      commit: ff00e737090e0f015059e59829aaa58565b16321
[4/4] KVM: arm64: selftests: Add a test case for KVM_GUESTDBG_SINGLESTEP
      commit: b18e4d4aebdddd05810ceb2f73d7f72afcd11b41

Cheers,

	M.
-- 
Marc Zyngier <maz@kernel.org>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 4/4] KVM: arm64: selftests: Add a test case for KVM_GUESTDBG_SINGLESTEP
  2022-09-19  9:36   ` Marc Zyngier
@ 2022-09-20  2:13     ` Reiji Watanabe
  0 siblings, 0 replies; 8+ messages in thread
From: Reiji Watanabe @ 2022-09-20  2:13 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: kvmarm, kvm, Linux ARM, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata

Hi Marc,

On Mon, Sep 19, 2022 at 2:36 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On Sat, 17 Sep 2022 02:06:00 +0100,
> Reiji Watanabe <reijiw@google.com> wrote:
> >
> > Add a test case for KVM_GUESTDBG_SINGLESTEP to the debug-exceptions test.
> > The test enables single-step execution from userspace, and check if the
> > exit to userspace occurs for each instruction that is stepped.
> > Set the default number of the test iterations to a number of iterations
> > sufficient to always reproduce the problem that the previous patch fixes
> > on an Ampere Altra machine.
>
> A possibly more aggressive version of this test would be to force a
> (short lived) timer to fire on the same CPU, forcing an exit. This
> should hopefully result in a more predictable way to trigger the
> issue. But that's a reasonable test as a start.

Yes, that could result in a more predictable way to cause the specific case!
I will consider this at a future opportunity.

Thank you,
Reiji

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-09-20  2:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-17  1:05 [PATCH v2 0/4] KVM: arm64: Fix bugs of single-step execution enabled by userspace Reiji Watanabe
2022-09-17  1:05 ` [PATCH v2 1/4] KVM: arm64: Preserve PSTATE.SS for the guest while single-step is enabled Reiji Watanabe
2022-09-17  1:05 ` [PATCH v2 2/4] KVM: arm64: Clear PSTATE.SS when the Software Step state was Active-pending Reiji Watanabe
2022-09-17  1:05 ` [PATCH v2 3/4] KVM: arm64: selftests: Refactor debug-exceptions to make it amenable to new test cases Reiji Watanabe
2022-09-17  1:06 ` [PATCH v2 4/4] KVM: arm64: selftests: Add a test case for KVM_GUESTDBG_SINGLESTEP Reiji Watanabe
2022-09-19  9:36   ` Marc Zyngier
2022-09-20  2:13     ` Reiji Watanabe
2022-09-19 10:04 ` [PATCH v2 0/4] KVM: arm64: Fix bugs of single-step execution enabled by userspace Marc Zyngier

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).