RE: [PATCH for-4.14 v2 1/2] x86/passthrough: do not assert edge triggered GSIs for PVH dom0

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Roger Pau Monne <roger.pau@xxxxxxxxxx>
> Sent: 10 June 2020 15:29
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: paul@xxxxxxx; Roger Pau Monne <roger.pau@xxxxxxxxxx>; Jan Beulich 
> <jbeulich@xxxxxxxx>; Andrew
> Cooper <andrew.cooper3@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>
> Subject: [PATCH for-4.14 v2 1/2] x86/passthrough: do not assert edge 
> triggered GSIs for PVH dom0
> 
> Edge triggered interrupts do not assert the line, so the handling done
> in Xen should also avoid asserting it. Asserting the line prevents
> further edge triggered interrupts on the same vIO-APIC pin from being
> delivered, since the line is not de-asserted.
> 
> One case of such kind of interrupt is the RTC timer, which is edge
> triggered and available to a PVH dom0. Note this should not affect
> domUs, as it only modifies the behavior of IDENTITY_GSI kind of passed
> through interrupts.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Paul Durrant <paul@xxxxxxx>
Release-acked-by: Paul Durrant <paul@xxxxxxx>

> ---
> Changes since v1:
>  - Compare the triggering against VIOAPIC_{EDGE/LEVEL}_TRIG.
> ---
>  xen/arch/x86/hvm/irq.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
> index 9c8adbc495..fd02cf2e8d 100644
> --- a/xen/arch/x86/hvm/irq.c
> +++ b/xen/arch/x86/hvm/irq.c
> @@ -169,9 +169,10 @@ void hvm_pci_intx_deassert(
> 
>  void hvm_gsi_assert(struct domain *d, unsigned int gsi)
>  {
> +    int trig = vioapic_get_trigger_mode(d, gsi);
>      struct hvm_irq *hvm_irq = hvm_domain_irq(d);
> 
> -    if ( gsi >= hvm_irq->nr_gsis )
> +    if ( gsi >= hvm_irq->nr_gsis || trig < 0 )
>      {
>          ASSERT_UNREACHABLE();
>          return;
> @@ -186,9 +187,10 @@ void hvm_gsi_assert(struct domain *d, unsigned int gsi)
>       * to know if the GSI is pending or not.
>       */
>      spin_lock(&d->arch.hvm.irq_lock);
> -    if ( !hvm_irq->gsi_assert_count[gsi] )
> +    if ( trig == VIOAPIC_EDGE_TRIG || !hvm_irq->gsi_assert_count[gsi] )
>      {
> -        hvm_irq->gsi_assert_count[gsi] = 1;
> +        if ( trig == VIOAPIC_LEVEL_TRIG )
> +            hvm_irq->gsi_assert_count[gsi] = 1;
>          assert_gsi(d, gsi);
>      }
>      spin_unlock(&d->arch.hvm.irq_lock);
> @@ -196,11 +198,12 @@ void hvm_gsi_assert(struct domain *d, unsigned int gsi)
> 
>  void hvm_gsi_deassert(struct domain *d, unsigned int gsi)
>  {
> +    int trig = vioapic_get_trigger_mode(d, gsi);
>      struct hvm_irq *hvm_irq = hvm_domain_irq(d);
> 
> -    if ( gsi >= hvm_irq->nr_gsis )
> +    if ( trig <= VIOAPIC_EDGE_TRIG || gsi >= hvm_irq->nr_gsis )
>      {
> -        ASSERT_UNREACHABLE();
> +        ASSERT(trig == VIOAPIC_EDGE_TRIG && gsi < hvm_irq->nr_gsis);
>          return;
>      }
> 
> --
> 2.26.2