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]
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.