linux-kernel - Re: [PATCH v3 1/4] x86/tdx: Add tdx_mcall

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ce0c1d0b-b60f-79e1-b602-450def91ae77@linux.intel.com>
Date:   Mon, 18 Apr 2022 20:37:25 -0700
From:   Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
To:     Kai Huang <kai.huang@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        Hans de Goede <hdegoede@...hat.com>,
        Mark Gross <mgross@...ux.intel.com>
Cc:     "H . Peter Anvin" <hpa@...or.com>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Tony Luck <tony.luck@...el.com>,
        Andi Kleen <ak@...ux.intel.com>, linux-kernel@...r.kernel.org,
        platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v3 1/4] x86/tdx: Add tdx_mcall_tdreport() API support



On 4/18/22 7:29 PM, Kai Huang wrote:
> On Fri, 2022-04-15 at 15:01 -0700, Kuppuswamy Sathyanarayanan wrote:
>> In TDX guest, attestation is mainly used to verify the trustworthiness
>> of a TD to the 3rd party key servers.
>>
> 
> "key servers" is only a use case of using the attestation service. This sentence
> looks not accurate.

I thought it is mainly used for this use case. If it is not accurate,
how about following?

Attestation is used to verify the trustworthiness of a TD to the other
3rd party entities (like key servers) before exchanging sensitive
information.

> 
>> First step in attestation process
>> is to get the TDREPORT data and the generated data is further used in
>> subsequent steps of the attestation process. TDREPORT data contains
>> details like TDX module version information, measurement of the TD,
>> along with a TD-specified nonce
>>
>> Add a wrapper function (tdx_mcall_tdreport()) to get the TDREPORT from
>> the TDX Module.
>>
>> More details about the TDREPORT TDCALL can be found in TDX Guest-Host
>> Communication Interface (GHCI) for Intel TDX 1.5, section titled
>> "TDCALL [MR.REPORT]".
> 
> Attestation is a must for TDX architecture, so The TDCALL[MR.REPORT] is
> available in TDX 1.0.  I don't think we should use TDX 1.5 here.  And this

Yes. It is also part of v1.0. Since the feature is similar between v1.0
and v1.5, I have included one link. If v1.0 reference is preferred, I
will update it.

> TDCALL is defined in the TDX module spec 1.0.  You can find it in the public TDX
> module 1.0 spec (22.3.3. TDG.MR.REPORT Leaf):

It looks like in the latest update, they have moved this section from
ABI spec. I will update the specification reference.


> 
> https://www.intel.com/content/dam/develop/external/us/en/documents/tdx-module-1.0-public-spec-v0.931.pdf
> 
>>
>> Steps involved in attestation process can be found in TDX Guest-Host
>> Communication Interface (GHCI) for Intel TDX 1.5, section titled
>> "TD attestation"
> 
> It's strange we need to use GHCI for TDX 1.5 to get some idea about the
> attestation process.  Looking at the GHCI 1.0 spec, it seems it already has one
> section to talk about attestation process (5.4 TD attestation).

Both are same. I will change it to 1.0 reference.

> 
>>
>> This API will be mainly used by the attestation driver. Attestation
>> driver support will be added by upcoming patches.
>>
>> Reviewed-by: Tony Luck <tony.luck@...el.com>
>> Reviewed-by: Andi Kleen <ak@...ux.intel.com>
>> Acked-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
>> ---
>>   arch/x86/coco/tdx/tdx.c    | 46 ++++++++++++++++++++++++++++++++++++++
>>   arch/x86/include/asm/tdx.h |  2 ++
>>   2 files changed, 48 insertions(+)
>>
>> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
>> index 03deb4d6920d..3e409b618d3f 100644
>> --- a/arch/x86/coco/tdx/tdx.c
>> +++ b/arch/x86/coco/tdx/tdx.c
>> @@ -11,10 +11,12 @@
>>   #include <asm/insn.h>
>>   #include <asm/insn-eval.h>
>>   #include <asm/pgtable.h>
>> +#include <asm/io.h>
>>   
>>   /* TDX module Call Leaf IDs */
>>   #define TDX_GET_INFO			1
>>   #define TDX_GET_VEINFO			3
>> +#define TDX_GET_REPORT			4
>>   #define TDX_ACCEPT_PAGE			6
>>   
>>   /* TDX hypercall Leaf IDs */
>> @@ -34,6 +36,10 @@
>>   #define VE_GET_PORT_NUM(e)	((e) >> 16)
>>   #define VE_IS_IO_STRING(e)	((e) & BIT(4))
>>   
>> +/* TDX Module call error codes */
>> +#define TDCALL_RETURN_CODE_MASK		0xffffffff00000000
>> +#define TDCALL_RETURN_CODE(a)		((a) & TDCALL_RETURN_CODE_MASK)
>> +
>>   /*
>>    * Wrapper for standard use of __tdx_hypercall with no output aside from
>>    * return code.
>> @@ -98,6 +104,46 @@ static inline void tdx_module_call(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
>>   		panic("TDCALL %lld failed (Buggy TDX module!)\n", fn);
>>   }
>>   
>> +/*
>> + * tdx_mcall_tdreport() - Generate TDREPORT_STRUCT using TDCALL.
>> + *
>> + * @data        : Address of 1024B aligned data to store
>> + *                TDREPORT_STRUCT.
>> + * @reportdata  : Address of 64B aligned report data
>> + *
>> + * return 0 on success or failure error number.
>> + */
>> +long tdx_mcall_tdreport(void *data, void *reportdata)
>> +{
>> +	u64 ret;
>> +
>> +	/*
>> +	 * Check for a valid TDX guest to ensure this API is only
>> +	 * used by TDX guest platform. Also make sure "data" and
>> +	 * "reportdata" pointers are valid.
>> +	 */
>> +	if (!data || !reportdata || !cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
>> +		return -EINVAL;
> 
> Do we need to manually check the alignment since it is mentioned in the comment
> of this function?

Users are responsible to allocate aligned data. I don't think we need
to add a check for it. If it is unaligned, TDCALL will return error.

> 
>> +
>> +	/*
>> +	 * Pass the physical address of user generated reportdata
>> +	 * and the physical address of out pointer to store the
>> +	 * TDREPORT data to the TDX module to generate the
>> +	 * TD report. Generated data contains measurements/configuration
>> +	 * data of the TD guest. More info about ABI can be found in TDX
>> +	 * Guest-Host-Communication Interface (GHCI), sec titled
>> +	 * "TDG.MR.REPORT".
> 
> If you agree with my above comments, then this comment should be updated too:
> TDG.MR.REPORT is defined in TDX module 1.0 spec.

Ok. will change it.



-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer