All of lore.kernel.org
 help / color / mirror / Atom feed
From: Xiaoyao Li <xiaoyao.li@intel.com>
To: Sean Christopherson <seanjc@google.com>
Cc: Rick P Edgecombe <rick.p.edgecombe@intel.com>,
	"sashiko-reviews@lists.linux.dev"
	<sashiko-reviews@lists.linux.dev>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: Re: [PATCH v3 3/4] KVM: TDX: Don't assume exit_reason[31:16] as all-0 in tdx_to_vmx_exit_reason()
Date: Mon, 17 Aug 2026 10:49:01 +0800	[thread overview]
Message-ID: <d67eb097-1223-4b68-92d4-1510d3d7be7c@intel.com> (raw)
In-Reply-To: <an8wUfZ9JxvEhtHS@google.com>

On 8/14/2026 11:12 PM, Sean Christopherson wrote:
> On Fri, Aug 14, 2026, Xiaoyao Li wrote:
>> On 8/14/2026 7:44 AM, Sean Christopherson wrote:
>>> On Wed, Aug 12, 2026, Rick P Edgecombe wrote:
>>>> Side note. I really dislike how tangled this area is for something that seems
>>>> like it should be much more straightforward. Deriving partially I think from the
>>>> overloading of the TDVMCALL leafs with the exit reasons. So we have things like:
>>>> ...
>>>> 	case EXIT_REASON_EPT_VIOLATION:
>>>> 		return EXIT_REASON_EPT_MISCONFIG;
>>>> ...
>>>
>>> I peeked at that code again, and FWIW I still think swizzling the exit_reason for
>>> TDVMCALL is the least awful solution.  If we don't do that, then we'll have to
>>> update every single use of the exit_reason to demux TDVMCALL into the "real" exit
>>> reason, which will be a mess.
>>
>> I'm not sure if you read my idea[1]?
>>
>> I think there is only one place KVM cares about the exit_reason TDVMCALL,
>> just the
>>
>> 	case EXIT_REASON_TDCALL:
> 
> No, the massaged exit_reason is also subtley consumed via trace_kvm_exit().

yeah. This is exactly the one I don't like, it looks like 
trace_kvm_exit() is tracing the wrong exit reason, though it is by 
intentional.

> It's also consumed by tdx_complete_emulated_msr():
> 
> 	if (vmx_get_exit_reason(vcpu).basic == EXIT_REASON_MSR_READ)
> 
> and by tdx_interrupt_allowed()
> 
> 	return vmx_get_exit_reason(vcpu).basic != EXIT_REASON_HLT ||
> 	       !to_tdx(vcpu)->vp_enter_args.r12;
> 
> and by tdx_protected_apic_has_interrupt():
> 
> 	if (vmx_get_exit_reason(vcpu).basic != EXIT_REASON_HLT ||
> 	    to_tdx(vcpu)->vp_enter_args.r12)
> 		return false;

Ah, I really should have checked the code more carefully.

>> in tdx_handle_exit().
>> [1]
>> https://lore.kernel.org/all/646f9595-459f-4224-b4e5-4ec2eecc0bc6@intel.com/
>>> And once we track the exit_reason separately from vp_enter_ret, IMO it all becomes
>>> more logical and easier to follow.  vp_enter_ret holds the information about why
>>> VP.ENTER returned/exited, while exit_reason holds information about why the _guest_
>>> exited.  Obviously it's imperfect since we're still fudging EXIT_REASON_EPT_MISCONFIG,
>>> but again, I think that's a better alternative than demuxing exit_reason in multiple
>>> locations.
>>
>> The question do we really need to swizzle EXIT_REASON_TDCALL to other exit
>> reasons ahead? why cannot them just be handled in the central handler for
>> EXIT_REASON_TDCALL?
> 
> Because as above, it's not as central as you think.  If we want to not swizzle
> the exit_reason, then IMO the only sane way to do that is to not track exit_reason
> for TDX vCPUs, i.e. move vcpu_vt.exit_reason back to vcpu_vmx and force TDX to
> always demux vp_enter_ret every time.

How about adding a specific field to track the TDVMCALL leaf? Full diff 
as below (the EPT MISCONFIG part can be split into a separate one)


