kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test
@ 2021-06-11  1:10 Ricardo Koller
  2021-06-11  1:10 ` [PATCH v4 1/6] KVM: selftests: Rename vm_handle_exception Ricardo Koller
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Ricardo Koller @ 2021-06-11  1:10 UTC (permalink / raw)
  To: kvm, kvmarm
  Cc: pbonzini, maz, drjones, alexandru.elisei, eric.auger, yuzenghui,
	vkuznets, Ricardo Koller

Hi,

These patches add a debug exception test in aarch64 KVM selftests while
also adding basic exception handling support.

The structure of the exception handling is based on its x86 counterpart.
Tests use the same calls to initialize exception handling and both
architectures allow tests to override the handler for a particular
vector, or (vector, ec) for synchronous exceptions in the arm64 case.

The debug test is similar to x86_64/debug_regs, except that the x86 one
controls the debugging from outside the VM. This proposed arm64 test
controls and handles debug exceptions from the inside.

Thanks,
Ricardo

v3 -> v4:

V3 was dropped because it was breaking x86 selftests builds (reported by
the kernel test robot).
- rename vm_handle_exception to vm_install_sync_handler instead of
  vm_install_vector_handlers. [Sean]
- use a single level of routing for exception handling. [Sean]
- fix issue in x86_64/sync_regs_test when switching to ucalls for unhandled
  exceptions reporting.

v2 -> v3:

Addressed comments from Andrew and Marc (thanks again). Also, many thanks for
the reviews and tests from Eric and Zenghui.
- add missing ISBs after writing into debug registers.
- not store/restore of sp_el0 on exceptions.
- add default handlers for Error and FIQ.
- change multiple TEST_ASSERT(false, ...) to TEST_FAIL.
- use Andrew's suggestion regarding __GUEST_ASSERT modifications
  in order to easier implement GUEST_ASSERT_EQ (Thanks Andrew).

v1 -> v2:

Addressed comments from Andrew and Marc (thank you very much):
- rename vm_handle_exception in all tests.
- introduce UCALL_UNHANDLED in x86 first.
- move GUEST_ASSERT_EQ to common utils header.
- handle sync and other exceptions separately: use two tables (like
  kvm-unit-tests).
- add two separate functions for installing sync versus other exceptions
- changes in handlers.S: use the same layout as user_pt_regs, treat the
  EL1t vectors as invalid, refactor the vector table creation to not use
  manual numbering, add comments, remove LR from the stored registers.
- changes in debug-exceptions.c: remove unused headers, use the common
  GUEST_ASSERT_EQ, use vcpu_run instead of _vcpu_run.
- changes in processor.h: write_sysreg with support for xzr, replace EL1
  with current in macro names, define ESR_EC_MASK as ESR_EC_NUM-1.

Ricardo Koller (6):
  KVM: selftests: Rename vm_handle_exception
  KVM: selftests: Complete x86_64/sync_regs_test ucall
  KVM: selftests: Introduce UCALL_UNHANDLED for unhandled vector
    reporting
  KVM: selftests: Move GUEST_ASSERT_EQ to utils header
  KVM: selftests: Add exception handling support for aarch64
  KVM: selftests: Add aarch64/debug-exceptions test

 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   3 +-
 .../selftests/kvm/aarch64/debug-exceptions.c  | 250 ++++++++++++++++++
 .../selftests/kvm/include/aarch64/processor.h |  83 +++++-
 .../testing/selftests/kvm/include/kvm_util.h  |  23 +-
 .../selftests/kvm/include/x86_64/processor.h  |   4 +-
 .../selftests/kvm/lib/aarch64/handlers.S      | 126 +++++++++
 .../selftests/kvm/lib/aarch64/processor.c     |  97 +++++++
 .../selftests/kvm/lib/x86_64/processor.c      |  23 +-
 .../testing/selftests/kvm/x86_64/evmcs_test.c |   4 +-
 .../selftests/kvm/x86_64/kvm_pv_test.c        |   2 +-
 .../selftests/kvm/x86_64/sync_regs_test.c     |   7 +-
 .../selftests/kvm/x86_64/tsc_msrs_test.c      |   9 -
 .../kvm/x86_64/userspace_msr_exit_test.c      |   8 +-
 .../selftests/kvm/x86_64/xapic_ipi_test.c     |   2 +-
 15 files changed, 592 insertions(+), 50 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/aarch64/debug-exceptions.c
 create mode 100644 tools/testing/selftests/kvm/lib/aarch64/handlers.S

-- 
2.32.0.272.g935e593368-goog


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

* [PATCH v4 1/6] KVM: selftests: Rename vm_handle_exception
  2021-06-11  1:10 [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test Ricardo Koller
@ 2021-06-11  1:10 ` Ricardo Koller
  2021-06-11  1:10 ` [PATCH v4 2/6] KVM: selftests: Complete x86_64/sync_regs_test ucall Ricardo Koller
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Ricardo Koller @ 2021-06-11  1:10 UTC (permalink / raw)
  To: kvm, kvmarm
  Cc: pbonzini, maz, drjones, alexandru.elisei, eric.auger, yuzenghui,
	vkuznets, Ricardo Koller, kernel test robot

Rename the vm_handle_exception function to a name that indicates more
clearly that it installs something: vm_install_exception_handler.

Reported-by: kernel test robot <oliver.sang@intel.com>
Suggested-by: Marc Zyngier <maz@kernel.org>
Suggested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Ricardo Koller <ricarkol@google.com>
---
 tools/testing/selftests/kvm/include/x86_64/processor.h    | 2 +-
 tools/testing/selftests/kvm/lib/x86_64/processor.c        | 4 ++--
 tools/testing/selftests/kvm/x86_64/evmcs_test.c           | 4 ++--
 tools/testing/selftests/kvm/x86_64/kvm_pv_test.c          | 2 +-
 .../selftests/kvm/x86_64/userspace_msr_exit_test.c        | 8 ++++----
 tools/testing/selftests/kvm/x86_64/xapic_ipi_test.c       | 2 +-
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index 0b30b4e15c38..e9f584991332 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -391,7 +391,7 @@ struct ex_regs {
 
 void vm_init_descriptor_tables(struct kvm_vm *vm);
 void vcpu_init_descriptor_tables(struct kvm_vm *vm, uint32_t vcpuid);
-void vm_handle_exception(struct kvm_vm *vm, int vector,
+void vm_install_exception_handler(struct kvm_vm *vm, int vector,
 			void (*handler)(struct ex_regs *));
 
 /*
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index efe235044421..257c5c33d04e 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -1244,8 +1244,8 @@ void vcpu_init_descriptor_tables(struct kvm_vm *vm, uint32_t vcpuid)
 	*(vm_vaddr_t *)addr_gva2hva(vm, (vm_vaddr_t)(&exception_handlers)) = vm->handlers;
 }
 
-void vm_handle_exception(struct kvm_vm *vm, int vector,
-			 void (*handler)(struct ex_regs *))
+void vm_install_exception_handler(struct kvm_vm *vm, int vector,
+			       void (*handler)(struct ex_regs *))
 {
 	vm_vaddr_t *handlers = (vm_vaddr_t *)addr_gva2hva(vm, vm->handlers);
 
diff --git a/tools/testing/selftests/kvm/x86_64/evmcs_test.c b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
index 63096cea26c6..0864b2e3fd9e 100644
--- a/tools/testing/selftests/kvm/x86_64/evmcs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
@@ -154,8 +154,8 @@ int main(int argc, char *argv[])
 
 	vm_init_descriptor_tables(vm);
 	vcpu_init_descriptor_tables(vm, VCPU_ID);
-	vm_handle_exception(vm, UD_VECTOR, guest_ud_handler);
-	vm_handle_exception(vm, NMI_VECTOR, guest_nmi_handler);
+	vm_install_exception_handler(vm, UD_VECTOR, guest_ud_handler);
+	vm_install_exception_handler(vm, NMI_VECTOR, guest_nmi_handler);
 
 	pr_info("Running L1 which uses EVMCS to run L2\n");
 
diff --git a/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c b/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c
index 732b244d6956..04ed975662c9 100644
--- a/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c
+++ b/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c
@@ -227,7 +227,7 @@ int main(void)
 
 	vm_init_descriptor_tables(vm);
 	vcpu_init_descriptor_tables(vm, VCPU_ID);
-	vm_handle_exception(vm, GP_VECTOR, guest_gp_handler);
+	vm_install_exception_handler(vm, GP_VECTOR, guest_gp_handler);
 
 	enter_guest(vm);
 	kvm_vm_free(vm);
diff --git a/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c b/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
index 72c0d0797522..e3e20e8848d0 100644
--- a/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
+++ b/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
@@ -574,7 +574,7 @@ static void test_msr_filter_allow(void) {
 	vm_init_descriptor_tables(vm);
 	vcpu_init_descriptor_tables(vm, VCPU_ID);
 
-	vm_handle_exception(vm, GP_VECTOR, guest_gp_handler);
+	vm_install_exception_handler(vm, GP_VECTOR, guest_gp_handler);
 
 	/* Process guest code userspace exits. */
 	run_guest_then_process_rdmsr(vm, MSR_IA32_XSS);
@@ -588,12 +588,12 @@ static void test_msr_filter_allow(void) {
 	run_guest_then_process_wrmsr(vm, MSR_NON_EXISTENT);
 	run_guest_then_process_rdmsr(vm, MSR_NON_EXISTENT);
 
-	vm_handle_exception(vm, UD_VECTOR, guest_ud_handler);
+	vm_install_exception_handler(vm, UD_VECTOR, guest_ud_handler);
 	run_guest(vm);
-	vm_handle_exception(vm, UD_VECTOR, NULL);
+	vm_install_exception_handler(vm, UD_VECTOR, NULL);
 
 	if (process_ucall(vm) != UCALL_DONE) {
-		vm_handle_exception(vm, GP_VECTOR, guest_fep_gp_handler);
+		vm_install_exception_handler(vm, GP_VECTOR, guest_fep_gp_handler);
 
 		/* Process emulated rdmsr and wrmsr instructions. */
 		run_guest_then_process_rdmsr(vm, MSR_IA32_XSS);
diff --git a/tools/testing/selftests/kvm/x86_64/xapic_ipi_test.c b/tools/testing/selftests/kvm/x86_64/xapic_ipi_test.c
index 2f964cdc273c..ed27269a01bb 100644
--- a/tools/testing/selftests/kvm/x86_64/xapic_ipi_test.c
+++ b/tools/testing/selftests/kvm/x86_64/xapic_ipi_test.c
@@ -462,7 +462,7 @@ int main(int argc, char *argv[])
 
 	vm_init_descriptor_tables(vm);
 	vcpu_init_descriptor_tables(vm, HALTER_VCPU_ID);
-	vm_handle_exception(vm, IPI_VECTOR, guest_ipi_handler);
+	vm_install_exception_handler(vm, IPI_VECTOR, guest_ipi_handler);
 
 	virt_pg_map(vm, APIC_DEFAULT_GPA, APIC_DEFAULT_GPA, 0);
 
-- 
2.32.0.272.g935e593368-goog


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

* [PATCH v4 2/6] KVM: selftests: Complete x86_64/sync_regs_test ucall
  2021-06-11  1:10 [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test Ricardo Koller
  2021-06-11  1:10 ` [PATCH v4 1/6] KVM: selftests: Rename vm_handle_exception Ricardo Koller
