Re: [PATCH RFC 6/6] x86/ioapic: mask entry while updating

[Jan Beulich <jbeulich@xxxxxxxx>]

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