Re: [PATCH v5 5/7] KVM: VMX: Factor out wakeup list handling code to KVM

[Sairaj Kodilkar <sarunkod@amd.com>]

On 10/15/2026 12:30 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] kvm: uninitialized return value from vcpu_irq_pending due to KVM_X86_OP_OPTIONAL
> --
> 
> commit 56cc0b2c3b271017f40149e674efc58c033886d7
> Author: Sairaj Kodilkar <sarunkod@amd.com>
> 
> KVM: VMX: Factor out wakeup list handling code to KVM
> 
> This commit moves the per-pCPU list of blocked vCPUs and the wakeup
> handler from VMX to common KVM x86 code to support AMD. It introduces
> a new optional vcpu_irq_pending callback so Intel and AMD can
> determine if a queued vCPU has a notification pending.
> 
>> diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
>> index e213c9ae3e301..5d337f9336c5d 100644
>> --- a/arch/x86/include/asm/kvm-x86-ops.h
>> +++ b/arch/x86/include/asm/kvm-x86-ops.h
>> @@ -30,6 +30,7 @@ KVM_X86_OP(vcpu_reset)
>>  KVM_X86_OP(prepare_switch_to_guest)
>>  KVM_X86_OP(vcpu_load)
>>  KVM_X86_OP(vcpu_put)
>> +KVM_X86_OP_OPTIONAL(vcpu_irq_pending)
> 
> [Severity: Medium]
> Since vcpu_irq_pending() returns a boolean, should this be declared with
> KVM_X86_OP_OPTIONAL_RET0 instead?
> 
> For optional KVM operations, if the active vendor module does not implement
> the callback, KVM patches the static call site to a dummy trampoline. Because
> KVM_X86_OP_OPTIONAL is used here, the trampoline does not clear the return
> register.
> 

Yes this is a valid concern, it should be OPTIONAL_RET0.

Thanks
Sairaj

> [ ... ]
> 
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index aad065d035fbb..0c637a18bcfa4 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
> 
> [ ... ]
> 
>> @@ -2688,6 +2709,79 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> 
> [ ... ]
> 
>> +/*
>> + * Handler for POSTED_INTERRUPT_WAKEUP_VECTOR.
>> + */
>> +static void kvm_pi_wakeup_handler(void)
>> +{
>> +	int cpu = smp_processor_id();
>> +	struct list_head *wakeup_list = &per_cpu(wakeup_vcpus_on_cpu, cpu);
>> +	raw_spinlock_t *spinlock = &per_cpu(wakeup_vcpus_on_cpu_lock, cpu);
>> +	struct kvm_vcpu *vcpu;
>> +
>> +	raw_spin_lock(spinlock);
>> +	list_for_each_entry(vcpu, wakeup_list, pi_wakeup_list) {
>> +		if (kvm_x86_call(vcpu_irq_pending)(vcpu))
> 
> [Severity: Medium]
> Could this evaluate uninitialized garbage if the vendor omits the callback?
> 
> Because KVM_X86_OP_OPTIONAL does not clear the return register, this check
> will read whatever was left in the register. This could lead to unpredictable
> behavior, such as spurious vCPU wakeups or missed wakeups on platforms
> lacking the implementation.
> 
>> +			kvm_vcpu_wake_up(vcpu);
>> +	}
>> +	raw_spin_unlock(spinlock);
>> +}
>