Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 31 May 2023 13:30:12 -0700
From:   Isaku Yamahata <isaku.yamahata@gmail.com>
To:     Zhi Wang <zhi.wang.linux@gmail.com>
Cc:     isaku.yamahata@intel.com, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, isaku.yamahata@gmail.com,
        Paolo Bonzini <pbonzini@redhat.com>, erdemaktas@google.com,
        Sean Christopherson <seanjc@google.com>,
        Sagi Shahar <sagis@google.com>,
        David Matlack <dmatlack@google.com>,
        Kai Huang <kai.huang@intel.com>, chen.bo@intel.com
Subject: Re: [PATCH v14 003/113] KVM: x86/vmx: Refactor KVM VMX module
 init/exit functions
Message-ID: <20230531203012.GG1234772@ls.amr.corp.intel.com>
References: <cover.1685333727.git.isaku.yamahata@intel.com>
 <4ef61085333e97e0ae48c3d7603042b9801e3608.1685333727.git.isaku.yamahata@intel.com>
 <20230531093950.00007e7d.zhi.wang.linux@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20230531093950.00007e7d.zhi.wang.linux@gmail.com>
Precedence: bulk

On Wed, May 31, 2023 at 09:57:18AM +0800,
Zhi Wang <zhi.wang.linux@gmail.com> 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.
-- 
Isaku Yamahata <isaku.yamahata@gmail.com>