All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Xiaoyao Li <xiaoyao.li@intel.com>
Cc: sashiko-reviews@lists.linux.dev, kvm@vger.kernel.org
Subject: Re: [PATCH 2/2] KVM: TDX: Enable Bus Lock VM exit
Date: Wed, 5 Aug 2026 07:56:01 -0700	[thread overview]
Message-ID: <anNPAXShf1gjDdeu@google.com> (raw)
In-Reply-To: <4fedcaa6-d710-48f8-84c4-de682350bb2f@intel.com>

On Wed, Aug 05, 2026, Xiaoyao Li wrote:
> On 8/5/2026 11:46 AM, sashiko-bot@kernel.org wrote:
> > This also appears to affect tdx_to_vmx_exit_reason(), where comparing the raw
> > exit reason directly against 16-bit constants like EXIT_REASON_TDCALL will
> > fail to match if the bus lock bit is set, leading to incorrect emulation.
> 
> This is valid. We need to adjust tdx_to_vmx_exit_reason().
> 
> However, there is a more important problem. Since Bus Lock VM exit makes bit
> 26 possible in EXIT REASON, the trick of "return -1" in
> tdx_to_vmx_exit_reason() will introduce false-positive in the following
> check of if(vmx_get_exit_reason(vcpu).bus_lock_detected) added by this
> patch.
> 
> how about something like below:

Way too subtle.  tdx_to_vmx_exit_reason() should return the actual union, not a
raw u32, otherwise it's going to be extremely difficult to avoid reintroducing
similar bugs.

And looking at this all again, we should change the handling of actual
EXIT_REASON_EPT_MISCONFIG exits.  Stuffing a bogus value into the exit_reason
is "fine", but as Sashiko points out, it's extremely brittle.  Rather than
stuff the exit reason, we should stuff the status to signal TDX_SW_ERROR.

And to do that without introducing more fragility, we should flag the raw
vp_enter_ret as "unsafe", and explicitly track vp_enter_status.  I.e. separate
the status from the exit_reason immediately after VP.ENTER, instead of mixing
and matching the two concepts.

The fastpath "handler" is also all kinds of messed up.  KVM fails to trace_kvm_exit()
EPT misconfigs and software errors; even though the exit reason is undefined, it
should still be captured in the trace, otherwise it's a huge blindspot.  And AFAICT,
OPERAND_BUSY should be mutually exclusive with actual VM-Entry failures, so manually
checking for VM-Entry failure is completely unnecessary, just handle OPERAND_BUSY.
If TDX ever gains fastpath handlers, then we can add a true fastpath handler at
that time.  But OPERAND_BUSY should be a "never do the fastpath", because AIUI,
VM-Enter wasn't attempted, i.e. there's nothing to handle.

Compile tested only, and it should be chunked over several patches, but this?

diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 299c051d648e..19e8e3703ce1 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -921,12 +921,12 @@ static __always_inline u32 tdcall_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
 	return EXIT_REASON_TDCALL;
 }
 
