KVM: selftests: Automatically do init_ucall() for non-barebones VMs
authorSean Christopherson <seanjc@google.com>
Thu, 6 Oct 2022 00:34:05 +0000 (00:34 +0000)
committerSean Christopherson <seanjc@google.com>
Thu, 17 Nov 2022 00:58:51 +0000 (16:58 -0800)
Do init_ucall() automatically during VM creation to kill two (three?)
birds with one stone.

First, initializing ucall immediately after VM creations allows forcing
aarch64's MMIO ucall address to immediately follow memslot0.  This is
still somewhat fragile as tests could clobber the MMIO address with a
new memslot, but it's safe-ish since tests have to be conversative when
accounting for memslot0.  And this can be hardened in the future by
creating a read-only memslot for the MMIO page (KVM ARM exits with MMIO
if the guest writes to a read-only memslot).  Add a TODO to document that
selftests can and should use a memslot for the ucall MMIO (doing so
requires yet more rework because tests assumes thay can use all memslots
except memslot0).

Second, initializing ucall for all VMs prepares for making ucall
initialization meaningful on all architectures.  aarch64 is currently the
only arch that needs to do any setup, but that will change in the future
by switching to a pool-based implementation (instead of the current
stack-based approach).

Lastly, defining the ucall MMIO address from common code will simplify
switching all architectures (except s390) to a common MMIO-based ucall
implementation (if there's ever sufficient motivation to do so).

Cc: Oliver Upton <oliver.upton@linux.dev>
Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
Tested-by: Peter Gonda <pgonda@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/r/20221006003409.649993-4-seanjc@google.com
20 files changed:
tools/testing/selftests/kvm/aarch64/aarch32_id_regs.c
tools/testing/selftests/kvm/aarch64/arch_timer.c
tools/testing/selftests/kvm/aarch64/debug-exceptions.c
tools/testing/selftests/kvm/aarch64/hypercalls.c
tools/testing/selftests/kvm/aarch64/psci_test.c
tools/testing/selftests/kvm/aarch64/vgic_init.c
tools/testing/selftests/kvm/aarch64/vgic_irq.c
tools/testing/selftests/kvm/dirty_log_test.c
tools/testing/selftests/kvm/include/ucall_common.h
tools/testing/selftests/kvm/kvm_page_table_test.c
tools/testing/selftests/kvm/lib/aarch64/ucall.c
tools/testing/selftests/kvm/lib/kvm_util.c
tools/testing/selftests/kvm/lib/memstress.c
tools/testing/selftests/kvm/lib/riscv/ucall.c
tools/testing/selftests/kvm/lib/s390x/ucall.c
tools/testing/selftests/kvm/lib/x86_64/ucall.c
tools/testing/selftests/kvm/memslot_perf_test.c
tools/testing/selftests/kvm/rseq_test.c
tools/testing/selftests/kvm/steal_time.c
tools/testing/selftests/kvm/system_counter_offset_test.c

index 6f9c1f19c7f6469536d7bd5131612186b8d20e06..03f6b3af6b4d7ac0b985c0a9dccda4ad37c57cab 100644 (file)
@@ -158,8 +158,6 @@ int main(void)
 
        TEST_REQUIRE(vcpu_aarch64_only(vcpu));
 
-       ucall_init(vm, NULL);
-
        test_user_raz_wi(vcpu);
        test_user_raz_invariant(vcpu);
        test_guest_raz(vcpu);
index 9409617fce9c44034aed224a8675a29eecba1b11..54016cdd8a0987df3d855a3654e44fe9c537ba87 100644 (file)
@@ -375,7 +375,6 @@ static struct kvm_vm *test_vm_create(void)
        for (i = 0; i < nr_vcpus; i++)
                vcpu_init_descriptor_tables(vcpus[i]);
 
-       ucall_init(vm, NULL);
        test_init_timer_irq(vm);
        gic_fd = vgic_v3_setup(vm, nr_vcpus, 64, GICD_BASE_GPA, GICR_BASE_GPA);
        __TEST_REQUIRE(gic_fd >= 0, "Failed to create vgic-v3");
index a3c2216b65fb3511b39b323ee42cebceef49f6e4..d86c4e4d1c8268e80488845fa72e035bf446e2b7 100644 (file)
@@ -292,7 +292,6 @@ static void test_guest_debug_exceptions(void)
        int stage;
 
        vm = vm_create_with_one_vcpu(&vcpu, guest_code);
-       ucall_init(vm, NULL);
 
        vm_init_descriptor_tables(vm);
        vcpu_init_descriptor_tables(vcpu);
@@ -343,7 +342,6 @@ void test_single_step_from_userspace(int test_cnt)
        struct kvm_guest_debug debug = {};
 
        vm = vm_create_with_one_vcpu(&vcpu, guest_code_ss);
-       ucall_init(vm, NULL);
        run = vcpu->run;
        vcpu_args_set(vcpu, 1, test_cnt);
 
index a39da3fe49525858baec07585cc00900160c6c60..3dceecfd1f62865a864ea94a4bb5f47e9c3726fe 100644 (file)
@@ -236,7 +236,6 @@ static struct kvm_vm *test_vm_create(struct kvm_vcpu **vcpu)
 
        vm = vm_create_with_one_vcpu(vcpu, guest_code);
 
-       ucall_init(vm, NULL);
        steal_time_init(*vcpu);
 
        return vm;
index e0b9e81a3e0918210868b352f6f3038ebf493368..cfa36f38794844078b8f429f845b427831b326c4 100644 (file)
@@ -79,7 +79,6 @@ static struct kvm_vm *setup_vm(void *guest_code, struct kvm_vcpu **source,
        struct kvm_vm *vm;
 
        vm = vm_create(2);
-       ucall_init(vm, NULL);
 
        vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init);
        init.features[0] |= (1 << KVM_ARM_VCPU_PSCI_0_2);
index 9c131d977a1b55ddef944f3e6ad377b5f7df9934..eef816b80993f528569f0de79b5f5e5fcda0e289 100644 (file)
@@ -68,8 +68,6 @@ static void guest_code(void)
 /* we don't want to assert on run execution, hence that helper */
 static int run_vcpu(struct kvm_vcpu *vcpu)
 {
-       ucall_init(vcpu->vm, NULL);
-
        return __vcpu_run(vcpu) ? -errno : 0;
 }
 
index 4ead42a072b7d9e988649364e46726a33f6bbbf6..e0310ebc313ca741351824011bbba9a5618f3747 100644 (file)
@@ -756,7 +756,6 @@ static void test_vgic(uint32_t nr_irqs, bool level_sensitive, bool eoi_split)
        print_args(&args);
 
        vm = vm_create_with_one_vcpu(&vcpu, guest_code);
-       ucall_init(vm, NULL);
 
        vm_init_descriptor_tables(vm);
        vcpu_init_descriptor_tables(vcpu);
index b5234d6efbe1509a62ece9670123d22c9eb1ec3e..b458a2701634bac4ade217f7d53e0ff1b546e797 100644 (file)
@@ -756,8 +756,6 @@ static void run_test(enum vm_guest_mode mode, void *arg)
        /* Cache the HVA pointer of the region */
        host_test_mem = addr_gpa2hva(vm, (vm_paddr_t)guest_test_phys_mem);
 
-       ucall_init(vm, NULL);
-
        /* Export the shared variables to the guest */
        sync_global_to_guest(vm, host_page_size);
        sync_global_to_guest(vm, guest_page_size);
index 63bfc60be995ebc8056f85312ca8d7fc779d03b4..8077a6d8b1ba58aae7370caf37dc8e2ed67b64e5 100644 (file)
@@ -24,7 +24,7 @@ struct ucall {
        uint64_t args[UCALL_MAX_ARGS];
 };
 
-void ucall_arch_init(struct kvm_vm *vm, void *arg);
+void ucall_arch_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa);
 void ucall_arch_uninit(struct kvm_vm *vm);
 void ucall_arch_do_ucall(vm_vaddr_t uc);
 void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu);
