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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.12.2019 11:53, Varad Gautam wrote:
> 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.

There must be more to this, seeing the cleanup_domain_irq_pirq()
invocation at the end of pirq_guest_unbind(), which ought to be
reached in the case you describe.

> --- 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 ) {

Brace on its own line please.

> +        ASSERT(action->nr_guests > 0) ;

Stray blank.

> +        /* 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 )

Long lines and malformed comment.

Do you perhaps also want to check that you take this path only
for dying guests?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel