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.