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

["hao_zhang_kdev@163.com" <hao_zhang_kdev@163.com>]

On Fri, Aug 07, 2026, Huang, Kai wrote:
> On Thu, 2026-08-06 at 21:39 +0800, Hao Zhang wrote:
> > From: Hao Zhang <zhanghao1@kylinos.cn>
> > 
> > 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 <zhanghao1@kylinos.cn>
> > ---
> >  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?
>

I didn't hit this from a production workload; I found it while testing the
failed-delivery path. But the state corruption is guest/userspace visible,
especially across KVM_GET_IRQCHIP/KVM_SET_IRQCHIP.

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

Thanks, yes, I think you're right. irr_delivered has the same problem for
edge-triggered interrupts: it is used to hide delivered edge IRQs from
KVM_GET_IRQCHIP, but currently it is set before knowing whether delivery
actually succeeded. If delivery fails, KVM_GET_IRQCHIP can incorrectly
drop the pending IRR bit.

I'll send a v2 that gates both remote_irr and irr_delivered on ret > 0, and
extend the selftest to cover the edge-triggered irr_delivered case too.

Thanks,
Hao