* [PATCH 1/8] KVM: selftests: Fix mostly theoretical leak of VM's binary stats FD
2024-12-20 1:38 [PATCH 0/8] KVM: selftests: Binary stats fixes and infra updates Sean Christopherson
@ 2024-12-20 1:38 ` Sean Christopherson
2024-12-20 1:39 ` [PATCH 2/8] KVM: selftests: Close VM's binary stats FD when releasing VM Sean Christopherson
` (7 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Sean Christopherson @ 2024-12-20 1:38 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Sean Christopherson, Paolo Bonzini
Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
linux-kernel
When allocating and freeing a VM's cached binary stats info, check for a
NULL descriptor, not a '0' file descriptor, as '0' is a legal FD. E.g. in
the unlikely scenario the kernel installs the stats FD at entry '0',
selftests would reallocate on the next __vm_get_stat() and/or fail to free
the stats in kvm_vm_free().
Fixes: 83f6e109f562 ("KVM: selftests: Cache binary stats metadata for duration of test")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/lib/kvm_util.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 33fefeb3ca44..91d295ef5d02 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -749,7 +749,7 @@ void kvm_vm_free(struct kvm_vm *vmp)
return;
/* Free cached stats metadata and close FD */
- if (vmp->stats_fd) {
+ if (vmp->stats_desc) {
free(vmp->stats_desc);
close(vmp->stats_fd);
}
@@ -2218,7 +2218,7 @@ void __vm_get_stat(struct kvm_vm *vm, const char *stat_name, uint64_t *data,
size_t size_desc;
int i;
- if (!vm->stats_fd) {
+ if (!vm->stats_desc) {
vm->stats_fd = vm_get_stats_fd(vm);
read_stats_header(vm->stats_fd, &vm->stats_header);
vm->stats_desc = read_stats_descriptors(vm->stats_fd,
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/8] KVM: selftests: Close VM's binary stats FD when releasing VM
2024-12-20 1:38 [PATCH 0/8] KVM: selftests: Binary stats fixes and infra updates Sean Christopherson
2024-12-20 1:38 ` [PATCH 1/8] KVM: selftests: Fix mostly theoretical leak of VM's binary stats FD Sean Christopherson
@ 2024-12-20 1:39 ` Sean Christopherson
2024-12-20 1:39 ` [PATCH 3/8] KVM: selftests: Assert that __vm_get_stat() actually finds a stat Sean Christopherson
` (6 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Sean Christopherson @ 2024-12-20 1:39 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Sean Christopherson, Paolo Bonzini
Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
linux-kernel
Close/free a VM's binary stats cache when the VM is released, not when the
VM is fully freed. When a VM is re-created, e.g. for state save/restore
tests, the stats FD and descriptor points at the old, defunct VM. The FD
is still valid, in that the underlying stats file won't be freed until the
FD is closed, but reading stats will always pull information from the old
VM.
Note, this is a benign bug in the current code base as none of the tests
that recreate VMs use binary stats.
Fixes: 83f6e109f562 ("KVM: selftests: Cache binary stats metadata for duration of test")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/lib/kvm_util.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 91d295ef5d02..9138801ecb60 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -709,6 +709,15 @@ void kvm_vm_release(struct kvm_vm *vmp)
ret = close(vmp->kvm_fd);
TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("close()", ret));
+
+ /* Free cached stats metadata and close FD */
+ if (vmp->stats_desc) {
+ free(vmp->stats_desc);
+ vmp->stats_desc = NULL;
+
+ ret = close(vmp->stats_fd);
+ TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("close()", ret));
+ }
}
static void __vm_mem_region_delete(struct kvm_vm *vm,
@@ -748,12 +757,6 @@ void kvm_vm_free(struct kvm_vm *vmp)
if (vmp == NULL)
return;
- /* Free cached stats metadata and close FD */
- if (vmp->stats_desc) {
- free(vmp->stats_desc);
- close(vmp->stats_fd);
- }
-
/* Free userspace_mem_regions. */
hash_for_each_safe(vmp->regions.slot_hash, ctr, node, region, slot_node)
__vm_mem_region_delete(vmp, region);
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 3/8] KVM: selftests: Assert that __vm_get_stat() actually finds a stat
2024-12-20 1:38 [PATCH 0/8] KVM: selftests: Binary stats fixes and infra updates Sean Christopherson
2024-12-20 1:38 ` [PATCH 1/8] KVM: selftests: Fix mostly theoretical leak of VM's binary stats FD Sean Christopherson
2024-12-20 1:39 ` [PATCH 2/8] KVM: selftests: Close VM's binary stats FD when releasing VM Sean Christopherson
@ 2024-12-20 1:39 ` Sean Christopherson
2024-12-20 1:39 ` [PATCH 4/8] KVM: selftests: Macrofy vm_get_stat() to auto-generate stat name string Sean Christopherson
` (5 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Sean Christopherson @ 2024-12-20 1:39 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Sean Christopherson, Paolo Bonzini
Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
linux-kernel
Fail the test if it attempts to read a stat that doesn't exist, e.g. due
to a typo, or because the test tried to get a stat for the wrong scope.
As is, there's no indiciation of failure and @data is left untouched,
e.g. holds '0' or random stack data in most cases.
Fixes: 8448ec5993be ("KVM: selftests: Add NX huge pages test")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/lib/kvm_util.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 9138801ecb60..21b5a6261106 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -2238,9 +2238,10 @@ void __vm_get_stat(struct kvm_vm *vm, const char *stat_name, uint64_t *data,
read_stat_data(vm->stats_fd, &vm->stats_header, desc,
data, max_elements);
-
- break;
+ return;
}
+
+ TEST_FAIL("Unabled to find stat '%s'", stat_name);
}
__weak void kvm_arch_vm_post_create(struct kvm_vm *vm)
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 4/8] KVM: selftests: Macrofy vm_get_stat() to auto-generate stat name string
2024-12-20 1:38 [PATCH 0/8] KVM: selftests: Binary stats fixes and infra updates Sean Christopherson
` (2 preceding siblings ...)
2024-12-20 1:39 ` [PATCH 3/8] KVM: selftests: Assert that __vm_get_stat() actually finds a stat Sean Christopherson
@ 2024-12-20 1:39 ` Sean Christopherson
2024-12-20 1:39 ` [PATCH 5/8] KVM: selftests: Add struct and helpers to wrap binary stats cache Sean Christopherson
` (4 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Sean Christopherson @ 2024-12-20 1:39 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Sean Christopherson, Paolo Bonzini
Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
linux-kernel
Turn vm_get_stat() into a macro that generates a string for the stat name,
as opposed to taking a string. This will allow hardening stat usage in
the future to generate errors on unknown stats at compile time.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/include/kvm_util.h | 14 +++++++-------
.../kvm/x86/dirty_log_page_splitting_test.c | 6 +++---
.../testing/selftests/kvm/x86/nx_huge_pages_test.c | 4 ++--
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 4c4e5a847f67..044c2231431e 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -534,13 +534,13 @@ void read_stat_data(int stats_fd, struct kvm_stats_header *header,
void __vm_get_stat(struct kvm_vm *vm, const char *stat_name, uint64_t *data,
size_t max_elements);
-static inline uint64_t vm_get_stat(struct kvm_vm *vm, const char *stat_name)
-{
- uint64_t data;
-
- __vm_get_stat(vm, stat_name, &data, 1);
- return data;
-}
+#define vm_get_stat(vm, stat) \
+({ \
+ uint64_t data; \
+ \
+ __vm_get_stat(vm, #stat, &data, 1); \
+ data; \
+})
void vm_create_irqchip(struct kvm_vm *vm);
diff --git a/tools/testing/selftests/kvm/x86/dirty_log_page_splitting_test.c b/tools/testing/selftests/kvm/x86/dirty_log_page_splitting_test.c
index 2929c067c207..b0d2b04a7ff2 100644
--- a/tools/testing/selftests/kvm/x86/dirty_log_page_splitting_test.c
+++ b/tools/testing/selftests/kvm/x86/dirty_log_page_splitting_test.c
@@ -41,9 +41,9 @@ struct kvm_page_stats {
static void get_page_stats(struct kvm_vm *vm, struct kvm_page_stats *stats, const char *stage)
{
- stats->pages_4k = vm_get_stat(vm, "pages_4k");
- stats->pages_2m = vm_get_stat(vm, "pages_2m");
- stats->pages_1g = vm_get_stat(vm, "pages_1g");
+ stats->pages_4k = vm_get_stat(vm, pages_4k);
+ stats->pages_2m = vm_get_stat(vm, pages_2m);
+ stats->pages_1g = vm_get_stat(vm, pages_1g);
stats->hugepages = stats->pages_2m + stats->pages_1g;
pr_debug("\nPage stats after %s: 4K: %ld 2M: %ld 1G: %ld huge: %ld\n",
diff --git a/tools/testing/selftests/kvm/x86/nx_huge_pages_test.c b/tools/testing/selftests/kvm/x86/nx_huge_pages_test.c
index e7efb2b35f8b..c0d84827f736 100644
--- a/tools/testing/selftests/kvm/x86/nx_huge_pages_test.c
+++ b/tools/testing/selftests/kvm/x86/nx_huge_pages_test.c
@@ -73,7 +73,7 @@ static void check_2m_page_count(struct kvm_vm *vm, int expected_pages_2m)
{
int actual_pages_2m;
- actual_pages_2m = vm_get_stat(vm, "pages_2m");
+ actual_pages_2m = vm_get_stat(vm, pages_2m);
TEST_ASSERT(actual_pages_2m == expected_pages_2m,
"Unexpected 2m page count. Expected %d, got %d",
@@ -84,7 +84,7 @@ static void check_split_count(struct kvm_vm *vm, int expected_splits)
{
int actual_splits;
- actual_splits = vm_get_stat(vm, "nx_lpage_splits");
+ actual_splits = vm_get_stat(vm, nx_lpage_splits);
TEST_ASSERT(actual_splits == expected_splits,
"Unexpected NX huge page split count. Expected %d, got %d",
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 5/8] KVM: selftests: Add struct and helpers to wrap binary stats cache
2024-12-20 1:38 [PATCH 0/8] KVM: selftests: Binary stats fixes and infra updates Sean Christopherson
` (3 preceding siblings ...)
2024-12-20 1:39 ` [PATCH 4/8] KVM: selftests: Macrofy vm_get_stat() to auto-generate stat name string Sean Christopherson
@ 2024-12-20 1:39 ` Sean Christopherson
2024-12-20 1:39 ` [PATCH 6/8] KVM: selftests: Get VM's binary stats FD when opening VM Sean Christopherson
` (3 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Sean Christopherson @ 2024-12-20 1:39 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Sean Christopherson, Paolo Bonzini
Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
linux-kernel
Add a struct and helpers to manage the binary stats cache, which is
currently used only for VM-scoped stats. This will allow expanding the
selftests infrastructure to provide support for vCPU-scoped binary stats,
which, except for the ioctl to get the stats FD are identical to VM-scoped
stats.
Defer converting __vm_get_stat() to a scope-agnostic helper to a future
patch, as getting the stats FD from KVM needs to be moved elsewhere
before it can be made completely scope-agnostic.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
.../testing/selftests/kvm/include/kvm_util.h | 11 +++--
tools/testing/selftests/kvm/lib/kvm_util.c | 47 +++++++++++--------
2 files changed, 34 insertions(+), 24 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 044c2231431e..9a64bab42f89 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -46,6 +46,12 @@ struct userspace_mem_region {
struct hlist_node slot_node;
};
+struct kvm_binary_stats {
+ int fd;
+ struct kvm_stats_header header;
+ struct kvm_stats_desc *desc;
+};
+
struct kvm_vcpu {
struct list_head list;
uint32_t id;
@@ -99,10 +105,7 @@ struct kvm_vm {
struct kvm_vm_arch arch;
- /* Cache of information for binary stats interface */
- int stats_fd;
- struct kvm_stats_header stats_header;
- struct kvm_stats_desc *stats_desc;
+ struct kvm_binary_stats stats;
/*
* KVM region slots. These are the default memslots used by page
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 21b5a6261106..c88f5e7871f7 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -657,6 +657,20 @@ userspace_mem_region_find(struct kvm_vm *vm, uint64_t start, uint64_t end)
return NULL;
}
+static void kvm_stats_release(struct kvm_binary_stats *stats)
+{
+ int ret;
+
+ if (!stats->desc)
+ return;
+
+ free(stats->desc);
+ stats->desc = NULL;
+
+ ret = close(stats->fd);
+ TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("close()", ret));
+}
+
__weak void vcpu_arch_free(struct kvm_vcpu *vcpu)
{
@@ -711,13 +725,7 @@ void kvm_vm_release(struct kvm_vm *vmp)
TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("close()", ret));
/* Free cached stats metadata and close FD */
- if (vmp->stats_desc) {
- free(vmp->stats_desc);
- vmp->stats_desc = NULL;
-
- ret = close(vmp->stats_fd);
- TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("close()", ret));
- }
+ kvm_stats_release(&vmp->stats);
}
static void __vm_mem_region_delete(struct kvm_vm *vm,
@@ -2214,34 +2222,33 @@ void read_stat_data(int stats_fd, struct kvm_stats_header *header,
*
* Read the data values of a specified stat from the binary stats interface.
*/
-void __vm_get_stat(struct kvm_vm *vm, const char *stat_name, uint64_t *data,
+void __vm_get_stat(struct kvm_vm *vm, const char *name, uint64_t *data,
size_t max_elements)
{
+ struct kvm_binary_stats *stats = &vm->stats;
struct kvm_stats_desc *desc;
size_t size_desc;
int i;
- if (!vm->stats_desc) {
- vm->stats_fd = vm_get_stats_fd(vm);
- read_stats_header(vm->stats_fd, &vm->stats_header);
- vm->stats_desc = read_stats_descriptors(vm->stats_fd,
- &vm->stats_header);
+ if (!stats->desc) {
+ stats->fd = vm_get_stats_fd(vm);
+ read_stats_header(stats->fd, &stats->header);
+ stats->desc = read_stats_descriptors(stats->fd, &stats->header);
}
- size_desc = get_stats_descriptor_size(&vm->stats_header);
+ size_desc = get_stats_descriptor_size(&stats->header);
- for (i = 0; i < vm->stats_header.num_desc; ++i) {
- desc = (void *)vm->stats_desc + (i * size_desc);
+ for (i = 0; i < stats->header.num_desc; ++i) {
+ desc = (void *)stats->desc + (i * size_desc);
- if (strcmp(desc->name, stat_name))
+ if (strcmp(desc->name, name))
continue;
- read_stat_data(vm->stats_fd, &vm->stats_header, desc,
- data, max_elements);
+ read_stat_data(stats->fd, &stats->header, desc, data, max_elements);
return;
}
- TEST_FAIL("Unabled to find stat '%s'", stat_name);
+ TEST_FAIL("Unabled to find stat '%s'", name);
}
__weak void kvm_arch_vm_post_create(struct kvm_vm *vm)
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 6/8] KVM: selftests: Get VM's binary stats FD when opening VM
2024-12-20 1:38 [PATCH 0/8] KVM: selftests: Binary stats fixes and infra updates Sean Christopherson
` (4 preceding siblings ...)
2024-12-20 1:39 ` [PATCH 5/8] KVM: selftests: Add struct and helpers to wrap binary stats cache Sean Christopherson
@ 2024-12-20 1:39 ` Sean Christopherson
2024-12-20 1:39 ` [PATCH 7/8] KVM: selftests: Add infrastructure for getting vCPU binary stats Sean Christopherson
` (2 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Sean Christopherson @ 2024-12-20 1:39 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Sean Christopherson, Paolo Bonzini
Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
linux-kernel
Get and cache a VM's binary stats FD when the VM is opened, as opposed to
waiting until the stats are first used. Opening the stats FD outside of
__vm_get_stat() will allow converting it to a scope-agnostic helper.
Note, this doesn't interfere with kvm_binary_stats_test's testcase that
verifies a stats FD can be used after its own VM's FD is closed, as the
cached FD is also closed during kvm_vm_free().
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/lib/kvm_util.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index c88f5e7871f7..16ee03e76d66 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -196,6 +196,11 @@ static void vm_open(struct kvm_vm *vm)
vm->fd = __kvm_ioctl(vm->kvm_fd, KVM_CREATE_VM, (void *)vm->type);
TEST_ASSERT(vm->fd >= 0, KVM_IOCTL_ERROR(KVM_CREATE_VM, vm->fd));
+
+ if (kvm_has_cap(KVM_CAP_BINARY_STATS_FD))
+ vm->stats.fd = vm_get_stats_fd(vm);
+ else
+ vm->stats.fd = -1;
}
const char *vm_guest_mode_string(uint32_t i)
@@ -661,14 +666,17 @@ static void kvm_stats_release(struct kvm_binary_stats *stats)
{
int ret;
- if (!stats->desc)
+ if (stats->fd < 0)
return;
- free(stats->desc);
- stats->desc = NULL;
+ if (stats->desc) {
+ free(stats->desc);
+ stats->desc = NULL;
+ }
ret = close(stats->fd);
TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("close()", ret));
+ stats->fd = -1;
}
__weak void vcpu_arch_free(struct kvm_vcpu *vcpu)
@@ -2231,7 +2239,6 @@ void __vm_get_stat(struct kvm_vm *vm, const char *name, uint64_t *data,
int i;
if (!stats->desc) {
- stats->fd = vm_get_stats_fd(vm);
read_stats_header(stats->fd, &stats->header);
stats->desc = read_stats_descriptors(stats->fd, &stats->header);
}
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 7/8] KVM: selftests: Add infrastructure for getting vCPU binary stats
2024-12-20 1:38 [PATCH 0/8] KVM: selftests: Binary stats fixes and infra updates Sean Christopherson
` (5 preceding siblings ...)
2024-12-20 1:39 ` [PATCH 6/8] KVM: selftests: Get VM's binary stats FD when opening VM Sean Christopherson
@ 2024-12-20 1:39 ` Sean Christopherson
2024-12-20 1:39 ` [PATCH 8/8] KVM: selftests: Add compile-time assertions to guard against stats typos Sean Christopherson
2025-01-09 19:47 ` [PATCH 0/8] KVM: selftests: Binary stats fixes and infra updates Sean Christopherson
8 siblings, 0 replies; 11+ messages in thread
From: Sean Christopherson @ 2024-12-20 1:39 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Sean Christopherson, Paolo Bonzini
Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
linux-kernel
Now that the binary stats cache infrastructure is largely scope agnostic,
add support for vCPU-scoped stats. Like VM stats, open and cache the
stats FD when the vCPU is created so that it's guaranteed to be valid when
vcpu_get_stats() is invoked.
To sanity check that the infrastructure actually works, and to get a bit
of bonus coverage, add an assert in x86's xapic_ipi_test to verify that
the number of HLTs executed by the test matches the number of HLT exits
observed by KVM.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
.../testing/selftests/kvm/include/kvm_util.h | 20 +++++++++------
tools/testing/selftests/kvm/lib/kvm_util.c | 25 +++++++------------
.../selftests/kvm/x86/xapic_ipi_test.c | 2 ++
3 files changed, 23 insertions(+), 24 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 9a64bab42f89..e0d23873158e 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -61,6 +61,7 @@ struct kvm_vcpu {
#ifdef __x86_64__
struct kvm_cpuid2 *cpuid;
#endif
+ struct kvm_binary_stats stats;
struct kvm_dirty_gfn *dirty_gfns;
uint32_t fetch_index;
uint32_t dirty_gfns_count;
@@ -534,17 +535,20 @@ void read_stat_data(int stats_fd, struct kvm_stats_header *header,
struct kvm_stats_desc *desc, uint64_t *data,
size_t max_elements);
-void __vm_get_stat(struct kvm_vm *vm, const char *stat_name, uint64_t *data,
- size_t max_elements);
+void kvm_get_stat(struct kvm_binary_stats *stats, const char *name,
+ uint64_t *data, size_t max_elements);
-#define vm_get_stat(vm, stat) \
-({ \
- uint64_t data; \
- \
- __vm_get_stat(vm, #stat, &data, 1); \
- data; \
+#define __get_stat(stats, stat) \
+({ \
+ uint64_t data; \
+ \
+ kvm_get_stat(stats, #stat, &data, 1); \
+ data; \
})
+#define vm_get_stat(vm, stat) __get_stat(&(vm)->stats, stat)
+#define vcpu_get_stat(vcpu, stat) __get_stat(&(vcpu)->stats, stat)
+
void vm_create_irqchip(struct kvm_vm *vm);
static inline int __vm_create_guest_memfd(struct kvm_vm *vm, uint64_t size,
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 16ee03e76d66..99fe8bcd2346 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -712,6 +712,8 @@ static void vm_vcpu_rm(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
ret = close(vcpu->fd);
TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("close()", ret));
+ kvm_stats_release(&vcpu->stats);
+
list_del(&vcpu->list);
vcpu_arch_free(vcpu);
@@ -1305,6 +1307,11 @@ struct kvm_vcpu *__vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
TEST_ASSERT(vcpu->run != MAP_FAILED,
__KVM_SYSCALL_ERROR("mmap()", (int)(unsigned long)MAP_FAILED));
+ if (kvm_has_cap(KVM_CAP_BINARY_STATS_FD))
+ vcpu->stats.fd = vcpu_get_stats_fd(vcpu);
+ else
+ vcpu->stats.fd = -1;
+
/* Add to linked-list of VCPUs. */
list_add(&vcpu->list, &vm->vcpus);
@@ -2217,23 +2224,9 @@ void read_stat_data(int stats_fd, struct kvm_stats_header *header,
desc->name, size, ret);
}
-/*
- * Read the data of the named stat
- *
- * Input Args:
- * vm - the VM for which the stat should be read
- * stat_name - the name of the stat to read
- * max_elements - the maximum number of 8-byte values to read into data
- *
- * Output Args:
- * data - the buffer into which stat data should be read
- *
- * Read the data values of a specified stat from the binary stats interface.
- */
-void __vm_get_stat(struct kvm_vm *vm, const char *name, uint64_t *data,
- size_t max_elements)
+void kvm_get_stat(struct kvm_binary_stats *stats, const char *name,
+ uint64_t *data, size_t max_elements)
{
- struct kvm_binary_stats *stats = &vm->stats;
struct kvm_stats_desc *desc;
size_t size_desc;
int i;
diff --git a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
index a76078a08ff8..574a944763b7 100644
--- a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
+++ b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
@@ -465,6 +465,8 @@ int main(int argc, char *argv[])
cancel_join_vcpu_thread(threads[0], params[0].vcpu);
cancel_join_vcpu_thread(threads[1], params[1].vcpu);
+ TEST_ASSERT_EQ(data->hlt_count, vcpu_get_stat(params[0].vcpu, halt_exits));
+
fprintf(stderr,
"Test successful after running for %d seconds.\n"
"Sending vCPU sent %lu IPIs to halting vCPU\n"
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 8/8] KVM: selftests: Add compile-time assertions to guard against stats typos
2024-12-20 1:38 [PATCH 0/8] KVM: selftests: Binary stats fixes and infra updates Sean Christopherson
` (6 preceding siblings ...)
2024-12-20 1:39 ` [PATCH 7/8] KVM: selftests: Add infrastructure for getting vCPU binary stats Sean Christopherson
@ 2024-12-20 1:39 ` Sean Christopherson
2025-01-09 19:47 ` [PATCH 0/8] KVM: selftests: Binary stats fixes and infra updates Sean Christopherson
8 siblings, 0 replies; 11+ messages in thread
From: Sean Christopherson @ 2024-12-20 1:39 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Sean Christopherson, Paolo Bonzini
Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
linux-kernel
Add compile-time assertions to the main binary stats accessor to verify
the stat is a known KVM stat of the appropriate scope, and "add" all
known stats for all architectures.
To build the set of known stats, define enums for each stat, with a
completely arbitrary magic value to specify the scope of the stat. With
the assert in place, misuse of stat (or usage of a new stat) generates
error like so:
In file included from include/x86/kvm_util_arch.h:8,
from include/kvm_util.h:23,
from x86/dirty_log_page_splitting_test.c:16:
x86/dirty_log_page_splitting_test.c: In function ‘get_page_stats’:
include/kvm_util.h:563:42: error: ‘VM_STAT_pages_4m’ undeclared
(first use in this function); did you mean ‘VM_STAT_pages_2m’?
563 | #define vm_get_stat(vm, stat) __get_stat(VM, &(vm)->stats, stat)
| ^~
...
x86/dirty_log_page_splitting_test.c:45:27: note: in expansion of macro ‘vm_get_stat’
45 | stats->pages_2m = vm_get_stat(vm, pages_4m);
| ^~~~~~~~~~~
Using pre-defined lists of stats doesn't completely eliminate human error,
e.g. it's obviously possible to make a typo when adding a state. And
while there is also a non-zero cost to maintaining the set of stats,
adding stats in KVM is relatively uncommon, and removing stats is extremely
rare. On the flip side, providing a list of known stats should make it
easier to use stats in test, at which point detecting goofs at compile-time
will also be more valuable.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
.../kvm/include/arm64/kvm_util_arch.h | 12 ++
.../testing/selftests/kvm/include/kvm_util.h | 24 +++-
.../selftests/kvm/include/kvm_util_types.h | 6 +
.../kvm/include/riscv/kvm_util_arch.h | 14 +++
.../kvm/include/s390/kvm_util_arch.h | 113 ++++++++++++++++++
.../selftests/kvm/include/x86/kvm_util_arch.h | 52 ++++++++
6 files changed, 218 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/arm64/kvm_util_arch.h b/tools/testing/selftests/kvm/include/arm64/kvm_util_arch.h
index e43a57d99b56..12097262f585 100644
--- a/tools/testing/selftests/kvm/include/arm64/kvm_util_arch.h
+++ b/tools/testing/selftests/kvm/include/arm64/kvm_util_arch.h
@@ -2,6 +2,18 @@
#ifndef SELFTEST_KVM_UTIL_ARCH_H
#define SELFTEST_KVM_UTIL_ARCH_H
+#include "kvm_util_types.h"
+
struct kvm_vm_arch {};
+enum kvm_arm64_stats {
+ VCPU_STAT(hvc_exit_stat),
+ VCPU_STAT(wfe_exit_stat),
+ VCPU_STAT(wfi_exit_stat),
+ VCPU_STAT(mmio_exit_user),
+ VCPU_STAT(mmio_exit_kernel),
+ VCPU_STAT(signal_exits),
+ VCPU_STAT(exits),
+};
+
#endif // SELFTEST_KVM_UTIL_ARCH_H
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index e0d23873158e..f4f0e27cea27 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -535,19 +535,37 @@ void read_stat_data(int stats_fd, struct kvm_stats_header *header,
struct kvm_stats_desc *desc, uint64_t *data,
size_t max_elements);
+enum kvm_common_stats {
+ VM_STAT(remote_tlb_flush),
+ VM_STAT(remote_tlb_flush_requests),
+
+ VCPU_STAT(halt_successfull_poll),
+ VCPU_STAT(halt_attempted_poll),
+ VCPU_STAT(halt_poll_invalid),
+ VCPU_STAT(halt_wakeup),
+ VCPU_STAT(halt_poll_success_ns),
+ VCPU_STAT(halt_poll_fail_ns),
+ VCPU_STAT(halt_wait_ns),
+ VCPU_STAT(halt_poll_success_hist),
+ VCPU_STAT(halt_poll_fail_hist),
+ VCPU_STAT(halt_wait_hist),
+ VCPU_STAT(blocking),
+};
+
void kvm_get_stat(struct kvm_binary_stats *stats, const char *name,
uint64_t *data, size_t max_elements);
-#define __get_stat(stats, stat) \
+#define __get_stat(type, stats, stat) \
({ \
uint64_t data; \
\
+ kvm_static_assert(type##_STAT_##stat == type##_STAT_MAGIC_NUMBER); \
kvm_get_stat(stats, #stat, &data, 1); \
data; \
})
-#define vm_get_stat(vm, stat) __get_stat(&(vm)->stats, stat)
-#define vcpu_get_stat(vcpu, stat) __get_stat(&(vcpu)->stats, stat)
+#define vm_get_stat(vm, stat) __get_stat(VM, &(vm)->stats, stat)
+#define vcpu_get_stat(vcpu, stat) __get_stat(VCPU, &(vcpu)->stats, stat)
void vm_create_irqchip(struct kvm_vm *vm);
diff --git a/tools/testing/selftests/kvm/include/kvm_util_types.h b/tools/testing/selftests/kvm/include/kvm_util_types.h
index ec787b97cf18..20e6717a0d24 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_types.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_types.h
@@ -17,4 +17,10 @@
typedef uint64_t vm_paddr_t; /* Virtual Machine (Guest) physical address */
typedef uint64_t vm_vaddr_t; /* Virtual Machine (Guest) virtual address */
+#define VM_STAT_MAGIC_NUMBER 1
+#define VM_STAT(stat) VM_STAT_##stat = VM_STAT_MAGIC_NUMBER
+
+#define VCPU_STAT_MAGIC_NUMBER 2
+#define VCPU_STAT(stat) VCPU_STAT_##stat = VCPU_STAT_MAGIC_NUMBER
+
#endif /* SELFTEST_KVM_UTIL_TYPES_H */
diff --git a/tools/testing/selftests/kvm/include/riscv/kvm_util_arch.h b/tools/testing/selftests/kvm/include/riscv/kvm_util_arch.h
index e43a57d99b56..ea53d6aeb693 100644
--- a/tools/testing/selftests/kvm/include/riscv/kvm_util_arch.h
+++ b/tools/testing/selftests/kvm/include/riscv/kvm_util_arch.h
@@ -2,6 +2,20 @@
#ifndef SELFTEST_KVM_UTIL_ARCH_H
#define SELFTEST_KVM_UTIL_ARCH_H
+#include "kvm_util_types.h"
+
struct kvm_vm_arch {};
+enum kvm_riscv_stats {
+ VCPU_STAT(ecall_exit_stat),
+ VCPU_STAT(wfi_exit_stat),
+ VCPU_STAT(wrs_exit_stat),
+ VCPU_STAT(mmio_exit_user),
+ VCPU_STAT(mmio_exit_kernel),
+ VCPU_STAT(csr_exit_user),
+ VCPU_STAT(csr_exit_kernel),
+ VCPU_STAT(signal_exits),
+ VCPU_STAT(exits),
+};
+
#endif // SELFTEST_KVM_UTIL_ARCH_H
diff --git a/tools/testing/selftests/kvm/include/s390/kvm_util_arch.h b/tools/testing/selftests/kvm/include/s390/kvm_util_arch.h
index e43a57d99b56..64d4de333e09 100644
--- a/tools/testing/selftests/kvm/include/s390/kvm_util_arch.h
+++ b/tools/testing/selftests/kvm/include/s390/kvm_util_arch.h
@@ -2,6 +2,119 @@
#ifndef SELFTEST_KVM_UTIL_ARCH_H
#define SELFTEST_KVM_UTIL_ARCH_H
+#include "kvm_util_types.h"
+
struct kvm_vm_arch {};
+enum kvm_s390_stats {
+ VM_STAT(inject_io),
+ VM_STAT(inject_float_mchk),
+ VM_STAT(inject_pfault_done),
+ VM_STAT(inject_service_signal),
+ VM_STAT(inject_virtio),
+ VM_STAT(aen_forward),
+ VM_STAT(gmap_shadow_reuse),
+ VM_STAT(gmap_shadow_create),
+ VM_STAT(gmap_shadow_r1_entry),
+ VM_STAT(gmap_shadow_r2_entry),
+ VM_STAT(gmap_shadow_r3_entry),
+ VM_STAT(gmap_shadow_sg_entry),
+ VM_STAT(gmap_shadow_pg_entry),
+
+ VCPU_STAT(exit_userspace),
+ VCPU_STAT(exit_null),
+ VCPU_STAT(exit_external_request),
+ VCPU_STAT(exit_io_request),
+ VCPU_STAT(exit_external_interrupt),
+ VCPU_STAT(exit_stop_request),
+ VCPU_STAT(exit_validity),
+ VCPU_STAT(exit_instruction),
+ VCPU_STAT(exit_pei),
+ VCPU_STAT(halt_no_poll_steal),
+ VCPU_STAT(instruction_lctl),
+ VCPU_STAT(instruction_lctlg),
+ VCPU_STAT(instruction_stctl),
+ VCPU_STAT(instruction_stctg),
+ VCPU_STAT(exit_program_interruption),
+ VCPU_STAT(exit_instr_and_program),
+ VCPU_STAT(exit_operation_exception),
+ VCPU_STAT(deliver_ckc),
+ VCPU_STAT(deliver_cputm),
+ VCPU_STAT(deliver_external_call),
+ VCPU_STAT(deliver_emergency_signal),
+ VCPU_STAT(deliver_service_signal),
+ VCPU_STAT(deliver_virtio),
+ VCPU_STAT(deliver_stop_signal),
+ VCPU_STAT(deliver_prefix_signal),
+ VCPU_STAT(deliver_restart_signal),
+ VCPU_STAT(deliver_program),
+ VCPU_STAT(deliver_io),
+ VCPU_STAT(deliver_machine_check),
+ VCPU_STAT(exit_wait_state),
+ VCPU_STAT(inject_ckc),
+ VCPU_STAT(inject_cputm),
+ VCPU_STAT(inject_external_call),
+ VCPU_STAT(inject_emergency_signal),
+ VCPU_STAT(inject_mchk),
+ VCPU_STAT(inject_pfault_init),
+ VCPU_STAT(inject_program),
+ VCPU_STAT(inject_restart),
+ VCPU_STAT(inject_set_prefix),
+ VCPU_STAT(inject_stop_signal),
+ VCPU_STAT(instruction_epsw),
+ VCPU_STAT(instruction_gs),
+ VCPU_STAT(instruction_io_other),
+ VCPU_STAT(instruction_lpsw),
+ VCPU_STAT(instruction_lpswe),
+ VCPU_STAT(instruction_lpswey),
+ VCPU_STAT(instruction_pfmf),
+ VCPU_STAT(instruction_ptff),
+ VCPU_STAT(instruction_sck),
+ VCPU_STAT(instruction_sckpf),
+ VCPU_STAT(instruction_stidp),
+ VCPU_STAT(instruction_spx),
+ VCPU_STAT(instruction_stpx),
+ VCPU_STAT(instruction_stap),
+ VCPU_STAT(instruction_iske),
+ VCPU_STAT(instruction_ri),
+ VCPU_STAT(instruction_rrbe),
+ VCPU_STAT(instruction_sske),
+ VCPU_STAT(instruction_ipte_interlock),
+ VCPU_STAT(instruction_stsi),
+ VCPU_STAT(instruction_stfl),
+ VCPU_STAT(instruction_tb),
+ VCPU_STAT(instruction_tpi),
+ VCPU_STAT(instruction_tprot),
+ VCPU_STAT(instruction_tsch),
+ VCPU_STAT(instruction_sie),
+ VCPU_STAT(instruction_essa),
+ VCPU_STAT(instruction_sthyi),
+ VCPU_STAT(instruction_sigp_sense),
+ VCPU_STAT(instruction_sigp_sense_running),
+ VCPU_STAT(instruction_sigp_external_call),
+ VCPU_STAT(instruction_sigp_emergency),
+ VCPU_STAT(instruction_sigp_cond_emergency),
+ VCPU_STAT(instruction_sigp_start),
+ VCPU_STAT(instruction_sigp_stop),
+ VCPU_STAT(instruction_sigp_stop_store_status),
+ VCPU_STAT(instruction_sigp_store_status),
+ VCPU_STAT(instruction_sigp_store_adtl_status),
+ VCPU_STAT(instruction_sigp_arch),
+ VCPU_STAT(instruction_sigp_prefix),
+ VCPU_STAT(instruction_sigp_restart),
+ VCPU_STAT(instruction_sigp_init_cpu_reset),
+ VCPU_STAT(instruction_sigp_cpu_reset),
+ VCPU_STAT(instruction_sigp_unknown),
+ VCPU_STAT(instruction_diagnose_10),
+ VCPU_STAT(instruction_diagnose_44),
+ VCPU_STAT(instruction_diagnose_9c),
+ VCPU_STAT(diag_9c_ignored),
+ VCPU_STAT(diag_9c_forward),
+ VCPU_STAT(instruction_diagnose_258),
+ VCPU_STAT(instruction_diagnose_308),
+ VCPU_STAT(instruction_diagnose_500),
+ VCPU_STAT(instruction_diagnose_other),
+ VCPU_STAT(pfault_sync),
+};
+
#endif // SELFTEST_KVM_UTIL_ARCH_H
diff --git a/tools/testing/selftests/kvm/include/x86/kvm_util_arch.h b/tools/testing/selftests/kvm/include/x86/kvm_util_arch.h
index 972bb1c4ab4c..f9c4aedddbd0 100644
--- a/tools/testing/selftests/kvm/include/x86/kvm_util_arch.h
+++ b/tools/testing/selftests/kvm/include/x86/kvm_util_arch.h
@@ -48,4 +48,56 @@ do { \
} \
} while (0)
+enum kvm_x86_stats {
+ VM_STAT(mmu_shadow_zapped),
+ VM_STAT(mmu_pte_write),
+ VM_STAT(mmu_pde_zapped),
+ VM_STAT(mmu_flooded),
+ VM_STAT(mmu_recycled),
+ VM_STAT(mmu_cache_miss),
+ VM_STAT(mmu_unsync),
+ VM_STAT(pages_4k),
+ VM_STAT(pages_2m),
+ VM_STAT(pages_1g),
+ VM_STAT(pages),
+ VM_STAT(nx_lpage_splits),
+ VM_STAT(max_mmu_page_hash_collisions),
+ VM_STAT(max_mmu_rmap_size),
+
+ VCPU_STAT(pf_taken),
+ VCPU_STAT(pf_fixed),
+ VCPU_STAT(pf_emulate),
+ VCPU_STAT(pf_spurious),
+ VCPU_STAT(pf_fast),
+ VCPU_STAT(pf_mmio_spte_created),
+ VCPU_STAT(pf_guest),
+ VCPU_STAT(tlb_flush),
+ VCPU_STAT(invlpg),
+ VCPU_STAT(exits),
+ VCPU_STAT(io_exits),
+ VCPU_STAT(mmio_exits),
+ VCPU_STAT(signal_exits),
+ VCPU_STAT(irq_window_exits),
+ VCPU_STAT(nmi_window_exits),
+ VCPU_STAT(l1d_flush),
+ VCPU_STAT(halt_exits),
+ VCPU_STAT(request_irq_exits),
+ VCPU_STAT(irq_exits),
+ VCPU_STAT(host_state_reload),
+ VCPU_STAT(fpu_reload),
+ VCPU_STAT(insn_emulation),
+ VCPU_STAT(insn_emulation_fail),
+ VCPU_STAT(hypercalls),
+ VCPU_STAT(irq_injections),
+ VCPU_STAT(nmi_injections),
+ VCPU_STAT(req_event),
+ VCPU_STAT(nested_run),
+ VCPU_STAT(directed_yield_attempted),
+ VCPU_STAT(directed_yield_successful),
+ VCPU_STAT(preemption_reported),
+ VCPU_STAT(preemption_other),
+ VCPU_STAT(guest_mode),
+ VCPU_STAT(notify_window_exits),
+};
+
#endif // SELFTEST_KVM_UTIL_ARCH_H
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 0/8] KVM: selftests: Binary stats fixes and infra updates
2024-12-20 1:38 [PATCH 0/8] KVM: selftests: Binary stats fixes and infra updates Sean Christopherson
` (7 preceding siblings ...)
2024-12-20 1:39 ` [PATCH 8/8] KVM: selftests: Add compile-time assertions to guard against stats typos Sean Christopherson
@ 2025-01-09 19:47 ` Sean Christopherson
2025-01-10 1:15 ` Sean Christopherson
8 siblings, 1 reply; 11+ messages in thread
From: Sean Christopherson @ 2025-01-09 19:47 UTC (permalink / raw)
To: Sean Christopherson, Marc Zyngier, Oliver Upton, Anup Patel,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Christian Borntraeger,
Janosch Frank, Claudio Imbrenda, Paolo Bonzini
Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
linux-kernel
On Thu, 19 Dec 2024 17:38:58 -0800, Sean Christopherson wrote:
> Fix a handful of bugs in the binary stats infrastructure, expand support
> to vCPU-scoped stats, enumerate all KVM stats in selftests, and use the
> enumerated stats to assert at compile-time that {vm,vcpu}_get_stat() is
> getting a stat that actually exists.
>
> Most of the bugs are benign, and AFAICT, none actually cause problems in
> the current code base. The worst of the bugs is lack of validation that
> the requested stat actually exists, which is quite annoying if someone
> fat fingers a stat name, tries to get a vCPU stat on a VM FD, etc.
>
> [...]
Applied 1-7 to kvm-x86 selftests (x86 wants to build tests on the vCPU-scoped
stats infrastructure).
I'll hold off on the compile-time assertions stuff until there's consensus that
we want to go that route for all architectures (not at all urgent).
[1/8] KVM: selftests: Fix mostly theoretical leak of VM's binary stats FD
https://github.com/kvm-x86/linux/commit/b68ec5b6869f
[2/8] KVM: selftests: Close VM's binary stats FD when releasing VM
https://github.com/kvm-x86/linux/commit/a59768d6cb64
[3/8] KVM: selftests: Assert that __vm_get_stat() actually finds a stat
https://github.com/kvm-x86/linux/commit/52ef723593fe
[4/8] KVM: selftests: Macrofy vm_get_stat() to auto-generate stat name string
https://github.com/kvm-x86/linux/commit/7884da344973
[5/8] KVM: selftests: Add struct and helpers to wrap binary stats cache
https://github.com/kvm-x86/linux/commit/384544c026f6
[6/8] KVM: selftests: Get VM's binary stats FD when opening VM
https://github.com/kvm-x86/linux/commit/6d22ccb1c309
[7/8] KVM: selftests: Add infrastructure for getting vCPU binary stats
https://github.com/kvm-x86/linux/commit/60d432517838
[8/8] KVM: selftests: Add compile-time assertions to guard against stats typos
not applied
--
https://github.com/kvm-x86/linux/tree/next
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 0/8] KVM: selftests: Binary stats fixes and infra updates
2025-01-09 19:47 ` [PATCH 0/8] KVM: selftests: Binary stats fixes and infra updates Sean Christopherson
@ 2025-01-10 1:15 ` Sean Christopherson
0 siblings, 0 replies; 11+ messages in thread
From: Sean Christopherson @ 2025-01-10 1:15 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Paolo Bonzini
Cc: linux-arm-kernel, kvmarm, kvm, kvm-riscv, linux-riscv,
linux-kernel
On Thu, Jan 09, 2025, Sean Christopherson wrote:
> On Thu, 19 Dec 2024 17:38:58 -0800, Sean Christopherson wrote:
> > Fix a handful of bugs in the binary stats infrastructure, expand support
> > to vCPU-scoped stats, enumerate all KVM stats in selftests, and use the
> > enumerated stats to assert at compile-time that {vm,vcpu}_get_stat() is
> > getting a stat that actually exists.
> >
> > Most of the bugs are benign, and AFAICT, none actually cause problems in
> > the current code base. The worst of the bugs is lack of validation that
> > the requested stat actually exists, which is quite annoying if someone
> > fat fingers a stat name, tries to get a vCPU stat on a VM FD, etc.
> >
> > [...]
>
> Applied 1-7 to kvm-x86 selftests (x86 wants to build tests on the vCPU-scoped
> stats infrastructure).
>
> I'll hold off on the compile-time assertions stuff until there's consensus that
> we want to go that route for all architectures (not at all urgent).
>
> [1/8] KVM: selftests: Fix mostly theoretical leak of VM's binary stats FD
> https://github.com/kvm-x86/linux/commit/b68ec5b6869f
> [2/8] KVM: selftests: Close VM's binary stats FD when releasing VM
> https://github.com/kvm-x86/linux/commit/a59768d6cb64
> [3/8] KVM: selftests: Assert that __vm_get_stat() actually finds a stat
> https://github.com/kvm-x86/linux/commit/52ef723593fe
> [4/8] KVM: selftests: Macrofy vm_get_stat() to auto-generate stat name string
> https://github.com/kvm-x86/linux/commit/7884da344973
> [5/8] KVM: selftests: Add struct and helpers to wrap binary stats cache
> https://github.com/kvm-x86/linux/commit/384544c026f6
> [6/8] KVM: selftests: Get VM's binary stats FD when opening VM
> https://github.com/kvm-x86/linux/commit/6d22ccb1c309
> [7/8] KVM: selftests: Add infrastructure for getting vCPU binary stats
> https://github.com/kvm-x86/linux/commit/60d432517838
Argh, apparently I only tested this series on platforms with high RLIMIT_NOFILE
values. Creating the stats fd for all vCPUs causes kvm_create_max_vcpus to fail
on some of my systems due to doubling the number of fds needed.
One option would be to figure out a clean way to avoid creating the stats fds
for barebones VMs, but x86's recalc_apic_map_test also fails. That test creates
512 (max "supported" by selftests) using vm_create_with_vcpus().
Doubling the rlimit from 1024+100 to 2048+100 for the max test doesn't seem insane,
so my plan is to move the rlimit twiddling into common code, and then account for
the vCPU stats fds.
I'll drop the above commits and post a v2.
^ permalink raw reply [flat|nested] 11+ messages in thread