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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The mandatory `.vcpu_pre_run` hook is conditionally assigned `NULL` for VMX on non-TDX builds, causing a WARN on module load and breaking vCPU execution.
--

--- Patch [1]: [PATCH v4 1/6] KVM: x86: Extract VMX's unhandleable emulation check to common x86 ---
commit 1ae0061cc0844907be897cd2b6f318482c190f81
Author: Sean Christopherson <seanjc@google.com>

KVM: x86: Extract VMX's unhandleable emulation check to common x86

Expose VMX's check for unhandleable emulation as its own kvm_x86_ops hook,
and move the actual pre-KVM_RUN check into common x86.  This will allow
sharing the core logic with KVM's RSM emulation without needed to add a
post-RSM hook, and is a step towards removing the .vcpu_pre_run() hook
entirely.

> 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.

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728004351.887076-1-seanjc@google.com?part=1