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

[Huang, Kai]

On Wed, 2023-06-28 at 16:10 +0200, Peter Zijlstra wrote:
> On Tue, Jun 27, 2023 at 02:12:38AM +1200, Kai Huang wrote:
> > +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: attributes 0x%x, vendor_id 0x%x, major_version %u, minor_version %u, build_date %u, build_num %u",
> > +		sysinfo->attributes,	sysinfo->vendor_id,
> > +		sysinfo->major_version, sysinfo->minor_version,
> > +		sysinfo->build_date,	sysinfo->build_num);
> > +
> > +	/* R9 contains the actual entries written to the CMR array. */
> 
> So I'm vexed by this comment; it's either not enough or too much.
> 
> I mean, as given you assume we all know about the magic parameters to
> TDH_SYS_INFO but then somehow need an explanation for how %r9 is changed
> from the array size to the number of used entries.
> 
> Either describe the whole thing or none of it.
> 
> Me, I would prefer all of it, because I've no idea where to begin
> looking for any of this, 
> 

Sure.  How about below?

+       /*
+        * TDH.SYS.INFO writes the TDSYSINFO_STRUCT and the CMR array
+        * to the buffers provided by the kernel (via RCX and R8
+        * respectively).  The buffer size of the TDSYSINFO_STRUCT
+        * (via RDX) and the maximum entries of the CMR array (via R9)
+        * passed to this SEAMCALL must be at least the size of
+        * TDSYSINFO_STRUCT and MAX_CMRS respectively.
+        *
+        * Upon a successful return, R9 contains the actual entries
+        * written to the CMR array.
+        */
        sysinfo_pa = __pa(sysinfo);
        cmr_array_pa = __pa(cmr_array);
        ret = seamcall(TDH_SYS_INFO, sysinfo_pa, TDSYSINFO_STRUCT_SIZE,
@@ -228,7 +239,6 @@ static int tdx_get_sysinfo(struct tdsysinfo_struct *sysinfo,
                sysinfo->major_version, sysinfo->minor_version,
                sysinfo->build_date,    sysinfo->build_num);
 
-       /* R9 contains the actual entries written to the CMR array. */
        print_cmrs(cmr_array, out.r9);

Or should I just repeat the spec like below?

+       /*
+        * TDH.SYS.INFO writes the TDSYSINFO_STRUCT and the CMR array
+        * to the buffers provided by the kernel:
+        *
+        * Input:
+        *  - RCX: The buffer of TDSYSINFO_STRUCT
+        *  - RDX: The size of the TDSYSINFO_STRUCT buffer, must be at
+        *         at least the size of TDSYSINFO_STRUCT
+        *  - R8: The buffer of the CMR array
+        *  - R9: The entry number of the array, must be at least
+        *        MAX_CMRS.
+        *
+        * Output (successful):
+        *  - RDX: The actual bytes written to the TDSYSINFO_STRUCT
+        *         buffer
+        *  - R9: The actual entries written to the CMR array.
+        */
        sysinfo_pa = __pa(sysinfo);
        cmr_array_pa = __pa(cmr_array);
        ret = seamcall(TDH_SYS_INFO, sysinfo_pa, TDSYSINFO_STRUCT_SIZE,
@@ -228,7 +245,6 @@ static int tdx_get_sysinfo(struct tdsysinfo_struct *sysinfo,
                sysinfo->major_version, sysinfo->minor_version,
                sysinfo->build_date,    sysinfo->build_num);
 
-       /* R9 contains the actual entries written to the CMR array. */
        print_cmrs(cmr_array, out.r9);

> SDM doesn't seem to be the place. That doesn't
> even list TDCALL/SEAMCALL in Volume 2 :-( Let alone describe the magic
> values.
> 

TDX has it's own specs at here:

https://www.intel.com/content/www/us/en/developer/articles/technical/intel-trust-domain-extensions.html

For this one you can find it in here:

https://cdrdv2.intel.com/v1/dl/getContent/733568