@ 2021-06-11  1:10 ` Ricardo Koller
  2021-06-11  1:10 ` [PATCH v4 3/6] KVM: selftests: Introduce UCALL_UNHANDLED for unhandled vector reporting Ricardo Koller
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Ricardo Koller @ 2021-06-11  1:10 UTC (permalink / raw)
  To: kvm, kvmarm
  Cc: pbonzini, maz, drjones, alexandru.elisei, eric.auger, yuzenghui,
	vkuznets, Ricardo Koller

The guest in sync_regs_test does raw ucalls by directly accessing the
ucall IO port. It makes these ucalls without setting %rdi to a `struct
ucall`, which is what a ucall uses to pass messages.  The issue is that
if the host did a get_ucall (the receiver side), it would try to access
the `struct ucall` at %rdi=0 which would lead to an error ("No mapping
for vm virtual address, gva: 0x0").

This issue is currently benign as there is no get_ucall in
sync_regs_test; however, that will change in the next commit as it
changes the unhandled exception reporting mechanism to use ucalls.  In
that case, every vcpu_run is followed by a get_ucall to check if the
guest is trying to report an unhandled exception.

Fix this in advance by setting %rdi to a UCALL_NONE struct ucall for the
sync_regs_test guest.

Tested with gcc-[8,9,10], and clang-[9,11].

Signed-off-by: Ricardo Koller <ricarkol@google.com>
---
 tools/testing/selftests/kvm/x86_64/sync_regs_test.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c
index d672f0a473f8..fc03a150278d 100644
--- a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c
@@ -24,6 +24,10 @@
 
 #define UCALL_PIO_PORT ((uint16_t)0x1000)
 
+struct ucall uc_none = {
+	.cmd = UCALL_NONE,
+};
+
 /*
  * ucall is embedded here to protect against compiler reshuffling registers
  * before calling a function. In this test we only need to get KVM_EXIT_IO
@@ -34,7 +38,8 @@ void guest_code(void)
 	asm volatile("1: in %[port], %%al\n"
 		     "add $0x1, %%rbx\n"
 		     "jmp 1b"
-		     : : [port] "d" (UCALL_PIO_PORT) : "rax", "rbx");
+		     : : [port] "d" (UCALL_PIO_PORT), "D" (&uc_none)
+		     : "rax", "rbx");
 }
 
 static void compare_regs(struct kvm_regs *left, struct kvm_regs *right)
-- 
2.32.0.272.g935e593368-goog


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

* [PATCH v4 3/6] KVM: selftests: Introduce UCALL_UNHANDLED for unhandled vector reporting
  2021-06-11  1:10 [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test Ricardo Koller
  2021-06-11  1:10 ` [PATCH v4 1/6] KVM: selftests: Rename vm_handle_exception Ricardo Koller
  2021-06-11  1:10 ` [PATCH v4 2/6] KVM: selftests: Complete x86_64/sync_regs_test ucall Ricardo Koller
@ 2021-06-11  1:10 ` Ricardo Koller
  2021-07-29 18:15   ` Sean Christopherson
  2021-06-11  1:10 ` [PATCH v4 4/6] KVM: selftests: Move GUEST_ASSERT_EQ to utils header Ricardo Koller
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Ricardo Koller @ 2021-06-11  1:10 UTC (permalink / raw)
  To: kvm, kvmarm
  Cc: pbonzini, maz, drjones, alexandru.elisei, eric.auger, yuzenghui,
	vkuznets, Ricardo Koller

x86, the only arch implementing exception handling, reports unhandled
vectors using port IO at a specific port number. This replicates what
ucall already does.

Introduce a new ucall type, UCALL_UNHANDLED, for guests to report
unhandled exceptions. Then replace the x86 unhandled vector exception
reporting to use it instead of port IO.  This new ucall type will be
used in the next commits by arm64 to report unhandled vectors as well.

Tested: Forcing a page fault in the ./x86_64/xapic_ipi_test
	halter_guest_code() shows this:

	$ ./x86_64/xapic_ipi_test
	...
	  Unexpected vectored event in guest (vector:0xe)

Signed-off-by: Ricardo Koller <ricarkol@google.com>
---
 .../testing/selftests/kvm/include/kvm_util.h  |  1 +
 .../selftests/kvm/include/x86_64/processor.h  |  2 --
 .../selftests/kvm/lib/x86_64/processor.c      | 19 ++++++++-----------
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index fcd8e3855111..beb76d6deaa9 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -349,6 +349,7 @@ enum {
 	UCALL_SYNC,
 	UCALL_ABORT,
 	UCALL_DONE,
+	UCALL_UNHANDLED,
 };
 
 #define UCALL_MAX_ARGS 6
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index e9f584991332..92a62c6999bc 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -53,8 +53,6 @@
 #define CPUID_PKU		(1ul << 3)
 #define CPUID_LA57		(1ul << 16)
 
-#define UNEXPECTED_VECTOR_PORT 0xfff0u
-
 /* General Registers in 64-Bit Mode */
 struct gpr64_regs {
 	u64 rax;
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index 257c5c33d04e..a217515a9bc2 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -1201,7 +1201,7 @@ static void set_idt_entry(struct kvm_vm *vm, int vector, unsigned long addr,
 
 void kvm_exit_unexpected_vector(uint32_t value)
 {
-	outl(UNEXPECTED_VECTOR_PORT, value);
+	ucall(UCALL_UNHANDLED, 1, value);
 }
 
 void route_exception(struct ex_regs *regs)
@@ -1254,16 +1254,13 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector,
 
 void assert_on_unhandled_exception(struct kvm_vm *vm, uint32_t vcpuid)
 {
-	if (vcpu_state(vm, vcpuid)->exit_reason == KVM_EXIT_IO
-		&& vcpu_state(vm, vcpuid)->io.port == UNEXPECTED_VECTOR_PORT
-		&& vcpu_state(vm, vcpuid)->io.size == 4) {
-		/* Grab pointer to io data */
-		uint32_t *data = (void *)vcpu_state(vm, vcpuid)
-			+ vcpu_state(vm, vcpuid)->io.data_offset;
-
-		TEST_ASSERT(false,
-			    "Unexpected vectored event in guest (vector:0x%x)",
-			    *data);
+	struct ucall uc;
+
+	if (get_ucall(vm, vcpuid, &uc) == UCALL_UNHANDLED) {
+		uint64_t vector = uc.args[0];
+
+		TEST_FAIL("Unexpected vectored event in guest (vector:0x%lx)",
+			  vector);
 	}
 }
 
-- 
2.32.0.272.g935e593368-goog


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

* [PATCH v4 4/6] KVM: selftests: Move GUEST_ASSERT_EQ to utils header
  2021-06-11  1:10 [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test Ricardo Koller
                   ` (2 preceding siblings ...)
  2021-06-11  1:10 ` [PATCH v4 3/6] KVM: selftests: Introduce UCALL_UNHANDLED for unhandled vector reporting Ricardo Koller
@ 2021-06-11  1:10 ` Ricardo Koller
  2021-06-11  1:10 ` [PATCH v4 5/6] KVM: selftests: Add exception handling support for aarch64 Ricardo Koller
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Ricardo Koller @ 2021-06-11  1:10 UTC (permalink / raw)
  To: kvm, kvmarm
  Cc: pbonzini, maz, drjones, alexandru.elisei, eric.auger, yuzenghui,
	vkuznets, Ricardo Koller

Move GUEST_ASSERT_EQ to a common header, kvm_util.h, for other
architectures and tests to use. Also modify __GUEST_ASSERT so it can be
reused to implement GUEST_ASSERT_EQ.

Signed-off-by: Ricardo Koller <ricarkol@google.com>
---
 .../testing/selftests/kvm/include/kvm_util.h  | 22 ++++++++++---------
 .../selftests/kvm/x86_64/tsc_msrs_test.c      |  9 --------
 2 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index beb76d6deaa9..ce49e22843d8 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -368,26 +368,28 @@ uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc);
 				ucall(UCALL_SYNC, 6, "hello", stage, arg1, arg2, arg3, arg4)
 #define GUEST_SYNC(stage)	ucall(UCALL_SYNC, 2, "hello", stage)
 #define GUEST_DONE()		ucall(UCALL_DONE, 0)
-#define __GUEST_ASSERT(_condition, _nargs, _args...) do {	\
-	if (!(_condition))					\
-		ucall(UCALL_ABORT, 2 + _nargs,			\
-			"Failed guest assert: "			\
-			#_condition, __LINE__, _args);		\
+#define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {    \
+	if (!(_condition))                                              \
+		ucall(UCALL_ABORT, 2 + _nargs,                          \
+			"Failed guest assert: "                         \
+			_condstr, __LINE__, _args);                     \
 } while (0)
 
 #define GUEST_ASSERT(_condition) \
-	__GUEST_ASSERT((_condition), 0, 0)
+	__GUEST_ASSERT(_condition, #_condition, 0, 0)
 
 #define GUEST_ASSERT_1(_condition, arg1) \
-	__GUEST_ASSERT((_condition), 1, (arg1))
+	__GUEST_ASSERT(_condition, #_condition, 1, (arg1))
 
 #define GUEST_ASSERT_2(_condition, arg1, arg2) \
-	__GUEST_ASSERT((_condition), 2, (arg1), (arg2))
+	__GUEST_ASSERT(_condition, #_condition, 2, (arg1), (arg2))
 
 #define GUEST_ASSERT_3(_condition, arg1, arg2, arg3) \
-	__GUEST_ASSERT((_condition), 3, (arg1), (arg2), (arg3))
+	__GUEST_ASSERT(_condition, #_condition, 3, (arg1), (arg2), (arg3))
 
 #define GUEST_ASSERT_4(_condition, arg1, arg2, arg3, arg4) \
-	__GUEST_ASSERT((_condition), 4, (arg1), (arg2), (arg3), (arg4))
+	__GUEST_ASSERT(_condition, #_condition, 4, (arg1), (arg2), (arg3), (arg4))
+
+#define GUEST_ASSERT_EQ(a, b) __GUEST_ASSERT((a) == (b), #a " == " #b, 2, a, b)
 
 #endif /* SELFTEST_KVM_UTIL_H */
diff --git a/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c b/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c
index e357d8e222d4..5a6a662f2e59 100644
--- a/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c
@@ -18,15 +18,6 @@
 #define rounded_rdmsr(x)       ROUND(rdmsr(x))
 #define rounded_host_rdmsr(x)  ROUND(vcpu_get_msr(vm, 0, x))
 
-#define GUEST_ASSERT_EQ(a, b) do {				\
-	__typeof(a) _a = (a);					\
-	__typeof(b) _b = (b);					\
-	if (_a != _b)						\
-                ucall(UCALL_ABORT, 4,				\
-                        "Failed guest assert: "			\
-                        #a " == " #b, __LINE__, _a, _b);	\
-  } while(0)
-
 static void guest_code(void)
 {
 	u64 val = 0;
-- 
2.32.0.272.g935e593368-goog


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

* [PATCH v4 5/6] KVM: selftests: Add exception handling support for aarch64
  2021-06-11  1:10 [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test Ricardo Koller
                   ` (3 preceding siblings ...)
  2021-06-11  1:10 ` [PATCH v4 4/6] KVM: selftests: Move GUEST_ASSERT_EQ to utils header Ricardo Koller
@ 2021-06-11  1:10 ` Ricardo Koller
  2021-07-02  6:46   ` Zenghui Yu
  2021-06-11  1:10 ` [PATCH v4 6/6] KVM: selftests: Add aarch64/debug-exceptions test Ricardo Koller
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Ricardo Koller @ 2021-06-11  1:10 UTC (permalink / raw)
  To: kvm, kvmarm
  Cc: pbonzini, maz, drjones, alexandru.elisei, eric.auger, yuzenghui,
	vkuznets, Ricardo Koller

Add the infrastructure needed to enable exception handling in aarch64
selftests. The exception handling defaults to an unhandled-exception
handler which aborts the test, just like x86. These handlers can be
overridden by calling vm_install_exception_handler(vector) or
vm_install_sync_handler(vector, ec). The unhandled exception reporting
from the guest is done using the ucall type introduced in a previous
commit, UCALL_UNHANDLED.

The exception handling code is inspired on kvm-unit-tests.

Signed-off-by: Ricardo Koller <ricarkol@google.com>
---
 tools/testing/selftests/kvm/Makefile          |   2 +-
 .../selftests/kvm/include/aarch64/processor.h |  63 +++++++++
 .../selftests/kvm/lib/aarch64/handlers.S      | 126 ++++++++++++++++++
 .../selftests/kvm/lib/aarch64/processor.c     |  97 ++++++++++++++
 4 files changed, 287 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/kvm/lib/aarch64/handlers.S

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index daaee1888b12..a77e6063f7e9 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -35,7 +35,7 @@ endif
 
 LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/rbtree.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c lib/perf_test_util.c
 LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S
-LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c
+LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c lib/aarch64/handlers.S
 LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c lib/s390x/diag318_test_handler.c
 
 TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
diff --git a/tools/testing/selftests/kvm/include/aarch64/processor.h b/tools/testing/selftests/kvm/include/aarch64/processor.h
index b7fa0c8551db..b2b3e9d626cb 100644
--- a/tools/testing/selftests/kvm/include/aarch64/processor.h
+++ b/tools/testing/selftests/kvm/include/aarch64/processor.h
@@ -8,6 +8,7 @@
 #define SELFTEST_KVM_PROCESSOR_H
 
 #include "kvm_util.h"
+#include <linux/stringify.h>
 
 
 #define ARM64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
@@ -18,6 +19,7 @@
 #define MAIR_EL1	3, 0, 10, 2, 0
 #define TTBR0_EL1	3, 0,  2, 0, 0
 #define SCTLR_EL1	3, 0,  1, 0, 0
+#define VBAR_EL1	3, 0, 12, 0, 0
 
 /*
  * Default MAIR
@@ -56,4 +58,65 @@ void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *ini
 void aarch64_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid,
 			      struct kvm_vcpu_init *init, void *guest_code);
 
+struct ex_regs {
+	u64 regs[31];
+	u64 sp;
+	u64 pc;
+	u64 pstate;
+};
+
+#define VECTOR_NUM	16
+
+enum {
+	VECTOR_SYNC_CURRENT_SP0,
+	VECTOR_IRQ_CURRENT_SP0,
+	VECTOR_FIQ_CURRENT_SP0,
+	VECTOR_ERROR_CURRENT_SP0,
+
+	VECTOR_SYNC_CURRENT,
+	VECTOR_IRQ_CURRENT,
+	VECTOR_FIQ_CURRENT,
+	VECTOR_ERROR_CURRENT,
+
+	VECTOR_SYNC_LOWER_64,
+	VECTOR_IRQ_LOWER_64,
+	VECTOR_FIQ_LOWER_64,
+	VECTOR_ERROR_LOWER_64,
+
+	VECTOR_SYNC_LOWER_32,
+	VECTOR_IRQ_LOWER_32,
+	VECTOR_FIQ_LOWER_32,
+	VECTOR_ERROR_LOWER_32,
+};
+
+#define VECTOR_IS_SYNC(v) ((v) == VECTOR_SYNC_CURRENT_SP0 || \
+			   (v) == VECTOR_SYNC_CURRENT     || \
+			   (v) == VECTOR_SYNC_LOWER_64    || \
+			   (v) == VECTOR_SYNC_LOWER_32)
+
+#define ESR_EC_NUM		64
+#define ESR_EC_SHIFT		26
+#define ESR_EC_MASK		(ESR_EC_NUM - 1)
+
+void vm_init_descriptor_tables(struct kvm_vm *vm);
+void vcpu_init_descriptor_tables(struct kvm_vm *vm, uint32_t vcpuid);
+
+typedef void(*handler_fn)(struct ex_regs *);
+void vm_install_exception_handler(struct kvm_vm *vm,
+		int vector, handler_fn handler);
+void vm_install_sync_handler(struct kvm_vm *vm,
+		int vector, int ec, handler_fn handler);
+
+#define write_sysreg(reg, val)						  \
+({									  \
+	u64 __val = (u64)(val);						  \
+	asm volatile("msr " __stringify(reg) ", %x0" : : "rZ" (__val));	  \
+})
+
+#define read_sysreg(reg)						  \
+({	u64 val;							  \
+	asm volatile("mrs %0, "__stringify(reg) : "=r"(val) : : "memory");\
+	val;								  \
+})
+
 #endif /* SELFTEST_KVM_PROCESSOR_H */
diff --git a/tools/testing/selftests/kvm/lib/aarch64/handlers.S b/tools/testing/selftests/kvm/lib/aarch64/handlers.S
new file mode 100644
index 000000000000..0e443eadfac6
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/aarch64/handlers.S
@@ -0,0 +1,126 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+.macro save_registers
+	add	sp, sp, #-16 * 17
+
+	stp	x0, x1, [sp, #16 * 0]
+	stp	x2, x3, [sp, #16 * 1]
+	stp	x4, x5, [sp, #16 * 2]
+	stp	x6, x7, [sp, #16 * 3]
+	stp	x8, x9, [sp, #16 * 4]
+	stp	x10, x11, [sp, #16 * 5]
+	stp	x12, x13, [sp, #16 * 6]
+	stp	x14, x15, [sp, #16 * 7]
+	stp	x16, x17, [sp, #16 * 8]
+	stp	x18, x19, [sp, #16 * 9]
+	stp	x20, x21, [sp, #16 * 10]
+	stp	x22, x23, [sp, #16 * 11]
+	stp	x24, x25, [sp, #16 * 12]
+	stp	x26, x27, [sp, #16 * 13]
+	stp	x28, x29, [sp, #16 * 14]
+
+	/*
+	 * This stores sp_el1 into ex_regs.sp so exception handlers can "look"
+	 * at it. It will _not_ be used to restore the sp on return from the
+	 * exception so handlers can not update it.
+	 */
+	add	x1, sp, #16 * 17
+	stp	x30, x1, [sp, #16 * 15] /* x30, SP */
+
+	mrs	x1, elr_el1
+	mrs	x2, spsr_el1
+	stp	x1, x2, [sp, #16 * 16] /* PC, PSTATE */
+.endm
+
+.macro restore_registers
+	ldp	x1, x2, [sp, #16 * 16] /* PC, PSTATE */
+	msr	elr_el1, x1
+	msr	spsr_el1, x2
+
+	/* sp is not restored */
+	ldp	x30, xzr, [sp, #16 * 15] /* x30, SP */
+
+	ldp	x28, x29, [sp, #16 * 14]
+	ldp	x26, x27, [sp, #16 * 13]
+	ldp	x24, x25, [sp, #16 * 12]
+	ldp	x22, x23, [sp, #16 * 11]
+	ldp	x20, x21, [sp, #16 * 10]
+	ldp	x18, x19, [sp, #16 * 9]
+	ldp	x16, x17, [sp, #16 * 8]
+	ldp	x14, x15, [sp, #16 * 7]
+	ldp	x12, x13, [sp, #16 * 6]
+	ldp	x10, x11, [sp, #16 * 5]
+	ldp	x8, x9, [sp, #16 * 4]
+	ldp	x6, x7, [sp, #16 * 3]
+	ldp	x4, x5, [sp, #16 * 2]
+	ldp	x2, x3, [sp, #16 * 1]
+	ldp	x0, x1, [sp, #16 * 0]
+
+	add	sp, sp, #16 * 17
+
+	eret
+.endm
+
+.pushsection ".entry.text", "ax"
+.balign 0x800
+.global vectors
+vectors:
+.popsection
+
+.set	vector, 0
+
+/*
+ * Build an exception handler for vector and append a jump to it into
+ * vectors (while making sure that it's 0x80 aligned).
+ */
+.macro HANDLER, label
+handler_\label:
+	save_registers
+	mov	x0, sp
+	mov	x1, #vector
+	bl	route_exception
+	restore_registers
+
+.pushsection ".entry.text", "ax"
+.balign 0x80
+	b	handler_\label
+.popsection
+
+.set	vector, vector + 1
+.endm
+
+.macro HANDLER_INVALID
+.pushsection ".entry.text", "ax"
+.balign 0x80
+/* This will abort so no need to save and restore registers. */
+	mov	x0, #vector
+	mov	x1, #0 /* ec */
+	mov	x2, #0 /* valid_ec */
+	b	kvm_exit_unexpected_exception
+.popsection
+
+.set	vector, vector + 1
+.endm
+
+/*
+ * Caution: be sure to not add anything between the declaration of vectors
+ * above and these macro calls that will build the vectors table below it.
+ */
+	HANDLER_INVALID                         // Synchronous EL1t
+	HANDLER_INVALID                         // IRQ EL1t
+	HANDLER_INVALID                         // FIQ EL1t
+	HANDLER_INVALID                         // Error EL1t
+
+	HANDLER	el1h_sync                       // Synchronous EL1h
+	HANDLER	el1h_irq                        // IRQ EL1h
+	HANDLER el1h_fiq                        // FIQ EL1h
+	HANDLER	el1h_error                      // Error EL1h
+
+	HANDLER	el0_sync_64                     // Synchronous 64-bit EL0
+	HANDLER	el0_irq_64                      // IRQ 64-bit EL0
+	HANDLER	el0_fiq_64                      // FIQ 64-bit EL0
+	HANDLER	el0_error_64                    // Error 64-bit EL0
+
+	HANDLER	el0_sync_32                     // Synchronous 32-bit EL0
+	HANDLER	el0_irq_32                      // IRQ 32-bit EL0
+	HANDLER	el0_fiq_32                      // FIQ 32-bit EL0
+	HANDLER	el0_error_32                    // Error 32-bit EL0
diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index cee92d477dc0..48b55c93f858 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/compiler.h>
+#include <assert.h>
 
 #include "kvm_util.h"
 #include "../kvm_util_internal.h"
@@ -14,6 +15,8 @@
 #define KVM_GUEST_PAGE_TABLE_MIN_PADDR		0x180000
 #define DEFAULT_ARM64_GUEST_STACK_VADDR_MIN	0xac0000
 
+static vm_vaddr_t exception_handlers;
+
 static uint64_t page_align(struct kvm_vm *vm, uint64_t v)
 {
 	return (v + vm->page_size) & ~(vm->page_size - 1);
@@ -334,6 +337,100 @@ void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
 	va_end(ap);
 }
 
+void kvm_exit_unexpected_exception(int vector, uint64_t ec, bool valid_ec)
+{
+	ucall(UCALL_UNHANDLED, 3, vector, ec, valid_ec);
+	while (1)
+		;
+}
+
 void assert_on_unhandled_exception(struct kvm_vm *vm, uint32_t vcpuid)
 {
+	struct ucall uc;
+
+	if (get_ucall(vm, vcpuid, &uc) != UCALL_UNHANDLED)
+		return;
+
+	if (uc.args[2]) /* valid_ec */ {
+		assert(VECTOR_IS_SYNC(uc.args[0]));
+		TEST_FAIL("Unexpected exception (vector:0x%lx, ec:0x%lx)",
+			  uc.args[0], uc.args[1]);
+	} else {
+		assert(!VECTOR_IS_SYNC(uc.args[0]));
+		TEST_FAIL("Unexpected exception (vector:0x%lx)",
+			  uc.args[0]);
+	}
+}
+
+struct handlers {
+	handler_fn exception_handlers[VECTOR_NUM][ESR_EC_NUM];
+};
+
+void vcpu_init_descriptor_tables(struct kvm_vm *vm, uint32_t vcpuid)
+{
+	extern char vectors;
+
+	set_reg(vm, vcpuid, ARM64_SYS_REG(VBAR_EL1), (uint64_t)&vectors);
+}
+
+void route_exception(struct ex_regs *regs, int vector)
+{
+	struct handlers *handlers = (struct handlers *)exception_handlers;
+	bool valid_ec;
+	int ec = 0;
+
+	switch (vector) {
+	case VECTOR_SYNC_CURRENT:
+	case VECTOR_SYNC_LOWER_64:
+		ec = (read_sysreg(esr_el1) >> ESR_EC_SHIFT) & ESR_EC_MASK;
+		valid_ec = true;
+		break;
+	case VECTOR_IRQ_CURRENT:
+	case VECTOR_IRQ_LOWER_64:
+	case VECTOR_FIQ_CURRENT:
+	case VECTOR_FIQ_LOWER_64:
+	case VECTOR_ERROR_CURRENT:
+	case VECTOR_ERROR_LOWER_64:
+		ec = 0;
+		valid_ec = false;
+		break;
+	default:
+		valid_ec = false;
+		goto unexpected_exception;
+	}
+
+	if (handlers && handlers->exception_handlers[vector][ec])
+		return handlers->exception_handlers[vector][ec](regs);
+
+unexpected_exception:
+	kvm_exit_unexpected_exception(vector, ec, valid_ec);
+}
+
+void vm_init_descriptor_tables(struct kvm_vm *vm)
+{
+	vm->handlers = vm_vaddr_alloc(vm, sizeof(struct handlers),
+			vm->page_size, 0, 0);
+
+	*(vm_vaddr_t *)addr_gva2hva(vm, (vm_vaddr_t)(&exception_handlers)) = vm->handlers;
+}
+
+void vm_install_sync_handler(struct kvm_vm *vm, int vector, int ec,
+			 void (*handler)(struct ex_regs *))
+{
+	struct handlers *handlers = addr_gva2hva(vm, vm->handlers);
+
+	assert(VECTOR_IS_SYNC(vector));
+	assert(vector < VECTOR_NUM);
+	assert(ec < ESR_EC_NUM);
+	handlers->exception_handlers[vector][ec] = handler;
+}
+
+void vm_install_exception_handler(struct kvm_vm *vm, int vector,
+			 void (*handler)(struct ex_regs *))
+{
+	struct handlers *handlers = addr_gva2hva(vm, vm->handlers);
+
+	assert(!VECTOR_IS_SYNC(vector));
+	assert(vector < VECTOR_NUM);
+	handlers->exception_handlers[vector][0] = handler;
 }
-- 
2.32.0.272.g935e593368-goog


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

* [PATCH v4 6/6] KVM: selftests: Add aarch64/debug-exceptions test
  2021-06-11  1:10 [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test Ricardo Koller
                   ` (4 preceding siblings ...)
  2021-06-11  1:10 ` [PATCH v4 5/6] KVM: selftests: Add exception handling support for aarch64 Ricardo Koller
@ 2021-06-11  1:10 ` Ricardo Koller
  2021-06-11 12:55 ` [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test Marc Zyngier
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Ricardo Koller @ 2021-06-11  1:10 UTC (permalink / raw)
  To: kvm, kvmarm
  Cc: pbonzini, maz, drjones, alexandru.elisei, eric.auger, yuzenghui,
	vkuznets, Ricardo Koller

Covers fundamental tests for debug exceptions. The guest installs and
handle its debug exceptions itself, without KVM_SET_GUEST_DEBUG.

Signed-off-by: Ricardo Koller <ricarkol@google.com>
---
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/aarch64/debug-exceptions.c  | 250 ++++++++++++++++++
 .../selftests/kvm/include/aarch64/processor.h |  22 +-
 4 files changed, 268 insertions(+), 6 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/aarch64/debug-exceptions.c

diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index 524c857a049c..7e2c66155b06 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
+/aarch64/debug-exceptions
 /aarch64/get-reg-list
 /aarch64/get-reg-list-sve
 /aarch64/vgic_init
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index a77e6063f7e9..36e4ebcc82f0 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -78,6 +78,7 @@ TEST_GEN_PROGS_x86_64 += memslot_perf_test
 TEST_GEN_PROGS_x86_64 += set_memory_region_test
 TEST_GEN_PROGS_x86_64 += steal_time
 
+TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
 TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
 TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list-sve
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_init
diff --git a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
new file mode 100644
index 000000000000..e5e6c92b60da
--- /dev/null
+++ b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_util.h>
+#include <kvm_util.h>
+#include <processor.h>
+
+#define VCPU_ID 0
+
+#define MDSCR_KDE	(1 << 13)
+#define MDSCR_MDE	(1 << 15)
+#define MDSCR_SS	(1 << 0)
+
+#define DBGBCR_LEN8	(0xff << 5)
+#define DBGBCR_EXEC	(0x0 << 3)
+#define DBGBCR_EL1	(0x1 << 1)
+#define DBGBCR_E	(0x1 << 0)
+
+#define DBGWCR_LEN8	(0xff << 5)
+#define DBGWCR_RD	(0x1 << 3)
+#define DBGWCR_WR	(0x2 << 3)
+#define DBGWCR_EL1	(0x1 << 1)
+#define DBGWCR_E	(0x1 << 0)
+
+#define SPSR_D		(1 << 9)
+#define SPSR_SS		(1 << 21)
+
+extern unsigned char sw_bp, hw_bp, bp_svc, bp_brk, hw_wp, ss_start;
+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;
+static volatile uint64_t ss_addr[4], ss_idx;
+#define  PC(v)  ((uint64_t)&(v))
+
+static void reset_debug_state(void)
+{
+	asm volatile("msr daifset, #8");
+
+	write_sysreg(osdlr_el1, 0);
+	write_sysreg(oslar_el1, 0);
+	isb();
+
+	write_sysreg(mdscr_el1, 0);
+	/* This test only uses the first bp and wp slot. */
+	write_sysreg(dbgbvr0_el1, 0);
+	write_sysreg(dbgbcr0_el1, 0);
+	write_sysreg(dbgwcr0_el1, 0);
+	write_sysreg(dbgwvr0_el1, 0);
+	isb();
+}
+
+static void install_wp(uint64_t addr)
+{
+	uint32_t wcr;
+	uint32_t mdscr;
+
+	wcr = DBGWCR_LEN8 | DBGWCR_RD | DBGWCR_WR | DBGWCR_EL1 | DBGWCR_E;
+	write_sysreg(dbgwcr0_el1, wcr);
+	write_sysreg(dbgwvr0_el1, addr);
+	isb();
+
+	asm volatile("msr daifclr, #8");
+
+	mdscr = read_sysreg(mdscr_el1) | MDSCR_KDE | MDSCR_MDE;
+	write_sysreg(mdscr_el1, mdscr);
+	isb();
+}
+
+static void install_hw_bp(uint64_t addr)
+{
+	uint32_t bcr;
+	uint32_t mdscr;
+
+	bcr = DBGBCR_LEN8 | DBGBCR_EXEC | DBGBCR_EL1 | DBGBCR_E;
+	write_sysreg(dbgbcr0_el1, bcr);
+	write_sysreg(dbgbvr0_el1, addr);
+	isb();
+
+	asm volatile("msr daifclr, #8");
+
+	mdscr = read_sysreg(mdscr_el1) | MDSCR_KDE | MDSCR_MDE;
+	write_sysreg(mdscr_el1, mdscr);
+	isb();
+}
+
+static void install_ss(void)
+{
+	uint32_t mdscr;
+
+	asm volatile("msr daifclr, #8");
+
+	mdscr = read_sysreg(mdscr_el1) | MDSCR_KDE | MDSCR_SS;
+	write_sysreg(mdscr_el1, mdscr);
+	isb();
+}
+
+static volatile char write_data;
+
+static void guest_code(void)
+{
+	GUEST_SYNC(0);
+
+	/* Software-breakpoint */
+	asm volatile("sw_bp: brk #0");
+	GUEST_ASSERT_EQ(sw_bp_addr, PC(sw_bp));
+
+	GUEST_SYNC(1);
+
+	/* Hardware-breakpoint */
+	reset_debug_state();
+	install_hw_bp(PC(hw_bp));
+	asm volatile("hw_bp: nop");
+	GUEST_ASSERT_EQ(hw_bp_addr, PC(hw_bp));
+
+	GUEST_SYNC(2);
+
+	/* Hardware-breakpoint + svc */
+	reset_debug_state();
+	install_hw_bp(PC(bp_svc));
+	asm volatile("bp_svc: svc #0");
+	GUEST_ASSERT_EQ(hw_bp_addr, PC(bp_svc));
+	GUEST_ASSERT_EQ(svc_addr, PC(bp_svc) + 4);
+
+	GUEST_SYNC(3);
+
+	/* Hardware-breakpoint + software-breakpoint */
+	reset_debug_state();
+	install_hw_bp(PC(bp_brk));
+	asm volatile("bp_brk: brk #0");
+	GUEST_ASSERT_EQ(sw_bp_addr, PC(bp_brk));
+	GUEST_ASSERT_EQ(hw_bp_addr, PC(bp_brk));
+
+	GUEST_SYNC(4);
+
+	/* Watchpoint */
+	reset_debug_state();
+	install_wp(PC(write_data));
+	write_data = 'x';
+	GUEST_ASSERT_EQ(write_data, 'x');
+	GUEST_ASSERT_EQ(wp_data_addr, PC(write_data));
+
+	GUEST_SYNC(5);
+
+	/* Single-step */
+	reset_debug_state();
+	install_ss();
+	ss_idx = 0;
+	asm volatile("ss_start:\n"
+		     "mrs x0, esr_el1\n"
+		     "add x0, x0, #1\n"
+		     "msr daifset, #8\n"
+		     : : : "x0");
+	GUEST_ASSERT_EQ(ss_addr[0], PC(ss_start));
+	GUEST_ASSERT_EQ(ss_addr[1], PC(ss_start) + 4);
+	GUEST_ASSERT_EQ(ss_addr[2], PC(ss_start) + 8);
+
+	GUEST_DONE();
+}
+
+static void guest_sw_bp_handler(struct ex_regs *regs)
+{
+	sw_bp_addr = regs->pc;
+	regs->pc += 4;
+}
+
+static void guest_hw_bp_handler(struct ex_regs *regs)
+{
+	hw_bp_addr = regs->pc;
+	regs->pstate |= SPSR_D;
+}
+
+static void guest_wp_handler(struct ex_regs *regs)
+{
+	wp_data_addr = read_sysreg(far_el1);
+	wp_addr = regs->pc;
+	regs->pstate |= SPSR_D;
+}
+
+static void guest_ss_handler(struct ex_regs *regs)
+{
+	GUEST_ASSERT_1(ss_idx < 4, ss_idx);
+	ss_addr[ss_idx++] = regs->pc;
+	regs->pstate |= SPSR_SS;
+}
+
+static void guest_svc_handler(struct ex_regs *regs)
+{
+	svc_addr = regs->pc;
+}
+
+static int debug_version(struct kvm_vm *vm)
+{
+	uint64_t id_aa64dfr0;
+
+	get_reg(vm, VCPU_ID, ARM64_SYS_REG(ID_AA64DFR0_EL1), &id_aa64dfr0);
+	return id_aa64dfr0 & 0xf;
+}
+
+int main(int argc, char *argv[])
+{
+	struct kvm_vm *vm;
+	struct ucall uc;
+	int stage;
+
+	vm = vm_create_default(VCPU_ID, 0, guest_code);
+	ucall_init(vm, NULL);
+
+	vm_init_descriptor_tables(vm);
+	vcpu_init_descriptor_tables(vm, VCPU_ID);
+
+	if (debug_version(vm) < 6) {
+		print_skip("Armv8 debug architecture not supported.");
+		kvm_vm_free(vm);
+		exit(KSFT_SKIP);
+	}
+
+	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
+				ESR_EC_BRK_INS, guest_sw_bp_handler);
+	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
+				ESR_EC_HW_BP_CURRENT, guest_hw_bp_handler);
+	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
+				ESR_EC_WP_CURRENT, guest_wp_handler);
+	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
+				ESR_EC_SSTEP_CURRENT, guest_ss_handler);
+	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
+				ESR_EC_SVC64, guest_svc_handler);
+
+	for (stage = 0; stage < 7; stage++) {
+		vcpu_run(vm, VCPU_ID);
+
+		switch (get_ucall(vm, VCPU_ID, &uc)) {
+		case UCALL_SYNC:
+			TEST_ASSERT(uc.args[1] == stage,
+				"Stage %d: Unexpected sync ucall, got %lx",
+				stage, (ulong)uc.args[1]);
+			break;
+		case UCALL_ABORT:
+			TEST_FAIL("%s at %s:%ld\n\tvalues: %#lx, %#lx",
+				(const char *)uc.args[0],
+				__FILE__, uc.args[1], uc.args[2], uc.args[3]);
+			break;
+		case UCALL_DONE:
+			goto done;
+		default:
+			TEST_FAIL("Unknown ucall %lu", uc.cmd);
+		}
+	}
+
+done:
+	kvm_vm_free(vm);
+	return 0;
+}
diff --git a/tools/testing/selftests/kvm/include/aarch64/processor.h b/tools/testing/selftests/kvm/include/aarch64/processor.h
index b2b3e9d626cb..27dc5c2e56b9 100644
--- a/tools/testing/selftests/kvm/include/aarch64/processor.h
+++ b/tools/testing/selftests/kvm/include/aarch64/processor.h
@@ -14,12 +14,14 @@
 #define ARM64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
 			   KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
 
-#define CPACR_EL1	3, 0,  1, 0, 2
-#define TCR_EL1		3, 0,  2, 0, 2
-#define MAIR_EL1	3, 0, 10, 2, 0
-#define TTBR0_EL1	3, 0,  2, 0, 0
-#define SCTLR_EL1	3, 0,  1, 0, 0
-#define VBAR_EL1	3, 0, 12, 0, 0
+#define CPACR_EL1               3, 0,  1, 0, 2
+#define TCR_EL1                 3, 0,  2, 0, 2
+#define MAIR_EL1                3, 0, 10, 2, 0
+#define TTBR0_EL1               3, 0,  2, 0, 0
+#define SCTLR_EL1               3, 0,  1, 0, 0
+#define VBAR_EL1                3, 0, 12, 0, 0
+
+#define ID_AA64DFR0_EL1         3, 0,  0, 5, 0
 
 /*
  * Default MAIR
@@ -98,6 +100,12 @@ enum {
 #define ESR_EC_SHIFT		26
 #define ESR_EC_MASK		(ESR_EC_NUM - 1)
 
+#define ESR_EC_SVC64		0x15
+#define ESR_EC_HW_BP_CURRENT	0x31
+#define ESR_EC_SSTEP_CURRENT	0x33
+#define ESR_EC_WP_CURRENT	0x35
+#define ESR_EC_BRK_INS		0x3c
+
 void vm_init_descriptor_tables(struct kvm_vm *vm);
 void vcpu_init_descriptor_tables(struct kvm_vm *vm, uint32_t vcpuid);
 
@@ -119,4 +127,6 @@ void vm_install_sync_handler(struct kvm_vm *vm,
 	val;								  \
 })
 
+#define isb()	asm volatile("isb" : : : "memory")
+
 #endif /* SELFTEST_KVM_PROCESSOR_H */
-- 
2.32.0.272.g935e593368-goog


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

* Re: [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test
  2021-06-11  1:10 [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test Ricardo Koller
                   ` (5 preceding siblings ...)
  2021-06-11  1:10 ` [PATCH v4 6/6] KVM: selftests: Add aarch64/debug-exceptions test Ricardo Koller
@ 2021-06-11 12:55 ` Marc Zyngier
  2021-06-14  7:40 ` Andrew Jones
  2021-06-14  8:08 ` Marc Zyngier
  8 siblings, 0 replies; 15+ messages in thread
From: Marc Zyngier @ 2021-06-11 12:55 UTC (permalink / raw)
  To: Ricardo Koller
  Cc: kvm, kvmarm, pbonzini, drjones, alexandru.elisei, eric.auger,
	yuzenghui, vkuznets

On 2021-06-11 02:10, Ricardo Koller wrote:
> Hi,
> 
> These patches add a debug exception test in aarch64 KVM selftests while
> also adding basic exception handling support.
> 
> The structure of the exception handling is based on its x86 
> counterpart.
> Tests use the same calls to initialize exception handling and both
> architectures allow tests to override the handler for a particular
> vector, or (vector, ec) for synchronous exceptions in the arm64 case.
> 
> The debug test is similar to x86_64/debug_regs, except that the x86 one
> controls the debugging from outside the VM. This proposed arm64 test
> controls and handles debug exceptions from the inside.
> 
> Thanks,
> Ricardo
> 
> v3 -> v4:
> 
> V3 was dropped because it was breaking x86 selftests builds (reported 
> by
> the kernel test robot).
> - rename vm_handle_exception to vm_install_sync_handler instead of
>   vm_install_vector_handlers. [Sean]
> - use a single level of routing for exception handling. [Sean]
> - fix issue in x86_64/sync_regs_test when switching to ucalls for 
> unhandled
>   exceptions reporting.

This looks good to me. If I can get an Ack from any of the x86 
maintainers,
I'll queue the series.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

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

* Re: [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test
  2021-06-11  1:10 [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test Ricardo Koller
                   ` (6 preceding siblings ...)
  2021-06-11 12:55 ` [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test Marc Zyngier
@ 2021-06-14  7:40 ` Andrew Jones
  2021-06-14  8:08 ` Marc Zyngier
  8 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2021-06-14  7:40 UTC (permalink / raw)
  To: Ricardo Koller
  Cc: kvm, kvmarm, pbonzini, maz, alexandru.elisei, eric.auger,
	yuzenghui, vkuznets

On Thu, Jun 10, 2021 at 06:10:14PM -0700, Ricardo Koller wrote:
> Hi,
> 
> These patches add a debug exception test in aarch64 KVM selftests while
> also adding basic exception handling support.
> 
> The structure of the exception handling is based on its x86 counterpart.
> Tests use the same calls to initialize exception handling and both
> architectures allow tests to override the handler for a particular
> vector, or (vector, ec) for synchronous exceptions in the arm64 case.
> 
> The debug test is similar to x86_64/debug_regs, except that the x86 one
> controls the debugging from outside the VM. This proposed arm64 test
> controls and handles debug exceptions from the inside.
> 
> Thanks,
> Ricardo
> 
> v3 -> v4:
> 
> V3 was dropped because it was breaking x86 selftests builds (reported by
> the kernel test robot).
> - rename vm_handle_exception to vm_install_sync_handler instead of
>   vm_install_vector_handlers. [Sean]
> - use a single level of routing for exception handling. [Sean]
> - fix issue in x86_64/sync_regs_test when switching to ucalls for unhandled
>   exceptions reporting.
> 
> v2 -> v3:
> 
> Addressed comments from Andrew and Marc (thanks again). Also, many thanks for
> the reviews and tests from Eric and Zenghui.
> - add missing ISBs after writing into debug registers.
> - not store/restore of sp_el0 on exceptions.
> - add default handlers for Error and FIQ.
> - change multiple TEST_ASSERT(false, ...) to TEST_FAIL.
> - use Andrew's suggestion regarding __GUEST_ASSERT modifications
>   in order to easier implement GUEST_ASSERT_EQ (Thanks Andrew).
> 
> v1 -> v2:
> 
> Addressed comments from Andrew and Marc (thank you very much):
> - rename vm_handle_exception in all tests.
> - introduce UCALL_UNHANDLED in x86 first.
> - move GUEST_ASSERT_EQ to common utils header.
> - handle sync and other exceptions separately: use two tables (like
>   kvm-unit-tests).
> - add two separate functions for installing sync versus other exceptions
> - changes in handlers.S: use the same layout as user_pt_regs, treat the
>   EL1t vectors as invalid, refactor the vector table creation to not use
>   manual numbering, add comments, remove LR from the stored registers.
> - changes in debug-exceptions.c: remove unused headers, use the common
>   GUEST_ASSERT_EQ, use vcpu_run instead of _vcpu_run.
> - changes in processor.h: write_sysreg with support for xzr, replace EL1
>   with current in macro names, define ESR_EC_MASK as ESR_EC_NUM-1.
> 
> Ricardo Koller (6):
>   KVM: selftests: Rename vm_handle_exception
>   KVM: selftests: Complete x86_64/sync_regs_test ucall
>   KVM: selftests: Introduce UCALL_UNHANDLED for unhandled vector
>     reporting
>   KVM: selftests: Move GUEST_ASSERT_EQ to utils header
>   KVM: selftests: Add exception handling support for aarch64
>   KVM: selftests: Add aarch64/debug-exceptions test
> 
>  tools/testing/selftests/kvm/.gitignore        |   1 +
>  tools/testing/selftests/kvm/Makefile          |   3 +-
>  .../selftests/kvm/aarch64/debug-exceptions.c  | 250 ++++++++++++++++++
>  .../selftests/kvm/include/aarch64/processor.h |  83 +++++-
>  .../testing/selftests/kvm/include/kvm_util.h  |  23 +-
>  .../selftests/kvm/include/x86_64/processor.h  |   4 +-
>  .../selftests/kvm/lib/aarch64/handlers.S      | 126 +++++++++
>  .../selftests/kvm/lib/aarch64/processor.c     |  97 +++++++
>  .../selftests/kvm/lib/x86_64/processor.c      |  23 +-
>  .../testing/selftests/kvm/x86_64/evmcs_test.c |   4 +-
>  .../selftests/kvm/x86_64/kvm_pv_test.c        |   2 +-
>  .../selftests/kvm/x86_64/sync_regs_test.c     |   7 +-
>  .../selftests/kvm/x86_64/tsc_msrs_test.c      |   9 -
>  .../kvm/x86_64/userspace_msr_exit_test.c      |   8 +-
>  .../selftests/kvm/x86_64/xapic_ipi_test.c     |   2 +-
>  15 files changed, 592 insertions(+), 50 deletions(-)
>  create mode 100644 tools/testing/selftests/kvm/aarch64/debug-exceptions.c
>  create mode 100644 tools/testing/selftests/kvm/lib/aarch64/handlers.S
> 
> -- 
> 2.32.0.272.g935e593368-goog
>

For the series

Reviewed-by: Andrew Jones <drjones@redhat.com>

Thanks,
drew


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

* Re: [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test
  2021-06-11  1:10 [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test Ricardo Koller
                   ` (7 preceding siblings ...)
  2021-06-14  7:40 ` Andrew Jones
@ 2021-06-14  8:08 ` Marc Zyngier
  8 siblings, 0 replies; 15+ messages in thread
From: Marc Zyngier @ 2021-06-14  8:08 UTC (permalink / raw)
  To: Ricardo Koller, kvm, kvmarm
  Cc: vkuznets, pbonzini, alexandru.elisei, eric.auger, drjones, yuzenghui

On Thu, 10 Jun 2021 18:10:14 -0700, Ricardo Koller wrote:
> These patches add a debug exception test in aarch64 KVM selftests while
> also adding basic exception handling support.
> 
> The structure of the exception handling is based on its x86 counterpart.
> Tests use the same calls to initialize exception handling and both
> architectures allow tests to override the handler for a particular
> vector, or (vector, ec) for synchronous exceptions in the arm64 case.
> 
> [...]

Applied to next, thanks!

[1/6] KVM: selftests: Rename vm_handle_exception
      commit: b78f4a596692f6805e796a4c13f2d921b8a95166
[2/6] KVM: selftests: Complete x86_64/sync_regs_test ucall
      commit: b7326c01122683b88e273a0cc826cd4c01234470
[3/6] KVM: selftests: Introduce UCALL_UNHANDLED for unhandled vector reporting
      commit: 75275d7fbef47805b77e8af81a4d51e2d92db70f
[4/6] KVM: selftests: Move GUEST_ASSERT_EQ to utils header
      commit: 67f709f52bf0b5c19f24d1234163123cbb6af545
[5/6] KVM: selftests: Add exception handling support for aarch64
      commit: e3db7579ef355a0b2bfef4448b84d9ac882c8f2c
[6/6] KVM: selftests: Add aarch64/debug-exceptions test
      commit: 4f05223acaeaabe0a1a188e25fab334735d85c5e

Cheers,

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



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

* Re: [PATCH v4 5/6] KVM: selftests: Add exception handling support for aarch64
  2021-06-11  1:10 ` [PATCH v4 5/6] KVM: selftests: Add exception handling support for aarch64 Ricardo Koller
@ 2021-07-02  6:46   ` Zenghui Yu
  2021-07-02 18:56     ` Ricardo Koller
  0 siblings, 1 reply; 15+ messages in thread
From: Zenghui Yu @ 2021-07-02  6:46 UTC (permalink / raw)
  To: Ricardo Koller
  Cc: kvm, kvmarm, pbonzini, maz, drjones, alexandru.elisei,
	eric.auger, vkuznets, Sean Christopherson

[+Sean]

On 2021/6/11 9:10, Ricardo Koller wrote:
> Add the infrastructure needed to enable exception handling in aarch64
> selftests. The exception handling defaults to an unhandled-exception
> handler which aborts the test, just like x86. These handlers can be
> overridden by calling vm_install_exception_handler(vector) or
> vm_install_sync_handler(vector, ec). The unhandled exception reporting
> from the guest is done using the ucall type introduced in a previous
> commit, UCALL_UNHANDLED.
> 
> The exception handling code is inspired on kvm-unit-tests.
> 
> Signed-off-by: Ricardo Koller <ricarkol@google.com>
> ---
>  tools/testing/selftests/kvm/Makefile          |   2 +-
>  .../selftests/kvm/include/aarch64/processor.h |  63 +++++++++
>  .../selftests/kvm/lib/aarch64/handlers.S      | 126 ++++++++++++++++++
>  .../selftests/kvm/lib/aarch64/processor.c     |  97 ++++++++++++++
>  4 files changed, 287 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/kvm/lib/aarch64/handlers.S

[...]

> +void vm_init_descriptor_tables(struct kvm_vm *vm)
> +{
> +	vm->handlers = vm_vaddr_alloc(vm, sizeof(struct handlers),
> +			vm->page_size, 0, 0);

This raced with commit a75a895e6457 ("KVM: selftests: Unconditionally
use memslot 0 for vaddr allocations") which dropped memslot parameters
from vm_vaddr_alloc().

We can remove the related comments on top of vm_vaddr_alloc() as well.

Zenghui

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

* Re: [PATCH v4 5/6] KVM: selftests: Add exception handling support for aarch64
  2021-07-02  6:46   ` Zenghui Yu
@ 2021-07-02 18:56     ` Ricardo Koller
  0 siblings, 0 replies; 15+ messages in thread
From: Ricardo Koller @ 2021-07-02 18:56 UTC (permalink / raw)
  To: Zenghui Yu
  Cc: kvm, kvmarm, pbonzini, maz, drjones, alexandru.elisei,
	eric.auger, vkuznets, Sean Christopherson

On Fri, Jul 02, 2021 at 02:46:57PM +0800, Zenghui Yu wrote:
> [+Sean]
> 
> On 2021/6/11 9:10, Ricardo Koller wrote:
> > Add the infrastructure needed to enable exception handling in aarch64
> > selftests. The exception handling defaults to an unhandled-exception
> > handler which aborts the test, just like x86. These handlers can be
> > overridden by calling vm_install_exception_handler(vector) or
> > vm_install_sync_handler(vector, ec). The unhandled exception reporting
> > from the guest is done using the ucall type introduced in a previous
> > commit, UCALL_UNHANDLED.
> > 
> > The exception handling code is inspired on kvm-unit-tests.
> > 
> > Signed-off-by: Ricardo Koller <ricarkol@google.com>
> > ---
> >  tools/testing/selftests/kvm/Makefile          |   2 +-
> >  .../selftests/kvm/include/aarch64/processor.h |  63 +++++++++
> >  .../selftests/kvm/lib/aarch64/handlers.S      | 126 ++++++++++++++++++
> >  .../selftests/kvm/lib/aarch64/processor.c     |  97 ++++++++++++++
> >  4 files changed, 287 insertions(+), 1 deletion(-)
> >  create mode 100644 tools/testing/selftests/kvm/lib/aarch64/handlers.S
> 
> [...]
> 
> > +void vm_init_descriptor_tables(struct kvm_vm *vm)
> > +{
> > +	vm->handlers = vm_vaddr_alloc(vm, sizeof(struct handlers),
> > +			vm->page_size, 0, 0);
> 
> This raced with commit a75a895e6457 ("KVM: selftests: Unconditionally
> use memslot 0 for vaddr allocations") which dropped memslot parameters
> from vm_vaddr_alloc().

Will send a fix to use vm_vaddr_alloc() without the memslot parameters.

> 
> We can remove the related comments on top of vm_vaddr_alloc() as well.

Can do this as well, will send a separate patch removing this. Unless
somebody was about to.

> 
> Zenghui

Thanks,
Ricardo

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

* Re: [PATCH v4 3/6] KVM: selftests: Introduce UCALL_UNHANDLED for unhandled vector reporting
  2021-06-11  1:10 ` [PATCH v4 3/6] KVM: selftests: Introduce UCALL_UNHANDLED for unhandled vector reporting Ricardo Koller
@ 2021-07-29 18:15   ` Sean Christopherson
  2021-07-30  1:10     ` Ricardo Koller
  0 siblings, 1 reply; 15+ messages in thread
From: Sean Christopherson @ 2021-07-29 18:15 UTC (permalink / raw)
  To: Ricardo Koller
  Cc: kvm, kvmarm, pbonzini, maz, drjones, alexandru.elisei,
	eric.auger, yuzenghui, vkuznets

On Thu, Jun 10, 2021, Ricardo Koller wrote:
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index fcd8e3855111..beb76d6deaa9 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -349,6 +349,7 @@ enum {
>  	UCALL_SYNC,
>  	UCALL_ABORT,
>  	UCALL_DONE,
> +	UCALL_UNHANDLED,
>  };

...

> @@ -1254,16 +1254,13 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector,
>  
>  void assert_on_unhandled_exception(struct kvm_vm *vm, uint32_t vcpuid)
>  {
> -	if (vcpu_state(vm, vcpuid)->exit_reason == KVM_EXIT_IO
> -		&& vcpu_state(vm, vcpuid)->io.port == UNEXPECTED_VECTOR_PORT
> -		&& vcpu_state(vm, vcpuid)->io.size == 4) {
> -		/* Grab pointer to io data */
> -		uint32_t *data = (void *)vcpu_state(vm, vcpuid)
> -			+ vcpu_state(vm, vcpuid)->io.data_offset;
> -
> -		TEST_ASSERT(false,
> -			    "Unexpected vectored event in guest (vector:0x%x)",
> -			    *data);
> +	struct ucall uc;
> +
> +	if (get_ucall(vm, vcpuid, &uc) == UCALL_UNHANDLED) {

UCALL_UNHANDLED is a bit of an odd name.  Without the surrounding context, I would
have no idea that it's referring to an unhandled event, e.g. my gut reaction would
be that it means the ucall itself was unhandled.  Maybe UCALL_UNHANDLED_EVENT?
It's rather long, but I don't think that will be problematic for any of the code.


> +		uint64_t vector = uc.args[0];
> +
> +		TEST_FAIL("Unexpected vectored event in guest (vector:0x%lx)",
> +			  vector);
>  	}
>  }
>  
> -- 
> 2.32.0.272.g935e593368-goog
> 

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

* Re: [PATCH v4 3/6] KVM: selftests: Introduce UCALL_UNHANDLED for unhandled vector reporting
  2021-07-29 18:15   ` Sean Christopherson
@ 2021-07-30  1:10     ` Ricardo Koller
  2021-07-30 16:24       ` Sean Christopherson
  0 siblings, 1 reply; 15+ messages in thread
From: Ricardo Koller @ 2021-07-30  1:10 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: kvm, kvmarm, pbonzini, maz, drjones, alexandru.elisei,
	eric.auger, yuzenghui, vkuznets

On Thu, Jul 29, 2021 at 06:15:27PM +0000, Sean Christopherson wrote:
> On Thu, Jun 10, 2021, Ricardo Koller wrote:
> > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > index fcd8e3855111..beb76d6deaa9 100644
> > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > @@ -349,6 +349,7 @@ enum {
> >  	UCALL_SYNC,
> >  	UCALL_ABORT,
> >  	UCALL_DONE,
> > +	UCALL_UNHANDLED,
> >  };
> 
> ...
> 
> > @@ -1254,16 +1254,13 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector,
> >  
> >  void assert_on_unhandled_exception(struct kvm_vm *vm, uint32_t vcpuid)
> >  {
> > -	if (vcpu_state(vm, vcpuid)->exit_reason == KVM_EXIT_IO
> > -		&& vcpu_state(vm, vcpuid)->io.port == UNEXPECTED_VECTOR_PORT
> > -		&& vcpu_state(vm, vcpuid)->io.size == 4) {
> > -		/* Grab pointer to io data */
> > -		uint32_t *data = (void *)vcpu_state(vm, vcpuid)
> > -			+ vcpu_state(vm, vcpuid)->io.data_offset;
> > -
> > -		TEST_ASSERT(false,
> > -			    "Unexpected vectored event in guest (vector:0x%x)",
> > -			    *data);
> > +	struct ucall uc;
> > +
> > +	if (get_ucall(vm, vcpuid, &uc) == UCALL_UNHANDLED) {
> 
> UCALL_UNHANDLED is a bit of an odd name.  Without the surrounding context, I would
> have no idea that it's referring to an unhandled event, e.g. my gut reaction would
> be that it means the ucall itself was unhandled. Maybe UCALL_UNHANDLED_EVENT?

I see. I can send a new patch (this was commited as 75275d7fbe) with a
new name. The only name I can think of that's more descriptive would be
UCALL_UNHANDLED_EXCEPTION, but that's even longer.

> It's rather long, but I don't think that will be problematic for any of the code.
> 
> 
> > +		uint64_t vector = uc.args[0];
> > +
> > +		TEST_FAIL("Unexpected vectored event in guest (vector:0x%lx)",
> > +			  vector);
> >  	}
> >  }
> >  
> > -- 
> > 2.32.0.272.g935e593368-goog
> > 

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

* Re: [PATCH v4 3/6] KVM: selftests: Introduce UCALL_UNHANDLED for unhandled vector reporting
  2021-07-30  1:10     ` Ricardo Koller
@ 2021-07-30 16:24       ` Sean Christopherson
  0 siblings, 0 replies; 15+ messages in thread
From: Sean Christopherson @ 2021-07-30 16:24 UTC (permalink / raw)
  To: Ricardo Koller
  Cc: kvm, kvmarm, pbonzini, maz, drjones, alexandru.elisei,
	eric.auger, yuzenghui, vkuznets

On Thu, Jul 29, 2021, Ricardo Koller wrote:
> On Thu, Jul 29, 2021 at 06:15:27PM +0000, Sean Christopherson wrote:
> > On Thu, Jun 10, 2021, Ricardo Koller wrote:
> > > +	struct ucall uc;
> > > +
> > > +	if (get_ucall(vm, vcpuid, &uc) == UCALL_UNHANDLED) {
> > 
> > UCALL_UNHANDLED is a bit of an odd name.  Without the surrounding context, I would
> > have no idea that it's referring to an unhandled event, e.g. my gut reaction would
> > be that it means the ucall itself was unhandled. Maybe UCALL_UNHANDLED_EVENT?
> 
> I see. I can send a new patch (this was commited as 75275d7fbe) with a
> new name.

Eh, no need to post another patch.  If it can be fixed up in tree, great, if not,
no big deal.

> The only name I can think of that's more descriptive would be
> UCALL_UNHANDLED_EXCEPTION, but that's even longer.

Unfortunately, EXCEPTION is incorrect as x86 will route unexpected IRQs through
this as well.

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

end of thread, other threads:[~2021-07-30 16:24 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-11  1:10 [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test Ricardo Koller
2021-06-11  1:10 ` [PATCH v4 1/6] KVM: selftests: Rename vm_handle_exception Ricardo Koller
2021-06-11  1:10 ` [PATCH v4 2/6] KVM: selftests: Complete x86_64/sync_regs_test ucall Ricardo Koller
2021-06-11  1:10 ` [PATCH v4 3/6] KVM: selftests: Introduce UCALL_UNHANDLED for unhandled vector reporting Ricardo Koller
2021-07-29 18:15   ` Sean Christopherson
2021-07-30  1:10     ` Ricardo Koller
2021-07-30 16:24       ` Sean Christopherson
2021-06-11  1:10 ` [PATCH v4 4/6] KVM: selftests: Move GUEST_ASSERT_EQ to utils header Ricardo Koller
2021-06-11  1:10 ` [PATCH v4 5/6] KVM: selftests: Add exception handling support for aarch64 Ricardo Koller
2021-07-02  6:46   ` Zenghui Yu
2021-07-02 18:56     ` Ricardo Koller
2021-06-11  1:10 ` [PATCH v4 6/6] KVM: selftests: Add aarch64/debug-exceptions test Ricardo Koller
2021-06-11 12:55 ` [PATCH v4 0/6] KVM: selftests: arm64 exception handling and debug test Marc Zyngier
2021-06-14  7:40 ` Andrew Jones
2021-06-14  8:08 ` 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).