Re: [RFC PATCH 1/4] KVM: selftests: Add support for creating non-default type VMs

[Xiaoyao Li]

On 8/4/2021 10:24 PM, Maxim Levitsky wrote:
> On Wed, 2021-08-04 at 14:09 +0800, Xiaoyao Li wrote:
> > On 7/27/2021 2:37 AM, Erdem Aktas wrote:
> > > Currently vm_create function only creates KVM_X86_LEGACY_VM type VMs.
> > > Changing the vm_create function to accept type parameter to create
> > > new VM types.
> > >
> > > Signed-off-by: Erdem Aktas <erdemaktas@xxxxxxxxxx>
> > > Reviewed-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > > Reviewed-by: Peter Gonda <pgonda@xxxxxxxxxx>
> > > Reviewed-by: Marc Orr <marcorr@xxxxxxxxxx>
> > > Reviewed-by: Sagi Shahar <sagis@xxxxxxxxxx>
> > > ---
> > >    .../testing/selftests/kvm/include/kvm_util.h  |  1 +
> > >    tools/testing/selftests/kvm/lib/kvm_util.c    | 29 +++++++++++++++++--
> > >    2 files changed, 27 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > > index d53bfadd2..c63df42d6 100644
> > > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > > @@ -88,6 +88,7 @@ int vcpu_enable_cap(struct kvm_vm *vm, uint32_t vcpu_id,
> > >    void vm_enable_dirty_ring(struct kvm_vm *vm, uint32_t ring_size);
> > >    
> > >    struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm);
> > > +struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm, int type);
> > >    void kvm_vm_free(struct kvm_vm *vmp);
> > >    void kvm_vm_restart(struct kvm_vm *vmp, int perm);
> > >    void kvm_vm_release(struct kvm_vm *vmp);
> > > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> > > index e5fbf16f7..70caa3882 100644
> > > --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> > > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> > > @@ -180,13 +180,36 @@ _Static_assert(sizeof(vm_guest_mode_params)/sizeof(struct vm_guest_mode_params)
> > >     * Return:
> > >     *   Pointer to opaque structure that describes the created VM.
> > >     *
> > > - * Creates a VM with the mode specified by mode (e.g. VM_MODE_P52V48_4K).
> > > + * Wrapper VM Create function to create a VM with default type (0).
> >
> > Can we pass KVM_X86_LEGACY_VM (whatever name when it's upstreamed)
> > instead of 0?
>
> To be honest I would prefer this to be called something like KVM_X86_STANDARD_VM,
> or something.
>
> I don't think that normal unencrypted virtualization is already legacy, even if TDX
> docs claim that.

I'm not proposing to use this specific name introduced in TDX RFC 
series, but proposing to use the name defined in KVM in the future 
instead of hard-coded 0.

Yes, KVM_X86_STANDARD_VM or KVM_X86_NORMAL_VM (proposed by Paolo) is 
better than KVM_X86_LEGACY_VM.

> Just my personal opinion.
>
> Best regards,
> 	Maxim Levitsky
>
> > > + */
> > > +struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
> > > +{
> > > +	return __vm_create(mode, phy_pages, perm, 0);
> > > +}
> > > +