Re: [Xen-devel] [PATCH 1/7] xen/arm: fix rank calculation in vgic_vcpu_inject_irq

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 13 Nov 2012, Ian Campbell wrote:
> On Tue, 2012-11-13 at 11:57 +0000, Stefano Stabellini wrote:
> > On Sat, 27 Oct 2012, Tim Deegan wrote:
> > > At 21:47 +0100 on 26 Oct (1351288040), Ian Campbell wrote:
> > > > On Fri, 2012-10-26 at 19:42 +0100, Stefano Stabellini wrote:
> > > > > I think that the problem is the usage of vgic_irq_rank with registers
> > > > > that have 1 bit per interrupt.
> > > > 
> > > > That's very plausible indeed.
> > > > 
> > > > > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> > > > > index 3c3983f..92731b6 100644
> > > > > --- a/xen/arch/arm/vgic.c
> > > > > +++ b/xen/arch/arm/vgic.c
> > > > > @@ -42,13 +42,7 @@
> > > > >   */
> > > > >  static inline int REG_RANK_NR(int b, uint32_t n)
> > > > >  {
> > > > > -    switch ( b )
> > > > > -    {
> > > > > -    case 8: return n >> 3;
> > > > > -    case 4: return n >> 2;
> > > > > -    case 2: return n >> 1;
> > > > > -    default: BUG();
> > > > > -    }
> > > > > +    return n / b;
> > > > 
> > > > All the infrastructure will fall apart if b isn't a power of two, that's
> > > > why I used the switch. Can you just add the appropriate case 1 instead?
> > > > Probably the bug should be a call to an undefined function to make this
> > > > a compile time rather than runtime failure too.
> > > 
> > > BUILD_BUG_ON((b & -b) != b); ?
> > 
> > I can't do that:
> > 
> > error: expression in static assertion is not constant
> 
> I suppose you could make REG_RANK_NR a macro (as it's name implies it
> ought to be, oops!).
> 

REG_RANK_NR is called by vgic_irq_rank, so that would need to become a
macro too


> If not then given that b must be in {1,2,4,8} maybe the switch is
> ok/tolerable? If we end up supporting b=16 or 32 then more thought is
> probably required around the place anyway.

yeah I think is OK

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