RFC on writel and writel_relaxed

[Benjamin Herrenschmidt]

On Mon, 2018-03-26 at 18:08 -0400, Sinan Kaya wrote:
> On 3/26/2018 6:01 PM, Benjamin Herrenschmidt wrote:
> > On Mon, 2018-03-26 at 17:46 -0400, Sinan Kaya wrote:
> > > On 3/26/2018 5:30 PM, Arnd Bergmann wrote:
> > > > > But that was never a requirement of writel(),
> > > > > Documentation/memory-barriers.txt gives an explicit example demanding
> > > > > the wmb() before writel() for ordering system memory against writel.
> > > > 
> > > > Indeed, but it's in an example for when to use dma_wmb(), not wmb().
> > > > Adding Alexander Duyck to Cc, he added that section as part of
> > > > 1077fa36f23e ("arch: Add lightweight memory barriers dma_rmb() and
> > > > dma_wmb()"). Also adding the other people that were involved with that.
> > > > 
> > > 
> > > ARM developers can get away with not including wmb() in their code and use
> > > writel() to observe memory writes due to implicit barriers.
> > > 
> > > However, same code will not work on Intel.
> > 
> > Wrong. It will.
> > 
> > You do NOT need wmb between writes to memory and writel.
> 
> If writel() provides such a guarantee, why do I see code sequences like
> 
> wmb()
> writel() 
> 
> all over the place.

Because it was badly documented and people didn't know what to do, or
maybe the underlying mapping is WC ?

I don't know for sure but I can tell you Linus opinion on the matter
back in the days was very clear and that's why we implemented writel
the way we did on powerpc.

> > 
> > > writel() has a compiler barrier in it for x86.
> > > wmb() has a sync operation in it for x86. 
> > > 
> > > Unless wmb() is called, PCIe device won't observe memory updates from the CPU.
> > 
> > This is completely wrong. They will. Intel provides the necessary
> > ordering guarantees without an explicit wmb.
> > 
> 
> I'm still reserving my doubts here. I was told about an explicit
> wmb() requirement last week.

By whome ?

> > Otherwise almost all drivers out there are broken which I very much
> > doubt :-)
> > 
> > Cheers,
> > Ben.
> > 
> > 
> > 
> 
>