Re: [PATCH v4 1/6] KVM: x86: Extract VMX's unhandleable emulation check to common x86

Sean Christopherson <[email protected]>
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.kvm
Message-ID <[email protected]>
On Tue, Jul 28, 2026, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
> > index 04f986e3d439a..924a629c21e2f 100644
> > --- a/arch/x86/kvm/vmx/main.c
> > +++ b/arch/x86/kvm/vmx/main.c
> [ ... ]
> > @@ -939,11 +949,12 @@ struct kvm_x86_ops vt_x86_ops __initdata = {
> >  	.flush_tlb_gva = vt_op(flush_tlb_gva),
> >  	.flush_tlb_guest = vt_op(flush_tlb_guest),
> >  
> > -	.vcpu_pre_run = vt_op(vcpu_pre_run),
> > +	.vcpu_pre_run = vt_op_tdx_only(vcpu_pre_run),
> 
> [Severity: High]
> Does this conditionally assigned NULL pointer cause a module load warning on
> systems without CONFIG_KVM_INTEL_TDX enabled?
> 
> Since vcpu_pre_run is declared as a mandatory hook (KVM_X86_OP(vcpu_pre_run))
> in arch/x86/include/asm/kvm-x86-ops.h, setting it to NULL when TDX is
> disabled will trigger a WARN in kvm_ops_update().
> 
> [ ... ]
> 
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 960ff7fcd8dba..8c3efc1be1fa4 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -8867,6 +8867,11 @@ static int kvm_x86_vcpu_pre_run(struct kvm_vcpu *vcpu)
> >  	    !kvm_apic_init_sipi_allowed(vcpu))
> >  		return -EINVAL;
> >  
> > +	if (kvm_x86_call(unhandleable_emulation_required)(vcpu)) {
> > +		kvm_prepare_emulation_failure_exit(vcpu);
> > +		return 0;
> > +	}
> > +
> >  	return kvm_x86_call(vcpu_pre_run)(vcpu);
> 
> [Severity: High]
> Could this break KVM_RUN by returning an uninitialized register value when TDX
> is disabled?
> 
> When kvm_x86_vcpu_pre_run() calls this static call with a NULL target, the
> static call machinery will patch the call site with a NOP or RET. The CPU will
> execute the NOP and fall through, leaving the return value in the RAX register
> unmodified.
> 
> Since the preceding call to unhandleable_emulation_required() is a boolean
> function that typically returns false (setting AL=0), the uninitialized RAX
> will evaluate as <= 0. This would cause kvm_arch_vcpu_ioctl_run() to
> immediately exit to userspace instead of entering the guest.

Gah, that's what I get for splitting the diff just before posting.  I'll fix
both of these in v2, and make sure to test patch 1 in isolation.
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.