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
