Re: [PATCH v3 4/4] KVM: TDX: Enable Bus Lock VM exit
Xiaoyao Li <[email protected]>
| Newsgroups | dev.linux.lists.linux-coco,org.kernel.vger.kvm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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:
>>> On Wed, 2026-08-12 at 16:02 +0800, Xiaoyao Li wrote:
>>>> - if (unlikely(READ_ONCE(to_kvm_tdx(vcpu->kvm)->wait_for_sept_zap)))
>>>> + if (unlikely(READ_ONCE(to_kvm_tdx(vcpu->kvm)->wait_for_sept_zap))) {
>>>> + vt->exit_reason.bus_lock_detected = 0;
>>>> return EXIT_FASTPATH_EXIT_HANDLED;
>>>> + }
>>>
>>> This still feels hacky to me. It at least deserves a comment I think. If you
>>> spin another version.
>>
>> I spent more time on this today and I find there is an existing issue. When
>> the previous Exit Reason is EXIT_REASON_EXTERNAL_INTERRUPT, the early return
>> here can go to the path due to the stale exit_reason.
>>
>> kvm_x86_call(handle_exit_irqoff)(vcpu)
>> vmx_handle_exit_irqoff()
>> handle_external_interrupt_irqoff()
>>
>> This makes the host process the external interrupt twice, and maybe more
>> times if the wait_for_sept_zap remains longer.
>>
>> I think we need an separate fix to set
>>
>> vt->exit_reasons.full = TDX_INVALID_EXIT_REASON;
>>
>> so that if some patch is going to consume the stale Exit Reason, it can be
>> caught by TDX_INVALID_EXIT_REASON.
>
> 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.
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? 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)
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.
> + return;
> +
> + vmx_handle_exit_irqoff();
> +}
> +
> static int vt_handle_exit(struct kvm_vcpu *vcpu,
> enum exit_fastpath_completion fastpath)
> {
> @@ -1071,7 +1079,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = {
> .load_mmu_pgd = vt_op(load_mmu_pgd),
>
> .check_intercept = vmx_check_intercept,
> - .handle_exit_irqoff = vmx_handle_exit_irqoff,
> + .handle_exit_irqoff = vt_op(handle_exit_irqoff),
>
> .update_cpu_dirty_logging = vt_op(update_cpu_dirty_logging),