Re: [PATCH v14 003/113] KVM: x86/vmx: Refactor KVM VMX module init/exit functions

[Isaku Yamahata]

On Wed, May 31, 2023 at 01:30:12PM -0700,
Isaku Yamahata <isaku.yamahata@xxxxxxxxx> wrote:

> On Wed, May 31, 2023 at 09:57:18AM +0800,
> Zhi Wang <zhi.wang.linux@xxxxxxxxx> wrote:
> 
> > > +static int __init vt_init(void)
> > > +{
> > > +	unsigned int vcpu_size, vcpu_align;
> > > +	int r;
> > > +
> > > +	if (!kvm_is_vmx_supported())
> > > +		return -EOPNOTSUPP;
> > > +
> > > +	/*
> > > +	 * Note, hv_init_evmcs() touches only VMX knobs, i.e. there's nothing
> > > +	 * to unwind if a later step fails.
> > > +	 */
> > > +	hv_init_evmcs();
> > > +
> > > +	r = kvm_x86_vendor_init(&vt_init_ops);
> > > +	if (r)
> > > +		return r;
> > > +
> > > +	r = vmx_init();
> > > +	if (r)
> > > +		goto err_vmx_init;
> > > +
> > > +	/*
> > > +	 * Common KVM initialization _must_ come last, after this, /dev/kvm is
> > > +	 * exposed to userspace!
> > > +	 */
> > > +	vcpu_size = sizeof(struct vcpu_vmx);
> > > +	vcpu_align = __alignof__(struct vcpu_vmx);
> > > +	r = kvm_init(vcpu_size, vcpu_align, THIS_MODULE);
> > > +	if (r)
> > > +		goto err_kvm_init;
> > > +
> > > +	return 0;
> > > +
> > ---------------------------------
> > > +err_kvm_init:
> > > +	vmx_exit();
> > > +err_vmx_init:
> > > +	kvm_x86_vendor_exit();
> > > +	return r;
> > > +}
> > > +module_init(vt_init);
> > > +
> > ----------------------------------
> > > +static void vt_exit(void)
> > > +{
> > > +	kvm_exit();
> > > +	kvm_x86_vendor_exit();
> > > +	vmx_exit();
> > ----------------------------------
> > 
> > It seems the exiting sequences above are a little bit different with
> > each other (PS: It is not a prob introduced in this patch):
> > 
> > vmx_exit()
> > kvm_x86_vendor_exit()
> > ....
> > 
> > and
> > 
> > ...
> > kvm_x86_vnedor_exit()
> > vmx_exit()
> > 
> > I was wondering which one should be correct. Literally, the exiting
> > sequence would be in reversing order of the initialization sequence.
> 
> In theory, I think kvm_x86_vendor_exit() => vmx_exit() and
> vmx_init() => kvm_x86_vendor_exit() should be the right order.
> But in practice, it doesn't matter and I didn't want to touch the order with
> this patch series.

Here is the patch for it. But I'd like to hold the patch after this patch
series.