Re: [PATCH 2/2] KVM: TDX: Enable Bus Lock VM exit

[Xiaoyao Li <xiaoyao.li@intel.com>]

On 8/5/2026 10:56 PM, Sean Christopherson wrote:
> 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?

Basically, it looks good except some nits.

I'll try to split into a formal sereis. Please let me know if you want 
to do if yourself.

> @@ -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);

We need assign tdx->vp_enter_ret__unsafe somewhere after here. I think 
we can just drop the local vp_enter_ret and make it

	tdx->vp_enter_ret__unsafe = tdx_vcpu_enter_exit(vcpu);

>   
>   	if (vcpu->arch.host_debugctl & ~TDX_DEBUGCTL_PRESERVED)
>   		update_debugctlmsr(vcpu->arch.host_debugctl);

<...>

> -	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))

We need to check tdx->vp_enter_ret__unsafe instead of exit_reason becase 
tdcall_to_vmx_exit_reason() translates 
TDVMCALL(EXIT_REASON_EPT_VIOLATION) from guest to EXIT_REASON_EPT_MISCONFIG

<...>
> 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);

So the purpose is forcing people to think twice when using it because 
they see "__unsafe"? Maybe it's more for the reviewers and maintainers.

>   
>   	enum vcpu_tdx_state state;