| Message ID | 20220421132114.35118-7-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/ioapic: fix edge triggered interrupt migration | expand |
On 21.04.2022 15:21, Roger Pau Monne wrote: > --- a/xen/arch/x86/io_apic.c > +++ b/xen/arch/x86/io_apic.c > @@ -267,12 +267,47 @@ void __ioapic_write_entry( > unsigned int apic, unsigned int pin, bool raw, > struct IO_APIC_route_entry e) > { > - union entry_union eu = { .entry = e }; > - > if ( raw || !iommu_intremap ) > { > - __io_apic_write(apic, 0x11 + 2 * pin, eu.w2); > - __io_apic_write(apic, 0x10 + 2 * pin, eu.w1); > + union entry_union eu = { .entry = e }; > + union entry_union curr = { > + .entry = __ioapic_read_entry(apic, pin, true), > + }; > + bool masked = true; > + > + if ( curr.entry.mask ) > + { > + /* > + * If pin is currently masked we can update the high part first > + * without worrying about the RTE being in an inconsistent state. > + */ > + if ( curr.w2 != eu.w2 ) > + __io_apic_write(apic, 0x11 + 2 * pin, eu.w2); > + if ( curr.w1 != eu.w1 ) > + __io_apic_write(apic, 0x10 + 2 * pin, eu.w1); > + return; > + } > + > + if ( curr.w1 != eu.w1 && curr.w2 != eu.w2 && !eu.entry.mask ) > + { > + /* > + * If updating both halves mask the entry while updating so > + * interrupts are not injected with an inconsistent RTE. > + */ > + eu.entry.mask = 1; > + masked = false; > + } > + > + if ( curr.w1 != eu.w1 ) > + __io_apic_write(apic, 0x10 + 2 * pin, eu.w1); > + if ( curr.w2 != eu.w2 ) > + __io_apic_write(apic, 0x11 + 2 * pin, eu.w2); > + > + if ( !masked ) > + { > + eu.entry.mask = 0; > + __io_apic_write(apic, 0x10 + 2 * pin, eu.w1); > + } For the write avoidance don't you want to hide differences in the r/o delivery_status field, e.g. by setting curr's to eu's after having read curr? Jan
diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c index f61e56f3d1..1860af7353 100644 --- a/xen/arch/x86/io_apic.c +++ b/xen/arch/x86/io_apic.c @@ -267,12 +267,47 @@ void __ioapic_write_entry( unsigned int apic, unsigned int pin, bool raw, struct IO_APIC_route_entry e) { - union entry_union eu = { .entry = e }; - if ( raw || !iommu_intremap ) { - __io_apic_write(apic, 0x11 + 2 * pin, eu.w2); - __io_apic_write(apic, 0x10 + 2 * pin, eu.w1); + union entry_union eu = { .entry = e }; + union entry_union curr = { + .entry = __ioapic_read_entry(apic, pin, true), + }; + bool masked = true; + + if ( curr.entry.mask ) + { + /* + * If pin is currently masked we can update the high part first + * without worrying about the RTE being in an inconsistent state. + */ + if ( curr.w2 != eu.w2 ) + __io_apic_write(apic, 0x11 + 2 * pin, eu.w2); + if ( curr.w1 != eu.w1 ) + __io_apic_write(apic, 0x10 + 2 * pin, eu.w1); + return; + } + + if ( curr.w1 != eu.w1 && curr.w2 != eu.w2 && !eu.entry.mask ) + { + /* + * If updating both halves mask the entry while updating so + * interrupts are not injected with an inconsistent RTE. + */ + eu.entry.mask = 1; + masked = false; + } + + if ( curr.w1 != eu.w1 ) + __io_apic_write(apic, 0x10 + 2 * pin, eu.w1); + if ( curr.w2 != eu.w2 ) + __io_apic_write(apic, 0x11 + 2 * pin, eu.w2); + + if ( !masked ) + { + eu.entry.mask = 0; + __io_apic_write(apic, 0x10 + 2 * pin, eu.w1); + } } else iommu_update_ire_from_apic(apic, pin, e.raw); @@ -1780,7 +1815,7 @@ static hw_irq_controller ioapic_edge_type = { .startup = startup_edge_ioapic_irq, .shutdown = irq_shutdown_none, .enable = unmask_IO_APIC_irq, - .disable = mask_IO_APIC_irq, + .disable = irq_disable_none, .ack = ack_edge_ioapic_irq, .set_affinity = set_ioapic_affinity_irq, };
When writing an IO-APIC RTE entry make sure incoming interrupts never see a partially updated entry, by masking the pin while doing the update when necessary. Add some logic to attempt to limit the number of writes. With the masking now handled by __ioapic_write_entry itself when necessary, we can drop the setting of the disable hook for IO-APIC edge triggered interrupts. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Note this includes a revert of the first patch in the series. --- xen/arch/x86/io_apic.c | 45 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-)