Re: [PATCH v3 3/3] KVM: x86: ioapic: Preserve irr_delivered for duplicate edge interrupts

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but changing an IOAPIC pin's trigger mode from edge-triggered to level-triggered via MMIO does not clear `irr_delivered`, causing subsequent level-triggered interrupts to be masked and lost across live migration.
--

--- Patch [3]: [PATCH v3 3/3] KVM: x86: ioapic: Preserve irr_delivered for duplicate edge interrupts ---
commit 591f7b002947a1810eb5f06b6ab21c5e4d90fcae
Author: Hao Zhang <[email protected]>

KVM: x86: ioapic: Preserve irr_delivered for duplicate edge interrupts

This commit moves the state update of irr_delivered after the duplicate edge
check in ioapic_set_irq() to prevent duplicate edge interrupts from leaving
the interrupt visible in the KVM_GET_IRQCHIP state. It also adds a selftest
to verify this behavior.

> diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
> index 24a7cc3b8b7ef..676effd674f9e 100644
> --- a/arch/x86/kvm/ioapic.c
> +++ b/arch/x86/kvm/ioapic.c
> @@ -230,11 +230,11 @@ static int ioapic_set_irq(struct kvm_ioapic *ioapic, unsigned int irq,
>  	old_irr = ioapic->irr;
>  	ioapic->irr |= mask;
>  	if (edge) {
> -		ioapic->irr_delivered &= ~mask;
>  		if (old_irr == ioapic->irr) {
>  			ret = 0;
>  			goto out;
>  		}
> +		ioapic->irr_delivered &= ~mask;
>  	}

[Severity: High]
This is a pre-existing issue, but does changing an IOAPIC pin's trigger mode
from edge-triggered to level-triggered via MMIO leave a stale irr_delivered
bit?

If a guest configures a pin as edge-triggered, an interrupt sets the bit in
ioapic->irr_delivered. If the guest then reconfigures the pin to
level-triggered in ioapic_write_indirect(), the stale irr_delivered bit is
not cleared.

When a subsequent level interrupt is asserted, ioapic_set_irq() sets the irr
bit but skips clearing irr_delivered because the pin is now level-triggered
and edge is false.

During VM migration, kvm_get_ioapic() extracts the state:

	state->irr &= ~ioapic->irr_delivered;

Could this stale irr_delivered bit incorrectly mask out the pending level
interrupt and permanently lose it across live migration?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3