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

[Nikolay Borisov]

On 11.11.24 г. 12: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).

nit: Is it safe to assume that null CMRs are going to be sequential and 
always at the end? Nothing in the TDX module spec suggests this. I.e 
can't we have :


1. Valid CMR region
2. ZERO CMR
3. Valid CMR

Sure, it might be a dummy and pointless but nothing prevents such CMR 
records. In any case I think the mentioning of "tail" is a bit too much 
detail and adds to unnecessary mental overload. Simply say you trim 
empty CMR's and that such regions will be sequential (if that's the 
case) and be done with it.

Because having "tail null cmr" can be interpreted as also having  there 
might be "non-tail null CMR", which doesn't seem to be the case?

> 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.
>
> Link: https://cdrdv2.intel.com/v1/dl/getContent/733575 [1]
> Link: https://cdrdv2.intel.com/v1/dl/getContent/733579 [2]
> Signed-off-by: Kai Huang <kai.huang@xxxxxxxxx>
> Reviewed-by: Dan Williams <dan.j.williams@xxxxxxxxx>

Reviewed-by: Nikolay Borisov <nik.borisov@xxxxxxxx>