The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Xiaoyao Li <xiaoyao.li@intel.com>
Cc: Rick P Edgecombe <rick.p.edgecombe@intel.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	 "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	 "linux-coco@lists.linux.dev" <linux-coco@lists.linux.dev>,
	"kas@kernel.org" <kas@kernel.org>,
	 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	 "nik.borisov@suse.com" <nik.borisov@suse.com>
Subject: Re: [PATCH v3 4/4] KVM: TDX: Enable Bus Lock VM exit
Date: Fri, 14 Aug 2026 15:46:57 +0000	[thread overview]
Message-ID: <an84cenJjf7EqhVM@google.com> (raw)
In-Reply-To: <77567574-d26c-4d2c-b5ce-5da28a7178d6@intel.com>

On Fri, Aug 14, 2026, Xiaoyao Li wrote:
> On 8/13/2026 10:43 PM, Sean Christopherson wrote:
> > On Thu, Aug 13, 2026, Xiaoyao Li wrote:
> > > On 8/13/2026 6:58 AM, Edgecombe, Rick P wrote:

...

> > The more I look at this, the more I'm against shoving garbage into vt->exit_reasons.
> > With tdx_is_exit_reason_valid(), this is trivially easy to handle, *and* explicitly
> > captures the logic instead of subtly rerouting KVM away from meaningful handling.
> 
> tdx_is_exit_reason_valid() doesn't work unless we update the vp_enter_ret to
> one that isn't one of the status code with valid Exit Reason. See below.

Correct.

> About the idea, isn't the purpose of setting the Exit Reason to a synthesized
> one is to avoid the undefined Exit Reason causing false-positives when it
> happens to match one of the existing ones?

Yes.  The nuance is that I'm against clobbering the *full* exit reason.  I am
not against setting exit_reason.basic to something like EXIT_REASON_INVALID_STATE.

> If we are going to use tdx_is_exit_reason_valid(), does it mean we can drop
> the synthesized Exit Reason entirely and check tdx_is_exit_reason_valid()
> everywhere when KVM is going to consume Exit Reason?  e.g., for this initial
> bus_lock_detected bit handling here. we can avoid it by below. (Which makes
> it different for VMX, and somehow prevents consolidating the code between VMX
> and TDX around the exit handlers that we plan to do after series)

I'd rather not, as it would make checking for tdx_is_exit_reason_valid() much
more load bearing.  I.e. I want to minimize the number of flows that *need* to
check tdx_is_exit_reason_valid(), and an easy way to help on that front is to
ensure that exit_reason[31:16] is accurate with respect to what to the last
(attempted) entry to the guest.

> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index b6c30c4c6b84..52360313dd82 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -2169,7 +2169,8 @@ int tdx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t
> fastpath)
>         int ret = __tdx_handle_exit(vcpu, fastpath);
> 
>         /* Exit to user space when bus lock was detected */
> -       if (vmx_get_exit_reason(vcpu).bus_lock_detected) {
> +       if (tdx_is_exit_reason_valid(vcpu) &&
> +           vmx_get_exit_reason(vcpu).bus_lock_detected) {
>                 if (ret > 0) {
>                         vcpu->run->exit_reason = KVM_EXIT_X86_BUS_LOCK;
>                         ret = 0;
> 
> > diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
> > index 95d89d809c19..a1f5b5dc1fa3 100644
> > --- a/arch/x86/kvm/vmx/main.c
> > +++ b/arch/x86/kvm/vmx/main.c
> > @@ -157,6 +157,14 @@ static fastpath_t vt_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags)
> >          return vmx_vcpu_run(vcpu, run_flags);
> >   }
> > +static void vt_handle_exit_irqoff(struct kvm_vcpu *vcpu)
> > +{
> > +       if (is_td_vcpu(vcpu) && !tdx_is_exit_reason_valid(vcpu))
> 
> No, this doesn't work.
> 
> The previous vp_enter_ret was TDX_SUCCESS with
> EXIT_REASON_EXTERNAL_INTERRUPT. KVM handled it and is going to re-enter the
> TD. But wait_for_sept_zap is set by other vCPU thread, KVM decides to return
> directly but the vp_entere_ret isn't updated.

Oh, right, it really is the same situation as emulation_required, sort of.  The
key difference is that wait_for_sept_zap is per-VM, not per-VM, and so if it's
*cleared* between tdx_vcpu_run() and vt_handle_exit_irqoff(), then KVM would get
a false negative and again synthesize a duplicate IRQ.

Actually, can't we just do this?  Logically it's sound: if the exit was handled,
then there's nothing to be done.

diff --git arch/x86/kvm/x86.c arch/x86/kvm/x86.c
index 0626e835e9eb..6cace57e45fd 100644
--- arch/x86/kvm/x86.c
+++ arch/x86/kvm/x86.c
@@ -8374,7 +8374,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
        if (vcpu->arch.xfd_no_write_intercept)
                fpu_sync_guest_vmexit_xfd_state();
 
-       kvm_x86_call(handle_exit_irqoff)(vcpu);
+       if (exit_fastpath != EXIT_FASTPATH_EXIT_HANDLED)
+               kvm_x86_call(handle_exit_irqoff)(vcpu);
 
        if (vcpu->arch.guest_fpu.xfd_err)
                wrmsrq(MSR_IA32_XFD_ERR, 0);

      reply	other threads:[~2026-08-14 15:46 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-12  8:02 [PATCH v3 0/4] KVM: TDX: Enable VM-DoS Prevention Features for TDX Xiaoyao Li
2026-08-12  8:02 ` [PATCH v3 1/4] KVM: TDX: Enable Notify VM exit Xiaoyao Li
2026-08-12  8:02 ` [PATCH v3 2/4] KVM: TDX: Set bits 31:16 to 0 for the synthesized Exit Reason Xiaoyao Li
2026-08-12 21:58   ` Edgecombe, Rick P
2026-08-12  8:02 ` [PATCH v3 3/4] KVM: TDX: Don't assume exit_reason[31:16] as all-0 in tdx_to_vmx_exit_reason() Xiaoyao Li
2026-08-12  8:02 ` [PATCH v3 4/4] KVM: TDX: Enable Bus Lock VM exit Xiaoyao Li
2026-08-12 22:58   ` Edgecombe, Rick P
2026-08-13 11:17     ` Xiaoyao Li
2026-08-13 14:43       ` Sean Christopherson
2026-08-14  0:49         ` Xiaoyao Li
2026-08-14 15:46           ` Sean Christopherson [this message]

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=an84cenJjf7EqhVM@google.com \
    --to=seanjc@google.com \
    --cc=kas@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-coco@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nik.borisov@suse.com \
    --cc=pbonzini@redhat.com \
    --cc=rick.p.edgecombe@intel.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox