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

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

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Jan 
> Beulich
> Sent: 05 March 2020 09:37
> To: Gautam, Varad <vrd@xxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Andrew Cooper 
> <andrew.cooper3@xxxxxxxxxx>; Julien Grall
> <julien@xxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Subject: Re: [Xen-devel] [PATCH v3] x86: irq: Do not BUG_ON multiple unbind 
> calls for shared pirqs
> 
> On 29.01.2020 12:47, Jan Beulich wrote:
> > On 29.01.2020 11:30, Roger Pau Monné wrote:
> >> Hello,
> >>
> >> Thanks for the patch! Next time could you please try to reply to the
> >> previous questions before sending a new version:
> >>
> >> https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg00257.html
> >>
> >> On Wed, Jan 29, 2020 at 10:28:07AM +0100, 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.
> >>>
> >>> 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.
> >>
> >> So AFAICT this is because pt_pirq_softirq_active() returns true in
> >> pci_clean_dpci_irq() and hence the iteration is stopped and
> >> hvm_domain_irq(d)->dpci is not set to NULL.
> >>
> >> Would it be possible to clean the already processed IRQs from the
> >> domain pirq_tree?
> >
> > This might work, perhaps by way of invoking unmap_domain_pirq()
> > right after pirq_guest_unbind(), as long as hvm_dirq_assist() (as
> > called from dpci_softirq()) can be made skip all actual work it
> > means to do in such a case. Unfortunately the two ->masked fields
> > acted upon are different between __pirq_guest_unbind() and
> > hvm_dirq_assist().
> 
> Ping? Unless I hear back soon, I'm afraid I'm going to drop this
> patch from my "pending" mail folder, as not being agreed whether
> to stick to the current version or whether to go this alternative
> route. A more "natural" approach to fixing the issue would be
> quite nice, after all.

I'll try to pick this up tomorrow as I helped diagnose the problem being fixed.

  Paul

> 
> Jan
> 
> _______________________________________________
> 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