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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 24 Jun 2014, Julien Grall wrote:
> 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.

OK


> 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.

OK

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel