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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan

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