Re: [PATCH] MIPS: io: Add barrier after register read in inX()

[okaya@codeaurora.org]

On 2018-04-23 00:51, Huacai Chen wrote:
> Your patch add rmb() before read in readX(), why inX() need rmb() after 
> read?
> 

I had to double check what ioswab macro does.

/*
  * Raw operations are never swapped in software.  OTOH values that raw
  * operations are working on may or may not have been swapped by the bus
  * hardware.  An example use would be for flash memory that's used for
  * execute in place.
  */
# define __raw_ioswabb(a, x)	(x)
# define __raw_ioswabw(a, x)	(x)
# define __raw_ioswabl(a, x)	(x)
# define __raw_ioswabq(a, x)	(x)
# define ____raw_ioswabq(a, x)	(x)

/* ioswab[bwlq], __mem_ioswab[bwlq] are defined in mangle-port.h */

So, neither my patch nor yours places rmb before read.

Both are placing it after read and it is the right thing.

ioswab is just an endianness conversion macro.

> Huacai
> 
> On Mon, Apr 23, 2018 at 12:31 PM,  <okaya@codeaurora.org> wrote:
>> On 2018-04-22 23:53, Huacai Chen wrote:
>>> 
>>> While a barrier is present in the outX() functions before the 
>>> register
>>> write, a similar barrier is missing in the inX() functions after the
>>> register read. This could allow memory accesses following inX() to
>>> observe stale data.
>>> 
>>> This patch is very similar to commit a1cc7034e33d12dc1 ("MIPS: io: 
>>> Add
>>> barrier after register read in readX()"). Because 
>>> war_io_reorder_wmb()
>>> is both used by writeX() and outX(), if readX() need a barrier then 
>>> so
>>> does inX().
>>> 
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Huacai Chen <chenhc@lemote.com>
>>> ---
>>>  arch/mips/include/asm/io.h | 2 ++
>>>  1 file changed, 2 insertions(+)
>>> 
>>> diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h
>>> index a7d0b83..cea8ad8 100644
>>> --- a/arch/mips/include/asm/io.h
>>> +++ b/arch/mips/include/asm/io.h
>>> @@ -414,6 +414,8 @@ static inline type pfx##in##bwlq##p(unsigned long
>>> port)                     \
>>>         __val = *__addr;                                              
>>>   \
>>>         slow;                                                         
>>>   \
>>>                                                                       
>>>   \
>>> +       /* prevent prefetching of coherent DMA data prematurely */    
>>>   \
>>> +       rmb();                                                        
>>>   \
>>>         return pfx##ioswab##bwlq(__addr, __val);                      
>>>   \
>>>  }
>> 
>> 
>> Typically read barrier is applied after register read.