Re: [PATCH v13 003/113] KVM: TDX: Initialize the TDX module when loading the KVM intel kernel module

[Isaku Yamahata]

On Tue, Mar 14, 2023 at 02:38:06AM +0000,
"Huang, Kai" <kai.huang@xxxxxxxxx> wrote:

> On Sun, 2023-03-12 at 10:55 -0700, isaku.yamahata@xxxxxxxxx wrote:
> > +int __init tdx_hardware_setup(struct kvm_x86_ops *x86_ops)
> > +{
> > +	int r;
> > +
> > +	if (!enable_ept) {
> > +		pr_warn("Cannot enable TDX with EPT disabled\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* tdx_enable() in tdx_module_setup() requires cpus lock. */
> > +	cpus_read_lock();
> > +	/* TDX requires VMX. */
> > +	r = vmxon_all();
> 
> Why not using hardware_enable_all()?
> 
> > +	if (!r) {
> > +		int cpu;
> > +
> > +		/*
> > +		 * Because tdx_cpu_enabel() acquire spin locks, on_each_cpu()
> > +		 * can't be used.
> > +		 */
> > +		for_each_online_cpu(cpu) {
> > +			if (smp_call_on_cpu(cpu, tdx_cpu_enable_cpu, NULL, false))
> > +				r = -EIO;
> > +		}
> > +		if (!r)
> > +			r = tdx_module_setup();
> > +	}
> > +	vmxoff_all();
> > +	cpus_read_unlock();
> > +
> > +	return r;
> > +}
> 
> I think you should use hardware_enable_all(), and just do something similar to
> below in vmx_hardware_enable():

The use of hardware_enable_all() make us circle back to refactoring KVM
hardware initialization topic.  I'd like to stay away from it for now for TDX.
I find that vmxon_all() is unnecessary and we can move VMXON to
tdx_cpu_enable_cpu().
Here is the version of dropping vmxon_all().