Re: [PATCH v7 07/10] x86/virt/tdx: Trim away tail null CMRs

["Huang, Kai" <kai.huang@intel.com>]

On Mon, 2024-11-11 at 11:41 -0800, Dave Hansen wrote:
> On 11/11/24 02:39, Kai Huang wrote:
> > TDX architecturally supports up to 32 CMRs.  The global metadata field
> > "NUM_CMRS" reports the number of CMR entries that can be read by the
> > kernel.  However, that field may just report the maximum number of CMRs
> > albeit the actual number of CMRs is smaller, in which case there are
> > tail null CMRs (size is 0).
> > 
> > Trim away those null CMRs, and print valid CMRs since they are useful
> > at least to developers.
> > 
> > More information about CMR can be found at "Intel TDX ISA Background:
> > Convertible Memory Ranges (CMRs)" in TDX 1.5 base spec [1], and
> > "CMR_INFO" in TDX 1.5 ABI spec [2].
> > 
> > Now get_tdx_sys_info() just reads kernel-needed global metadata to
> > kernel structure, and it is auto-generated.  Add a wrapper function
> > init_tdx_sys_info() to invoke get_tdx_sys_info() and provide room to do
> > additional things like dealing with CMRs.
> 
> I'm not sure I understand why this patch is here.
> 
> What does trimming buy us in the first place?

I think the global metadata provided by the core kernel should only reflect the
real valid CMRs via 'num_cmrs', so when the kernel uses CMRs it can just get
valid ones.

One immediate need is the next patch will need to loop over CMRs to set up
reserved areas for TDMRs.  If we don't trim here, we will need to explicitly
skip all null CMRs in each loop.  This will result in more duplicated code and
is not as clean as trimming at early IMO.

I should call this out here though.  

How about I clarify this in the changelog like below?

"
A future fix to a module initialization failure issue will need to loop over all
CMRs.  Trim away those null CMRs once for all here so that the kernel doesn't
need to explicitly skip them each time when it uses CMRs in later time.  Also
print valid CMRs since they are useful at least to developers.
"