-----8<--------
diff --git a/arch/x86/include/asm/shared/tdx.h 
b/arch/x86/include/asm/shared/tdx.h
index f20e91d7ac35..61d00100a2b1 100644
--- a/arch/x86/include/asm/shared/tdx.h
+++ b/arch/x86/include/asm/shared/tdx.h
@@ -69,6 +69,13 @@
  #define TD_CTLS_LOCK                   BIT_ULL(TD_CTLS_LOCK_BIT)

  /* TDX hypercall Leaf IDs */
+#define TDVMCALL_CPUID                 10 /* EXIT_REASON_CPUID */
+#define TDVMCALL_HLT                   12 /* EXIT_REASON_HLT */
+#define TDVMCALL_IO_INSTRUCTION                30 /* 
EXIT_REASON_IO_INSTRUCTION */
+#define TDVMCALL_MSR_READ              31 /* EXIT_REASON_MSR_READ */
+#define TDVMCALL_MSR_WRITE             32 /* EXIT_REASON_MSR_WRITE */
+#define TDVMCALL_MMIO                  48 /* EXIT_REASON_EPT_VIOLATION */
+
  #define TDVMCALL_GET_TD_VM_CALL_INFO   0x10000
  #define TDVMCALL_MAP_GPA               0x10001
  #define TDVMCALL_GET_QUOTE             0x10002
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 545b03d9d10b..113bcf301ec9 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -734,6 +734,12 @@ void tdx_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
         local_irq_enable();
  }

+static bool tdx_is_tdvmcall(struct kvm_vcpu *vcpu, u64 
tdvmcall_exit_reason)
+{
+       return vmx_get_exit_reason(vcpu).basic == EXIT_REASON_TDCALL &&
+              to_tdx(vcpu)->tdvmcall_exit_reason == tdvmcall_exit_reason;
+}
+
  bool tdx_interrupt_allowed(struct kvm_vcpu *vcpu)
  {
         /*
@@ -741,7 +747,7 @@ bool tdx_interrupt_allowed(struct kvm_vcpu *vcpu)
          * interrupt is always allowed unless TDX guest calls TDVMCALL 
with HLT,
          * which passes the interrupt blocked flag.
          */
-       return vmx_get_exit_reason(vcpu).basic != EXIT_REASON_HLT ||
+       return !tdx_is_tdvmcall(vcpu, TDVMCALL_HLT) ||
                !to_tdx(vcpu)->vp_enter_args.r12;
  }

@@ -759,7 +765,7 @@ static bool tdx_protected_apic_has_interrupt(struct 
kvm_vcpu *vcpu)
          * otherwise the interrupt would have been serviced at the 
instruction
          * boundary.
          */
-       if (vmx_get_exit_reason(vcpu).basic != EXIT_REASON_HLT ||
+       if (!tdx_is_tdvmcall(vcpu, TDVMCALL_HLT) ||
             to_tdx(vcpu)->vp_enter_args.r12)
                 return false;

@@ -906,28 +912,9 @@ int tdx_vcpu_pre_run(struct kvm_vcpu *vcpu)
         return 1;
  }

-static __always_inline u32 tdcall_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
-{
-       switch (tdvmcall_leaf(vcpu)) {
-       case EXIT_REASON_CPUID:
-       case EXIT_REASON_HLT:
-       case EXIT_REASON_IO_INSTRUCTION:
-       case EXIT_REASON_MSR_READ:
-       case EXIT_REASON_MSR_WRITE:
-               return tdvmcall_leaf(vcpu);
-       case EXIT_REASON_EPT_VIOLATION:
-               return EXIT_REASON_EPT_MISCONFIG;
-       default:
-               break;
-       }
-
-       return EXIT_REASON_TDCALL;
-}
-
  static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
  {
         struct vcpu_tdx *tdx = to_tdx(vcpu);
-       u32 exit_reason;

         switch (tdx->vp_enter_ret & TDX_SEAMCALL_STATUS_MASK) {
         case TDX_SUCCESS:
@@ -935,30 +922,10 @@ static __always_inline u32 
tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
         case TDX_NON_RECOVERABLE_TD:
         case TDX_NON_RECOVERABLE_TD_NON_ACCESSIBLE:
         case TDX_NON_RECOVERABLE_TD_WRONG_APIC_MODE:
-               break;
+               return (u32)tdx->vp_enter_ret;
         default:
                 return -1u;
         }
-
-       exit_reason = tdx->vp_enter_ret;
-
-       switch (exit_reason) {
-       case 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;
-       }
-
-       return exit_reason;
  }

  static noinstr void tdx_vcpu_enter_exit(struct kvm_vcpu *vcpu)
