diff mbox

[v3,21/32] powerpc: include default ioremap_nopost() implementation

Message ID 20170411122923.6285-22-lorenzo.pieralisi@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lorenzo Pieralisi April 11, 2017, 12:29 p.m. UTC
The PCI specifications (Rev 3.0, 3.2.5 "Transaction Ordering and Posting")
mandate non-posted configuration transactions. As further highlighted in
the PCIe specifications (4.0 - Rev0.3, "Ordering Considerations for the
Enhanced Configuration Access Mechanism"), through ECAM and
ECAM-derivative configuration mechanism, the memory mapped transactions
from the host CPU into Configuration Requests on the PCI express fabric
may create ordering problems for software because writes to memory
address are typically posted transactions (unless the architecture can
enforce through virtual address mapping non-posted write transactions
behaviour) but writes to Configuration Space are not posted on the PCI
express fabric.

Include the asm-generic ioremap_nopost() implementation (currently
falling back to ioremap_nocache()) to provide a non-posted writes
ioremap interface to kernel subsystems.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
---
 arch/powerpc/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

Comments

Lorenzo Pieralisi April 11, 2017, 2:24 p.m. UTC | #1
On Tue, Apr 11, 2017 at 11:38:48PM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2017-04-11 at 13:29 +0100, Lorenzo Pieralisi wrote:
> > The PCI specifications (Rev 3.0, 3.2.5 "Transaction Ordering and Posting")
> > mandate non-posted configuration transactions. As further highlighted in
> > the PCIe specifications (4.0 - Rev0.3, "Ordering Considerations for the
> > Enhanced Configuration Access Mechanism"), through ECAM and
> > ECAM-derivative configuration mechanism, the memory mapped transactions
> > from the host CPU into Configuration Requests on the PCI express fabric
> > may create ordering problems for software because writes to memory
> > address are typically posted transactions (unless the architecture can
> > enforce through virtual address mapping non-posted write transactions
> > behaviour) but writes to Configuration Space are not posted on the PCI
> > express fabric.
> > 
> > Include the asm-generic ioremap_nopost() implementation (currently
> > falling back to ioremap_nocache()) to provide a non-posted writes
> > ioremap interface to kernel subsystems.
> 
> NAK. As explained in my reply to patch 0.

Ok, point taken. BTW, may I ask you guys to have a look into this
please ?

https://lkml.org/lkml/2017/4/6/743

It is a side effect of this thread (v2), not sure why <asm/io.h>
on powerpc has to include <linux/io.h>.

Thanks,
Lorenzo

> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Cc: Paul Mackerras <paulus@samba.org>
> > ---
> >  arch/powerpc/include/asm/io.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
> > index 5ed2924..6dcd0e2 100644
> > --- a/arch/powerpc/include/asm/io.h
> > +++ b/arch/powerpc/include/asm/io.h
> > @@ -757,6 +757,7 @@ extern void __iomem *ioremap_prot(phys_addr_t address, unsigned long size,
> >  extern void __iomem *ioremap_wc(phys_addr_t address, unsigned long size);
> > >  #define ioremap_nocache(addr, size)	ioremap((addr), (size))
> > >  #define ioremap_uc(addr, size)		ioremap((addr), (size))
> > +#include <asm-generic/ioremap-nopost.h>
> >  
> >  extern void iounmap(volatile void __iomem *addr);
> >
Benjamin Herrenschmidt April 11, 2017, 11:15 p.m. UTC | #2
On Tue, 2017-04-11 at 15:24 +0100, Lorenzo Pieralisi wrote:
> Ok, point taken. BTW, may I ask you guys to have a look into this
> please ?
> 
> https://lkml.org/lkml/2017/4/6/743
> 
> It is a side effect of this thread (v2), not sure why <asm/io.h>
> on powerpc has to include <linux/io.h>.

Not sure how we ended up with that... it's odd indeed.

Michael ? Any reason we can't just remove it ?

Cheers,
Ben.
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index 5ed2924..6dcd0e2 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -757,6 +757,7 @@  extern void __iomem *ioremap_prot(phys_addr_t address, unsigned long size,
 extern void __iomem *ioremap_wc(phys_addr_t address, unsigned long size);
 #define ioremap_nocache(addr, size)	ioremap((addr), (size))
 #define ioremap_uc(addr, size)		ioremap((addr), (size))
+#include <asm-generic/ioremap-nopost.h>
 
 extern void iounmap(volatile void __iomem *addr);