Re: [Xen-devel] [PATCH v4 05/10] xen/arm: keep track of the GICH_LR used for the irq in struct pending_irq

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-03-21 at 16:19 +0000, Stefano Stabellini wrote:
> On Fri, 21 Mar 2014, Ian Campbell wrote:
> > On Wed, 2014-03-19 at 12:32 +0000, Stefano Stabellini wrote:
> > 
> > Strictly you are tracking the last LR which this interrupt was in, since
> > you don't clear p->lr AFAICT. Maybe this is OK and things never get
> > confused by it, but it might have surprising results...
> 
> Actually the patch is clearing p->lr by setting it to nr_lrs, that of
> course is invalid.

Ah, that is a bit non-obvious. Better would be
        #define INVALID_LR ~(type_t)0
and use that.

> > >      }
> > >  
> > >  }
> > > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> > > index bc20a15..ea89057 100644
> > > --- a/xen/include/asm-arm/domain.h
> > > +++ b/xen/include/asm-arm/domain.h
> > > @@ -59,6 +59,7 @@ struct pending_irq
> > >  #define GIC_IRQ_GUEST_VISIBLE  1
> > >  #define GIC_IRQ_GUEST_ENABLED  2
> > >      unsigned long status;
> > > +    uint8_t lr;
> > 
> > Put this next to priority to improve the packing, you've just added
> > another 7 byte hole to the struct on arm64 (and 3 on arm32).
> >
> > (pulling int irq from just out of scope down into the same area might
> > also improve packing on arm64, since irq is just 4 bytes).
> 
> Good idea

There was a tool which would tell you about the holes in your structs.
Now what was it called...



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