Re: [Xen-devel] [PATCH v2 07/12] x86/IRQ: fix locking around vector management

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.05.19 at 16:45, <roger.pau@xxxxxxxxxx> wrote:
> On Mon, May 13, 2019 at 08:19:04AM -0600, Jan Beulich wrote:
>> >>> On 13.05.19 at 15:48, <roger.pau@xxxxxxxxxx> wrote:
>> > On Wed, May 08, 2019 at 07:10:59AM -0600, Jan Beulich wrote:
>> >> --- a/xen/drivers/passthrough/vtd/iommu.c
>> >> +++ b/xen/drivers/passthrough/vtd/iommu.c
>> >> @@ -2134,11 +2134,16 @@ static void adjust_irq_affinity(struct a
>> >>      unsigned int node = rhsa ? pxm_to_node(rhsa->proximity_domain)
>> >>                               : NUMA_NO_NODE;
>> >>      const cpumask_t *cpumask = &cpu_online_map;
>> >> +    struct irq_desc *desc;
>> >>  
>> >>      if ( node < MAX_NUMNODES && node_online(node) &&
>> >>           cpumask_intersects(&node_to_cpumask(node), cpumask) )
>> >>          cpumask = &node_to_cpumask(node);
>> >> -    dma_msi_set_affinity(irq_to_desc(drhd->iommu->msi.irq), cpumask);
>> >> +
>> >> +    desc = irq_to_desc(drhd->iommu->msi.irq);
>> >> +    spin_lock_irq(&desc->lock);
>> > 
>> > I would use the irqsave/irqrestore variants here for extra safety.
>> 
>> Hmm, maybe. But I think we're in bigger trouble if IRQs indeed
>> ended up enabled at any of the two points where this function
>> gets called.
> 
> I think I'm misreading the above, but if you expect
> adjust_irq_affinity to always be called with interrupts disabled using
> spin_unlock_irq is wrong as it unconditionally enables interrupts.

Oops - s/enabled/disabled/ in my earlier reply.

Jan



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