@@ -1096,9 +1063,6 @@ 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;

@@ -1589,27 +1553,6 @@ static int 
tdx_setup_event_notify_interrupt(struct kvm_vcpu *vcpu)
         return 0;
  }

-static int handle_tdvmcall(struct kvm_vcpu *vcpu)
-{
-       switch (tdvmcall_leaf(vcpu)) {
-       case TDVMCALL_MAP_GPA:
-               return tdx_map_gpa(vcpu);
-       case TDVMCALL_REPORT_FATAL_ERROR:
-               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);
-       case TDVMCALL_SETUP_EVENT_NOTIFY_INTERRUPT:
-               return tdx_setup_event_notify_interrupt(vcpu);
-       default:
-               break;
-       }
-
-       tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_SUBFUNC_UNSUPPORTED);
-       return 1;
-}
-
  void tdx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa, int 
pgd_level)
  {
         u64 shared_bit = (pgd_level == 5) ? TDX_SHARED_BIT_PWL_5 :
@@ -2024,12 +1967,56 @@ int tdx_complete_emulated_msr(struct kvm_vcpu 
*vcpu, int err)
                 return 1;
         }

-       if (vmx_get_exit_reason(vcpu).basic == EXIT_REASON_MSR_READ)
+       if (tdx_is_tdvmcall(vcpu, TDVMCALL_MSR_READ))
                 tdvmcall_set_return_val(vcpu, kvm_read_edx_eax(vcpu));

         return 1;
  }

