Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 24 Mar 2023 18:35:21 +0200
From:   Zhi Wang <zhi.wang.linux@gmail.com>
To:     isaku.yamahata@intel.com
Cc:     kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        isaku.yamahata@gmail.com, Paolo Bonzini <pbonzini@redhat.com>,
        erdemaktas@google.com, Sean Christopherson <seanjc@google.com>,
        Sagi Shahar <sagis@google.com>,
        David Matlack <dmatlack@google.com>,
        Kai Huang <kai.huang@intel.com>
Subject: Re: [PATCH v13 014/113] x86/cpu: Add helper functions to
 allocate/free TDX private host key id
Message-ID: <20230324183521.000010ad.zhi.wang.linux@gmail.com>
In-Reply-To: <c88b3dc84d64f0e73039670acecbbb07dc5c6a54.1678643052.git.isaku.yamahata@intel.com>
References: <cover.1678643051.git.isaku.yamahata@intel.com>
        <c88b3dc84d64f0e73039670acecbbb07dc5c6a54.1678643052.git.isaku.yamahata@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Sun, 12 Mar 2023 10:55:38 -0700
isaku.yamahata@intel.com wrote:

> From: Isaku Yamahata <isaku.yamahata@intel.com>
> 
> TDX private host key id (HKID) is assigned to guest TD.  The memory
> controller encrypts guest TD memory with the assigned TDX HKID.  Add helper
> functions to allocate/free TDX private HKID so that TDX KVM can manage it.
> 
^ This sentence should go to the below paragraph, which is "problem/way to go"
paragraph. For "background" paragraph, you can talk more about the usage of
HKID, like global/private HKID. Then you don't need to introduce them again in
the below paragraph and just talk about the solution.

> Also export the global TDX private HKID that is used to encrypt TDX module,
> its memory and some dynamic data (TDR).  When VMM releasing encrypted page
> to reuse it, the page needs to be flushed with the used HKID.  VMM needs
> the global TDX private HKID to flush such pages.
> 
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
>  arch/x86/include/asm/tdx.h  | 12 ++++++++++++
>  arch/x86/virt/vmx/tdx/tdx.c | 34 +++++++++++++++++++++++++++++++++-
>  2 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 6c01ab572c1f..7d99a48a98cc 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -104,6 +104,16 @@ static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1,
>  bool platform_tdx_enabled(void);
>  int tdx_cpu_enable(void);
>  int tdx_enable(void);
> +/*
> + * Key id globally used by TDX module: TDX module maps TDR with this TDX global
> + * key id.  TDR includes key id assigned to the TD.  Then TDX module maps other
> + * TD-related pages with the assigned key id.  TDR requires this TDX global key
> + * id for cache flush unlike other TD-related pages.
> + */
> +extern u32 tdx_global_keyid __ro_after_init;
> +int tdx_guest_keyid_alloc(void);
> +void tdx_guest_keyid_free(int keyid);
> +
>  u64 __seamcall(u64 op, u64 rcx, u64 rdx, u64 r8, u64 r9,
>  	       struct tdx_module_output *out);
>  #else	/* !CONFIG_INTEL_TDX_HOST */
> @@ -112,6 +122,8 @@ static inline int tdx_cpu_enable(void) { return -EINVAL; }
>  static inline int tdx_enable(void)  { return -EINVAL; }
>  static inline u64 __seamcall(u64 op, u64 rcx, u64 rdx, u64 r8, u64 r9,
>  			     struct tdx_module_output *out) { return TDX_SEAMCALL_UD; };
> +static inline int tdx_guest_keyid_alloc(void) { return -EOPNOTSUPP; }
> +static inline void tdx_guest_keyid_free(int keyid) { }
>  #endif	/* CONFIG_INTEL_TDX_HOST */
>  
>  #endif /* !__ASSEMBLY__ */
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index ee94a7327d93..3251fdf74667 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -33,7 +33,8 @@
>  #include <asm/tdx.h>
>  #include "tdx.h"
>  
> -static u32 tdx_global_keyid __ro_after_init;
> +u32 tdx_global_keyid __ro_after_init;
> +EXPORT_SYMBOL_GPL(tdx_global_keyid);
>  static u32 tdx_guest_keyid_start __ro_after_init;
>  static u32 tdx_nr_guest_keyids __ro_after_init;
>  
> @@ -138,6 +139,31 @@ static struct notifier_block tdx_memory_nb = {
>  	.notifier_call = tdx_memory_notifier,
>  };
>  
> +/* TDX KeyID pool */
> +static DEFINE_IDA(tdx_guest_keyid_pool);
> +
> +int tdx_guest_keyid_alloc(void)
> +{
> +	if (WARN_ON_ONCE(!tdx_guest_keyid_start || !tdx_nr_guest_keyids))
> +		return -EINVAL;
> +
> +	/* The first keyID is reserved for the global key. */
> +	return ida_alloc_range(&tdx_guest_keyid_pool, tdx_guest_keyid_start + 1,
> +			       tdx_guest_keyid_start + tdx_nr_guest_keyids - 1,
> +			       GFP_KERNEL);
> +}
> +EXPORT_SYMBOL_GPL(tdx_guest_keyid_alloc);
> +
> +void tdx_guest_keyid_free(int keyid)
> +{
> +	/* keyid = 0 is reserved. */
> +	if (WARN_ON_ONCE(keyid <= 0))
> +		return;
> +
> +	ida_free(&tdx_guest_keyid_pool, keyid);
> +}
> +EXPORT_SYMBOL_GPL(tdx_guest_keyid_free);
> +
>  static int __init tdx_init(void)
>  {
>  	u32 tdx_keyid_start, nr_tdx_keyids;
> @@ -1218,6 +1244,12 @@ static int init_tdx_module(void)
>  	if (ret)
>  		goto out_free_pamts;
>  
> +	/*
> +	 * Reserve the first TDX KeyID as global KeyID to protect
> +	 * TDX module metadata.
> +	 */
> +	tdx_global_keyid = tdx_keyid_start;
> +
>  	/* Initialize TDMRs to complete the TDX module initialization */
>  	ret = init_tdmrs(&tdmr_list);
>