Re: [PATCH v6 18/21] x86/virt/tdx: Configure global KeyID on all packages

[Huang, Kai]

Thanks for the review!

> >   
> > +/*
> > + * Call one SEAMCALL on one (any) cpu for each physical package in
> > + * serialized way.  Return immediately in case of any error if
> > + * SEAMCALL fails on any cpu.
> 
> 
> It's not clear what are you serializing against (against itself or other 
> calls of this functions)
> 
> I assume its because the TDX module errors out for parallel calls 
> instead of waiting.

Yes.

> 
> The code seems to only do itself, so where is the check against others? 
> I assume in the callers but that would need to be explained. 
> 

Yes in the callers.  In short there's no need, or it doesn't make sense to check
against others as SEAMCALLs involved during the module initialization are not
supposed can be made in random.

> Also could 
> it need serialization against other kinds of seam calls?


The TDX module initialization is essentially a state machine -- it involves a
couple of steps to finish the process and each step moves the TDX module's
current state to a later state.

Each step involves a different SEAMCALL, but this SEAMCALL may be only called
one (any) cpu, or needs to be called for all cpus, or one (any) cpu for each
package.

The TDX module initialization code do the whole process step by step, so the
caller guarantees no random SEAMCALLs will be made when it is doing one step of
the initialization.

> 
> Perhaps it might be more efficient to just broad cast and handle a retry 
> with some synchronization in the low level code.
> 
> That likely would cause less review thrash than just reimplementing a 
> common function like this here.

As mentioned above the whole initialization process is just a machine state, so
not all SEAMCALLs are subject to the logic of retry.  To me the retry only makes
sense for one particular SEAMCALL which must be done on multiple cpus but
requires some serialization.

Does all above make sense?