Re: [PATCH 27/34] KVM: selftests: Introduce VM "shape" to allow tests to specify the VM type
From: Fuad Tabba
Date: Mon Nov 06 2023 - 06:55:26 EST
On Sun, Nov 5, 2023 at 4:34 PM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>
> From: Sean Christopherson <seanjc@xxxxxxxxxx>
>
> Add a "vm_shape" structure to encapsulate the selftests-defined "mode",
> along with the KVM-defined "type" for use when creating a new VM. "mode"
> tracks physical and virtual address properties, as well as the preferred
> backing memory type, while "type" corresponds to the VM type.
>
> Taking the VM type will allow adding tests for KVM_CREATE_GUEST_MEMFD,
> a.k.a. guest private memory, without needing an entirely separate set of
> helpers. Guest private memory is effectively usable only by confidential
> VM types, and it's expected that x86 will double down and require unique
> VM types for TDX and SNP guests.
>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> Message-Id: <20231027182217.3615211-30-seanjc@xxxxxxxxxx>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
nit: as in a prior selftest commit messages, references in the commit
message to guest _private_ memory. Should these be changed to just
guest memory?
Reviewed-by: Fuad Tabba <tabba@xxxxxxxxxx>
Tested-by: Fuad Tabba <tabba@xxxxxxxxxx>
Cheers,
/fuad
> tools/testing/selftests/kvm/dirty_log_test.c | 2 +-
> .../selftests/kvm/include/kvm_util_base.h | 54 +++++++++++++++----
> .../selftests/kvm/kvm_page_table_test.c | 2 +-
> tools/testing/selftests/kvm/lib/kvm_util.c | 43 +++++++--------
> tools/testing/selftests/kvm/lib/memstress.c | 3 +-
> .../kvm/x86_64/ucna_injection_test.c | 2 +-
> 6 files changed, 72 insertions(+), 34 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
> index 936f3a8d1b83..6cbecf499767 100644
> --- a/tools/testing/selftests/kvm/dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_test.c
> @@ -699,7 +699,7 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, struct kvm_vcpu **vcpu,
>
> pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
>
> - vm = __vm_create(mode, 1, extra_mem_pages);
> + vm = __vm_create(VM_SHAPE(mode), 1, extra_mem_pages);
>
> log_mode_create_vm_done(vm);
> *vcpu = vm_vcpu_add(vm, 0, guest_code);
> diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
> index 1441fca6c273..157508c071f3 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util_base.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
> @@ -188,6 +188,23 @@ enum vm_guest_mode {
> NUM_VM_MODES,
> };
>
> +struct vm_shape {
> + enum vm_guest_mode mode;
> + unsigned int type;
> +};
> +
> +#define VM_TYPE_DEFAULT 0
> +
> +#define VM_SHAPE(__mode) \
> +({ \
> + struct vm_shape shape = { \
> + .mode = (__mode), \
> + .type = VM_TYPE_DEFAULT \
> + }; \
> + \
> + shape; \
> +})
> +
> #if defined(__aarch64__)
>
> extern enum vm_guest_mode vm_mode_default;
> @@ -220,6 +237,8 @@ extern enum vm_guest_mode vm_mode_default;
>
> #endif
>
> +#define VM_SHAPE_DEFAULT VM_SHAPE(VM_MODE_DEFAULT)
> +
> #define MIN_PAGE_SIZE (1U << MIN_PAGE_SHIFT)
> #define PTES_PER_MIN_PAGE ptes_per_page(MIN_PAGE_SIZE)
>
> @@ -784,21 +803,21 @@ vm_paddr_t vm_alloc_page_table(struct kvm_vm *vm);
> * __vm_create() does NOT create vCPUs, @nr_runnable_vcpus is used purely to
> * calculate the amount of memory needed for per-vCPU data, e.g. stacks.
> */
> -struct kvm_vm *____vm_create(enum vm_guest_mode mode);
> -struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint32_t nr_runnable_vcpus,
> +struct kvm_vm *____vm_create(struct vm_shape shape);
> +struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
> uint64_t nr_extra_pages);
>
> static inline struct kvm_vm *vm_create_barebones(void)
> {
> - return ____vm_create(VM_MODE_DEFAULT);
> + return ____vm_create(VM_SHAPE_DEFAULT);
> }
>
> static inline struct kvm_vm *vm_create(uint32_t nr_runnable_vcpus)
> {
> - return __vm_create(VM_MODE_DEFAULT, nr_runnable_vcpus, 0);
> + return __vm_create(VM_SHAPE_DEFAULT, nr_runnable_vcpus, 0);
> }
>
> -struct kvm_vm *__vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus,
> +struct kvm_vm *__vm_create_with_vcpus(struct vm_shape shape, uint32_t nr_vcpus,
> uint64_t extra_mem_pages,
> void *guest_code, struct kvm_vcpu *vcpus[]);
>
> @@ -806,17 +825,27 @@ static inline struct kvm_vm *vm_create_with_vcpus(uint32_t nr_vcpus,
> void *guest_code,
> struct kvm_vcpu *vcpus[])
> {
> - return __vm_create_with_vcpus(VM_MODE_DEFAULT, nr_vcpus, 0,
> + return __vm_create_with_vcpus(VM_SHAPE_DEFAULT, nr_vcpus, 0,
> guest_code, vcpus);
> }
>
> +
> +struct kvm_vm *__vm_create_shape_with_one_vcpu(struct vm_shape shape,
> + struct kvm_vcpu **vcpu,
> + uint64_t extra_mem_pages,
> + void *guest_code);
> +
> /*
> * Create a VM with a single vCPU with reasonable defaults and @extra_mem_pages
> * additional pages of guest memory. Returns the VM and vCPU (via out param).
> */
> -struct kvm_vm *__vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
> - uint64_t extra_mem_pages,
> - void *guest_code);
> +static inline struct kvm_vm *__vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
> + uint64_t extra_mem_pages,
> + void *guest_code)
> +{
> + return __vm_create_shape_with_one_vcpu(VM_SHAPE_DEFAULT, vcpu,
> + extra_mem_pages, guest_code);
> +}
>
> static inline struct kvm_vm *vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
> void *guest_code)
> @@ -824,6 +853,13 @@ static inline struct kvm_vm *vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
> return __vm_create_with_one_vcpu(vcpu, 0, guest_code);
> }
>
> +static inline struct kvm_vm *vm_create_shape_with_one_vcpu(struct vm_shape shape,
> + struct kvm_vcpu **vcpu,
> + void *guest_code)
> +{
> + return __vm_create_shape_with_one_vcpu(shape, vcpu, 0, guest_code);
> +}
> +
> struct kvm_vcpu *vm_recreate_with_one_vcpu(struct kvm_vm *vm);
>
> void kvm_pin_this_task_to_pcpu(uint32_t pcpu);
> diff --git a/tools/testing/selftests/kvm/kvm_page_table_test.c b/tools/testing/selftests/kvm/kvm_page_table_test.c
> index 69f26d80c821..e37dc9c21888 100644
> --- a/tools/testing/selftests/kvm/kvm_page_table_test.c
> +++ b/tools/testing/selftests/kvm/kvm_page_table_test.c
> @@ -254,7 +254,7 @@ static struct kvm_vm *pre_init_before_test(enum vm_guest_mode mode, void *arg)
>
> /* Create a VM with enough guest pages */
> guest_num_pages = test_mem_size / guest_page_size;
> - vm = __vm_create_with_vcpus(mode, nr_vcpus, guest_num_pages,
> + vm = __vm_create_with_vcpus(VM_SHAPE(mode), nr_vcpus, guest_num_pages,
> guest_code, test_args.vcpus);
>
> /* Align down GPA of the testing memslot */
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 95a553400ea9..1c74310f1d44 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -209,7 +209,7 @@ __weak void vm_vaddr_populate_bitmap(struct kvm_vm *vm)
> (1ULL << (vm->va_bits - 1)) >> vm->page_shift);
> }
>
> -struct kvm_vm *____vm_create(enum vm_guest_mode mode)
> +struct kvm_vm *____vm_create(struct vm_shape shape)
> {
> struct kvm_vm *vm;
>
> @@ -221,13 +221,13 @@ struct kvm_vm *____vm_create(enum vm_guest_mode mode)
> vm->regions.hva_tree = RB_ROOT;
> hash_init(vm->regions.slot_hash);
>
> - vm->mode = mode;
> - vm->type = 0;
> + vm->mode = shape.mode;
> + vm->type = shape.type;
>
> - vm->pa_bits = vm_guest_mode_params[mode].pa_bits;
> - vm->va_bits = vm_guest_mode_params[mode].va_bits;
> - vm->page_size = vm_guest_mode_params[mode].page_size;
> - vm->page_shift = vm_guest_mode_params[mode].page_shift;
> + vm->pa_bits = vm_guest_mode_params[vm->mode].pa_bits;
> + vm->va_bits = vm_guest_mode_params[vm->mode].va_bits;
> + vm->page_size = vm_guest_mode_params[vm->mode].page_size;
> + vm->page_shift = vm_guest_mode_params[vm->mode].page_shift;
>
> /* Setup mode specific traits. */
> switch (vm->mode) {
> @@ -265,7 +265,7 @@ struct kvm_vm *____vm_create(enum vm_guest_mode mode)
> /*
> * Ignore KVM support for 5-level paging (vm->va_bits == 57),
> * it doesn't take effect unless a CR4.LA57 is set, which it
> - * isn't for this VM_MODE.
> + * isn't for this mode (48-bit virtual address space).
> */
> TEST_ASSERT(vm->va_bits == 48 || vm->va_bits == 57,
> "Linear address width (%d bits) not supported",
> @@ -285,10 +285,11 @@ struct kvm_vm *____vm_create(enum vm_guest_mode mode)
> vm->pgtable_levels = 5;
> break;
> default:
> - TEST_FAIL("Unknown guest mode, mode: 0x%x", mode);
> + TEST_FAIL("Unknown guest mode: 0x%x", vm->mode);
> }
>
> #ifdef __aarch64__
> + TEST_ASSERT(!vm->type, "ARM doesn't support test-provided types");
> if (vm->pa_bits != 40)
> vm->type = KVM_VM_TYPE_ARM_IPA_SIZE(vm->pa_bits);
> #endif
> @@ -347,19 +348,19 @@ static uint64_t vm_nr_pages_required(enum vm_guest_mode mode,
> return vm_adjust_num_guest_pages(mode, nr_pages);
> }
>
> -struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint32_t nr_runnable_vcpus,
> +struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
> uint64_t nr_extra_pages)
> {
> - uint64_t nr_pages = vm_nr_pages_required(mode, nr_runnable_vcpus,
> + uint64_t nr_pages = vm_nr_pages_required(shape.mode, nr_runnable_vcpus,
> nr_extra_pages);
> struct userspace_mem_region *slot0;
> struct kvm_vm *vm;
> int i;
>
> - pr_debug("%s: mode='%s' pages='%ld'\n", __func__,
> - vm_guest_mode_string(mode), nr_pages);
> + pr_debug("%s: mode='%s' type='%d', pages='%ld'\n", __func__,
> + vm_guest_mode_string(shape.mode), shape.type, nr_pages);
>
> - vm = ____vm_create(mode);
> + vm = ____vm_create(shape);
>
> vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, 0, 0, nr_pages, 0);
> for (i = 0; i < NR_MEM_REGIONS; i++)
> @@ -400,7 +401,7 @@ struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint32_t nr_runnable_vcpus,
> * extra_mem_pages is only used to calculate the maximum page table size,
> * no real memory allocation for non-slot0 memory in this function.
> */
> -struct kvm_vm *__vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus,
> +struct kvm_vm *__vm_create_with_vcpus(struct vm_shape shape, uint32_t nr_vcpus,
> uint64_t extra_mem_pages,
> void *guest_code, struct kvm_vcpu *vcpus[])
> {
> @@ -409,7 +410,7 @@ struct kvm_vm *__vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus
>
> TEST_ASSERT(!nr_vcpus || vcpus, "Must provide vCPU array");
>
> - vm = __vm_create(mode, nr_vcpus, extra_mem_pages);
> + vm = __vm_create(shape, nr_vcpus, extra_mem_pages);
>
> for (i = 0; i < nr_vcpus; ++i)
> vcpus[i] = vm_vcpu_add(vm, i, guest_code);
> @@ -417,15 +418,15 @@ struct kvm_vm *__vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus
> return vm;
> }
>
> -struct kvm_vm *__vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
> - uint64_t extra_mem_pages,
> - void *guest_code)
> +struct kvm_vm *__vm_create_shape_with_one_vcpu(struct vm_shape shape,
> + struct kvm_vcpu **vcpu,
> + uint64_t extra_mem_pages,
> + void *guest_code)
> {
> struct kvm_vcpu *vcpus[1];
> struct kvm_vm *vm;
>
> - vm = __vm_create_with_vcpus(VM_MODE_DEFAULT, 1, extra_mem_pages,
> - guest_code, vcpus);
> + vm = __vm_create_with_vcpus(shape, 1, extra_mem_pages, guest_code, vcpus);
>
> *vcpu = vcpus[0];
> return vm;
> diff --git a/tools/testing/selftests/kvm/lib/memstress.c b/tools/testing/selftests/kvm/lib/memstress.c
> index df457452d146..d05487e5a371 100644
> --- a/tools/testing/selftests/kvm/lib/memstress.c
> +++ b/tools/testing/selftests/kvm/lib/memstress.c
> @@ -168,7 +168,8 @@ struct kvm_vm *memstress_create_vm(enum vm_guest_mode mode, int nr_vcpus,
> * The memory is also added to memslot 0, but that's a benign side
> * effect as KVM allows aliasing HVAs in meslots.
> */
> - vm = __vm_create_with_vcpus(mode, nr_vcpus, slot0_pages + guest_num_pages,
> + vm = __vm_create_with_vcpus(VM_SHAPE(mode), nr_vcpus,
> + slot0_pages + guest_num_pages,
> memstress_guest_code, vcpus);
>
> args->vm = vm;
> diff --git a/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c b/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c
> index 85f34ca7e49e..0ed32ec903d0 100644
> --- a/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c
> @@ -271,7 +271,7 @@ int main(int argc, char *argv[])
>
> kvm_check_cap(KVM_CAP_MCE);
>
> - vm = __vm_create(VM_MODE_DEFAULT, 3, 0);
> + vm = __vm_create(VM_SHAPE_DEFAULT, 3, 0);
>
> kvm_ioctl(vm->kvm_fd, KVM_X86_GET_MCE_CAP_SUPPORTED,
> &supported_mcg_caps);
> --
> 2.39.1
>
>