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