@@ -32,9 +32,9 @@ void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu);
 void ucall(uint64_t cmd, int nargs, ...);
 uint64_t get_ucall(struct kvm_vcpu *vcpu, struct ucall *uc);
 
-static inline void ucall_init(struct kvm_vm *vm, void *arg)
+static inline void ucall_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa)
 {
-       ucall_arch_init(vm, arg);
+       ucall_arch_init(vm, mmio_gpa);
 }
 
 static inline void ucall_uninit(struct kvm_vm *vm)
index 696b366be06b4b79809c2e91beed4b635e542b25..3db32d56787e8e482566f7776e7ebd1f10f812c4 100644 (file)
@@ -289,7 +289,6 @@ static struct kvm_vm *pre_init_before_test(enum vm_guest_mode mode, void *arg)
        host_test_mem = addr_gpa2hva(vm, (vm_paddr_t)guest_test_phys_mem);
 
        /* Export shared structure test_args to guest */
-       ucall_init(vm, NULL);
        sync_global_to_guest(vm, test_args);
 
        ret = sem_init(&test_stage_updated, 0, 0);
index f214f5cc53d3be117c5d84c9cf9c77b1f71ddc6e..f02ae27c3e43ae1bb21ec17ad097bd664f3627da 100644 (file)
@@ -8,60 +8,12 @@
 
 static vm_vaddr_t *ucall_exit_mmio_addr;
 
