by Ben Gardon

[permalink] [raw]

Subject: [PATCH 01/13] selftests: KVM: Dump VM stats in binary stats test

Add kvm_util library functions to read KVM stats through the binary
stats interface and then dump them to stdout when running the binary
stats test. Subsequent commits will extend the kvm_util code and use it
to make assertions in a test for NX hugepages.

CC: Jing Zhang <[email protected]>
Signed-off-by: Ben Gardon <[email protected]>
---
.../selftests/kvm/include/kvm_util_base.h | 1 +
.../selftests/kvm/kvm_binary_stats_test.c | 3 +
tools/testing/selftests/kvm/lib/kvm_util.c | 143 ++++++++++++++++++
3 files changed, 147 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index 92cef0ffb19e..c5f4a67772cb 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -400,6 +400,7 @@ void assert_on_unhandled_exception(struct kvm_vm *vm, uint32_t vcpuid);

int vm_get_stats_fd(struct kvm_vm *vm);
int vcpu_get_stats_fd(struct kvm_vm *vm, uint32_t vcpuid);
+void dump_vm_stats(struct kvm_vm *vm);

uint32_t guest_get_vcpuid(void);

diff --git a/tools/testing/selftests/kvm/kvm_binary_stats_test.c b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
index 17f65d514915..afc4701ce8dd 100644
--- a/tools/testing/selftests/kvm/kvm_binary_stats_test.c
+++ b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
@@ -174,6 +174,9 @@ static void vm_stats_test(struct kvm_vm *vm)
stats_test(stats_fd);
close(stats_fd);
TEST_ASSERT(fcntl(stats_fd, F_GETFD) == -1, "Stats fd not freed");
+
+ /* Dump VM stats */
+ dump_vm_stats(vm);
}

static void vcpu_stats_test(struct kvm_vm *vm, int vcpu_id)
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 1665a220abcb..4d21c3b46780 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -2556,3 +2556,146 @@ int vcpu_get_stats_fd(struct kvm_vm *vm, uint32_t vcpuid)

