[U-Boot] [PATCH] ARM926: Add mb to the cache invalidate/flush

[Scott Wood <scottwood@freescale.com>]

On 10/11/2012 06:37:40 PM, Albert ARIBAUD wrote:
> Hi Scott,
> 
> On Thu, 11 Oct 2012 15:21:28 -0500, Scott Wood
> <scottwood@freescale.com> wrote:
> 
> > > http://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Extended-Asm
> > >
> > > "If your assembler instructions access memory in an unpredictable
> > > fashion, add `memory' to the list of clobbered registers. This  
> will
> > > cause GCC to not keep memory values cached in registers across the
> > > assembler instruction and not optimize stores or loads to that  
> memory.
> > > You will also want to add the volatile keyword if the memory  
> affected
> > > is not listed in the inputs or outputs of the asm, as the `memory'
> > > clobber does not count as a side-effect of the asm".
> >
> > "and not optimize stores or loads to that memory".  It's not clear  
> what
> > "that" refers to, since the memory clobber does not refer to  
> specific
> > memory, but given that the purpose is "if your assembler  
> instructions
> > access memory in an unpredictable fashion", I don't see how it  
> could be
> > interpreted as anything other than "any memory which could possibly  
> be
> > modified by the program".  So it excludes constant data, but that's
> > about it.
> 
> It does not necessarily include "all memory". Besides, "that"  -- to  
> me
> -- cleary means "the memory mentioned in the statement, for instance  
> in
> the inputs or outputs.

The whole point of the memory clobber is for when you can't easily  
express the memory in terms of input/output constraints.

> > The only reference to volatile is to tell you to add it to the asm
> > statement (not to other memory accesses) so that the asm statement  
> does
> > not get removed altogether.
> 
> The memory clobber definition says no memory values are kept cached
> in registers across the instruction, that implies that if a volatile
> access was prepared (a memory value was cached in a register) it is
> finished before the asm statement executes, and similarly, since the
> desciption says "across" the instruction, volatiles reads or writes
> located after the instruction won't have been started before ithe
> instruction executest, or they would have needed to cache the value,
> which is contrary to the memory clobber definition.
> 
> This is not to say that only volatiles are affected by the barrier;  
> but
> volatiles certainly are.

Sure, but that first paragraph without the second is misleading as it  
implies that it is only volatiles that are affected.  Maybe not in  
terms of logical inference, but in terms of natural language and the  
meaning people are likely to take from it.

> Regarding Linux spinlocks vs these patches: it's not the same
> situation. spinlock functions are inlined, as you noted, thus a code
> sequence that takes a spinlock, does some accesses, then releases the
> spinlock ends up as a long sequence of instructions. On the contrary,
> the cache functions (which are not going to be inlined any time soon  
> as
> they are strong versions of weak symbols, incompatible with inlining)
> contain a single asm statement, thus adding a memory clobber to this
> statement won't have any effect for lack of preceding or following
> instructions to (not) reorder.

Fine.  I still think it's good practice to always use the memory  
clobber in these situations, as it doesn't hurt anything, and if  
nothing else you'd be setting a good example for others in situations  
that may not be as inline-averse -- but such style issues are up to you  
as maintainer.

-Scott