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

[Huang, Kai]

> > +static inline bool is_cmr_empty(struct cmr_info *cmr)
> > +{
> > +	return !cmr->size;
> > +}
> > +
> 
> Nit: maybe it's just me, but this function seems unnecessary.
> 
> If "!cmr->size" is not expressive, then I don't know why "is_cmr_empty" 
> should be. Just inline that into the single user.
> 
> .. after all the single caller also uses/prints cmr->size ...

Agreed.  I'll remove this function.  Thanks!

> 
> > +static void print_cmrs(struct cmr_info *cmr_array, int nr_cmrs)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < nr_cmrs; i++) {
> > +		struct cmr_info *cmr = &cmr_array[i];
> > +
> > +		/*
> > +		 * The array of CMRs reported via TDH.SYS.INFO can
> > +		 * contain tail empty CMRs.  Don't print them.
> > +		 */
> > +		if (is_cmr_empty(cmr))
> > +			break;
> > +
> > +		pr_info("CMR: [0x%llx, 0x%llx)\n", cmr->base,
> > +				cmr->base + cmr->size);
> > +	}
> > +}
> > +
> > +/*
> > + * Get the TDX module information (TDSYSINFO_STRUCT) and the array of
> > + * CMRs, and save them to @sysinfo and @cmr_array.  @sysinfo must have
> > + * been padded to have enough room to save the TDSYSINFO_STRUCT.
> > + */
> > +static int tdx_get_sysinfo(struct tdsysinfo_struct *sysinfo,
> > +			   struct cmr_info *cmr_array)
> > +{
> > +	struct tdx_module_output out;
> > +	u64 sysinfo_pa, cmr_array_pa;
> > +	int ret;
> > +
> > +	sysinfo_pa = __pa(sysinfo);
> > +	cmr_array_pa = __pa(cmr_array);
> > +	ret = seamcall(TDH_SYS_INFO, sysinfo_pa, TDSYSINFO_STRUCT_SIZE,
> > +			cmr_array_pa, MAX_CMRS, NULL, &out);
> > +	if (ret)
> > +		return ret;
> > +
> > +	pr_info("TDX module: atributes 0x%x, vendor_id 0x%x, major_version %u, minor_version %u, build_date %u, build_num %u",
> 
> 
> "attributes" ?

Appreciate! :)

[...]


> > +#define TDSYSINFO_STRUCT_SIZE		1024
> 
> So, it can never be larger than 1024 bytes? Not even with many cpuid 
> configs?

Correct.  The TDX module spec(s) says: 

	TDSYSINFO_STRUCT’s size is 1024B.

Which is an architectural sentence to me.

We (Intel) already published TDX IO, and TDSYSINFO_STRUCT is 1024B for all TDX
module versions.
>