-static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
+static __always_inline union vmx_exit_reason tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu,
+								    u64 vp_enter_ret)
 {
-	struct vcpu_tdx *tdx = to_tdx(vcpu);
-	u32 exit_reason;
+	union vmx_exit_reason exit_reason;
 
-	switch (tdx->vp_enter_ret & TDX_SEAMCALL_STATUS_MASK) {
+	switch (vp_enter_ret & TDX_SEAMCALL_STATUS_MASK) {
 	case TDX_SUCCESS:
 	case TDX_NON_RECOVERABLE_VCPU:
 	case TDX_NON_RECOVERABLE_TD:
@@ -934,40 +934,38 @@ static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
 	case TDX_NON_RECOVERABLE_TD_WRONG_APIC_MODE:
 		break;
 	default:
-		return -1u;
+		/*
+		 * Synthesize an invalid bogus Exit Reason, as the TDX-Module
+		 * never attempted to run the vCPU, i.e. the Exit Reason is
+		 * undefined, but this is NOT a failed VM-Enter.
+		 */
+		return (union vmx_exit_reason) {
+			.basic = -1,
+		};
 	}
 
-	exit_reason = tdx->vp_enter_ret;
+	exit_reason.full = (u32)vp_enter_ret;
 
-	switch (exit_reason) {
-	case EXIT_REASON_TDCALL:
+	if (exit_reason.basic == EXIT_REASON_TDCALL) {
 		if (tdvmcall_exit_type(vcpu))
-			return EXIT_REASON_VMCALL;
-
-		return tdcall_to_vmx_exit_reason(vcpu);
-	case EXIT_REASON_EPT_MISCONFIG:
-		/*
-		 * Defer KVM_BUG_ON() until tdx_handle_exit() because this is in
-		 * non-instrumentable code with interrupts disabled.
-		 */
-		return -1u;
-	default:
-		break;
+			exit_reason.basic = EXIT_REASON_VMCALL;
+		else
+			exit_reason.basic = tdcall_to_vmx_exit_reason(vcpu);
 	}
-
 	return exit_reason;
 }
 
-static noinstr void tdx_vcpu_enter_exit(struct kvm_vcpu *vcpu)
+static noinstr u64 tdx_vcpu_enter_exit(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_tdx *tdx = to_tdx(vcpu);
 	struct vcpu_vt *vt = to_vt(vcpu);
+	u64 ret;
 
 	guest_state_enter_irqoff();
 
-	tdx->vp_enter_ret = tdh_vp_enter(&tdx->vp, &tdx->vp_enter_args);
+	ret = tdh_vp_enter(&tdx->vp, &tdx->vp_enter_args);
 
-	vt->exit_reason.full = tdx_to_vmx_exit_reason(vcpu);
+	vt->exit_reason = tdx_to_vmx_exit_reason(vcpu, ret);
 
 	vt->exit_qualification = tdx->vp_enter_args.rcx;
 	tdx->ext_exit_qualification = tdx->vp_enter_args.rdx;
@@ -977,33 +975,8 @@ static noinstr void tdx_vcpu_enter_exit(struct kvm_vcpu *vcpu)
 	vmx_handle_nmi(vcpu);
 
 	guest_state_exit_irqoff();
-}
 
-static bool tdx_failed_vmentry(struct kvm_vcpu *vcpu)
-{
-	return vmx_get_exit_reason(vcpu).failed_vmentry &&
-	       vmx_get_exit_reason(vcpu).full != -1u;
-}
-
-static fastpath_t tdx_exit_handlers_fastpath(struct kvm_vcpu *vcpu)
-{
-	u64 vp_enter_ret = to_tdx(vcpu)->vp_enter_ret;
-
-	/*
-	 * TDX_OPERAND_BUSY could be returned for SEPT due to 0-step mitigation
-	 * or for TD EPOCH due to contention with TDH.MEM.TRACK on TDH.VP.ENTER.
-	 *
-	 * When KVM requests KVM_REQ_OUTSIDE_GUEST_MODE, which has both
-	 * KVM_REQUEST_WAIT and KVM_REQUEST_NO_ACTION set, it requires target
-	 * vCPUs leaving fastpath so that interrupt can be enabled to ensure the
-	 * IPIs can be delivered. Return EXIT_FASTPATH_EXIT_HANDLED instead of
-	 * EXIT_FASTPATH_REENTER_GUEST to exit fastpath, otherwise, the
-	 * requester may be blocked endlessly.
-	 */
-	if (unlikely(tdx_operand_busy(vp_enter_ret)))
-		return EXIT_FASTPATH_EXIT_HANDLED;
-
-	return EXIT_FASTPATH_NONE;
+	return ret;
 }
 
 #define TDX_REGS_AVAIL_SET	(BIT(VCPU_REG_EXIT_INFO_1) | \
@@ -1053,8 +1026,8 @@ static void tdx_load_host_xsave_state(struct kvm_vcpu *vcpu)
 
 fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags)
 {
-	struct vcpu_tdx *tdx = to_tdx(vcpu);
 	struct vcpu_vt *vt = to_vt(vcpu);
+	u64 vp_enter_ret;
 
 	/*
 	 * WARN if KVM wants to force an immediate exit, as the TDX module does
@@ -1084,7 +1057,7 @@ fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags)
 			kvm_wait_lapic_expire(vcpu);
 	}
 
-	tdx_vcpu_enter_exit(vcpu);
+	vp_enter_ret = tdx_vcpu_enter_exit(vcpu);
 
 	if (vcpu->arch.host_debugctl & ~TDX_DEBUGCTL_PRESERVED)
 		update_debugctlmsr(vcpu->arch.host_debugctl);
@@ -1093,18 +1066,23 @@ fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags)
 
 	kvm_clear_available_registers(vcpu, ~TDX_REGS_AVAIL_SET);
 
-	if (unlikely(tdx->vp_enter_ret == EXIT_REASON_EPT_MISCONFIG))
-		return EXIT_FASTPATH_NONE;
-
-	if (unlikely((tdx->vp_enter_ret & TDX_SW_ERROR) == TDX_SW_ERROR))
-		return EXIT_FASTPATH_NONE;
-
 	trace_kvm_exit(vcpu, KVM_ISA_VMX);
 
-	if (unlikely(tdx_failed_vmentry(vcpu)))
-		return EXIT_FASTPATH_NONE;
+	/*
+	 * TDX_OPERAND_BUSY could be returned for SEPT due to 0-step mitigation
+	 * or for TD EPOCH due to contention with TDH.MEM.TRACK on TDH.VP.ENTER.
+	 *
+	 * When KVM requests KVM_REQ_OUTSIDE_GUEST_MODE, which has both
+	 * KVM_REQUEST_WAIT and KVM_REQUEST_NO_ACTION set, it requires target
+	 * vCPUs leaving fastpath so that interrupt can be enabled to ensure the
+	 * IPIs can be delivered. Return EXIT_FASTPATH_EXIT_HANDLED instead of
+	 * EXIT_FASTPATH_REENTER_GUEST to exit fastpath, otherwise, the
+	 * requester may be blocked endlessly.
+	 */
+	if (unlikely(tdx_operand_busy(vp_enter_ret)))
+		return EXIT_FASTPATH_EXIT_HANDLED;
 
-	return tdx_exit_handlers_fastpath(vcpu);
+	return EXIT_FASTPATH_NONE;
 }
 
 void tdx_inject_nmi(struct kvm_vcpu *vcpu)
@@ -1300,7 +1278,7 @@ static int tdx_report_fatal_error(struct kvm_vcpu *vcpu)
 	vcpu->run->system_event.ndata = 16;
 
 	/* Dump 16 general-purpose registers to userspace in ascending order. */
-	regs[index++] = tdx->vp_enter_ret;
+	regs[index++] = tdx->vp_enter_ret__unsafe;
 	regs[index++] = tdx->vp_enter_args.rcx;
 	regs[index++] = tdx->vp_enter_args.rdx;
 	regs[index++] = tdx->vp_enter_args.rbx;
@@ -2030,48 +2008,44 @@ int tdx_complete_emulated_msr(struct kvm_vcpu *vcpu, int err)
 
 static int __tdx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t fastpath)
 {
-	struct vcpu_tdx *tdx = to_tdx(vcpu);
-	u64 vp_enter_ret = tdx->vp_enter_ret;
 	union vmx_exit_reason exit_reason = vmx_get_exit_reason(vcpu);
+	struct vcpu_tdx *tdx = to_tdx(vcpu);
+	u64 status = tdx->vp_enter_ret__unsafe & TDX_SEAMCALL_STATUS_MASK;
 
 	if (fastpath != EXIT_FASTPATH_NONE)
 		return 1;
 
-	if (unlikely(vp_enter_ret == EXIT_REASON_EPT_MISCONFIG)) {
-		KVM_BUG_ON(1, vcpu->kvm);
+	if (KVM_BUG_ON(exit_reason.basic == EXIT_REASON_EPT_MISCONFIG, vcpu->kvm))
 		return -EIO;
-	}
 
 	/*
 	 * Handle TDX SW errors, including TDX_SEAMCALL_UD, TDX_SEAMCALL_GP and
 	 * TDX_SEAMCALL_VMFAILINVALID.
 	 */
-	if (unlikely((vp_enter_ret & TDX_SW_ERROR) == TDX_SW_ERROR)) {
+	if (unlikely(status == TDX_SW_ERROR)) {
 		KVM_BUG_ON(!virt_rebooting, vcpu->kvm);
 		goto unhandled_exit;
 	}
 
-	if (unlikely(tdx_failed_vmentry(vcpu))) {
+	if (unlikely(exit_reason.failed_vmentry)) {
 		/*
 		 * If the guest state is protected, that means off-TD debug is
 		 * not enabled, TDX_NON_RECOVERABLE must be set.
 		 */
 		WARN_ON_ONCE(vcpu->arch.guest_state_protected &&
-				!(vp_enter_ret & TDX_NON_RECOVERABLE));
+			     !(status & TDX_NON_RECOVERABLE));
 		vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
 		vcpu->run->fail_entry.hardware_entry_failure_reason = exit_reason.full;
 		vcpu->run->fail_entry.cpu = vcpu->arch.last_vmentry_cpu;
 		return 0;
 	}
 
-	if (unlikely(vp_enter_ret & (TDX_ERROR | TDX_NON_RECOVERABLE)) &&
-		exit_reason.basic != EXIT_REASON_TRIPLE_FAULT) {
-		kvm_pr_unimpl("TD vp_enter_ret 0x%llx\n", vp_enter_ret);
+	if (unlikely(status & (TDX_ERROR | TDX_NON_RECOVERABLE)) &&
+	    exit_reason.basic != EXIT_REASON_TRIPLE_FAULT)
 		goto unhandled_exit;
-	}
 
-	WARN_ON_ONCE(exit_reason.basic != EXIT_REASON_TRIPLE_FAULT &&
-		     (vp_enter_ret & TDX_SEAMCALL_STATUS_MASK) != TDX_SUCCESS);
+	WARN_ON_ONCE(status != TDX_SUCCESS &&
+		     exit_reason.basic != EXIT_REASON_TRIPLE_FAULT);
 
 	switch (exit_reason.basic) {
 	case EXIT_REASON_TRIPLE_FAULT:
@@ -2136,7 +2110,8 @@ static int __tdx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t fastpath)
 	}
 
 unhandled_exit:
-	kvm_prepare_unexpected_reason_exit(vcpu, vp_enter_ret);
+	kvm_pr_unimpl("TD vp_enter_ret 0x%llx\n", tdx->vp_enter_ret__unsafe);
+	kvm_prepare_unexpected_reason_exit(vcpu, tdx->vp_enter_ret__unsafe);
 	return 0;
 }
 
