Re: [PATCH v2 2/3] KVM: TDX: Fix the exit reason handling
Xiaoyao Li <[email protected]>
| Newsgroups | org.kernel.vger.kvm,dev.linux.lists.sashiko-reviews |
|---|---|
| Message-ID | <[email protected]> |
On 8/11/2026 7:57 AM, Sean Christopherson wrote: > On Mon, Aug 10, 2026, Xiaoyao Li wrote: >> On 8/10/2026 7:39 PM, [email protected] wrote: >>>> @@ -2147,7 +2156,7 @@ void tdx_get_exit_info(struct kvm_vcpu *vcpu, u32 *reason, >>>> struct vcpu_tdx *tdx = to_tdx(vcpu); >>>> *reason = tdx->vt.exit_reason.full; >>>> - if (*reason != -1u) { >>>> + if (tdx->vt.exit_reason.basic != -1) { >>> [Severity: Medium] >>> Will this check always evaluate to true due to C integer promotion rules? >>> >>> The basic field in union vmx_exit_reason is a 16-bit unsigned bitfield. When >>> comparing it to -1, the unsigned 16-bit value is promoted to a signed 32-bit >>> integer. If the value was set to -1 (65535), the comparison evaluates as >>> 65535 != -1, which is always true. >>> >> >> Well, how about something below on top of this patch? > > No, we need to not rely on magic exit_reason.basic values. Can't this be? > > if ((tdx->vp_enter_ret & TDX_SW_ERROR) != TDX_SW_ERROR) { I think the reason of checking if (*reason != -1u) in the original commit 095b71a03f49 ("KVM: TDX: Add a place holder to handle TDX VM exit") was that, -1u means there is no valid Exit Reason in the lower 32 bits in vp_enter_ret. Change it to check TDX_SW_ERROR, doesn't look correct to me. Set the EPT_MISCONFIG magic handling aside, TDX_SW_ERROR only means the SEAMCALL instruction faults, e.g., hitting #UD, #GP, or VMFAILINVALID. Just a small subset of the cases where there is no valid Exit Reason. > <read values> > } else { > <zero values> > } > > We'd need to use the unsafe version if we go with my suggestion[*], but other > than that wrinkle, the above seems like the obviously correct fix (maybe too > obvious)? > > [*] https://lore.kernel.org/all/[email protected]