return ioctl(vcpu->fd, KVM_GET_STATS_FD, NULL);
}
+
+/* Caller is responsible for freeing the returned kvm_stats_header. */
+static struct kvm_stats_header *read_vm_stats_header(int stats_fd)
+{
+ struct kvm_stats_header *header;
+ ssize_t ret;
+
+ /* Read kvm stats header */
+ header = malloc(sizeof(*header));
+ TEST_ASSERT(header, "Allocate memory for stats header");
+
+ ret = read(stats_fd, header, sizeof(*header));
+ TEST_ASSERT(ret == sizeof(*header), "Read stats header");
+
+ return header;
+}
+
+static void dump_header(int stats_fd, struct kvm_stats_header *header)
+{
+ ssize_t ret;
+ char *id;
+
+ printf("flags: %u\n", header->flags);
+ printf("name size: %u\n", header->name_size);
+ printf("num_desc: %u\n", header->num_desc);
+ printf("id_offset: %u\n", header->id_offset);
+ printf("desc_offset: %u\n", header->desc_offset);
+ printf("data_offset: %u\n", header->data_offset);
+
+ /* Read kvm stats id string */
+ id = malloc(header->name_size);
+ TEST_ASSERT(id, "Allocate memory for id string");
+ ret = pread(stats_fd, id, header->name_size, header->id_offset);
+ TEST_ASSERT(ret == header->name_size, "Read id string");
+
+ printf("id: %s\n", id);
+
+ free(id);
+}
+
+static ssize_t stats_desc_size(struct kvm_stats_header *header)
+{
+ return sizeof(struct kvm_stats_desc) + header->name_size;
+}
+
+/* Caller is responsible for freeing the returned kvm_stats_desc. */
+static struct kvm_stats_desc *read_vm_stats_desc(int stats_fd,
+ struct kvm_stats_header *header)
+{
+ struct kvm_stats_desc *stats_desc;
+ size_t size_desc;
+ ssize_t ret;
+
+ size_desc = header->num_desc * stats_desc_size(header);
+
+ /* Allocate memory for stats descriptors */
+ stats_desc = malloc(size_desc);
+ TEST_ASSERT(stats_desc, "Allocate memory for stats descriptors");
+
+ /* Read kvm stats descriptors */
+ ret = pread(stats_fd, stats_desc, size_desc, header->desc_offset);
+ TEST_ASSERT(ret == size_desc, "Read KVM stats descriptors");
+
+ return stats_desc;
+}
+
+/* Caller is responsible for freeing the memory *data. */
+static int read_stat_data(int stats_fd, struct kvm_stats_header *header,
+ struct kvm_stats_desc *desc, uint64_t **data)
+{
+ u64 *stats_data;
+ ssize_t ret;
+
+ stats_data = malloc(desc->size * sizeof(*stats_data));
+
+ ret = pread(stats_fd, stats_data, desc->size * sizeof(*stats_data),
+ header->data_offset + desc->offset);
+
+ /* ret is in bytes. */
+ ret = ret / sizeof(*stats_data);
+
+ TEST_ASSERT(ret == desc->size,
+ "Read data of KVM stats: %s", desc->name);
+
+ *data = stats_data;
+
+ return ret;
+}
+
+static void dump_stat(int stats_fd, struct kvm_stats_header *header,
+ struct kvm_stats_desc *desc)
+{
+ u64 *stats_data;
+ ssize_t ret;
+ int i;
+
+ printf("\tflags: %u\n", desc->flags);
+ printf("\texponent: %u\n", desc->exponent);
+ printf("\tsize: %u\n", desc->size);
+ printf("\toffset: %u\n", desc->offset);
+ printf("\tbucket_size: %u\n", desc->bucket_size);
+ printf("\tname: %s\n", (char *)&desc->name);
+
+ ret = read_stat_data(stats_fd, header, desc, &stats_data);
+
+ printf("\tdata: %lu", *stats_data);
+ for (i = 1; i < ret; i++)
+ printf(", %lu", *(stats_data + i));
+ printf("\n\n");
+
+ free(stats_data);
+}
+
+void dump_vm_stats(struct kvm_vm *vm)
+{
+ struct kvm_stats_desc *stats_desc;
+ struct kvm_stats_header *header;
+ struct kvm_stats_desc *desc;
+ size_t size_desc;
+ int stats_fd;
+ int i;
+
+ stats_fd = vm_get_stats_fd(vm);
+
+ header = read_vm_stats_header(stats_fd);
+ dump_header(stats_fd, header);
+
+ stats_desc = read_vm_stats_desc(stats_fd, header);
+
+ size_desc = stats_desc_size(header);
+
+ /* Read kvm stats data one by one */
+ for (i = 0; i < header->num_desc; ++i) {
+ desc = (void *)stats_desc + (i * size_desc);
+ dump_stat(stats_fd, header, desc);
+ }
+
+ free(stats_desc);
+ free(header);
+
+ close(stats_fd);
+}
+
--
2.35.1.616.g0bdcbb4464-goog

2022-03-17 05:46:55

by Ben Gardon

[permalink] [raw]

Subject: Re: [PATCH 00/13] KVM: x86: Add a cap to disable NX hugepages on a VM

Okay, I'll hold off on the memslot refactor in v2, but if folks have
feedback on the other aspects of the v1 patch series, I'd appreciate
it.
If not, I'll try to get a v2 sent out.
I think that the commits adding utility functions for the binary stats
interface to the binary stats test could be queued separately from the
rest of this series and will be helpful for other folks working on new
selftests.

On Thu, Mar 10, 2022 at 8:19 PM Sean Christopherson <[email protected]> wrote:
>
> On Thu, Mar 10, 2022, Ben Gardon wrote:
> > Those patches are a lot of churn, but at least to me, they make the
> > code much more readable. Currently there are many functions which just
> > pass along 0 for the memslot, and often have multiple other numerical
> > arguments, which makes it hard to understand what the function is
> > doing.
>
> Yeah, my solution for that was to rip out all the params. E.g. the most used
> function I ended up with is
>
> static inline struct kvm_vm *vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
> void *guest_code)
> {
> return __vm_create_with_one_vcpu(vcpu, 0, guest_code);
> }
>
> and then the usage is
>
> vm = vm_create_with_one_vcpu(&vcpu, guest_main);
>
> supp_cpuid = kvm_get_supported_cpuid();
> cpuid2 = vcpu_get_cpuid(vcpu);
>
> My overarching complaint with the selftests is that they make the hard things hard,
> and the easy things harder. If a test wants to be backed by hugepages, it shouldn't
> have to manually specify a memslot.
>
> Let me post my selftests rework as RFC (_very_ RFC at this point). I was hoping to
> do more than compile test before posting anything, but it's going to be multiple
> weeks before I'll get back to it. Hopefully it'll start a discussion on actually
> rewriting the framework so that writing new tests is less painful, and so that every
> new thing that comes along doesn't require poking at 50 different tests.