diff --git a/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h
index ac8323a68b16..5564617fc12a 100644
--- a/arch/x86/kvm/vmx/tdx.h
+++ b/arch/x86/kvm/vmx/tdx.h
@@ -66,7 +66,12 @@ struct vcpu_tdx {
 
 	struct list_head cpu_list;
 
-	u64 vp_enter_ret;
+	/*
+	 * Discourage direct use of the raw VP.ENTER return value, as there are
+	 * several subtleties that need to be accounted for when working with
+	 * the raw value.
+	 */
+	u64 HINT_UNSAFE_IN_KVM(vp_enter_ret);
 
 	enum vcpu_tdx_state state;

  reply	other threads:[~2026-08-05 14:56 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-05  3:12 [PATCH 0/2] KVM: TDX: Enable VM-DoS Prevention Features for TDX Xiaoyao Li
2026-08-05  3:12 ` [PATCH 1/2] KVM: TDX: Enable Notify VM exit Xiaoyao Li
2026-08-05  3:38   ` sashiko-bot
2026-08-05  4:16     ` Xiaoyao Li
2026-08-06 13:33   ` Nikolay Borisov
2026-08-06 13:50     ` Sean Christopherson
2026-08-07  0:27       ` Edgecombe, Rick P
2026-08-07  0:32         ` Sean Christopherson
2026-08-07  1:07           ` Xiaoyao Li
2026-08-07  6:46           ` Nikolay Borisov
2026-08-07  1:06     ` Xiaoyao Li
2026-08-07 14:38       ` Sean Christopherson
2026-08-05  3:12 ` [PATCH 2/2] KVM: TDX: Enable Bus Lock " Xiaoyao Li
2026-08-05  3:46   ` sashiko-bot
2026-08-05  7:53     ` Xiaoyao Li
2026-08-05 14:56       ` Sean Christopherson [this message]
2026-08-06  6:10         ` Xiaoyao Li
2026-08-07 14:51           ` Sean Christopherson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=anNPAXShf1gjDdeu@google.com \
    --to=seanjc@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=xiaoyao.li@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.