Re: [PATCH v2 2/3] KVM: TDX: Fix the exit reason handling
Sean Christopherson <[email protected]>
| Newsgroups | org.kernel.vger.kvm,dev.linux.lists.sashiko-reviews |
|---|---|
| Message-ID | <[email protected]> |
On Mon, Aug 10, 2026, 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) { This would arguably be a bug fix as well, because the "real" EXIT_REASON_EPT_MISCONFIG path gets a false negative. E.g. when getting information for an EPT Misconfig for the tracepoint, KVM really should print all information, not zeros. At a glance, this exact change can probably be a separate patch too. > <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]