linux-kernel - Re: [PATCH v3 03/12] x86/tdx: Make macros of TDCALLs consistent with the spec

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: <b9fbbbb7-f855-5744-d6ad-c650b10ce135@linux.intel.com>
Date:   Thu, 27 Jul 2023 18:54:59 -0700
From:   Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
To:     Kai Huang <kai.huang@...el.com>, peterz@...radead.org,
        kirill.shutemov@...ux.intel.com, linux-kernel@...r.kernel.org
Cc:     dave.hansen@...el.com, tglx@...utronix.de, bp@...en8.de,
        mingo@...hat.com, hpa@...or.com, x86@...nel.org, seanjc@...gle.com,
        pbonzini@...hat.com, isaku.yamahata@...el.com,
        n.borisov.lkml@...il.com
Subject: Re: [PATCH v3 03/12] x86/tdx: Make macros of TDCALLs consistent with
 the spec



On 7/26/23 4:25 AM, Kai Huang wrote:
> The TDX spec names all TDCALLs with prefix "TDG".  Currently, the kernel
> doesn't follow such convention for the macros of those TDCALLs but uses
> prefix "TDX_" for all of them.  Although it's arguable whether the TDX
> spec names those TDCALLs properly, it's better for the kernel to follow
> the spec when naming those macros.
> 
> Change all macros of TDCALLs to make them consistent with the spec.  As
> a bonus, they get distinguished easily from the host-side SEAMCALLs,
> which all have prefix "TDH".
> 
> No functional change intended.
> 

When upstreaming the TDX guest patches, there was a discussion about using
TDG vs TDX. Final agreement is to use TDX_ prefix. I think it makes sense
to align with the spec, but it is up to the maintainer.

What about the function name prefix? Are you planning to change them to tdg_*?

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>

> Signed-off-by: Kai Huang <kai.huang@...el.com>
> ---
> 
> v2 -> v3:
>  - No change.
> 
> v1 -> v2:
>  - Rebase to 6.5-rc2.
> 
> ---
>  arch/x86/coco/tdx/tdx-shared.c    |  4 ++--
>  arch/x86/coco/tdx/tdx.c           |  8 ++++----
>  arch/x86/include/asm/shared/tdx.h | 10 +++++-----
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/coco/tdx/tdx-shared.c b/arch/x86/coco/tdx/tdx-shared.c
> index ef20ddc37b58..f10cd3e4a04e 100644
> --- a/arch/x86/coco/tdx/tdx-shared.c
> +++ b/arch/x86/coco/tdx/tdx-shared.c
> @@ -35,7 +35,7 @@ static unsigned long try_accept_one(phys_addr_t start, unsigned long len,
>  	}
>  
>  	tdcall_rcx = start | page_size;
> -	if (__tdx_module_call(TDX_ACCEPT_PAGE, tdcall_rcx, 0, 0, 0, NULL))
> +	if (__tdx_module_call(TDG_MEM_PAGE_ACCEPT, tdcall_rcx, 0, 0, 0, NULL))
>  		return 0;
>  
>  	return accept_size;
> @@ -45,7 +45,7 @@ bool tdx_accept_memory(phys_addr_t start, phys_addr_t end)
>  {
>  	/*
>  	 * For shared->private conversion, accept the page using
> -	 * TDX_ACCEPT_PAGE TDX module call.
> +	 * TDG_MEM_PAGE_ACCEPT TDX module call.
>  	 */
>  	while (start < end) {
>  		unsigned long len = end - start;
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 1d6b863c42b0..05785df66b1c 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -91,7 +91,7 @@ int tdx_mcall_get_report0(u8 *reportdata, u8 *tdreport)
>  {
>  	u64 ret;
>  
> -	ret = __tdx_module_call(TDX_GET_REPORT, virt_to_phys(tdreport),
> +	ret = __tdx_module_call(TDG_MR_REPORT, virt_to_phys(tdreport),
>  				virt_to_phys(reportdata), TDREPORT_SUBTYPE_0,
>  				0, NULL);
>  	if (ret) {
> @@ -152,7 +152,7 @@ static void tdx_parse_tdinfo(u64 *cc_mask)
>  	 * Guest-Host-Communication Interface (GHCI), section 2.4.2 TDCALL
>  	 * [TDG.VP.INFO].
>  	 */
> -	tdx_module_call(TDX_GET_INFO, 0, 0, 0, 0, &out);
> +	tdx_module_call(TDG_VP_INFO, 0, 0, 0, 0, &out);
>  
>  	/*
>  	 * The highest bit of a guest physical address is the "sharing" bit.
> @@ -594,7 +594,7 @@ void tdx_get_ve_info(struct ve_info *ve)
>  	 * Note, the TDX module treats virtual NMIs as inhibited if the #VE
>  	 * valid flag is set. It means that NMI=>#VE will not result in a #DF.
>  	 */
> -	tdx_module_call(TDX_GET_VEINFO, 0, 0, 0, 0, &out);
> +	tdx_module_call(TDG_VP_VEINFO_GET, 0, 0, 0, 0, &out);
>  
>  	/* Transfer the output parameters */
>  	ve->exit_reason = out.rcx;
> @@ -774,7 +774,7 @@ void __init tdx_early_init(void)
>  	cc_set_mask(cc_mask);
>  
>  	/* Kernel does not use NOTIFY_ENABLES and does not need random #VEs */
> -	tdx_module_call(TDX_WR, 0, TDCS_NOTIFY_ENABLES, 0, -1ULL, NULL);
> +	tdx_module_call(TDG_VM_WR, 0, TDCS_NOTIFY_ENABLES, 0, -1ULL, NULL);
>  
>  	/*
>  	 * All bits above GPA width are reserved and kernel treats shared bit
> diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> index 7513b3bb69b7..78f109446da6 100644
> --- a/arch/x86/include/asm/shared/tdx.h
> +++ b/arch/x86/include/asm/shared/tdx.h
> @@ -11,11 +11,11 @@
>  #define TDX_IDENT		"IntelTDX    "
>  
>  /* 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
> -#define TDX_WR				8
> +#define TDG_VP_INFO			1
> +#define TDG_VP_VEINFO_GET		3
> +#define TDG_MR_REPORT			4
> +#define TDG_MEM_PAGE_ACCEPT		6
> +#define TDG_VM_WR			8
>  
>  /* TDCS fields. To be used by TDG.VM.WR and TDG.VM.RD module calls */
>  #define TDCS_NOTIFY_ENABLES		0x9100000000000010

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer