On Mon, Apr 04, 2022 at 11:13:40AM -0700, Ben Gardon wrote:
> On Fri, Apr 1, 2022 at 4:37 PM David Matlack <dmatlack@xxxxxxxxxx> wrote:
> >
> > Introduce a new selftest, execute_perf_test, that uses the
> > perf_test_util framework to measure the performance of executing code
> > within a VM. This test is similar to the other perf_test_util-based
> > tests in that it spins up a variable number of vCPUs and runs them
> > concurrently, accessing memory.
> >
> > In order to support executiong, extend perf_test_util to populate guest
>
> *executing instructions in the data slot,
>
> > memory with return instructions rather than random garbage. This way
> > memory can be execute simply by calling it.
>
> *executed
>
> >
> > Currently only x86-64 supports execution, but other architectures can be
> > easily added by providing their return code instruction.
> >
> > Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx>
> > ---
> > tools/testing/selftests/kvm/.gitignore | 1 +
> > tools/testing/selftests/kvm/Makefile | 1 +
> > .../testing/selftests/kvm/execute_perf_test.c | 188 ++++++++++++++++++
> > .../selftests/kvm/include/perf_test_util.h | 2 +
> > .../selftests/kvm/lib/perf_test_util.c | 25 ++-
> > 5 files changed, 215 insertions(+), 2 deletions(-)
> > create mode 100644 tools/testing/selftests/kvm/execute_perf_test.c
> >
> > diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> > index 1f1b6c978bf7..3647ddacb103 100644
> > --- a/tools/testing/selftests/kvm/.gitignore
> > +++ b/tools/testing/selftests/kvm/.gitignore
> > @@ -56,6 +56,7 @@
> > /demand_paging_test
> > /dirty_log_test
> > /dirty_log_perf_test
> > +/execute_perf_test
> > /hardware_disable_test
> > /kvm_create_max_vcpus
> > /kvm_page_table_test
> > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> > index c9cdbd248727..3c67346b0766 100644
> > --- a/tools/testing/selftests/kvm/Makefile
> > +++ b/tools/testing/selftests/kvm/Makefile
> > @@ -92,6 +92,7 @@ TEST_GEN_PROGS_x86_64 += access_tracking_perf_test
> > TEST_GEN_PROGS_x86_64 += demand_paging_test
> > TEST_GEN_PROGS_x86_64 += dirty_log_test
> > TEST_GEN_PROGS_x86_64 += dirty_log_perf_test
> > +TEST_GEN_PROGS_x86_64 += execute_perf_test
> > TEST_GEN_PROGS_x86_64 += hardware_disable_test
> > TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
> > TEST_GEN_PROGS_x86_64 += kvm_page_table_test
> > diff --git a/tools/testing/selftests/kvm/execute_perf_test.c b/tools/testing/selftests/kvm/execute_perf_test.c
> > new file mode 100644
> > index 000000000000..fa78facf44e7
> > --- /dev/null
> > +++ b/tools/testing/selftests/kvm/execute_perf_test.c
> > @@ -0,0 +1,188 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <inttypes.h>
> > +#include <limits.h>
> > +#include <pthread.h>
> > +#include <sys/mman.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +
> > +#include "kvm_util.h"
> > +#include "test_util.h"
> > +#include "perf_test_util.h"
> > +#include "guest_modes.h"
> > +
> > +/* Global variable used to synchronize all of the vCPU threads. */
> > +static int iteration;
>
> Should this be volatile? (same for other globals)
Or atomic_t. This is a common pattern across almost all of the
perf_test_util-based tests that needs to be addressed.
>
> > +
> > +/* Set to true when vCPU threads should exit. */
> > +static bool done;
> > +
> > +/* The iteration that was last completed by each vCPU. */
> > +static int vcpu_last_completed_iteration[KVM_MAX_VCPUS];
> > +
> > +/* Whether to overlap the regions of memory vCPUs access. */
> > +static bool overlap_memory_access;
>
> Can this be factored into the perf test util framework / test params?
Yes. I'm planning to do a larger refactor of the perf_test_util
framework to consolidate code like this. But I want to leave that to a
separate series.
I'd be fine with deferring this test until that refactor is complete but
I don't think it's stricly necessary.
>
> > +
> > +struct test_params {
> > + /* The backing source for the region of memory. */
> > + enum vm_mem_backing_src_type backing_src;
> > +
> > + /* The amount of memory to allocate for each vCPU. */
> > + uint64_t vcpu_memory_bytes;
> > +
> > + /* The number of vCPUs to create in the VM. */
> > + int vcpus;
> > +};
> > +
> > +static void assert_ucall(struct kvm_vm *vm, uint32_t vcpu_id,
> > + uint64_t expected_ucall)
> > +{
> > + struct ucall uc;
> > + uint64_t actual_ucall = get_ucall(vm, vcpu_id, &uc);
> > +
> > + TEST_ASSERT(expected_ucall == actual_ucall,
> > + "Guest exited unexpectedly (expected ucall %" PRIu64
> > + ", got %" PRIu64 ")",
> > + expected_ucall, actual_ucall);
> > +}
> > +
> > +static bool spin_wait_for_next_iteration(int *current_iteration)
> > +{
> > + int last_iteration = *current_iteration;
> > +
> > + do {
> > + if (READ_ONCE(done))
> > + return false;
> > +
> > + *current_iteration = READ_ONCE(iteration);
> > + } while (last_iteration == *current_iteration);
> > +
> > + return true;
> > +}
> > +
> > +static void vcpu_thread_main(struct perf_test_vcpu_args *vcpu_args)
> > +{
> > + struct kvm_vm *vm = perf_test_args.vm;
> > + int vcpu_id = vcpu_args->vcpu_id;
> > + int current_iteration = 0;
> > +
> > + while (spin_wait_for_next_iteration(¤t_iteration)) {
> > + vcpu_run(vm, vcpu_id);
> > + assert_ucall(vm, vcpu_id, UCALL_SYNC);
> > + vcpu_last_completed_iteration[vcpu_id] = current_iteration;
> > + }
> > +}
> > +
> > +static void spin_wait_for_vcpu(int vcpu_id, int target_iteration)
> > +{
> > + while (READ_ONCE(vcpu_last_completed_iteration[vcpu_id]) !=
> > + target_iteration) {
> > + continue;
> > + }
> > +}
> > +
> > +static void run_iteration(struct kvm_vm *vm, int vcpus, const char *description)
> > +{
> > + struct timespec ts_start;
> > + struct timespec ts_elapsed;
> > + int next_iteration;
> > + int vcpu_id;
> > +
> > + /* Kick off the vCPUs by incrementing iteration. */
> > + next_iteration = ++iteration;
> > +
> > + clock_gettime(CLOCK_MONOTONIC, &ts_start);
> > +
> > + /* Wait for all vCPUs to finish the iteration. */
> > + for (vcpu_id = 0; vcpu_id < vcpus; vcpu_id++)
> > + spin_wait_for_vcpu(vcpu_id, next_iteration);
> > +
> > + ts_elapsed = timespec_elapsed(ts_start);
> > + pr_info("%-30s: %ld.%09lds\n",
> > + description, ts_elapsed.tv_sec, ts_elapsed.tv_nsec);
> > +}
> > +
> > +static void run_test(enum vm_guest_mode mode, void *arg)
> > +{
> > + struct test_params *params = arg;
> > + struct kvm_vm *vm;
> > + int vcpus = params->vcpus;
> > +
> > + vm = perf_test_create_vm(mode, vcpus, params->vcpu_memory_bytes, 1,
> > + params->backing_src, !overlap_memory_access);
> > +
> > + perf_test_start_vcpu_threads(vcpus, vcpu_thread_main);
> > +
> > + pr_info("\n");
> > +
> > + perf_test_set_wr_fract(vm, 1);
> > + run_iteration(vm, vcpus, "Populating memory");
> > +
> > + perf_test_set_execute(vm, true);
> > + run_iteration(vm, vcpus, "Executing from memory");
> > +
> > + /* Set done to signal the vCPU threads to exit */
> > + done = true;
> > +
> > + perf_test_join_vcpu_threads(vcpus);
> > + perf_test_destroy_vm(vm);
> > +}
> > +
> > +static void help(char *name)
> > +{
> > + puts("");
> > + printf("usage: %s [-h] [-m mode] [-b vcpu_bytes] [-v vcpus] [-o] [-s mem_type]\n",
> > + name);
> > + puts("");
> > + printf(" -h: Display this help message.");
> > + guest_modes_help();
> > + printf(" -b: specify the size of the memory region which should be\n"
> > + " dirtied by each vCPU. e.g. 10M or 3G.\n"
> > + " (default: 1G)\n");
> > + printf(" -v: specify the number of vCPUs to run.\n");
> > + printf(" -o: Overlap guest memory accesses instead of partitioning\n"
> > + " them into a separate region of memory for each vCPU.\n");
> > + backing_src_help("-s");
> > + puts("");
> > + exit(0);
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > + struct test_params params = {
> > + .backing_src = DEFAULT_VM_MEM_SRC,
> > + .vcpu_memory_bytes = DEFAULT_PER_VCPU_MEM_SIZE,
> > + .vcpus = 1,
> > + };
> > + int opt;
> > +
> > + guest_modes_append_default();
> > +
> > + while ((opt = getopt(argc, argv, "hm:b:v:os:")) != -1) {
> > + switch (opt) {
> > + case 'm':
> > + guest_modes_cmdline(optarg);
> > + break;
> > + case 'b':
> > + params.vcpu_memory_bytes = parse_size(optarg);
> > + break;
> > + case 'v':
> > + params.vcpus = atoi(optarg);
> > + break;
> > + case 'o':
> > + overlap_memory_access = true;
> > + break;
> > + case 's':
> > + params.backing_src = parse_backing_src_type(optarg);
> > + break;
> > + case 'h':
> > + default:
> > + help(argv[0]);
> > + break;
> > + }
> > + }
> > +
> > + for_each_guest_mode(run_test, ¶ms);
> > +
> > + return 0;
> > +}
> > diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
> > index a86f953d8d36..0a5a56539aff 100644
> > --- a/tools/testing/selftests/kvm/include/perf_test_util.h
> > +++ b/tools/testing/selftests/kvm/include/perf_test_util.h
> > @@ -33,6 +33,7 @@ struct perf_test_args {
> > uint64_t gpa;
> > uint64_t guest_page_size;
> > int wr_fract;
> > + bool execute;
> >
> > struct perf_test_vcpu_args vcpu_args[KVM_MAX_VCPUS];
> > };
> > @@ -46,6 +47,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus,
> > void perf_test_destroy_vm(struct kvm_vm *vm);
> >
> > void perf_test_set_wr_fract(struct kvm_vm *vm, int wr_fract);
> > +void perf_test_set_execute(struct kvm_vm *vm, bool execute);
> >
> > void perf_test_start_vcpu_threads(int vcpus, void (*vcpu_fn)(struct perf_test_vcpu_args *));
> > void perf_test_join_vcpu_threads(int vcpus);
> > diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
> > index 722df3a28791..1a5eb60b59da 100644
> > --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> > +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> > @@ -36,6 +36,16 @@ static void (*vcpu_thread_fn)(struct perf_test_vcpu_args *);
> > /* Set to true once all vCPU threads are up and running. */
> > static bool all_vcpu_threads_running;
> >
> > +/*
> > + * When writing to guest memory, write the opcode for the `ret` instruction so
> > + * that subsequent iteractions can exercise instruction fetch by calling the
> > + * memory.
> > + *
> > + * NOTE: Non-x86 architectures would to use different values here to support
> > + * execute.
> > + */
> > +#define RETURN_OPCODE 0xC3
> > +
>
> This should be defined in an arch-specific header or surrounded by
> ifdefs so that the build would fail for other archs.
Agreed, this should really go in an x86-specific header file. There's no
correctness issue (0xC3 works just as well as 0x0123456789ABCDEF and
non-x86 architectures are prevented from setting execute to true in
perf_test_set_execute()), but this is a lazy way to structure the code.
>
> > /*
> > * Continuously write to the first 8 bytes of each page in the
> > * specified region.
> > @@ -58,8 +68,10 @@ static void guest_code(uint32_t vcpu_id)
> > for (i = 0; i < pages; i++) {
> > uint64_t addr = gva + (i * pta->guest_page_size);
> >
> > - if (i % pta->wr_fract == 0)
> > - *(uint64_t *)addr = 0x0123456789ABCDEF;
> > + if (pta->execute)
> > + ((void (*)(void)) addr)();
> > + else if (i % pta->wr_fract == 0)
> > + *(uint64_t *)addr = RETURN_OPCODE;
>
> Oh interesting, you're using a write pass to set up the contents of
> memory. I suppose that probably ends up being faster than memset, but
> it introduces kind of a strange dependency.
It also allows the memory to be mapped in a huge pages first so then it
can be split via NX HugePages. But I agree it's a strange dependency.
I'll have to think more about how to better structure this code.
>
> > else
> > READ_ONCE(*(uint64_t *)addr);
> > }
> > @@ -198,6 +210,15 @@ void perf_test_set_wr_fract(struct kvm_vm *vm, int wr_fract)
> > sync_global_to_guest(vm, perf_test_args);
> > }
> >
> > +void perf_test_set_execute(struct kvm_vm *vm, bool execute)
> > +{
> > +#ifndef __x86_64__
> > + TEST_ASSERT(false, "Execute not supported on this architure; see RETURN_OPCODE.");
> > +#endif
> > + perf_test_args.execute = execute;
> > + sync_global_to_guest(vm, perf_test_args);
> > +}
> > +
> > static void *vcpu_thread_main(void *data)
> > {
> > struct vcpu_thread *vcpu = data;
> >
> > base-commit: d1fb6a1ca3e535f89628193ab94203533b264c8c
> > --
> > 2.35.1.1094.g7c7d902a7c-goog
> >
