Re: [Xen-devel] [RFC PATCH] xen/arm: Deliver interrupts to vcpu specified in IROUTER

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 8 Sep 2014, Ian Campbell wrote:
> On Mon, 2014-09-08 at 19:55 +0530, Vijay Kilari wrote:
> > On Mon, Sep 8, 2014 at 7:33 PM, Vijay Kilari <vijay.kilari@xxxxxxxxx> wrote:
> > > On Mon, Sep 8, 2014 at 7:17 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> 
> > > wrote:
> > >> On Mon, 2014-09-08 at 19:11 +0530, Vijay Kilari wrote:
> > >>
> > >> Why have we dropped the list?
> > >
> > > I have not dropped the list. I still see list in my email.
> > >
> > >>
> > >>> Hi Stefano,
> > >>>
> > >>>  Apart from your comments below, I encounter below scenario.
> > >>>
> > >>> The below function vgic_irq_rank() in vgic.c is not generic. The rank 
> > >>> is always
> > >>> depends on register type it is going to access. So it cannot be
> > >>> just hardcoded.
> > >>>
> > >>> struct vgic_irq_rank *(struct vcpu *v, unsigned int irq)
> > >>> {
> > >>>     return vgic_rank_offset(v, 8, irq, DABT_WORD);
> > >>> }
> > >>>
> > >>> This function works ok for GIC v2. But this cannot be used for
> > >>> GICv3 to access registers like IROUTER which are u64. The rank
> > >>> calcuation goes wrong and there by takes wrong rank lock.
> > >>
> > >> This sounds to me like a bug which should be fixed. A rank is just a
> > >> group of 32 consecutive IRQs and their associated register values, it
> > >> shouldn't matter whether those registers are 8-, 16-, 32- or 64-bits.
> > >
> > >  Now I think of creating a callback to return the right rank for given irq
> > > based on v2 or v3. This will fix and keep the old code intact.
> > 
> > This should fix the issue.
> 
> Looks plausible. Will you fold it into your series?

It looks OK to me too.


> >  struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, unsigned int irq)
> >  {
> > -    return vgic_rank_offset(v, 8, irq, DABT_WORD);
> > +    int rank = irq/32;
> > 
> > +    if ( rank == 0 )
> > +        return v->arch.vgic.private_irqs;
> > +    else if ( rank <= DOMAIN_NR_RANKS(v->domain) )
> > +        return &v->domain->arch.vgic.shared_irqs[rank - 1];
> > +    else
> > +        return NULL;
> > }
> 
> 

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