-static bool ucall_mmio_init(struct kvm_vm *vm, vm_paddr_t gpa)
+void ucall_arch_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa)
 {
-       if (kvm_userspace_memory_region_find(vm, gpa, gpa + 1))
-               return false;
+       virt_pg_map(vm, mmio_gpa, mmio_gpa);
 
-       virt_pg_map(vm, gpa, gpa);
-
-       ucall_exit_mmio_addr = (vm_vaddr_t *)gpa;
+       ucall_exit_mmio_addr = (vm_vaddr_t *)mmio_gpa;
        sync_global_to_guest(vm, ucall_exit_mmio_addr);
-
-       return true;
-}
-
-void ucall_arch_init(struct kvm_vm *vm, void *arg)
-{
-       vm_paddr_t gpa, start, end, step, offset;
-       unsigned int bits;
-       bool ret;
-
-       if (arg) {
-               gpa = (vm_paddr_t)arg;
-               ret = ucall_mmio_init(vm, gpa);
-               TEST_ASSERT(ret, "Can't set ucall mmio address to %lx", gpa);
-               return;
-       }
-
-       /*
-        * Find an address within the allowed physical and virtual address
-        * spaces, that does _not_ have a KVM memory region associated with
-        * it. Identity mapping an address like this allows the guest to
-        * access it, but as KVM doesn't know what to do with it, it
-        * will assume it's something userspace handles and exit with
-        * KVM_EXIT_MMIO. Well, at least that's how it works for AArch64.
-        * Here we start with a guess that the addresses around 5/8th
-        * of the allowed space are unmapped and then work both down and
-        * up from there in 1/16th allowed space sized steps.
-        *
-        * Note, we need to use VA-bits - 1 when calculating the allowed
-        * virtual address space for an identity mapping because the upper
-        * half of the virtual address space is the two's complement of the
-        * lower and won't match physical addresses.
-        */
-       bits = vm->va_bits - 1;
-       bits = min(vm->pa_bits, bits);
-       end = 1ul << bits;
-       start = end * 5 / 8;
-       step = end / 16;
-       for (offset = 0; offset < end - start; offset += step) {
-               if (ucall_mmio_init(vm, start - offset))
-                       return;
-               if (ucall_mmio_init(vm, start + offset))
-                       return;
-       }
-       TEST_FAIL("Can't find a ucall mmio address");
 }
 
 void ucall_arch_uninit(struct kvm_vm *vm)
index 3b7710fb378406eb77836537598d2990bc466584..07c8edd4e54861ee3c49a0a92c45c9550f0b8319 100644 (file)
@@ -335,15 +335,26 @@ struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint32_t nr_runnable_vcpus,
 {
        uint64_t nr_pages = vm_nr_pages_required(mode, nr_runnable_vcpus,
                                                 nr_extra_pages);
+       struct userspace_mem_region *slot0;
        struct kvm_vm *vm;
 
        vm = ____vm_create(mode, nr_pages);
 
        kvm_vm_elf_load(vm, program_invocation_name);
 
+       /*
+        * TODO: Add proper defines to protect the library's memslots, and then
+        * carve out memslot1 for the ucall MMIO address.  KVM treats writes to
+        * read-only memslots as MMIO, and creating a read-only memslot for the
+        * MMIO region would prevent silently clobbering the MMIO region.
+        */
+       slot0 = memslot2region(vm, 0);
+       ucall_init(vm, slot0->region.guest_phys_addr + slot0->region.memory_size);
+
 #ifdef __x86_64__
        vm_create_irqchip(vm);
 #endif
+
        return vm;
 }
 
