linux-kernel - Re: [RFC PATCH v5 089/104] KVM: TDX: Add a placeholder for handler of TDX hypercalls (TDG.VP.VMCALL)

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:   Thu, 07 Apr 2022 16:15:00 +1200
From:   Kai Huang <kai.huang@...el.com>
To:     isaku.yamahata@...el.com, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     isaku.yamahata@...il.com, Paolo Bonzini <pbonzini@...hat.com>,
        Jim Mattson <jmattson@...gle.com>, erdemaktas@...gle.com,
        Connor Kuehl <ckuehl@...hat.com>,
        Sean Christopherson <seanjc@...gle.com>
Subject: Re: [RFC PATCH v5 089/104] KVM: TDX: Add a placeholder for handler
 of TDX hypercalls (TDG.VP.VMCALL)

On Fri, 2022-03-04 at 11:49 -0800, isaku.yamahata@...el.com wrote:
> From: Sean Christopherson <sean.j.christopherson@...el.com>
> 
> The TDX module specification defines TDG.VP.VMCALL API (TDVMCALL for short)
> for the guest TD to call hypercall to VMM.  When the guest TD issues
> TDG.VP.VMCALL, the guest TD exits to VMM with a new exit reason of
> TDVMCALL.  The arguments from the guest TD and returned values from the VMM
> are passed in the guest registers.  The guest RCX registers indicates which
> registers are used.
> 
> Define the TDVMCALL exit reason, which is carved out from the VMX exit
> reason namespace as the TDVMCALL exit from TDX guest to TDX-SEAM is really
> just a VM-Exit.  Add a place holder to handle TDVMCALL exit.
> 
> Co-developed-by: Xiaoyao Li <xiaoyao.li@...el.com>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@...el.com>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@...el.com>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@...el.com>
> ---
>  arch/x86/include/uapi/asm/vmx.h |  4 +++-
>  arch/x86/kvm/vmx/tdx.c          | 27 ++++++++++++++++++++++++++-
>  arch/x86/kvm/vmx/tdx.h          | 13 +++++++++++++
>  3 files changed, 42 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/uapi/asm/vmx.h b/arch/x86/include/uapi/asm/vmx.h
> index 3d9b4598e166..cb0a0565219a 100644
> --- a/arch/x86/include/uapi/asm/vmx.h
> +++ b/arch/x86/include/uapi/asm/vmx.h
> @@ -92,6 +92,7 @@
>  #define EXIT_REASON_UMWAIT              67
>  #define EXIT_REASON_TPAUSE              68
>  #define EXIT_REASON_BUS_LOCK            74
> +#define EXIT_REASON_TDCALL              77
>  
>  #define VMX_EXIT_REASONS \
>  	{ EXIT_REASON_EXCEPTION_NMI,         "EXCEPTION_NMI" }, \
> @@ -154,7 +155,8 @@
>  	{ EXIT_REASON_XRSTORS,               "XRSTORS" }, \
>  	{ EXIT_REASON_UMWAIT,                "UMWAIT" }, \
>  	{ EXIT_REASON_TPAUSE,                "TPAUSE" }, \
> -	{ EXIT_REASON_BUS_LOCK,              "BUS_LOCK" }
> +	{ EXIT_REASON_BUS_LOCK,              "BUS_LOCK" }, \
> +	{ EXIT_REASON_TDCALL,                "TDCALL" }
>  
>  #define VMX_EXIT_REASON_FLAGS \
>  	{ VMX_EXIT_REASONS_FAILED_VMENTRY,	"FAILED_VMENTRY" }
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 8695836ce796..86daafd9eec0 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -780,7 +780,8 @@ static noinstr void tdx_vcpu_enter_exit(struct kvm_vcpu *vcpu,
>  					struct vcpu_tdx *tdx)
>  {
>  	guest_enter_irqoff();
> -	tdx->exit_reason.full = __tdx_vcpu_run(tdx->tdvpr.pa, vcpu->arch.regs, 0);
> +	tdx->exit_reason.full = __tdx_vcpu_run(tdx->tdvpr.pa, vcpu->arch.regs,
> +					tdx->tdvmcall.regs_mask);
>  	guest_exit_irqoff();
>  }
>  
> @@ -815,6 +816,11 @@ fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu)
>  
>  	if (tdx->exit_reason.error || tdx->exit_reason.non_recoverable)
>  		return EXIT_FASTPATH_NONE;
> +
> +	if (tdx->exit_reason.basic == EXIT_REASON_TDCALL)
> +		tdx->tdvmcall.rcx = vcpu->arch.regs[VCPU_REGS_RCX];
> +	else
> +		tdx->tdvmcall.rcx = 0;
>  	return EXIT_FASTPATH_NONE;
>  }
>  
> @@ -859,6 +865,23 @@ static int tdx_handle_triple_fault(struct kvm_vcpu *vcpu)
>  	return 0;
>  }
>  
> +static int handle_tdvmcall(struct kvm_vcpu *vcpu)
> +{
> +	struct vcpu_tdx *tdx = to_tdx(vcpu);
> +
> +	if (unlikely(tdx->tdvmcall.xmm_mask))
> +		goto unsupported;

Put a comment explaining this logic?

> +
> +	switch (tdvmcall_exit_reason(vcpu)) {

Could we rename tdxvmcall_exit_reason() to something like tdvmcall_leaf()?

Btw, why couldn't we merge previous patch to this one, so we don't have to look
back and forth to figure out exactly what do those functions do?


> +	default:
> +		break;
> +	}
> +
> +unsupported:
> +	tdvmcall_set_return_code(vcpu, TDG_VP_VMCALL_INVALID_OPERAND);
> +	return 1;
> +}
> +
>  void tdx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa, int pgd_level)
>  {
>  	td_vmcs_write64(to_tdx(vcpu), SHARED_EPT_POINTER, root_hpa & PAGE_MASK);
> @@ -1187,6 +1210,8 @@ int tdx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t fastpath)
>  		return tdx_handle_exception(vcpu);
>  	case EXIT_REASON_EXTERNAL_INTERRUPT:
>  		return tdx_handle_external_interrupt(vcpu);
> +	case EXIT_REASON_TDCALL:
> +		return handle_tdvmcall(vcpu);
>  	case EXIT_REASON_EPT_VIOLATION:
>  		return tdx_handle_ept_violation(vcpu);
>  	case EXIT_REASON_EPT_MISCONFIG:
> diff --git a/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h
> index 7cd81780f3fa..9e8ed9b3119e 100644
> --- a/arch/x86/kvm/vmx/tdx.h
> +++ b/arch/x86/kvm/vmx/tdx.h
> @@ -86,6 +86,19 @@ struct vcpu_tdx {
>  	/* Posted interrupt descriptor */
>  	struct pi_desc pi_desc;
>  
> +	union {
> +		struct {
> +			union {
> +				struct {
> +					u16 gpr_mask;
> +					u16 xmm_mask;
> +				};
> +				u32 regs_mask;
> +			};
> +			u32 reserved;
> +		};
> +		u64 rcx;
> +	} tdvmcall;
>  	union tdx_exit_reason exit_reason;
>  
>  	bool initialized;