Re: [PATCH v2 6.1.y 1/3] KVM: nVMX: Add a helper to get highest pending from Posted Interrupt vector

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The condition `if (max_irr > 0)` incorrectly ignores vector 0, preventing it from being synced to the guest's virtual APIC and leaving the PIR state corrupted.
--

commit 5c23c313090006d2eae5e26f892baf83bf9030f1
Author: Sean Christopherson <seanjc@google.com>

KVM: nVMX: Add a helper to get highest pending from Posted Interrupt vector

This commit adds a helper to retrieve the highest pending vector given a
Posted Interrupt descriptor. The helper returns -1 to indicate no interrupt
is pending, improving consistency with IRR and ISR handling.

Link: https://lore.kernel.org/r/20240607172609.3205077-2-seanjc@google.com

> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index bdc462944cb08..7d8e18dbe8531 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
[ ... ]
> @@ -3818,8 +3819,8 @@ static int vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu)
>  	if (!pi_test_and_clear_on(vmx->nested.pi_desc))
>  		return 0;
>  
> -	max_irr = find_last_bit((unsigned long *)vmx->nested.pi_desc->pir, 256);
> -	if (max_irr != 256) {
> +	max_irr = pi_find_highest_vector(vmx->nested.pi_desc);
> +	if (max_irr > 0) {

[Severity: Medium]
Does this condition inadvertently skip vector 0?

The commit message states that pi_find_highest_vector() returns -1 to indicate
no interrupt pending, so max_irr can be -1 or 0-255. By checking max_irr > 0,
the code skips processing if vector 0 is the highest pending interrupt.

While vector 0 is an invalid APIC interrupt that hardware typically ignores,
skipping it here means KVM does not call __kvm_apic_update_irr(). As a result,
vector 0 could remain stuck in the PIR array instead of being cleared, which
diverges from the lockless PIR/ON protocol.

Should this condition check for max_irr >= 0 or max_irr != -1 instead?

>  		vapic_page = vmx->nested.virtual_apic_map.hva;
>  		if (!vapic_page)
>  			goto mmio_needed;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619203107.2752678-1-main.kalliope@gmail.com?part=1