Re: [PATCH v6 4/5] xen/arm: physical irq follow virtual irq

[Julien Grall <julien.grall@linaro.org>]

Hi Stefano,

On 06/23/2014 05:37 PM, Stefano Stabellini wrote:
> -static void gic_irq_set_affinity(struct irq_desc *desc, const cpumask_t *mask)
> +static void gic_irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_mask)
>  {
> -    BUG();
> +    volatile unsigned char *bytereg;
> +    unsigned int mask;
> +
> +    if ( desc == NULL || cpumask_empty(cpu_mask) )
> +        return;

I think this check is pointless. Every irq_callback relies on the desc
is not NULL. Hence, you already check it in irq_set_affinity.

For the cpu_mask, we are in trouble is someone is calling with an empty
mask. I would replace it by an ASSERT.

The check here is pointless here. Every irq_callback rely on the desc is
not NULL. Why this

>  /* XXX different for level vs edge */
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index b2f922c..5a504ad 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -438,6 +438,32 @@ static void vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int ir
>      spin_unlock_irqrestore(&old->arch.vgic.lock, flags);
>  }
>  
> +static inline void irq_set_affinity(struct irq_desc *desc,
> +                                    const cpumask_t *cpu_mask)
> +{
> +    if ( desc != NULL )
> +        desc->handler->set_affinity(desc, cpu_mask);
> +}

This function belongs to the IRQ module. It might be useful for other
part in the future.

I would move this function in irq.c or in asm-arm/irq.h if you want to
keep it as inline function.

Regards,

-- 
Julien Grall