Re: [PATCH 1/2] KVM: x86: ioapic: Set remote_irr only after successful delivery

On Thu, 2026-08-06 at 21:39 +0800, Hao Zhang wrote:
> From: Hao Zhang <[email protected]>
> 
> The I/O APIC sets remote_irr for level-triggered interrupts to track that
> the interrupt has been accepted by a local APIC and that the I/O APIC must
> wait for the corresponding EOI before delivering the interrupt again.
> 
> But ioapic_service() currently sets remote_irr for any non-zero return
> from kvm_irq_delivery_to_apic(). The delivery helper can return -1 when
> no destination is found. Treating -1 as success causes KVM to set
> remote_irr even though no interrupt was delivered and no EOI will ever
> be generated, leaving the pin blocked.
> 
> Set remote_irr only when the delivery result is positive, i.e. when at
> least one local APIC accepted the interrupt.
> 
> Fixes: 4925663a079c ("KVM: Report IRQ injection status to userspace.")
> Signed-off-by: Hao Zhang <[email protected]>
> ---
>  arch/x86/kvm/ioapic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
> index 757667fb2bfa..24a7cc3b8b7e 100644
> --- a/arch/x86/kvm/ioapic.c
> +++ b/arch/x86/kvm/ioapic.c
> @@ -491,7 +491,7 @@ static int ioapic_service(struct kvm_ioapic *ioapic, int irq, bool line_status)
>  	} else
>  		ret = kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
>  
> -	if (ret && irqe.trig_mode == IOAPIC_LEVEL_TRIG)
> +	if (ret > 0 && irqe.trig_mode == IOAPIC_LEVEL_TRIG)
>  		entry->fields.remote_irr = 1;
>  
>  	return ret;
> 
> base-commit: c21bb4193868a8de71fc4693fa741e195fdf5d86

This seems reasonable to me.  Just wondering did you meet any real bug?

Btw, currently the IRQ is set to irr_delivered for level-triggered IRQ
regardless of the return value of kvm_irq_delivery_to_apic():

        if (irqe.trig_mode == IOAPIC_EDGE_TRIG)                                
                ioapic->irr_delivered |= 1 << irq;           

        if (irq == RTC_GSI && line_status) {                                   
		...                                                                          
        } else                                                                 
                ret = kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);

	...

Similarly, should this only be done when kvm_irq_delivery_to_apic() returns
positive?

E.g., kvm_get_ioapic() clears the bits in irr_delivered, so if one IRQ is set in
it but actually not accepted by LAPIC then kvm_get_ioapic() will not return such
IRQ in irr, therefore it can potentially be lost?