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

[Isaku Yamahata]

On Mon, Jun 12, 2023 at 11:55:14PM +0000,
"Huang, Kai" <kai.huang@xxxxxxxxx> wrote:

> On Wed, 2023-06-07 at 11:06 -0700, Isaku Yamahata wrote:
> > Thanks for pointing it out. The following is the fix.
> > 
> > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > index 8a1d0755d275..b0d3f646afb1 100644
> > --- a/arch/x86/kvm/vmx/tdx.c
> > +++ b/arch/x86/kvm/vmx/tdx.c
> > @@ -4499,26 +4499,39 @@ u64 tdx_non_arch_field_switch(u64 field)
> >  	}
> >  }
> >  
> > -static void __init vmx_tdx_on(void *info)
> > +struct vmx_tdx_enabled {
> > +	cpumask_var_t vmx_enabled;
> > +	atomic_t *err;
> > +};
> > +
> 
> Sorry for late reply.
> 
> I think you just need to mimic hardware_enable_all() -- using a per-cpu
> variable.  In this way you can get rid of this structure.
> 
> But again, we have listed a couple of options in the v13 discussion [1]:
> 
> 1) Call kvm_ops_update() twice before and after hardware_setup() in order to use
> hardware_enable_all() directly.
> 
> 2) Expose kvm_x86_ops as symbol so VMX can set hardware_{enable|disable}()
> callback before hardware_setup() in order to use hardware_enable_all().
> 
> 3) Implement VMX's own hardware_enable_all() logic as shown in this patch.
> 
> 4) ???
> 
> I think it would be better if Sean can provide some comments here, but until he
> does, we can keep using option 3) (this patch).
> 
> [1]
> https://lore.kernel.org/lkml/5dc84a2601a47ccc29ef43200cf3ec0d1b485d23.camel@xxxxxxxxx/

Ok, makes sense. Here is the updated version with the fix for the error you
pointed out.  Introduce cpu bitmap to track which cpu enable VMX(VMXON)
successfully.  Disable VMX off only for cpu with bit set.