Re: [Xen-devel] [PATCH v4 06/13] x86/IOMMU: don't restrict IRQ affinities to online CPUs

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Jul 16, 2019 at 07:40:57AM +0000, Jan Beulich wrote:
> In line with "x86/IRQ: desc->affinity should strictly represent the
> requested value" the internally used IRQ(s) also shouldn't be restricted
> to online ones. Make set_desc_affinity() (set_msi_affinity() then does
> by implication) cope with a NULL mask being passed (just like
> assign_irq_vector() does), and have IOMMU code pass NULL instead of
> &cpu_online_map (when, for VT-d, there's no NUMA node information
> available).
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

LGTM, just one patch style comment and one code comment:

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

> ---
> v4: New.
> 
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -796,18 +796,26 @@ unsigned int set_desc_affinity(struct ir
>       unsigned long flags;
>       cpumask_t dest_mask;
>   
> -    if (!cpumask_intersects(mask, &cpu_online_map))
> +    if ( mask && !cpumask_intersects(mask, &cpu_online_map) )
>           return BAD_APICID;
>   
>       spin_lock_irqsave(&vector_lock, flags);
> -    ret = _assign_irq_vector(desc, mask);
> +    ret = _assign_irq_vector(desc, mask ?: TARGET_CPUS);
>       spin_unlock_irqrestore(&vector_lock, flags);

I think the patch is somehow mangled at least on my end, there's one
prepended extra space in the non-modified lines AFAICT.

>   
> -    if (ret < 0)
> +    if ( ret < 0 )
>           return BAD_APICID;
>   
> -    cpumask_copy(desc->affinity, mask);

AFAICT you could also avoid the if and just do the same as in the
assign_irq_vector call above and pass TARGET_CPUS if mask is NULL?

Thanks, Roger.

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