+static int handle_tdcall(struct kvm_vcpu *vcpu)
+{
+       struct vcpu_tdx *tdx = to_tdx(vcpu);
+
+       if (tdvmcall_exit_type(vcpu)) {
+               tdx->tdvmcall_exit_reason = EXIT_REASON_VMCALL;
+               return tdx_emulate_vmcall(vcpu);
+       }
+
+       tdx->tdvmcall_exit_reason = tdvmcall_leaf(vcpu);
+       switch (tdvmcall_leaf(vcpu)) {
+       case TDVMCALL_CPUID:
+               return tdx_emulate_cpuid(vcpu);
+       case TDVMCALL_HLT:
+               return kvm_emulate_halt_noskip(vcpu);
+       case TDVMCALL_IO_INSTRUCTION:
+               return tdx_emulate_io(vcpu);
+       case TDVMCALL_MSR_READ:
+               kvm_ecx_write(vcpu, tdx->vp_enter_args.r12);
+               return kvm_emulate_rdmsr(vcpu);
+       case TDVMCALL_MSR_WRITE:
+               kvm_ecx_write(vcpu, tdx->vp_enter_args.r12);
+               kvm_eax_write(vcpu, tdx->vp_enter_args.r13);
+               kvm_edx_write(vcpu, tdx->vp_enter_args.r13 >> 32);
+               return kvm_emulate_wrmsr(vcpu);
+       case TDVMCALL_MMIO:
+               return tdx_emulate_mmio(vcpu);
+       case TDVMCALL_MAP_GPA:
+               return tdx_map_gpa(vcpu);
+       case TDVMCALL_REPORT_FATAL_ERROR:
+               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);
+       case TDVMCALL_SETUP_EVENT_NOTIFY_INTERRUPT:
+               return tdx_setup_event_notify_interrupt(vcpu);
+       default:
+               break;
+       }
+
+       tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_SUBFUNC_UNSUPPORTED);
+       return 1;
+}

  int tdx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t fastpath)
  {
@@ -2040,11 +2027,6 @@ int tdx_handle_exit(struct kvm_vcpu *vcpu, 
fastpath_t fastpath)
         if (fastpath != EXIT_FASTPATH_NONE)
                 return 1;

-       if (unlikely(vp_enter_ret == EXIT_REASON_EPT_MISCONFIG)) {
-               KVM_BUG_ON(1, vcpu->kvm);
-               return -EIO;
-       }
-
         /*
          * Handle TDX SW errors, including TDX_SEAMCALL_UD, 
TDX_SEAMCALL_GP and
          * TDX_SEAMCALL_VMFAILINVALID.
@@ -2086,26 +2068,12 @@ int tdx_handle_exit(struct kvm_vcpu *vcpu, 
fastpath_t fastpath)
         case EXIT_REASON_EXTERNAL_INTERRUPT:
                 ++vcpu->stat.irq_exits;
                 return 1;
-       case EXIT_REASON_CPUID:
-               return tdx_emulate_cpuid(vcpu);
-       case EXIT_REASON_HLT:
-               return kvm_emulate_halt_noskip(vcpu);
         case EXIT_REASON_TDCALL:
-               return handle_tdvmcall(vcpu);
-       case EXIT_REASON_VMCALL:
-               return tdx_emulate_vmcall(vcpu);
-       case EXIT_REASON_IO_INSTRUCTION:
-               return tdx_emulate_io(vcpu);
-       case EXIT_REASON_MSR_READ:
-               kvm_ecx_write(vcpu, tdx->vp_enter_args.r12);
-               return kvm_emulate_rdmsr(vcpu);
-       case EXIT_REASON_MSR_WRITE:
-               kvm_ecx_write(vcpu, tdx->vp_enter_args.r12);
-               kvm_eax_write(vcpu, tdx->vp_enter_args.r13);
-               kvm_edx_write(vcpu, tdx->vp_enter_args.r13 >> 32);
-               return kvm_emulate_wrmsr(vcpu);
+               return handle_tdcall(vcpu);
         case EXIT_REASON_EPT_MISCONFIG:
-               return tdx_emulate_mmio(vcpu);
+               /* EPT MISCONFIGs are *always* KVM/kernel bugs. */
+               KVM_BUG_ON(1, vcpu->kvm);
+               return -EIO;
         case EXIT_REASON_EPT_VIOLATION:
                 return tdx_handle_ept_violation(vcpu);
         case EXIT_REASON_OTHER_SMI:
diff --git a/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h
index ac8323a68b16..fc9c0c462f77 100644
--- a/arch/x86/kvm/vmx/tdx.h
+++ b/arch/x86/kvm/vmx/tdx.h
@@ -68,6 +68,22 @@ struct vcpu_tdx {

         u64 vp_enter_ret;

+       /*
+        * Valid only when exit_reason is EXIT_REASON_TDCALL.
+        *
+        * When r10 of TDCALL is 0. It means the TDCALL is invoked for a
+        * TDVMCALL leaf that is defined by the GHCI spec. In this case,
+        * tdvmcall_exit_reason is set to the value of r11.
+        *
+        * When r10 of TDCALL is not 0. KVM treats it as a KVM hypercall.
+        * In this case, tdvmcall_exit_reason is set to EXIT_REASON_VMCALL,
+        * as what to invoke KVM hypercall in VMX. If GHCI defines the
+        * number of EXIT_REASON_VMCALL to a new TDVMCALL leaf in the future
+        * and KVM is going to support it, KVM will need to choose a new 
value
+        * for tdvmcall_exit_reason to distinguish it from the KVM 
hypercall.
+        */
+       u64 tdvmcall_exit_reason;
+
         enum vcpu_tdx_state state;

         u64 map_gpa_next;

  reply	other threads:[~2026-08-17  2:49 UTC|newest]

Thread overview: 26+ 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:23   ` sashiko-bot
2026-08-12  9:07     ` Xiaoyao Li
2026-08-12 22:55       ` Edgecombe, Rick P
2026-08-13 23:44         ` Sean Christopherson
2026-08-14  1:02           ` Xiaoyao Li
2026-08-14 15:12             ` Sean Christopherson
2026-08-17  2:49               ` Xiaoyao Li [this message]
2026-08-17 18:19                 ` Sean Christopherson
2026-08-13  0:14       ` Sean Christopherson
2026-08-13  8:42         ` Xiaoyao Li
2026-08-12  8:02 ` [PATCH v3 4/4] KVM: TDX: Enable Bus Lock VM exit Xiaoyao Li
2026-08-12  8:20   ` sashiko-bot
2026-08-12  9:11     ` Xiaoyao Li
2026-08-13  0:06       ` Sean Christopherson
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
2026-08-15 13:28             ` Xiaoyao Li
2026-08-17 18:06               ` 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=d67eb097-1223-4b68-92d4-1510d3d7be7c@intel.com \
    --to=xiaoyao.li@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=rick.p.edgecombe@intel.com \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=seanjc@google.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.