Re: [Xen-devel] [PATCH v2] x86: irq: Do not BUG_ON multiple unbind calls for shared pirqs

["Durrant, Paul" <pdurrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of
> Varad Gautam
> Sent: 18 December 2019 10:54
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; Gautam, Varad
> <vrd@xxxxxxxxx>; Jan Beulich <jbeulich@xxxxxxxx>; Roger Pau Monné
> <roger.pau@xxxxxxxxxx>
> Subject: [Xen-devel] [PATCH v2] x86: irq: Do not BUG_ON multiple unbind
> calls for shared pirqs
> 
> XEN_DOMCTL_destroydomain creates a continuation if domain_kill -ERESTARTS.
> In that scenario, it is possible to receive multiple _pirq_guest_unbind
> calls for the same pirq from domain_kill, if the pirq has not yet been
> removed from the domain's pirq_tree, as:
>   domain_kill()
>     -> domain_relinquish_resources()
>       -> pci_release_devices()
>         -> pci_clean_dpci_irq()
>           -> pirq_guest_unbind()
>             -> __pirq_guest_unbind()
> 
> For a shared pirq (nr_guests > 1), the first call would zap the current
> domain from the pirq's guests[] list, but the action handler is never
> freed
> as there are other guests using this pirq. As a result, on the second
> call,
> __pirq_guest_unbind searches for the current domain which has been removed
> from the guests[] list, and hits a BUG_ON.
> 
> Make __pirq_guest_unbind safe to be called multiple times by letting xen
> continue if a shared pirq has already been unbound from this guest. The
> PIRQ will be cleaned up from the domain's pirq_tree during the destruction
> in complete_domain_destroy anyways.
> 
> Signed-off-by: Varad Gautam <vrd@xxxxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Paul Durrant <pdurrant@xxxxxxxxxx>

> 
> v2: Split the check on action->nr_guests > 0 and make it an ASSERT,
> reword.
> ---
>  xen/arch/x86/irq.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
> index 5d0d94c..3eb7b22 100644
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -1863,7 +1863,16 @@ static irq_guest_action_t *__pirq_guest_unbind(
> 
>      for ( i = 0; (i < action->nr_guests) && (action->guest[i] != d); i++
> )
>          continue;
> -    BUG_ON(i == action->nr_guests);
> +    if ( i == action->nr_guests ) {
> +        ASSERT(action->nr_guests > 0) ;
> +        /* In case the pirq was shared, unbound for this domain in an
> earlier call, but still
> +         * existed on the domain's pirq_tree, we still reach here if
> there are any later
> +         * unbind calls on the same pirq. Return if such an unbind
> happens. */
> +        if ( action->shareable )
> +            return NULL;
> +        BUG();
> +    }
> +
>      memmove(&action->guest[i], &action->guest[i+1],
>              (action->nr_guests-i-1) * sizeof(action->guest[0]));
>      action->nr_guests--;
> --
> 2.7.4
> 
> 
> 
> 
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
> Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
> Sitz: Berlin
> Ust-ID: DE 289 237 879
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel