Re: [Xen-devel] [PATCH ARM v8 2/4] mini-os: arm: interrupt controller

[Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>]

Ian Campbell, le Tue 21 Oct 2014 12:00:18 +0100, a écrit :
> On Fri, 2014-10-03 at 10:20 +0100, Thomas Leonard wrote:
> > +static inline uint32_t REG_READ32(volatile uint32_t *addr)
> > +{
> > +    uint32_t value;
> > +    __asm__ __volatile__("ldr %0, [%1]":"=&r"(value):"r"(addr));
> > +    rmb();
> 
> I'm not 100% convinced that you need this rmb().
> 
> > +    return value;
> > +}
> > +
> > +static inline void REG_WRITE32(volatile uint32_t *addr, unsigned int value)
> > +{
> > +    __asm__ __volatile__("str %0, [%1]"::"r"(value), "r"(addr));
> > +    wmb();
> > +}

I don't really see why such barriers are needed indeed. Are they needed
to actually push the values out? Or are they needed to synchronize with
others reads & writes in the source code (in which case REG_WRITE32 is
bogus, the barrier should be before the str...). Barriers should usually
be put in the code which needs them, not hidden in operations and thus
getting the thing working by luck only.

Samuel

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