Re: [PATCH 27/34] KVM: selftests: Introduce VM "shape" to allow tests to specify the VM type
On 11/9/23 00:37, Anish Moorthy wrote: On Wed, Nov 8, 2023 at 9:00 AM Anish Moorthy wrote: This commit breaks the arm64 selftests build btw: looks like a simple oversight? Yup, fix is a one-liner. Posted below. diff --git a/tools/testing/selftests/kvm/aarch64/page_fault_test.c b/tools/testing/selftests/kvm/aarch64/page_fault_test.c index eb4217b7c768..08a5ca5bed56 100644 --- a/tools/testing/selftests/kvm/aarch64/page_fault_test.c +++ b/tools/testing/selftests/kvm/aarch64/page_fault_test.c @@ -705,7 +705,7 @@ static void run_test(enum vm_guest_mode mode, void *arg) print_test_banner(mode, p); - vm = vm_create(mode); + vm = vm_create(VM_SHAPE(mode)); Yes, this is similar to the s390 patch I sent yesterday (https://patchew.org/linux/20231108094055.221234-1-pbonz...@redhat.com/). Paolo
Re: [PATCH 27/34] KVM: selftests: Introduce VM "shape" to allow tests to specify the VM type
On Wed, Nov 8, 2023 at 9:00 AM Anish Moorthy wrote: > > This commit breaks the arm64 selftests build btw: looks like a simple > oversight? Yup, fix is a one-liner. Posted below. diff --git a/tools/testing/selftests/kvm/aarch64/page_fault_test.c b/tools/testing/selftests/kvm/aarch64/page_fault_test.c index eb4217b7c768..08a5ca5bed56 100644 --- a/tools/testing/selftests/kvm/aarch64/page_fault_test.c +++ b/tools/testing/selftests/kvm/aarch64/page_fault_test.c @@ -705,7 +705,7 @@ static void run_test(enum vm_guest_mode mode, void *arg) print_test_banner(mode, p); - vm = vm_create(mode); + vm = vm_create(VM_SHAPE(mode)); setup_memslots(vm, p); kvm_vm_elf_load(vm, program_invocation_name); setup_ucall(vm);
Re: [PATCH 27/34] KVM: selftests: Introduce VM "shape" to allow tests to specify the VM type
This commit breaks the arm64 selftests build btw: looks like a simple oversight?
$ cd ${LINUX_ROOT}/tools/testing/selftests/kvm
$ CROSS_COMPILE=aarch64-linux-gnu- ARCH=arm64 make
# ...
aarch64/page_fault_test.c: In function ‘run_test’:
aarch64/page_fault_test.c:708:28: error: incompatible type for
argument 1 of ‘vm_create’
708 | vm = vm_create(mode);
|^~~~
||
|enum vm_guest_mode
In file included from include/kvm_util.h:10,
from aarch64/page_fault_test.c:14:
include/kvm_util_base.h:806:46: note: expected ‘struct vm_shape’ but
argument is of type ‘enum vm_guest_mode’
806 | struct kvm_vm *vm_create(struct vm_shape shape);
Re: [PATCH 27/34] KVM: selftests: Introduce VM "shape" to allow tests to specify the VM type
On Mon, Nov 6, 2023 at 4:04 PM Sean Christopherson wrote: > > On Mon, Nov 06, 2023, Fuad Tabba wrote: > > On Sun, Nov 5, 2023 at 4:34 PM Paolo Bonzini wrote: > > > > > > From: Sean Christopherson > > > > > > 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 > > > Message-Id: <20231027182217.3615211-30-sea...@google.com> > > > Signed-off-by: Paolo Bonzini > > > --- > > > > 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? > > Hmm, no, "private" is mostly appropriate here. At this point in time, only > x86 > supports KVM_CREATE_GUEST_MEMFD, and x86 only supports it for private memory. > And the purpose of letting x86 selftests specify KVM_X86_SW_PROTECTED_VM, i.e. > the reason this patch exists, is purely to get private memory. > > Maybe tweak the second paragraph to this? > > Taking the VM type will allow adding tests for KVM_CREATE_GUEST_MEMFD > without needing an entirely separate set of helpers. At this time, > guest_memfd is effectively usable only by confidential VM types in the > form of guest private memory, and it's expected that x86 will double down > and require unique VM types for TDX and SNP guests. sgtm /fuad
Re: [PATCH 27/34] KVM: selftests: Introduce VM "shape" to allow tests to specify the VM type
On Mon, Nov 06, 2023, Fuad Tabba wrote: > On Sun, Nov 5, 2023 at 4:34 PM Paolo Bonzini wrote: > > > > From: Sean Christopherson > > > > 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 > > Message-Id: <20231027182217.3615211-30-sea...@google.com> > > Signed-off-by: Paolo Bonzini > > --- > > 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? Hmm, no, "private" is mostly appropriate here. At this point in time, only x86 supports KVM_CREATE_GUEST_MEMFD, and x86 only supports it for private memory. And the purpose of letting x86 selftests specify KVM_X86_SW_PROTECTED_VM, i.e. the reason this patch exists, is purely to get private memory. Maybe tweak the second paragraph to this? Taking the VM type will allow adding tests for KVM_CREATE_GUEST_MEMFD without needing an entirely separate set of helpers. At this time, guest_memfd is effectively usable only by confidential VM types in the form of guest private memory, and it's expected that x86 will double down and require unique VM types for TDX and SNP guests.
Re: [PATCH 27/34] KVM: selftests: Introduce VM "shape" to allow tests to specify the VM type
On Sun, Nov 5, 2023 at 4:34 PM Paolo Bonzini wrote:
>
> From: Sean Christopherson
>
> 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
> Message-Id: <20231027182217.3615211-30-sea...@google.com>
> Signed-off-by: Paolo Bonzini
> ---
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
Tested-by: Fuad Tabba
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_DEFAULT0
> +
> +#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
