Re: [Xen-devel] [PATCH v3 06/15] x86/IRQ: fix locking around vector management

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 17/05/2019 11:47, Jan Beulich wrote:
> All of __{assign,bind,clear}_irq_vector() manipulate struct irq_desc
> fields, and hence ought to be called with the descriptor lock held in
> addition to vector_lock. This is currently the case for only
> set_desc_affinity() (in the common case) and destroy_irq(), which also
> clarifies what the nesting behavior between the locks has to be.
> Reflect the new expectation by having these functions all take a
> descriptor as parameter instead of an interrupt number.
>
> Also take care of the two special cases of calls to set_desc_affinity():
> set_ioapic_affinity_irq() and VT-d's dma_msi_set_affinity() get called
> directly as well, and in these cases the descriptor locks hadn't got
> acquired till now. For set_ioapic_affinity_irq() this means acquiring /
> releasing of the IO-APIC lock can be plain spin_{,un}lock() then.
>
> Drop one of the two leading underscores from all three functions at
> the same time.
>
> There's one case left where descriptors get manipulated with just
> vector_lock held: setup_vector_irq() assumes its caller to acquire
> vector_lock, and hence can't itself acquire the descriptor locks (wrong
> lock order). I don't currently see how to address this.

In practice, the only mutation is setting a bit in cpu_mask for the
shared high priority vectors, so looks to be safe in practice.  The
callers use of the vector_lock looks like a bodge though.

However,  this analysis needs to be added to the comment for
setup_vector_irq(), because the behaviour is extremely fragile and
mustn't change.

>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com> [VT-d]
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

With some form of adjustment to the comment for setup_vector_irq(), and
ideally to the commit message about safety in practice, Acked-by: Andrew
Cooper <andrew.cooper3@citrix.com>

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