From 215a681710a554b224d743e4a9cd68965889713c Mon Sep 17 00:00:00 2001 From: Aaron Lewis Date: Fri, 28 Jul 2023 17:36:15 -0700 Subject: KVM: selftests: Add additional pages to the guest to accommodate ucall Add additional pages to the guest to account for the number of pages the ucall headers need. The only reason things worked before is the ucall headers are fairly small. If they were ever to increase in size the guest could run out of memory. This is done in preparation for adding string formatting options to the guest through the ucall framework which increases the size of the ucall headers. Fixes: 426729b2cf2e ("KVM: selftests: Add ucall pool based implementation") Signed-off-by: Aaron Lewis Link: https://lore.kernel.org/r/20230729003643.1053367-7-seanjc@google.com Signed-off-by: Sean Christopherson --- tools/testing/selftests/kvm/lib/ucall_common.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'tools/testing/selftests/kvm/lib/ucall_common.c') diff --git a/tools/testing/selftests/kvm/lib/ucall_common.c b/tools/testing/selftests/kvm/lib/ucall_common.c index 2f0e2ea941cc..77ada362273d 100644 --- a/tools/testing/selftests/kvm/lib/ucall_common.c +++ b/tools/testing/selftests/kvm/lib/ucall_common.c @@ -11,6 +11,11 @@ struct ucall_header { struct ucall ucalls[KVM_MAX_VCPUS]; }; +int ucall_nr_pages_required(uint64_t page_size) +{ + return align_up(sizeof(struct ucall_header), page_size) / page_size; +} + /* * ucall_pool holds per-VM values (global data is duplicated by each VM), it * must not be accessed from host code. -- cgit From 57e5c1fef5ecc7c346ff971817ba6985643e0a24 Mon Sep 17 00:00:00 2001 From: Aaron Lewis Date: Fri, 28 Jul 2023 17:36:16 -0700 Subject: KVM: selftests: Add string formatting options to ucall Add more flexibility to guest debugging and testing by adding GUEST_PRINTF() and GUEST_ASSERT_FMT() to the ucall framework. Add a sized buffer to the ucall structure to hold the formatted string, i.e. to allow the guest to easily resolve the string, and thus avoid the ugly pattern of the host side having to make assumptions about the desired format, as well as having to pass around a large number of parameters. The buffer size was chosen to accommodate most use cases, and based on similar usage. E.g. printf() uses the same size buffer in arch/x86/boot/printf.c. And 1KiB ought to be enough for anybody. Signed-off-by: Aaron Lewis [sean: massage changelog, wrap macro param in ()] Link: https://lore.kernel.org/r/20230729003643.1053367-8-seanjc@google.com Signed-off-by: Sean Christopherson --- tools/testing/selftests/kvm/include/ucall_common.h | 7 +++++++ tools/testing/selftests/kvm/lib/ucall_common.c | 17 +++++++++++++++++ 2 files changed, 24 insertions(+) (limited to 'tools/testing/selftests/kvm/lib/ucall_common.c') diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h index bcbb362aa77f..b5548aeba9f0 100644 --- a/tools/testing/selftests/kvm/include/ucall_common.h +++ b/tools/testing/selftests/kvm/include/ucall_common.h @@ -13,15 +13,18 @@ enum { UCALL_NONE, UCALL_SYNC, UCALL_ABORT, + UCALL_PRINTF, UCALL_DONE, UCALL_UNHANDLED, }; #define UCALL_MAX_ARGS 7 +#define UCALL_BUFFER_LEN 1024 struct ucall { uint64_t cmd; uint64_t args[UCALL_MAX_ARGS]; + char buffer[UCALL_BUFFER_LEN]; /* Host virtual address of this struct. */ struct ucall *hva; @@ -32,6 +35,7 @@ void ucall_arch_do_ucall(vm_vaddr_t uc); void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu); void ucall(uint64_t cmd, int nargs, ...); +void ucall_fmt(uint64_t cmd, const char *fmt, ...); uint64_t get_ucall(struct kvm_vcpu *vcpu, struct ucall *uc); void ucall_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa); int ucall_nr_pages_required(uint64_t page_size); @@ -47,8 +51,11 @@ int ucall_nr_pages_required(uint64_t page_size); #define GUEST_SYNC_ARGS(stage, arg1, arg2, arg3, arg4) \ ucall(UCALL_SYNC, 6, "hello", stage, arg1, arg2, arg3, arg4) #define GUEST_SYNC(stage) ucall(UCALL_SYNC, 2, "hello", stage) +#define GUEST_PRINTF(_fmt, _args...) ucall_fmt(UCALL_PRINTF, _fmt, ##_args) #define GUEST_DONE() ucall(UCALL_DONE, 0) +#define REPORT_GUEST_PRINTF(ucall) pr_info("%s", (ucall).buffer) + enum guest_assert_builtin_args { GUEST_ERROR_STRING, GUEST_FILE, diff --git a/tools/testing/selftests/kvm/lib/ucall_common.c b/tools/testing/selftests/kvm/lib/ucall_common.c index 77ada362273d..b507db91139b 100644 --- a/tools/testing/selftests/kvm/lib/ucall_common.c +++ b/tools/testing/selftests/kvm/lib/ucall_common.c @@ -75,6 +75,23 @@ static void ucall_free(struct ucall *uc) clear_bit(uc - ucall_pool->ucalls, ucall_pool->in_use); } +void ucall_fmt(uint64_t cmd, const char *fmt, ...) +{ + struct ucall *uc; + va_list va; + + uc = ucall_alloc(); + uc->cmd = cmd; + + va_start(va, fmt); + guest_vsnprintf(uc->buffer, UCALL_BUFFER_LEN, fmt, va); + va_end(va); + + ucall_arch_do_ucall((vm_vaddr_t)uc->hva); + + ucall_free(uc); +} + void ucall(uint64_t cmd, int nargs, ...) { struct ucall *uc; -- cgit From 289c2b4db8f336b96d918eb0c548382aee71ed79 Mon Sep 17 00:00:00 2001 From: Sean Christopherson Date: Fri, 28 Jul 2023 17:36:17 -0700 Subject: KVM: selftests: Add formatted guest assert support in ucall framework Add printf-based GUEST_ASSERT macros and accompanying host-side support to provide an assert-specific versions of GUEST_PRINTF(). To make it easier to parse assert messages, for humans and bots alike, preserve/use the same layout as host asserts, e.g. in the example below, the reported expression, file, line number, and message are from the guest assertion, not the host reporting of the assertion. The call stack still captures the host reporting, but capturing the guest stack is a less pressing concern, i.e. can be done in the future, and an optimal solution would capture *both* the host and guest stacks, i.e. capturing the host stack isn't an outright bug. Running soft int test ==== Test Assertion Failure ==== x86_64/svm_nested_soft_inject_test.c:39: regs->rip != (unsigned long)l2_guest_code_int pid=214104 tid=214104 errno=4 - Interrupted system call 1 0x0000000000401b35: run_test at svm_nested_soft_inject_test.c:191 2 0x00000000004017d2: main at svm_nested_soft_inject_test.c:212 3 0x0000000000415b03: __libc_start_call_main at libc-start.o:? 4 0x000000000041714f: __libc_start_main_impl at ??:? 5 0x0000000000401660: _start at ??:? Expected IRQ at RIP 0x401e50, received IRQ at 0x401e50 Don't bother sharing code between ucall_assert() and ucall_fmt(), as forwarding the variable arguments would either require using macros or building a va_list, i.e. would make the code less readable and/or require just as much copy+paste code anyways. Gate the new macros with a flag so that tests can more or less be switched over one-by-one. The slow conversion won't be perfect, e.g. library code won't pick up the flag, but the only asserts in library code are of the vanilla GUEST_ASSERT() variety, i.e. don't print out variables. Add a temporary alias to GUEST_ASSERT_1() to fudge around ARM's arch_timer.h header using GUEST_ASSERT_1(), thus thwarting any attempt to convert tests one-by-one. Link: https://lore.kernel.org/r/20230729003643.1053367-9-seanjc@google.com Signed-off-by: Sean Christopherson --- tools/testing/selftests/kvm/include/ucall_common.h | 48 ++++++++++++++++++++++ tools/testing/selftests/kvm/lib/ucall_common.c | 22 ++++++++++ 2 files changed, 70 insertions(+) (limited to 'tools/testing/selftests/kvm/lib/ucall_common.c') diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h index b5548aeba9f0..4ce11c15285a 100644 --- a/tools/testing/selftests/kvm/include/ucall_common.h +++ b/tools/testing/selftests/kvm/include/ucall_common.h @@ -36,6 +36,8 @@ void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu); void ucall(uint64_t cmd, int nargs, ...); void ucall_fmt(uint64_t cmd, const char *fmt, ...); +void ucall_assert(uint64_t cmd, const char *exp, const char *file, + unsigned int line, const char *fmt, ...); uint64_t get_ucall(struct kvm_vcpu *vcpu, struct ucall *uc); void ucall_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa); int ucall_nr_pages_required(uint64_t page_size); @@ -63,6 +65,50 @@ enum guest_assert_builtin_args { GUEST_ASSERT_BUILTIN_NARGS }; +#ifdef USE_GUEST_ASSERT_PRINTF +#define ____GUEST_ASSERT(_condition, _exp, _fmt, _args...) \ +do { \ + if (!(_condition)) \ + ucall_assert(UCALL_ABORT, _exp, __FILE__, __LINE__, _fmt, ##_args); \ +} while (0) + +#define __GUEST_ASSERT(_condition, _fmt, _args...) \ + ____GUEST_ASSERT(_condition, #_condition, _fmt, ##_args) + +#define GUEST_ASSERT(_condition) \ + __GUEST_ASSERT(_condition, #_condition) + +#define GUEST_FAIL(_fmt, _args...) \ + ucall_assert(UCALL_ABORT, "Unconditional guest failure", \ + __FILE__, __LINE__, _fmt, ##_args) + +#define GUEST_ASSERT_EQ(a, b) \ +do { \ + typeof(a) __a = (a); \ + typeof(b) __b = (b); \ + ____GUEST_ASSERT(__a == __b, #a " == " #b, "%#lx != %#lx (%s != %s)", \ + (unsigned long)(__a), (unsigned long)(__b), #a, #b); \ +} while (0) + +#define GUEST_ASSERT_NE(a, b) \ +do { \ + typeof(a) __a = (a); \ + typeof(b) __b = (b); \ + ____GUEST_ASSERT(__a != __b, #a " != " #b, "%#lx == %#lx (%s == %s)", \ + (unsigned long)(__a), (unsigned long)(__b), #a, #b); \ +} while (0) + +#define REPORT_GUEST_ASSERT(ucall) \ + test_assert(false, (const char *)(ucall).args[GUEST_ERROR_STRING], \ + (const char *)(ucall).args[GUEST_FILE], \ + (ucall).args[GUEST_LINE], "%s", (ucall).buffer) + +/* FIXME: Drop this alias once the param-based guest asserts are gone. */ +#define GUEST_ASSERT_1(_condition, arg1) \ + __GUEST_ASSERT(_condition, "arg1 = 0x%lx", arg1) + +#else + #define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) \ do { \ if (!(_condition)) \ @@ -129,4 +175,6 @@ do { \ #define REPORT_GUEST_ASSERT_N(ucall, fmt, args...) \ __REPORT_GUEST_ASSERT((ucall), fmt, ##args) +#endif /* USE_GUEST_ASSERT_PRINTF */ + #endif /* SELFTEST_KVM_UCALL_COMMON_H */ diff --git a/tools/testing/selftests/kvm/lib/ucall_common.c b/tools/testing/selftests/kvm/lib/ucall_common.c index b507db91139b..816a3fa109bf 100644 --- a/tools/testing/selftests/kvm/lib/ucall_common.c +++ b/tools/testing/selftests/kvm/lib/ucall_common.c @@ -75,6 +75,28 @@ static void ucall_free(struct ucall *uc) clear_bit(uc - ucall_pool->ucalls, ucall_pool->in_use); } +void ucall_assert(uint64_t cmd, const char *exp, const char *file, + unsigned int line, const char *fmt, ...) +{ + struct ucall *uc; + va_list va; + + uc = ucall_alloc(); + uc->cmd = cmd; + + WRITE_ONCE(uc->args[GUEST_ERROR_STRING], (uint64_t)(exp)); + WRITE_ONCE(uc->args[GUEST_FILE], (uint64_t)(file)); + WRITE_ONCE(uc->args[GUEST_LINE], line); + + va_start(va, fmt); + guest_vsnprintf(uc->buffer, UCALL_BUFFER_LEN, fmt, va); + va_end(va); + + ucall_arch_do_ucall((vm_vaddr_t)uc->hva); + + ucall_free(uc); +} + void ucall_fmt(uint64_t cmd, const char *fmt, ...) { struct ucall *uc; -- cgit