Re: [PATCH v14 07/23] x86/virt/tdx: Add skeleton to enable TDX on demand

[Huang, Kai]

On Wed, 2023-10-18 at 11:39 +0300, Nikolay Borisov wrote:
> 
> On 18.10.23 г. 11:29 ч., Huang, Kai wrote:
> >   
> > > > +/*
> > > > + * Do the module global initialization once and return its result.
> > > > + * It can be done on any cpu.  It's always called with interrupts
> > > > + * disabled.
> > > > + */
> > > > +static int try_init_module_global(void)
> > > > +{
> > > 
> > > Any particular reason why this function is not called from the tdx
> > > module's tdx_init? It's global and must be called once when the module
> > > is initialised. Subsequently kvm which is supposed to call
> > > tdx_cpu_enable() must be sequenced _after_ tdx which shouldn't be that
> > > hard, no? This will eliminate the spinlock as well.
> > > 
> > 
> > Do you mean early_initcall(tdx_init)?
> > 
> > Because it requires VMXON being done to do SEAMCALL.  For now only KVM does
> > VMXON.
> > 
> 
> Right, then would it not make more sense too have this code as part of 
> the KVM bringup of TDX. In fact by keeping the 2 series separate you 
> might be adding complexity. What is the initial motivation to keep those 
> patches out of the KVM tdx host series support?

A simple answer is, for now only KVM uses this series, but later other kernel
components such as IOMMU will need to use this series too, so the core-kernel is
the right place to fit.

Another practical reason is, although for now KVM is the only user, but the
effect of enabling TDX module is system wide, because once TDX module is
initialized, it stays there no matter KVM is still present or not (KVM can
enable TDX and then unloaded permanently).

For instance, kexec() and #MC handler needs additional handling when TDX gets
enabled.  If we completely maintain this series in KVM, then after KVM is
unloaded we will lose some essential information to do these system-wide things.