Re: [PATCH v8 01/10] xen/arm: observe itargets setting in vgic_enable_irqs and vgic_disable_irqs

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Fri, 11 Jul 2014, Julien Grall wrote:
> Hi Stefano,
> 
> On 07/10/2014 07:13 PM, Stefano Stabellini wrote:
> > +/* the rank lock is already taken */
> > +static struct vcpu *_vgic_get_target_vcpu(struct vcpu *v, unsigned int irq)
> > +{
> > +    unsigned long target;
> > +    struct vcpu *v_target;
> > +    struct vgic_irq_rank *rank = vgic_rank_irq(v, irq);
> > +    ASSERT(spin_is_locked(&rank->lock));
> > +
> > +    target = vgic_byte_read(rank->itargets[(irq%32)/4], 0, irq % 4);
> > +    /* 1-N SPI should be delivered as pending to all the vcpus in the
> > +     * mask, but here we just return the first vcpu for simplicity and
> > +     * because it would be too slow to do otherwise. */
> > +    target = find_first_bit(&target, 8);
> > +    ASSERT(target >= 0 && target < v->domain->max_vcpus);
> > +    v_target = v->domain->vcpu[target];
> > +    return v_target;
> > +}
> > +
> > +/* takes the rank lock */
> > +struct vcpu *vgic_get_target_vcpu(struct vcpu *v, unsigned int irq)
> > +{
> > +    struct vcpu *v_target;
> > +    struct vgic_irq_rank *rank = vgic_rank_irq(v, irq);
> > +
> > +    vgic_lock_rank(v, rank);
> > +    v_target = _vgic_get_target_vcpu(v, irq);
> > +    vgic_unlock_rank(v, rank);
> > +    return v_target;
> > +}
> > +
> 
> itarget is gicv2 specific. GICv3 is using irouter. I don't understand
> why vijay change the rank structure in a later patch...
> 
> Those 2 functions should be moved in vgic-v2.c. You may also need to add
> a callback in the vgic structure.

Yes, you are right. I think I'll keep vgic_get_target_vcpu here and just
move _vgic_get_target_vcpu.

> > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> > index a0c07bf..6410280 100644
> > --- a/xen/include/asm-arm/gic.h
> > +++ b/xen/include/asm-arm/gic.h
> > @@ -320,6 +320,8 @@ struct gic_hw_operations {
> >  
> >  void register_gic_ops(const struct gic_hw_operations *ops);
> >  
> > +struct vcpu *vgic_get_target_vcpu(struct vcpu *v, unsigned int irq);
> > +
> 
> This should be moved in vgic.h.

OK