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;
From:   Zhi Wang <zhi.wang.linux@gmail.com>
Date:   Wed, 23 Nov 2022 12:00:47 +0200
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>,
        Sean Christopherson <sean.j.christopherson@intel.com>,
        zhi.a.wang@intel.com
Subject: Re: [PATCH v10 011/108] KVM: TDX: Add C wrapper functions for
 SEAMCALLs to the TDX module
Message-ID: <20221123120047.00001a9d@gmail.com>
In-Reply-To: <5f1a80e9ab037fa88d8821a6548638d282070f1d.1667110240.git.isaku.yamahata@intel.com>
References: <cover.1667110240.git.isaku.yamahata@intel.com>
        <5f1a80e9ab037fa88d8821a6548638d282070f1d.1667110240.git.isaku.yamahata@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Sat, 29 Oct 2022 23:22:12 -0700
isaku.yamahata@intel.com wrote:

> From: Isaku Yamahata <isaku.yamahata@intel.com>
> 
> A VMM interacts with the TDX module using a new instruction
> (SEAMCALL).  A TDX VMM uses SEAMCALLs where a VMX VMM would have
> directly interacted with VMX instructions.  For instance, a TDX VMM
> does not have full access to the VM control structure corresponding
> to VMX VMCS.  Instead, a VMM induces the TDX module to act on behalf
> via SEAMCALLs.
> 
> Export __seamcall and define C wrapper functions for SEAMCALLs for
> readability.  Some SEAMCALL APIs donates pages to TDX module or guest
> TD. The pages are encrypted with TDX private host key id set in high

It seems a little bit confusing here.

I guess you are talking about the host pages donated to TDX module will
be encrypted with TDX module global key id, which is used to
encrypted the pages only used by TDX module. For pages donated to the
TD guest, the pages are encrypted with TD guest prviate key id?

It might be better that you can talk about the actual reaason instead of
the key id stuff. For example Some SEMCALL APIs will zap or write the
pages donated by VMM via MOVDIR64B, which requires the VMM to flush the
cachelines.

> bits of physical address.  If any modified cache lines may exit for

typo exit->exist

> these pages, flush them to memory by clflush_cache_range().
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
>  arch/x86/include/asm/tdx.h       |   2 +
>  arch/x86/kvm/vmx/tdx_ops.h       | 185
> +++++++++++++++++++++++++++++++ arch/x86/virt/vmx/tdx/seamcall.S |
> 2 + 3 files changed, 189 insertions(+)
>  create mode 100644 arch/x86/kvm/vmx/tdx_ops.h
> 
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 5cff7ed5b11e..ba2e4c69fb9f 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -186,6 +186,8 @@ struct tdsysinfo_struct {
>  const struct tdsysinfo_struct *tdx_get_sysinfo(void);
>  bool platform_tdx_enabled(void);
>  int tdx_enable(void);
> +u64 __seamcall(u64 op, u64 rcx, u64 rdx, u64 r8, u64 r9,
> +	       struct tdx_module_output *out);
>  #else	/* !CONFIG_INTEL_TDX_HOST */
>  struct tdsysinfo_struct;
>  static inline const struct tdsysinfo_struct *tdx_get_sysinfo(void) {
> return NULL; } diff --git a/arch/x86/kvm/vmx/tdx_ops.h
> b/arch/x86/kvm/vmx/tdx_ops.h new file mode 100644
> index 000000000000..85adbf49c277
> --- /dev/null
> +++ b/arch/x86/kvm/vmx/tdx_ops.h
> @@ -0,0 +1,185 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* constants/data definitions for TDX SEAMCALLs */
> +
> +#ifndef __KVM_X86_TDX_OPS_H
> +#define __KVM_X86_TDX_OPS_H
> +
> +#include <linux/compiler.h>
> +
> +#include <asm/cacheflush.h>
> +#include <asm/asm.h>
> +#include <asm/kvm_host.h>
> +
> +#include "tdx_errno.h"
> +#include "tdx_arch.h"
> +
> +#ifdef CONFIG_INTEL_TDX_HOST
> +
> +static inline u64 tdh_mng_addcx(hpa_t tdr, hpa_t addr)
> +{
> +	clflush_cache_range(__va(addr), PAGE_SIZE);
> +	return __seamcall(TDH_MNG_ADDCX, addr, tdr, 0, 0, NULL);
> +}
> +
> +static inline u64 tdh_mem_page_add(hpa_t tdr, gpa_t gpa, hpa_t hpa,
> hpa_t source,
> +				   struct tdx_module_output *out)
> +{
> +	clflush_cache_range(__va(hpa), PAGE_SIZE);
> +	return __seamcall(TDH_MEM_PAGE_ADD, gpa, tdr, hpa, source,
> out); +}
> +
> +static inline u64 tdh_mem_sept_add(hpa_t tdr, gpa_t gpa, int level,
> hpa_t page,
> +				   struct tdx_module_output *out)
> +{
> +	clflush_cache_range(__va(page), PAGE_SIZE);
> +	return __seamcall(TDH_MEM_SEPT_ADD, gpa | level, tdr, page,
> 0, out); +}
> +
> +static inline u64 tdh_mem_sept_remove(hpa_t tdr, gpa_t gpa, int
> level,
> +				      struct tdx_module_output *out)
> +{
> +	return __seamcall(TDH_MEM_SEPT_REMOVE, gpa | level, tdr, 0,
> 0, out); +}
> +
> +static inline u64 tdh_vp_addcx(hpa_t tdvpr, hpa_t addr)
> +{
> +	clflush_cache_range(__va(addr), PAGE_SIZE);
> +	return __seamcall(TDH_VP_ADDCX, addr, tdvpr, 0, 0, NULL);
> +}
> +
> +static inline u64 tdh_mem_page_relocate(hpa_t tdr, gpa_t gpa, hpa_t
> hpa,
> +					struct tdx_module_output
> *out) +{
> +	clflush_cache_range(__va(hpa), PAGE_SIZE);
> +	return __seamcall(TDH_MEM_PAGE_RELOCATE, gpa, tdr, hpa, 0,
> out); +}
> +
> +static inline u64 tdh_mem_page_aug(hpa_t tdr, gpa_t gpa, hpa_t hpa,
> +				   struct tdx_module_output *out)
> +{
> +	clflush_cache_range(__va(hpa), PAGE_SIZE);
> +	return __seamcall(TDH_MEM_PAGE_AUG, gpa, tdr, hpa, 0, out);
> +}
> +
> +static inline u64 tdh_mem_range_block(hpa_t tdr, gpa_t gpa, int
> level,
> +				      struct tdx_module_output *out)
> +{
> +	return __seamcall(TDH_MEM_RANGE_BLOCK, gpa | level, tdr, 0,
> 0, out); +}
> +
> +static inline u64 tdh_mng_key_config(hpa_t tdr)
> +{
> +	return __seamcall(TDH_MNG_KEY_CONFIG, tdr, 0, 0, 0, NULL);
> +}
> +
> +static inline u64 tdh_mng_create(hpa_t tdr, int hkid)
> +{
> +	clflush_cache_range(__va(tdr), PAGE_SIZE);
> +	return __seamcall(TDH_MNG_CREATE, tdr, hkid, 0, 0, NULL);
> +}
> +
> +static inline u64 tdh_vp_create(hpa_t tdr, hpa_t tdvpr)
> +{
> +	clflush_cache_range(__va(tdvpr), PAGE_SIZE);
> +	return __seamcall(TDH_VP_CREATE, tdvpr, tdr, 0, 0, NULL);
> +}
> +
> +static inline u64 tdh_mng_rd(hpa_t tdr, u64 field, struct
> tdx_module_output *out) +{
> +	return __seamcall(TDH_MNG_RD, tdr, field, 0, 0, out);
> +}
> +
> +static inline u64 tdh_mr_extend(hpa_t tdr, gpa_t gpa,
> +				struct tdx_module_output *out)
> +{
> +	return __seamcall(TDH_MR_EXTEND, gpa, tdr, 0, 0, out);
> +}
> +
> +static inline u64 tdh_mr_finalize(hpa_t tdr)
> +{
> +	return __seamcall(TDH_MR_FINALIZE, tdr, 0, 0, 0, NULL);
> +}
> +
> +static inline u64 tdh_vp_flush(hpa_t tdvpr)
> +{
> +	return __seamcall(TDH_VP_FLUSH, tdvpr, 0, 0, 0, NULL);
> +}
> +
> +static inline u64 tdh_mng_vpflushdone(hpa_t tdr)
> +{
> +	return __seamcall(TDH_MNG_VPFLUSHDONE, tdr, 0, 0, 0, NULL);
> +}
> +
> +static inline u64 tdh_mng_key_freeid(hpa_t tdr)
> +{
> +	return __seamcall(TDH_MNG_KEY_FREEID, tdr, 0, 0, 0, NULL);
> +}
> +
> +static inline u64 tdh_mng_init(hpa_t tdr, hpa_t td_params,
> +			       struct tdx_module_output *out)
> +{
> +	return __seamcall(TDH_MNG_INIT, tdr, td_params, 0, 0, out);
> +}
> +
> +static inline u64 tdh_vp_init(hpa_t tdvpr, u64 rcx)
> +{
> +	return __seamcall(TDH_VP_INIT, tdvpr, rcx, 0, 0, NULL);
> +}
> +
> +static inline u64 tdh_vp_rd(hpa_t tdvpr, u64 field,
> +			    struct tdx_module_output *out)
> +{
> +	return __seamcall(TDH_VP_RD, tdvpr, field, 0, 0, out);
> +}
> +
> +static inline u64 tdh_mng_key_reclaimid(hpa_t tdr)
> +{
> +	return __seamcall(TDH_MNG_KEY_RECLAIMID, tdr, 0, 0, 0, NULL);
> +}
> +
> +static inline u64 tdh_phymem_page_reclaim(hpa_t page,
> +					  struct tdx_module_output
> *out) +{
> +	return __seamcall(TDH_PHYMEM_PAGE_RECLAIM, page, 0, 0, 0,
> out); +}
> +
> +static inline u64 tdh_mem_page_remove(hpa_t tdr, gpa_t gpa, int
> level,
> +				      struct tdx_module_output *out)
> +{
> +	return __seamcall(TDH_MEM_PAGE_REMOVE, gpa | level, tdr, 0,
> 0, out); +}
> +
> +static inline u64 tdh_sys_lp_shutdown(void)
> +{
> +	return __seamcall(TDH_SYS_LP_SHUTDOWN, 0, 0, 0, 0, NULL);
> +}
> +
> +static inline u64 tdh_mem_track(hpa_t tdr)
> +{
> +	return __seamcall(TDH_MEM_TRACK, tdr, 0, 0, 0, NULL);
> +}
> +
> +static inline u64 tdh_mem_range_unblock(hpa_t tdr, gpa_t gpa, int
> level,
> +					struct tdx_module_output
> *out) +{
> +	return __seamcall(TDH_MEM_RANGE_UNBLOCK, gpa | level, tdr,
> 0, 0, out); +}
> +
> +static inline u64 tdh_phymem_cache_wb(bool resume)
> +{
> +	return __seamcall(TDH_PHYMEM_CACHE_WB, resume ? 1 : 0, 0, 0,
> 0, NULL); +}
> +
> +static inline u64 tdh_phymem_page_wbinvd(hpa_t page)
> +{
> +	return __seamcall(TDH_PHYMEM_PAGE_WBINVD, page, 0, 0, 0,
> NULL); +}
> +
> +static inline u64 tdh_vp_wr(hpa_t tdvpr, u64 field, u64 val, u64
> mask,
> +			    struct tdx_module_output *out)
> +{
> +	return __seamcall(TDH_VP_WR, tdvpr, field, val, mask, out);
> +}
> +#endif /* CONFIG_INTEL_TDX_HOST */
> +
> +#endif /* __KVM_X86_TDX_OPS_H */
> diff --git a/arch/x86/virt/vmx/tdx/seamcall.S
> b/arch/x86/virt/vmx/tdx/seamcall.S index f81be6b9c133..b90a7fe05494
> 100644 --- a/arch/x86/virt/vmx/tdx/seamcall.S
> +++ b/arch/x86/virt/vmx/tdx/seamcall.S
> @@ -1,5 +1,6 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
>  #include <linux/linkage.h>
> +#include <asm/export.h>
>  #include <asm/frame.h>
>  
>  #include "tdxcall.S"
> @@ -50,3 +51,4 @@ SYM_FUNC_START(__seamcall)
>  	FRAME_END
>  	RET
>  SYM_FUNC_END(__seamcall)
> +EXPORT_SYMBOL_GPL(__seamcall)