[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d759c22d1360a56aa43796700a7fa3a9106d9fa9.camel@intel.com>
Date: Thu, 29 Jun 2023 09:15:39 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "peterz@...radead.org" <peterz@...radead.org>
CC: "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"Raj, Ashok" <ashok.raj@...el.com>,
"Hansen, Dave" <dave.hansen@...el.com>,
"david@...hat.com" <david@...hat.com>,
"bagasdotme@...il.com" <bagasdotme@...il.com>,
"Luck, Tony" <tony.luck@...el.com>,
"ak@...ux.intel.com" <ak@...ux.intel.com>,
"Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Christopherson,, Sean" <seanjc@...gle.com>,
"mingo@...hat.com" <mingo@...hat.com>,
"pbonzini@...hat.com" <pbonzini@...hat.com>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
"Chatre, Reinette" <reinette.chatre@...el.com>,
"Yamahata, Isaku" <isaku.yamahata@...el.com>,
"nik.borisov@...e.com" <nik.borisov@...e.com>,
"hpa@...or.com" <hpa@...or.com>, "Shahar, Sagi" <sagis@...gle.com>,
"imammedo@...hat.com" <imammedo@...hat.com>,
"bp@...en8.de" <bp@...en8.de>, "Gao, Chao" <chao.gao@...el.com>,
"Brown, Len" <len.brown@...el.com>,
"sathyanarayanan.kuppuswamy@...ux.intel.com"
<sathyanarayanan.kuppuswamy@...ux.intel.com>,
"Huang, Ying" <ying.huang@...el.com>,
"Williams, Dan J" <dan.j.williams@...el.com>,
"x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH v12 08/22] x86/virt/tdx: Get information about TDX module
and TDX-capable memory
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
Powered by blists - more mailing lists