Re: [PATCH v7 09/20] x86/virt/tdx: Get information about TDX module and TDX-capable memory

[Huang, Kai]

On Wed, 2022-11-23 at 22:53 +0000, Huang, Kai wrote:
> > > > > > > > > +
> > > > > > > > > +   /*
> > > > > > > > > +    * trim_empty_cmrs() updates the actual number of CMRs by
> > > > > > > > > +    * dropping all tail empty CMRs.
> > > > > > > > > +    */
> > > > > > > > > +   return trim_empty_cmrs(tdx_cmr_array, &tdx_cmr_num);
> > > > > > > > > +}
> > > > > > > 
> > > > > > > Why does this both need to respect the "tdx_cmr_num = out.r9"
> > > > > > > value
> > > > > > > *and* trim the empty ones?  Couldn't it just ignore the
> > > > > > > "tdx_cmr_num =
> > > > > > > out.r9" value and just trim the empty ones either way?  It's not
> > > > > > > like
> > > > > > > there is a billion of them.  It would simplify the code for sure.
> > > > > 
> > > > > OK.  Since spec says MAX_CMRs is 32, so I can use 32 instead of
> > > > > reading out from
> > > > > R9.
> > > 
> > > But then you still have the "trimming" code.  Why not just trust "r9"
> > > and then axe all the trimming code?  Heck, and most of the sanity checks.
> > > 
> > > This code could be a *lot* smaller.
> 
> As I said the only problem is there might be empty CMRs at the tail of the
> cmr_array[] following one or more valid CMRs.  

Hi Dave,

Probably I forgot to mention the "r9" in practice always returns 32, so there
will be empty CMRs at the tail of the cmr_array[].

> 
> But we can also do nothing here, but just skip empty CMRs when comparing the
> memory region to it (in next patch).
> 
> Or, we don't even need to explicitly check memory region against CMRs. If the
> memory regions that we provided in the TDMR doesn't fall into CMR, then
> TDH.SYS.CONFIG will fail. We can just depend on the SEAMCALL to do that.

Sorry to ping, but do you have any comments here?

How about we just don't do any check of TDX memory regions against CMRs, but 
just let the TDH.SYS.CONFIG SEAMCALL to determine?