| 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
| ||
|
Message-ID: <40f3dcc964bfb5d922cf09ddf080d53c97d82273.camel@intel.com>
Date: Wed, 2 Apr 2025 00:53:05 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "kvm@...r.kernel.org" <kvm@...r.kernel.org>, "pbonzini@...hat.com"
<pbonzini@...hat.com>, "seanjc@...gle.com" <seanjc@...gle.com>,
"binbin.wu@...ux.intel.com" <binbin.wu@...ux.intel.com>
CC: "Gao, Chao" <chao.gao@...el.com>, "Edgecombe, Rick P"
<rick.p.edgecombe@...el.com>, "mikko.ylinen@...ux.intel.com"
<mikko.ylinen@...ux.intel.com>, "Li, Xiaoyao" <xiaoyao.li@...el.com>,
"Lindgren, Tony" <tony.lindgren@...el.com>, "Hunter, Adrian"
<adrian.hunter@...el.com>, "Chatre, Reinette" <reinette.chatre@...el.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Zhao, Yan Y"
<yan.y.zhao@...el.com>, "Yamahata, Isaku" <isaku.yamahata@...el.com>
Subject: Re: [PATCH 1/2] KVM: TDX: Handle TDG.VP.VMCALL<GetQuote>
On Wed, 2025-04-02 at 08:15 +0800, Binbin Wu wrote:
> Handle TDVMCALL for GetQuote to generate a TD-Quote.
>
> GetQuote is a doorbell-like interface used by TDX guests to request VMM
> to generate a TD-Quote signed by a service hosting TD-Quoting Enclave
> operating on the host. A TDX guest passes a TD Report (TDREPORT_STRUCT) in
> a shared-memory area as parameter. Host VMM can access it and queue the
> operation for a service hosting TD-Quoting enclave. When completed, the
> Quote is returned via the same shared-memory area.
>
> KVM forwards the request to userspace VMM (e.g., QEMU) and userspace VMM
> queues the operation asynchronously.
>
I think the key is GetQuote is asynchronous therefore KVM will return to guest
immediately after forwarding to userspace. Whether *userspace* queues the
operation asynchronously doesn't matter.
> After the TDVMCALL is returned and
> back to TDX guest, TDX guest can poll the status field of the shared-memory
> area.
How about combing the above two paragraphs into:
GetQuote is an asynchronous request. KVM returns to userspace immediately after
forwarding the request to userspace. The TDX guest then needs to poll the
status field of the shared buffer (or wait for notification if enabled) to tell
whether the Quote is ready.
>
> Add KVM_EXIT_TDX_GET_QUOTE as a new exit reason to userspace and forward
> the request after some sanity checks.
>
> Signed-off-by: Binbin Wu <binbin.wu@...ux.intel.com>
> Tested-by: Mikko Ylinen <mikko.ylinen@...ux.intel.com>
> ---
> Documentation/virt/kvm/api.rst | 19 ++++++++++++++++++
> arch/x86/kvm/vmx/tdx.c | 35 ++++++++++++++++++++++++++++++++++
> include/uapi/linux/kvm.h | 7 +++++++
> 3 files changed, 61 insertions(+)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index b61371f45e78..90aa7a328dc8 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -7162,6 +7162,25 @@ The valid value for 'flags' is:
> - KVM_NOTIFY_CONTEXT_INVALID -- the VM context is corrupted and not valid
> in VMCS. It would run into unknown result if resume the target VM.
>
> +::
> +
> + /* KVM_EXIT_TDX_GET_QUOTE */
> + struct tdx_get_quote {
> + __u64 ret;
> + __u64 gpa;
> + __u64 size;
> + };
> +
> +If the exit reason is KVM_EXIT_TDX_GET_QUOTE, then it indicates that a TDX
> +guest has requested to generate a TD-Quote signed by a service hosting
> +TD-Quoting Enclave operating on the host. The 'gpa' field and 'size' specify
> +the guest physical address and size of a shared-memory buffer, in which the
> +TDX guest passes a TD report.
>
"TD report" -> "TD Report"? The changelog uses the latter.
> When completed, the generated quote is returned
"quote" -> "Quote"?
> +via the same buffer. The 'ret' field represents the return value.
>
return value of the GetQuote TDVMCALL?
> The userspace
> +should update the return value before resuming the vCPU according to TDX GHCI
> +spec.
>
I don't quite follow. Why userspace should "update" the return value?
> It's an asynchronous request. After the TDVMCALL is returned and back to
> +TDX guest, TDX guest can poll the status field of the shared-memory area.
> +
> ::
>
> /* Fix the size of the union. */
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index b952bc673271..535200446c21 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -1463,6 +1463,39 @@ static int tdx_get_td_vm_call_info(struct kvm_vcpu *vcpu)
> return 1;
> }
>
> +static int tdx_complete_get_quote(struct kvm_vcpu *vcpu)
> +{
> + tdvmcall_set_return_code(vcpu, vcpu->run->tdx_get_quote.ret);
> + return 1;
> +}
> +
> +static int tdx_get_quote(struct kvm_vcpu *vcpu)
> +{
> + struct vcpu_tdx *tdx = to_tdx(vcpu);
> +
> + u64 gpa = tdx->vp_enter_args.r12;
> + u64 size = tdx->vp_enter_args.r13;
> +
> + /* The buffer must be shared memory. */
> + if (vt_is_tdx_private_gpa(vcpu->kvm, gpa) || size == 0) {
> + tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_INVALID_OPERAND);
> + return 1;
> + }
It is a little bit confusing about the shared buffer check here. There are two
perspectives here:
1) the buffer has already been converted to shared, i.e., the attributes are
stored in the Xarray.
2) the GPA passed in the GetQuote must have the shared bit set.
The key is we need 1) here. From the spec, we need the 2) as well because it
*seems* that the spec requires GetQuote to provide the GPA with shared bit set,
as it says "Shared GPA as input".
The above check only does 2). I think we need to check 1) as well, because once
you forward this GetQuote to userspace, userspace is able to access it freely.
As a result, the comment
/* The buffer must be shared memory. */
should also be updated to something like:
/*
* The buffer must be shared. GetQuote requires the GPA to have
* shared bit set.
*/
> +
> + if (!PAGE_ALIGNED(gpa) || !PAGE_ALIGNED(size)) {
> + tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_ALIGN_ERROR);
> + return 1;
> + }
> +
> + vcpu->run->exit_reason = KVM_EXIT_TDX_GET_QUOTE;
> + vcpu->run->tdx_get_quote.gpa = gpa;
> + vcpu->run->tdx_get_quote.size = size;
> +
> + vcpu->arch.complete_userspace_io = tdx_complete_get_quote;
> +
> + return 0;
> +}
> +
> static int handle_tdvmcall(struct kvm_vcpu *vcpu)
> {
> switch (tdvmcall_leaf(vcpu)) {
> @@ -1472,6 +1505,8 @@ static int handle_tdvmcall(struct kvm_vcpu *vcpu)
> return tdx_report_fatal_error(vcpu);
> case TDVMCALL_GET_TD_VM_CALL_INFO:
> return tdx_get_td_vm_call_info(vcpu);
> + case TDVMCALL_GET_QUOTE:
> + return tdx_get_quote(vcpu);
> default:
> break;
> }
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index c6988e2c68d5..eca86b7f0cbc 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -178,6 +178,7 @@ struct kvm_xen_exit {
> #define KVM_EXIT_NOTIFY 37
> #define KVM_EXIT_LOONGARCH_IOCSR 38
> #define KVM_EXIT_MEMORY_FAULT 39
> +#define KVM_EXIT_TDX_GET_QUOTE 41
>
> /* For KVM_EXIT_INTERNAL_ERROR */
> /* Emulate instruction failed. */
> @@ -447,6 +448,12 @@ struct kvm_run {
> __u64 gpa;
> __u64 size;
> } memory_fault;
> + /* KVM_EXIT_TDX_GET_QUOTE */
> + struct {
> + __u64 ret;
> + __u64 gpa;
> + __u64 size;
> + } tdx_get_quote;
> /* Fix the size of the union. */
> char padding[256];
> };
Powered by blists - more mailing lists