Re: [PATCH v2 2/4] x86/vioapic: issue EOI to dpci when switching pin to edge trigger mode

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.01.2021 15:28, Roger Pau Monne wrote:
> --- a/xen/arch/x86/hvm/vioapic.c
> +++ b/xen/arch/x86/hvm/vioapic.c
> @@ -268,6 +268,17 @@ static void vioapic_write_redirent(
>  
>      spin_unlock(&d->arch.hvm.irq_lock);
>  
> +    if ( ent.fields.trig_mode == VIOAPIC_EDGE_TRIG &&
> +         ent.fields.remote_irr && is_iommu_enabled(d) )
> +            /*
> +             * Since IRR has been cleared and further interrupts can be
> +             * injected also attempt to deassert any virtual line of passed
> +             * through devices using this pin. Switching a pin from level to
> +             * trigger mode can be used as a way to EOI an interrupt at the
> +             * IO-APIC level.
> +             */
> +            hvm_dpci_eoi(d, gsi);
> +
>      if ( is_hardware_domain(d) && unmasked )
>      {
>          /*

I assume in the comment you mean "... from level to edge
mode ...". But this isn't reflected in the if() you add -
you do the same also when the mode doesn't change. Or do
you build on the assumption that ent.fields.remote_irr can
only be set if the prior mode was "level" (in which case
an assertion may be warranted, as I don't think this is
overly obvious)?

Also, looking at this code, is it correct to trigger an IRQ
upon the guest writing the upper half of an unmasked RTE
with remote_irr clear? I'd assume this needs to be strictly
limited to a 1->0 transition of the mask bit. If other code
indeed guarantees this in all cases, perhaps another place
where an assertion would be warranted?

Jan