index 503da78c558dd75da382ea33497e066f789b1300..b66404e56a3f878eff5047b815283f9af585c223 100644 (file)
@@ -221,8 +221,6 @@ struct kvm_vm *memstress_create_vm(enum vm_guest_mode mode, int nr_vcpus,
                memstress_setup_nested(vm, nr_vcpus, vcpus);
        }
 
-       ucall_init(vm, NULL);
-
        /* Export the shared variables to the guest. */
        sync_global_to_guest(vm, memstress_args);
 
index 37e091d4366e55cd845273344946e19140ee5e8e..c58ecb8a0981bd909fd689d2b8dce38dd8ef8681 100644 (file)
@@ -10,7 +10,7 @@
 #include "kvm_util.h"
 #include "processor.h"
 
-void ucall_arch_init(struct kvm_vm *vm, void *arg)
+void ucall_arch_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa)
 {
 }
 
index 0f695a031d35d635de238b1d8c06001c66be6d3d..208f0f04299bb073457e039a7b87d57c118f2606 100644 (file)
@@ -6,7 +6,7 @@
  */
 #include "kvm_util.h"
 
-void ucall_arch_init(struct kvm_vm *vm, void *arg)
+void ucall_arch_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa)
 {
 }
 
index ead9946399abf09d4327c9141cd7e1cb8ab4991d..016a0487cf7235c2f63e8c5996cb65dd3caa0d85 100644 (file)
@@ -8,7 +8,7 @@
 
 #define UCALL_PIO_PORT ((uint16_t)0x1000)
 
-void ucall_arch_init(struct kvm_vm *vm, void *arg)
+void ucall_arch_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa)
 {
 }
 
index 330aaef1c02fb85292886eaf4f011b6f818fb061..d771262ea584bdfd32bda8867f7ac1ecf565494f 100644 (file)
@@ -277,7 +277,6 @@ static bool prepare_vm(struct vm_data *data, int nslots, uint64_t *maxslots,
        TEST_ASSERT(data->hva_slots, "malloc() fail");
 
        data->vm = __vm_create_with_one_vcpu(&data->vcpu, mempages, guest_code);
-       ucall_init(data->vm, NULL);
 
        pr_info_v("Adding slots 1..%i, each slot with %"PRIu64" pages + %"PRIu64" extra pages last\n",
                max_mem_slots - 1, data->pages_per_slot, rempages);
index 6f88da7e60be63844a4c0f18f4ee0f84c5d1c0da..0e9e2b48a51f709b40b8fde6e141f55df470b4af 100644 (file)
@@ -224,7 +224,6 @@ int main(int argc, char *argv[])
         * CPU affinity.
         */
        vm = vm_create_with_one_vcpu(&vcpu, guest_code);
-       ucall_init(vm, NULL);
 
        pthread_create(&migration_thread, NULL, migration_worker,
                       (void *)(unsigned long)syscall(SYS_gettid));
index db8967f1a17bcc879a181ee6751e0dd56a0d24fd..c87f3871207361d8559b005e1bdb7c05090dee2d 100644 (file)
@@ -266,7 +266,6 @@ int main(int ac, char **av)
        gpages = vm_calc_num_guest_pages(VM_MODE_DEFAULT, STEAL_TIME_SIZE * NR_VCPUS);
        vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, ST_GPA_BASE, 1, gpages, 0);
        virt_map(vm, ST_GPA_BASE, ST_GPA_BASE, gpages);
-       ucall_init(vm, NULL);
 
        TEST_REQUIRE(is_steal_time_supported(vcpus[0]));
 
index 1c274933912bb9cf7fcb657ea596d4f1e33fd540..7f5b330b6a1b182f7a5890d3f7b67b7d2535accc 100644 (file)
@@ -121,7 +121,6 @@ int main(void)
 
        vm = vm_create_with_one_vcpu(&vcpu, guest_main);
        check_preconditions(vcpu);
-       ucall_init(vm, NULL);
 
        enter_guest(vcpu);
        kvm_vm_free(vm);