linux-kernel - Re: [PATCHv5 04/30] x86/tdx: Extend the confidential computing API to support TDX guests

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]

Date:   Tue, 8 Mar 2022 12:17:01 -0800
From:   Dave Hansen <dave.hansen@...el.com>
To:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        luto@...nel.org, peterz@...radead.org
Cc:     sathyanarayanan.kuppuswamy@...ux.intel.com, aarcange@...hat.com,
        ak@...ux.intel.com, dan.j.williams@...el.com, david@...hat.com,
        hpa@...or.com, jgross@...e.com, jmattson@...gle.com,
        joro@...tes.org, jpoimboe@...hat.com, knsathya@...nel.org,
        pbonzini@...hat.com, sdeep@...are.com, seanjc@...gle.com,
        tony.luck@...el.com, vkuznets@...hat.com, wanpengli@...cent.com,
        thomas.lendacky@....com, brijesh.singh@....com, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCHv5 04/30] x86/tdx: Extend the confidential computing API to
 support TDX guests

On 3/2/22 06:27, Kirill A. Shutemov wrote:
...
> Like AMD SME/SEV, TDX uses a bit in the page table entry to indicate
> encryption status of the page, but the polarity of the mask is
> opposite to AMD: if the bit is set the page is accessible to VMM.

I'd much rather this be in a code comment next to the weird-looking code
than in the changelog.

> Details about which bit in the page table entry to be used to indicate
> shared/private state can be determined by using the TDINFO TDCALL.

s/can be/are/

> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> ---
>  arch/x86/Kconfig     |  1 +
>  arch/x86/coco/core.c |  4 ++++
>  arch/x86/coco/tdx.c  | 38 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 43 insertions(+)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index c346d66b51fc..93e67842e369 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -884,6 +884,7 @@ config INTEL_TDX_GUEST
>  	bool "Intel TDX (Trust Domain Extensions) - Guest Support"
>  	depends on X86_64 && CPU_SUP_INTEL
>  	depends on X86_X2APIC
> +	select ARCH_HAS_CC_PLATFORM
>  	help
>  	  Support running as a guest under Intel TDX.  Without this support,
>  	  the guest kernel can not boot or run under TDX.
> diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
> index fc1365dd927e..9113baebbfd2 100644
> --- a/arch/x86/coco/core.c
> +++ b/arch/x86/coco/core.c
> @@ -90,6 +90,8 @@ u64 cc_mkenc(u64 val)
>  	switch (vendor) {
>  	case CC_VENDOR_AMD:
>  		return val | cc_mask;
> +	case CC_VENDOR_INTEL:
> +		return val & ~cc_mask;
>  	default:
>  		return val;
>  	}
> @@ -100,6 +102,8 @@ u64 cc_mkdec(u64 val)
>  	switch (vendor) {
>  	case CC_VENDOR_AMD:
>  		return val & ~cc_mask;
> +	case CC_VENDOR_INTEL:
> +		return val | cc_mask;
>  	default:
>  		return val;
>  	}
> diff --git a/arch/x86/coco/tdx.c b/arch/x86/coco/tdx.c
> index 17365fd40ba2..912ef12e434e 100644
> --- a/arch/x86/coco/tdx.c
> +++ b/arch/x86/coco/tdx.c
> @@ -5,8 +5,12 @@
>  #define pr_fmt(fmt)     "tdx: " fmt
>  
>  #include <linux/cpufeature.h>
> +#include <asm/coco.h>
>  #include <asm/tdx.h>
>  
> +/* TDX module Call Leaf IDs */
> +#define TDX_GET_INFO			1
> +
>  /*
>   * Wrapper for standard use of __tdx_hypercall with no output aside from
>   * return code.
> @@ -25,8 +29,32 @@ static inline u64 _tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14, u64 r15)
>  	return __tdx_hypercall(&args, 0);
>  }
>  
> +static inline void tdx_module_call(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
> +				   struct tdx_module_output *out)
> +{
> +	if (__tdx_module_call(fn, rcx, rdx, r8, r9, out))
> +		panic("TDCALL %lld failed (Buggy TDX module!)\n", fn);
> +}

I really think we need to document the panic()s that we add.  It might
mean duplicating a wee bit of the text from the SEAMCALL/TDCALL
assembly, but I think it's worth it so that folks don't think this is an
over-eager panic().