linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
@ 2017-04-11 12:28 Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 01/32] PCI: remove __weak tag from pci_remap_iospace() Lorenzo Pieralisi
                   ` (32 more replies)
  0 siblings, 33 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

This patch series[1] is a v3 of a previous version:

v2: https://lkml.org/lkml/2017/3/27/220

v2 -> v3:
	- Created a default ioremap_nopost() implementation in a separate
	  asm-generic header and patched all arches to make use of it
	- Removed PCI drivers patches from the series to simplify the
	  review, they will be posted separately once the ioremap_nopost()
	  interface is settled
	- Fixed devm_ioremap_* BUS offset comments and implemented
	  nopost interface on top of it
	- Added collected tags

v1: https://lkml.org/lkml/2017/2/27/228

v1 -> v2:
	- Changed pci_remap_cfgspace() to more generic ioremap_nopost()
	  interface
	- Added pgprot_nonposted
	- Fixed build errors on arches not relying on asm-generic headers
	- Added PCI versatile host controller driver patch
	- Added missing config space remapping to hisilicon host controller

---------------------
Original cover letter
---------------------

PCI local bus specifications (Rev3.0, 3.2.5 "Transaction Ordering
and Posting") strictly require PCI configuration and I/O Address space
write transactions to be non-posted.

Current crop of DT/ACPI PCI host controllers drivers relies on
the ioremap interface to map ECAM and ECAM-derivative PCI config
regions and pci_remap_iospace() to create a VMA for mapping
PCI host bridge I/O Address space transactions to CPU virtual address
space.

On some platforms (ie ARM/ARM64) ioremap fails to comply with the PCI
configuration non-posted write transactions requirement, because it
provides a memory mapping that issues "bufferable" or, in PCI terms
"posted" write transactions. Likewise, the current pci_remap_iospace()
implementation maps the physical address range that the PCI translates
to I/O space cycles to virtual address space through pgprot_device()
attributes that on eg ARM64 provides a memory mapping issuing
posted writes transactions, which is not PCI specifications compliant.

This patch series[1] addresses both issues in one go:

- It updates the pci_remap_iospace() function to use a page mapping
  that guarantees non-posted write transactions for I/O space addresses
- It adds a kernel API to remap PCI config space resources, so that
  architecture can override it with a mapping implementation that
  guarantees PCI specifications compliancy wrt non-posted write
  configuration transactions
- It updates all PCI host controller implementations (and the generic
  ECAM layer) to use the newly introduced mapping interface

Tested on Juno ECAM based interface (DT/ACPI).

Non-ECAM PCI host controller drivers patches need checking to make
sure that:

- I patched the correct resource region mapping for config space
- There are not any other ways to ensure posted-write completion
  in the respective pci_ops that make the relevant patch unnecessary

[1] git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git
pci/config-io-mappings-fix-v3

Lorenzo Pieralisi (32):
  PCI: remove __weak tag from pci_remap_iospace()
  asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute
  PCI: fix pci_remap_iospace() remap attribute
  asm-generic: add ioremap_nopost() remap interface
  alpha: include default ioremap_nopost() implementation
  avr32: include default ioremap_nopost() implementation
  arc: include default ioremap_nopost() implementation
  cris: include default ioremap_nopost() implementation
  frv: include default ioremap_nopost() implementation
  hexagon: include default ioremap_nopost() implementation
  ia64: include default ioremap_nopost() implementation
  m32r: include default ioremap_nopost() implementation
  m68k: include default ioremap_nopost() implementation
  metag: include default ioremap_nopost() implementation
  microblaze: include default ioremap_nopost() implementation
  mips: include default ioremap_nopost() implementation
  mn10300: include default ioremap_nopost() implementation
  nios2: include default ioremap_nopost() implementation
  openrisc: include default ioremap_nopost() implementation
  parisc: include default ioremap_nopost() implementation
  powerpc: include default ioremap_nopost() implementation
  s390: include default ioremap_nopost() implementation
  sh: include default ioremap_nopost() implementation
  sparc: include default ioremap_nopost() implementation
  tile: include default ioremap_nopost() implementation
  unicore32: include default ioremap_nopost() implementation
  x86: include default ioremap_nopost() implementation
  xtensa: include default ioremap_nopost() implementation
  arm64: implement ioremap_nopost() interface
  arm: implement ioremap_nopost() interface
  lib: fix Devres devm_ioremap_* offset parameter kerneldoc description
  lib: implement Devres ioremap_nopost() interface

 Documentation/driver-model/devres.txt |  3 ++
 arch/alpha/include/asm/io.h           |  1 +
 arch/arc/include/asm/io.h             |  1 +
 arch/arm/include/asm/io.h             |  9 ++++
 arch/arm/mm/ioremap.c                 |  7 +++
 arch/arm/mm/nommu.c                   |  9 ++++
 arch/arm64/include/asm/io.h           | 12 +++++
 arch/avr32/include/asm/io.h           |  1 +
 arch/cris/include/asm/io.h            |  1 +
 arch/frv/include/asm/io.h             |  1 +
 arch/hexagon/include/asm/io.h         |  2 +
 arch/ia64/include/asm/io.h            |  1 +
 arch/m32r/include/asm/io.h            |  1 +
 arch/m68k/include/asm/io.h            |  1 +
 arch/metag/include/asm/io.h           |  2 +
 arch/microblaze/include/asm/io.h      |  1 +
 arch/mips/include/asm/io.h            |  1 +
 arch/mn10300/include/asm/io.h         |  1 +
 arch/nios2/include/asm/io.h           |  1 +
 arch/openrisc/include/asm/io.h        |  2 +
 arch/parisc/include/asm/io.h          |  1 +
 arch/powerpc/include/asm/io.h         |  1 +
 arch/s390/include/asm/io.h            |  1 +
 arch/sh/include/asm/io.h              |  1 +
 arch/sparc/include/asm/io.h           |  1 +
 arch/tile/include/asm/io.h            |  1 +
 arch/unicore32/include/asm/io.h       |  1 +
 arch/x86/include/asm/io.h             |  1 +
 arch/xtensa/include/asm/io.h          |  1 +
 drivers/pci/pci.c                     |  4 +-
 include/asm-generic/ioremap-nopost.h  |  9 ++++
 include/asm-generic/pgtable.h         |  4 ++
 include/linux/device.h                |  2 +
 include/linux/io.h                    |  2 +
 lib/devres.c                          | 84 +++++++++++++++++++++++++++++++++--
 35 files changed, 167 insertions(+), 5 deletions(-)
 create mode 100644 include/asm-generic/ioremap-nopost.h

-- 
2.10.0

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 01/32] PCI: remove __weak tag from pci_remap_iospace()
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 02/32] asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute Lorenzo Pieralisi
                   ` (31 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

pci_remap_iospace() is marked as a weak symbol even though
no architecture is currently overriding it; given that its
implementation internals have already code paths that are
arch specific (ie PCI_IOBASE and ioremap_page_range() attributes)
there is no need to leave the weak symbol in the kernel since the
same functionality can be achieved by customizing per-arch the
corresponding functionality.

Remove the __weak symbol from pci_remap_iospace().

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7904d02..bd98674 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3363,7 +3363,7 @@ unsigned long __weak pci_address_to_pio(phys_addr_t address)
  *	Only architectures that have memory mapped IO functions defined
  *	(and the PCI_IOBASE value defined) should call this function.
  */
-int __weak pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
+int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
 {
 #if defined(PCI_IOBASE) && defined(CONFIG_MMU)
 	unsigned long vaddr = (unsigned long)PCI_IOBASE + res->start;
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 02/32] asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 01/32] PCI: remove __weak tag from pci_remap_iospace() Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 03/32] PCI: fix pci_remap_iospace() " Lorenzo Pieralisi
                   ` (30 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

According to the PCI local bus specifications (Revision 3.0, 3.2.5),
I/O Address space transactions are non-posted. On architectures where
I/O space is implemented through a chunk of memory mapped space mapped
to PCI address space (ie IA64/ARM/ARM64) the memory mapping for the
region backing I/O Address Space transactions determines the I/O
transactions attributes (before the transactions actually reaches the
PCI bus where it is handled according to the PCI specifications).

The kernel lacks a pgprot_* attribute to map memory with type
generating non-posted writes transactions, which therefore needs to
be added.

Add a pgprot_nonposted mapping prot to create a memory mapping for
memory areas requiring non-posted write transactions; make it default to
pgprot_noncached (which should provide a sane default behaviour) but
still allowing architectures on which pgprot_noncached results in posted
write transactions to override the prot with an arch specific
implementation that guarantees non-posted writes transactions.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 include/asm-generic/pgtable.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
index 1fad160..2070172 100644
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -399,6 +399,10 @@ static inline int pud_same(pud_t pud_a, pud_t pud_b)
 #define pgprot_device pgprot_noncached
 #endif
 
+#ifndef pgprot_nonposted
+#define pgprot_nonposted pgprot_noncached
+#endif
+
 #ifndef pgprot_modify
 #define pgprot_modify pgprot_modify
 static inline pgprot_t pgprot_modify(pgprot_t oldprot, pgprot_t newprot)
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 03/32] PCI: fix pci_remap_iospace() remap attribute
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 01/32] PCI: remove __weak tag from pci_remap_iospace() Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 02/32] asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 04/32] asm-generic: add ioremap_nopost() remap interface Lorenzo Pieralisi
                   ` (29 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

According to the PCI local bus specifications (Revision 3.0, 3.2.5),
I/O Address space transactions are non-posted. On architectures where
I/O space is implemented through a chunk of memory mapped space mapped
to PCI address space (ie IA64/ARM/ARM64) the memory mapping for the
region backing I/O Address Space transactions determines the I/O
transactions attributes (before the transactions actually reaches the
PCI bus where it is handled according to the PCI specifications).

Current pci_remap_iospace() interface, that is used to map the PCI I/O
Address Space into virtual address space, use pgprot_device() as memory
attribute for the virtual address mapping, that in some architectures
(ie ARM64) provides non-cacheable but write bufferable mappings (ie
posted writes), which clash with the non-posted write behaviour for I/O
Address Space mandated by the PCI specifications.

Update the prot ioremap_page_range() parameter in pci_remap_iospace()
to pgprot_nonposted to ensure that the virtual mapping backing
I/O Address Space guarantee non-posted write transactions issued
when addressing I/O Address Space through the MMIO mapping.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 drivers/pci/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index bd98674..9e084c0 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3375,7 +3375,7 @@ int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
 		return -EINVAL;
 
 	return ioremap_page_range(vaddr, vaddr + resource_size(res), phys_addr,
-				  pgprot_device(PAGE_KERNEL));
+				  pgprot_nonposted(PAGE_KERNEL));
 #else
 	/* this architecture does not have memory mapped I/O space,
 	   so this function should never be called */
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 04/32] asm-generic: add ioremap_nopost() remap interface
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (2 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 03/32] PCI: fix pci_remap_iospace() " Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 13:39   ` Benjamin Herrenschmidt
  2017-04-11 12:28 ` [PATCH v3 05/32] alpha: include default ioremap_nopost() implementation Lorenzo Pieralisi
                   ` (28 subsequent siblings)
  32 siblings, 1 reply; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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.

Current DT and ACPI host bridge controllers map PCI configuration space
(ECAM and ECAM-derivative) into the virtual address space through
ioremap() calls, that are non-cacheable device accesses on most
architectures, but may provide "bufferable" or "posted" write semantics
in architecture like eg ARM/ARM64 that allow ioremap'ed regions writes
to be buffered in the bus connecting the host CPU to the PCI fabric;
this behaviour, as underlined in the PCIe specifications, may trigger
transactions ordering rules and must be prevented.

Introduce a new generic and explicit API to create a memory mapping for
ECAM and ECAM-derivative config space area (and a corresponding generic
asm-generic header file) that defaults to ioremap_nocache() (which
should provide a sane default behaviour) and can be included by
all architectures that do not require an arch specific memory mapping
for ioremap_nopost() to guarantee non-posted writes behaviour.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
---
 include/asm-generic/ioremap-nopost.h | 9 +++++++++
 1 file changed, 9 insertions(+)
 create mode 100644 include/asm-generic/ioremap-nopost.h

diff --git a/include/asm-generic/ioremap-nopost.h b/include/asm-generic/ioremap-nopost.h
new file mode 100644
index 0000000..015911f
--- /dev/null
+++ b/include/asm-generic/ioremap-nopost.h
@@ -0,0 +1,9 @@
+#ifndef __ASM_GENERIC_IOREMAP_NOPOST_H
+#define __ASM_GENERIC_IOREMAP_NOPOST_H
+
+static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
+{
+	return ioremap_nocache(offset, size);
+}
+
+#endif /* __ASM_GENERIC_IOREMAP_NOPOST_H */
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 05/32] alpha: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (3 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 04/32] asm-generic: add ioremap_nopost() remap interface Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 06/32] avr32: " Lorenzo Pieralisi
                   ` (27 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
---
 arch/alpha/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
index ff40491..5d7cc11 100644
--- a/arch/alpha/include/asm/io.h
+++ b/arch/alpha/include/asm/io.h
@@ -300,6 +300,7 @@ static inline void __iomem * ioremap_nocache(unsigned long offset,
 }
 
 #define ioremap_uc ioremap_nocache
+#include <asm-generic/ioremap-nopost.h>
 
 static inline void iounmap(volatile void __iomem *addr)
 {
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 06/32] avr32: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (4 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 05/32] alpha: include default ioremap_nopost() implementation Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 13:55   ` Nicolas Ferre
  2017-04-11 12:28 ` [PATCH v3 07/32] arc: " Lorenzo Pieralisi
                   ` (26 subsequent siblings)
  32 siblings, 1 reply; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>
Cc: Haavard Skinnemoen <hskinnemoen@gmail.com>
---
 arch/avr32/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
index f855646..b2ff522 100644
--- a/arch/avr32/include/asm/io.h
+++ b/arch/avr32/include/asm/io.h
@@ -298,6 +298,7 @@ extern void __iounmap(void __iomem *addr);
 #define ioremap_wc ioremap_nocache
 #define ioremap_wt ioremap_nocache
 #define ioremap_uc ioremap_nocache
+#include <asm-generic/ioremap-nopost.h>
 
 #define cached(addr) P1SEGADDR(addr)
 #define uncached(addr) P2SEGADDR(addr)
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 07/32] arc: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (5 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 06/32] avr32: " Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 08/32] cris: " Lorenzo Pieralisi
                   ` (25 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
Cc: Vineet Gupta <vgupta@synopsys.com>
---
 arch/arc/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
index c22b181..58686c8 100644
--- a/arch/arc/include/asm/io.h
+++ b/arch/arc/include/asm/io.h
@@ -39,6 +39,7 @@ extern void iounmap(const void __iomem *addr);
 #define ioremap_nocache(phy, sz)	ioremap(phy, sz)
 #define ioremap_wc(phy, sz)		ioremap(phy, sz)
 #define ioremap_wt(phy, sz)		ioremap(phy, sz)
+#include <asm-generic/ioremap-nopost.h>
 
 /*
  * io{read,write}{16,32}be() macros
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 08/32] cris: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (6 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 07/32] arc: " Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 13:15   ` Jesper Nilsson
  2017-04-11 12:28 ` [PATCH v3 09/32] frv: " Lorenzo Pieralisi
                   ` (24 subsequent siblings)
  32 siblings, 1 reply; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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: Niklas Cassel <nks@flawful.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Jesper Nilsson <jesper.nilsson@axis.com>
Cc: Mikael Starvik <starvik@axis.com>
---
 arch/cris/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/cris/include/asm/io.h b/arch/cris/include/asm/io.h
index fe0b2a0..b9c9397 100644
--- a/arch/cris/include/asm/io.h
+++ b/arch/cris/include/asm/io.h
@@ -21,5 +21,6 @@ extern void iounmap(volatile void * __iomem addr);
 extern void __iomem * ioremap_nocache(unsigned long offset, unsigned long size);
 
 #include <asm-generic/io.h>
+#include <asm-generic/ioremap-nopost.h>
 
 #endif
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 09/32] frv: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (7 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 08/32] cris: " Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 10/32] hexagon: " Lorenzo Pieralisi
                   ` (23 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
---
 arch/frv/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/frv/include/asm/io.h b/arch/frv/include/asm/io.h
index 8062fc7..8808502 100644
--- a/arch/frv/include/asm/io.h
+++ b/arch/frv/include/asm/io.h
@@ -290,6 +290,7 @@ static inline void __iomem *ioremap_fullcache(unsigned long physaddr, unsigned l
 
 #define ioremap_wc ioremap_nocache
 #define ioremap_uc ioremap_nocache
+#include <asm-generic/ioremap-nopost.h>
 
 extern void iounmap(void volatile __iomem *addr);
 
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 10/32] hexagon: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (8 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 09/32] frv: " Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 11/32] ia64: " Lorenzo Pieralisi
                   ` (22 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
Cc: Richard Kuo <rkuo@codeaurora.org>
---
 arch/hexagon/include/asm/io.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/hexagon/include/asm/io.h b/arch/hexagon/include/asm/io.h
index 66f5e9a..be630bf 100644
--- a/arch/hexagon/include/asm/io.h
+++ b/arch/hexagon/include/asm/io.h
@@ -197,6 +197,8 @@ static inline void __iomem *ioremap(unsigned long phys_addr, unsigned long size)
 	return ioremap_nocache(phys_addr, size);
 }
 
+#include <asm-generic/ioremap-nopost.h>
+
 static inline void iounmap(volatile void __iomem *addr)
 {
 	__iounmap(addr);
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 11/32] ia64: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (9 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 10/32] hexagon: " Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 12/32] m32r: " Lorenzo Pieralisi
                   ` (21 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/ia64/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/ia64/include/asm/io.h b/arch/ia64/include/asm/io.h
index 5de673a..130956b 100644
--- a/arch/ia64/include/asm/io.h
+++ b/arch/ia64/include/asm/io.h
@@ -434,6 +434,7 @@ static inline void __iomem * ioremap_cache (unsigned long phys_addr, unsigned lo
 }
 #define ioremap_cache ioremap_cache
 #define ioremap_uc ioremap_nocache
+#include <asm-generic/ioremap-nopost.h>
 
 
 /*
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 12/32] m32r: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (10 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 11/32] ia64: " Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 13/32] m68k: " Lorenzo Pieralisi
                   ` (20 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
---
 arch/m32r/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/m32r/include/asm/io.h b/arch/m32r/include/asm/io.h
index 4b0f5e0..1577102 100644
--- a/arch/m32r/include/asm/io.h
+++ b/arch/m32r/include/asm/io.h
@@ -70,6 +70,7 @@ extern void iounmap(volatile void __iomem *addr);
 #define ioremap_wc ioremap_nocache
 #define ioremap_wt ioremap_nocache
 #define ioremap_uc ioremap_nocache
+#include <asm-generic/ioremap-nopost.h>
 
 /*
  * IO bus memory addresses are also 1:1 with the physical address
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 13/32] m68k: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (11 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 12/32] m32r: " Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 14/32] metag: " Lorenzo Pieralisi
                   ` (19 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
---
 arch/m68k/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/m68k/include/asm/io.h b/arch/m68k/include/asm/io.h
index bccd5a9..ce39ee5 100644
--- a/arch/m68k/include/asm/io.h
+++ b/arch/m68k/include/asm/io.h
@@ -3,6 +3,7 @@
 #else
 #include <asm/io_mm.h>
 #endif
+#include <asm-generic/ioremap-nopost.h>
 
 #define readb_relaxed(addr)	readb(addr)
 #define readw_relaxed(addr)	readw(addr)
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 14/32] metag: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (12 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 13/32] m68k: " Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 15/32] microblaze: " Lorenzo Pieralisi
                   ` (18 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
Cc: James Hogan <james.hogan@imgtec.com>
---
 arch/metag/include/asm/io.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/metag/include/asm/io.h b/arch/metag/include/asm/io.h
index 9890f21..c805e0d 100644
--- a/arch/metag/include/asm/io.h
+++ b/arch/metag/include/asm/io.h
@@ -166,4 +166,6 @@ extern void __iounmap(void __iomem *addr);
 #define iounmap(addr)                           \
 	__iounmap(addr)
 
+#include <asm-generic/ioremap-nopost.h>
+
 #endif  /* _ASM_METAG_IO_H */
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 15/32] microblaze: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (13 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 14/32] metag: " Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 16/32] mips: " Lorenzo Pieralisi
                   ` (17 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
Cc: Michal Simek <monstr@monstr.eu>
---
 arch/microblaze/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/microblaze/include/asm/io.h b/arch/microblaze/include/asm/io.h
index 39b6315..4714a6d 100644
--- a/arch/microblaze/include/asm/io.h
+++ b/arch/microblaze/include/asm/io.h
@@ -43,6 +43,7 @@ extern void __iomem *ioremap(phys_addr_t address, unsigned long size);
 #define ioremap_fullcache(addr, size)		ioremap((addr), (size))
 #define ioremap_wc(addr, size)			ioremap((addr), (size))
 #define ioremap_wt(addr, size)			ioremap((addr), (size))
+#include <asm-generic/ioremap-nopost.h>
 
 #endif /* CONFIG_MMU */
 
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 16/32] mips: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (14 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 15/32] microblaze: " Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 17/32] mn10300: " Lorenzo Pieralisi
                   ` (16 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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: Ralf Baechle <ralf@linux-mips.org>
Cc: Huacai Chen <chenhc@lemote.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
 arch/mips/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h
index ecabc00..d8d1bae 100644
--- a/arch/mips/include/asm/io.h
+++ b/arch/mips/include/asm/io.h
@@ -257,6 +257,7 @@ static inline void __iomem * __ioremap_mode(phys_addr_t offset, unsigned long si
 #define ioremap_nocache(offset, size)					\
 	__ioremap_mode((offset), (size), _CACHE_UNCACHED)
 #define ioremap_uc ioremap_nocache
+#include <asm-generic/ioremap-nopost.h>
 
 /*
  * ioremap_cachable -	map bus memory into CPU space
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 17/32] mn10300: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (15 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 16/32] mips: " Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 18/32] nios2: " Lorenzo Pieralisi
                   ` (15 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
Cc: David Howells <dhowells@redhat.com>
---
 arch/mn10300/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mn10300/include/asm/io.h b/arch/mn10300/include/asm/io.h
index 6218935..95a3c20 100644
--- a/arch/mn10300/include/asm/io.h
+++ b/arch/mn10300/include/asm/io.h
@@ -284,6 +284,7 @@ static inline void __iomem *ioremap_nocache(unsigned long offset, unsigned long
 #define ioremap_wc ioremap_nocache
 #define ioremap_wt ioremap_nocache
 #define ioremap_uc ioremap_nocache
+#include <asm-generic/ioremap-nopost.h>
 
 static inline void iounmap(void __iomem *addr)
 {
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 18/32] nios2: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (16 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 17/32] mn10300: " Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:28 ` [PATCH v3 19/32] openrisc: " Lorenzo Pieralisi
                   ` (14 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
Cc: Ley Foon Tan <lftan@altera.com>
---
 arch/nios2/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/nios2/include/asm/io.h b/arch/nios2/include/asm/io.h
index ce072ba..ff8ac76 100644
--- a/arch/nios2/include/asm/io.h
+++ b/arch/nios2/include/asm/io.h
@@ -47,6 +47,7 @@ static inline void iounmap(void __iomem *addr)
 
 #define ioremap_wc ioremap_nocache
 #define ioremap_wt ioremap_nocache
+#include <asm-generic/ioremap-nopost.h>
 
 /* Pages to physical address... */
 #define page_to_phys(page)	virt_to_phys(page_to_virt(page))
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 19/32] openrisc: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (17 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 18/32] nios2: " Lorenzo Pieralisi
@ 2017-04-11 12:28 ` Lorenzo Pieralisi
  2017-04-11 12:29 ` [PATCH v3 20/32] parisc: " Lorenzo Pieralisi
                   ` (13 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

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: Jonas Bonn <jonas@southpole.se>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Stafford Horne <shorne@gmail.com>
Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
---
 arch/openrisc/include/asm/io.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/openrisc/include/asm/io.h b/arch/openrisc/include/asm/io.h
index 7c69139..1cea257 100644
--- a/arch/openrisc/include/asm/io.h
+++ b/arch/openrisc/include/asm/io.h
@@ -48,5 +48,7 @@ static inline void __iomem *ioremap_nocache(phys_addr_t offset,
 			 __pgprot(pgprot_val(PAGE_KERNEL) | _PAGE_CI));
 }
 
+#include <asm-generic/ioremap-nopost.h>
+
 extern void iounmap(void *addr);
 #endif
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 20/32] parisc: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (18 preceding siblings ...)
  2017-04-11 12:28 ` [PATCH v3 19/32] openrisc: " Lorenzo Pieralisi
@ 2017-04-11 12:29 ` Lorenzo Pieralisi
  2017-04-11 12:29 ` [PATCH v3 21/32] powerpc: " Lorenzo Pieralisi
                   ` (12 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:29 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
Cc: "James E.J. Bottomley" <jejb@parisc-linux.org>
Cc: Helge Deller <deller@gmx.de>
---
 arch/parisc/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/parisc/include/asm/io.h b/arch/parisc/include/asm/io.h
index 1a16f1d..373ba75 100644
--- a/arch/parisc/include/asm/io.h
+++ b/arch/parisc/include/asm/io.h
@@ -139,6 +139,7 @@ static inline void __iomem * ioremap(unsigned long offset, unsigned long size)
 #define ioremap_nocache(off, sz)	ioremap((off), (sz))
 #define ioremap_wc			ioremap_nocache
 #define ioremap_uc			ioremap_nocache
+#include <asm-generic/ioremap-nopost.h>
 
 extern void iounmap(const volatile void __iomem *addr);
 
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 21/32] powerpc: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (19 preceding siblings ...)
  2017-04-11 12:29 ` [PATCH v3 20/32] parisc: " Lorenzo Pieralisi
@ 2017-04-11 12:29 ` Lorenzo Pieralisi
  2017-04-11 13:38   ` Benjamin Herrenschmidt
  2017-04-11 12:29 ` [PATCH v3 22/32] s390: " Lorenzo Pieralisi
                   ` (11 subsequent siblings)
  32 siblings, 1 reply; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:29 UTC (permalink / raw)
  To: linux-arm-kernel

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(+)

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);
 
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 22/32] s390: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (20 preceding siblings ...)
  2017-04-11 12:29 ` [PATCH v3 21/32] powerpc: " Lorenzo Pieralisi
@ 2017-04-11 12:29 ` Lorenzo Pieralisi
  2017-04-11 12:29 ` [PATCH v3 23/32] sh: " Lorenzo Pieralisi
                   ` (10 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:29 UTC (permalink / raw)
  To: linux-arm-kernel

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: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
---
 arch/s390/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/s390/include/asm/io.h b/arch/s390/include/asm/io.h
index 437e9af..374fb43 100644
--- a/arch/s390/include/asm/io.h
+++ b/arch/s390/include/asm/io.h
@@ -39,6 +39,7 @@ static inline void __iomem *ioremap(unsigned long offset, unsigned long size)
 static inline void iounmap(volatile void __iomem *addr)
 {
 }
+#include <asm-generic/ioremap-nopost.h>
 
 static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
 {
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 23/32] sh: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (21 preceding siblings ...)
  2017-04-11 12:29 ` [PATCH v3 22/32] s390: " Lorenzo Pieralisi
@ 2017-04-11 12:29 ` Lorenzo Pieralisi
  2017-04-11 12:29 ` [PATCH v3 24/32] sparc: " Lorenzo Pieralisi
                   ` (9 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:29 UTC (permalink / raw)
  To: linux-arm-kernel

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: Rich Felker <dalias@libc.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 arch/sh/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/sh/include/asm/io.h b/arch/sh/include/asm/io.h
index 3280a6b..23814cd 100644
--- a/arch/sh/include/asm/io.h
+++ b/arch/sh/include/asm/io.h
@@ -371,6 +371,7 @@ static inline int iounmap_fixed(void __iomem *addr) { return -EINVAL; }
 #define ioremap_nocache	ioremap
 #define ioremap_uc	ioremap
 #define iounmap		__iounmap
+#include <asm-generic/ioremap-nopost.h>
 
 /*
  * Convert a physical pointer to a virtual kernel pointer for /dev/mem
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 24/32] sparc: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (22 preceding siblings ...)
  2017-04-11 12:29 ` [PATCH v3 23/32] sh: " Lorenzo Pieralisi
@ 2017-04-11 12:29 ` Lorenzo Pieralisi
  2017-04-11 12:29 ` [PATCH v3 25/32] tile: " Lorenzo Pieralisi
                   ` (8 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:29 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
---
 arch/sparc/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/sparc/include/asm/io.h b/arch/sparc/include/asm/io.h
index f6902cf..09bb613 100644
--- a/arch/sparc/include/asm/io.h
+++ b/arch/sparc/include/asm/io.h
@@ -5,6 +5,7 @@
 #else
 #include <asm/io_32.h>
 #endif
+#include <asm-generic/ioremap-nopost.h>
 
 /*
  * Defines used for both SPARC32 and SPARC64
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 25/32] tile: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (23 preceding siblings ...)
  2017-04-11 12:29 ` [PATCH v3 24/32] sparc: " Lorenzo Pieralisi
@ 2017-04-11 12:29 ` Lorenzo Pieralisi
  2017-04-11 12:29 ` [PATCH v3 26/32] unicore32: " Lorenzo Pieralisi
                   ` (7 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:29 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
Cc: Chris Metcalf <cmetcalf@mellanox.com>
---
 arch/tile/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/tile/include/asm/io.h b/arch/tile/include/asm/io.h
index 30f4a21..d3f2b77 100644
--- a/arch/tile/include/asm/io.h
+++ b/arch/tile/include/asm/io.h
@@ -57,6 +57,7 @@ extern void iounmap(volatile void __iomem *addr);
 #define ioremap_wt(physaddr, size)		ioremap(physaddr, size)
 #define ioremap_uc(physaddr, size)		ioremap(physaddr, size)
 #define ioremap_fullcache(physaddr, size)	ioremap(physaddr, size)
+#include <asm-generic/ioremap-nopost.h>
 
 #define mmiowb()
 
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 26/32] unicore32: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (24 preceding siblings ...)
  2017-04-11 12:29 ` [PATCH v3 25/32] tile: " Lorenzo Pieralisi
@ 2017-04-11 12:29 ` Lorenzo Pieralisi
  2017-04-11 12:29 ` [PATCH v3 27/32] x86: " Lorenzo Pieralisi
                   ` (6 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:29 UTC (permalink / raw)
  To: linux-arm-kernel

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: Bjorn Helgaas <bhelgaas@google.com>
Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
---
 arch/unicore32/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/unicore32/include/asm/io.h b/arch/unicore32/include/asm/io.h
index cb1d8fd..71fdfa2 100644
--- a/arch/unicore32/include/asm/io.h
+++ b/arch/unicore32/include/asm/io.h
@@ -38,6 +38,7 @@ extern void __uc32_iounmap(volatile void __iomem *addr);
 #define ioremap_cached(cookie, size)	__uc32_ioremap_cached(cookie, size)
 #define ioremap_nocache(cookie, size)	__uc32_ioremap(cookie, size)
 #define iounmap(cookie)			__uc32_iounmap(cookie)
+#include <asm-generic/ioremap-nopost.h>
 
 #define readb_relaxed readb
 #define readw_relaxed readw
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 27/32] x86: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (25 preceding siblings ...)
  2017-04-11 12:29 ` [PATCH v3 26/32] unicore32: " Lorenzo Pieralisi
@ 2017-04-11 12:29 ` Lorenzo Pieralisi
  2017-04-11 12:29 ` [PATCH v3 28/32] xtensa: " Lorenzo Pieralisi
                   ` (5 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:29 UTC (permalink / raw)
  To: linux-arm-kernel

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: Thomas Gleixner <tglx@linutronix.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Ingo Molnar <mingo@redhat.com>
---
 arch/x86/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 7afb0e2..9d431ef 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -201,6 +201,7 @@ extern void set_iounmap_nonlazy(void);
 #ifdef __KERNEL__
 
 #include <asm-generic/iomap.h>
+#include <asm-generic/ioremap-nopost.h>
 
 /*
  * Convert a virtual cached pointer to an uncached pointer
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 28/32] xtensa: include default ioremap_nopost() implementation
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (26 preceding siblings ...)
  2017-04-11 12:29 ` [PATCH v3 27/32] x86: " Lorenzo Pieralisi
@ 2017-04-11 12:29 ` Lorenzo Pieralisi
  2017-04-11 12:29 ` [PATCH v3 29/32] arm64: implement ioremap_nopost() interface Lorenzo Pieralisi
                   ` (4 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:29 UTC (permalink / raw)
  To: linux-arm-kernel

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: Max Filippov <jcmvbkbc@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Chris Zankel <chris@zankel.net>
---
 arch/xtensa/include/asm/io.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/xtensa/include/asm/io.h b/arch/xtensa/include/asm/io.h
index c38e5a7..a469bef 100644
--- a/arch/xtensa/include/asm/io.h
+++ b/arch/xtensa/include/asm/io.h
@@ -55,6 +55,7 @@ static inline void __iomem *ioremap_cache(unsigned long offset,
 
 #define ioremap_wc ioremap_nocache
 #define ioremap_wt ioremap_nocache
+#include <asm-generic/ioremap-nopost.h>
 
 static inline void __iomem *ioremap(unsigned long offset, unsigned long size)
 {
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 29/32] arm64: implement ioremap_nopost() interface
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (27 preceding siblings ...)
  2017-04-11 12:29 ` [PATCH v3 28/32] xtensa: " Lorenzo Pieralisi
@ 2017-04-11 12:29 ` Lorenzo Pieralisi
  2017-04-11 12:29 ` [PATCH v3 30/32] arm: " Lorenzo Pieralisi
                   ` (3 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:29 UTC (permalink / raw)
  To: linux-arm-kernel

The PCI bus specifications (rev 3.0, 3.2.5 "Transaction Ordering
and Posting") defines rules for PCI configuration space transactions
ordering and posting, that state that configuration writes
are non-posted transactions.

This rule is reinforced by the ARM v8 architecture reference manual
(issue A.k, Early Write Acknowledgment) that explicitly recommends
that No Early Write Acknowledgment attribute should be used to map
PCI configuration (write) transactions.

Current ioremap interface on ARM64 implements mapping functions
where the Early Write Acknowledgment hint is enabled, so they
cannot be used to map PCI configuration space in a PCI specs
compliant way.

Implement an ARM64 specific ioremap_nopost() interface
that allows to map PCI config region with nGnRnE attributes, providing
a remap function that complies with PCI specifications and the ARMv8
architecture reference manual recommendations.

Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/io.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
index 0c00c87..1a703e5 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -173,6 +173,18 @@ extern void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size);
 #define iounmap				__iounmap
 
 /*
+ * ioremap implementation providing non-posted writes (ie v8 no Early
+ * Write Acknowledgment) semantics.
+ *
+ * PCI specifications disallows posted write configuration transactions.
+ * Add an arch specific ioremap_post definition that is implemented
+ * through nGnRnE device memory attribute as recommended by the ARM v8
+ * Architecture reference manual Issue A.k B2.8.2 "Device memory" so
+ * that it can be used to map PCI config space memory areas.
+ */
+#define ioremap_nopost(addr, size) __ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRnE))
+
+/*
  * io{read,write}{16,32,64}be() macros
  */
 #define ioread16be(p)		({ __u16 __v = be16_to_cpu((__force __be16)__raw_readw(p)); __iormb(); __v; })
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 30/32] arm: implement ioremap_nopost() interface
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (28 preceding siblings ...)
  2017-04-11 12:29 ` [PATCH v3 29/32] arm64: implement ioremap_nopost() interface Lorenzo Pieralisi
@ 2017-04-11 12:29 ` Lorenzo Pieralisi
  2017-04-11 12:29 ` [PATCH v3 31/32] lib: fix Devres devm_ioremap_* offset parameter kerneldoc description Lorenzo Pieralisi
                   ` (2 subsequent siblings)
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:29 UTC (permalink / raw)
  To: linux-arm-kernel

The PCI bus specifications (rev 3.0, 3.2.5 "Transaction Ordering
and Posting") define rules for PCI configuration space transactions
ordering and posting, that state that configuration writes have to
be non-posted transactions.

Current ioremap interface on ARM provides mapping functions that
provide "bufferable" writes transactions (ie ioremap uses MT_DEVICE
memory type) aka posted writes, so PCI host controller drivers have
no arch interface to remap PCI configuration space with memory
attributes that comply with the PCI specifications for configuration
space.

Implement an ARM specific ioremap_nopost() interface that allows to
map PCI config memory regions with MT_UNCACHED memory type (ie strongly
ordered - non-posted writes), providing a remap function that complies
with PCI specifications for config space transactions.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Russell King <linux@armlinux.org.uk>
---
 arch/arm/include/asm/io.h | 9 +++++++++
 arch/arm/mm/ioremap.c     | 7 +++++++
 arch/arm/mm/nommu.c       | 9 +++++++++
 3 files changed, 25 insertions(+)

diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 42871fb..28b15be 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -352,6 +352,7 @@ static inline void memcpy_toio(volatile void __iomem *to, const void *from,
  * mapping has specific properties.
  *
  * Function		Memory type	Cacheability	Cache hint
+ * ioremap_nopost()	SO		n/a		n/a
  * ioremap()		Device		n/a		n/a
  * ioremap_nocache()	Device		n/a		n/a
  * ioremap_cache()	Normal		Writeback	Read allocate
@@ -372,6 +373,12 @@ static inline void memcpy_toio(volatile void __iomem *to, const void *from,
  * compiler may generate unaligned accesses - eg, via inlining its own
  * memcpy.
  *
+ * ioremap_nopost() maps memory as strongly ordered, to be used for
+ * specific mappings (eg PCI config space) that require non-posted
+ * write transactions. Strongly ordered transactions are ordered wrt
+ * device mappings, which means that ioremap_nopost() is the same
+ * as ioremap() except for non-posted writes behaviour.
+ *
  * All normal memory mappings have the following properties:
  * - reads can be repeated with no side effects
  * - repeated reads return the last value written
@@ -407,6 +414,8 @@ void __iomem *ioremap_wc(resource_size_t res_cookie, size_t size);
 #define ioremap_wc ioremap_wc
 #define ioremap_wt ioremap_wc
 
+void __iomem *ioremap_nopost(resource_size_t res_cookie, size_t size);
+
 void iounmap(volatile void __iomem *iomem_cookie);
 #define iounmap iounmap
 
diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index ff0eed2..4ffaf16 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -463,6 +463,13 @@ void iounmap(volatile void __iomem *cookie)
 }
 EXPORT_SYMBOL(iounmap);
 
+void __iomem *ioremap_nopost(resource_size_t res_cookie, size_t size)
+{
+	return arch_ioremap_caller(res_cookie, size, MT_UNCACHED,
+				   __builtin_return_address(0));
+}
+EXPORT_SYMBOL_GPL(ioremap_nopost);
+
 #ifdef CONFIG_PCI
 static int pci_ioremap_mem_type = MT_DEVICE;
 
diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
index 3b5c7aa..dfd736a 100644
--- a/arch/arm/mm/nommu.c
+++ b/arch/arm/mm/nommu.c
@@ -21,6 +21,8 @@
 #include <asm/mpu.h>
 #include <asm/procinfo.h>
 
+#include <asm/mach/map.h>
+
 #include "mm.h"
 
 unsigned long vectors_base;
@@ -433,6 +435,13 @@ void __iomem *ioremap_wc(resource_size_t res_cookie, size_t size)
 }
 EXPORT_SYMBOL(ioremap_wc);
 
+void __iomem *ioremap_nopost(resource_size_t res_cookie, size_t size)
+{
+	return __arm_ioremap_caller(res_cookie, size, MT_UNCACHED,
+				    __builtin_return_address(0));
+}
+EXPORT_SYMBOL(ioremap_nopost);
+
 void *arch_memremap_wb(phys_addr_t phys_addr, size_t size)
 {
 	return (void *)phys_addr;
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 31/32] lib: fix Devres devm_ioremap_* offset parameter kerneldoc description
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (29 preceding siblings ...)
  2017-04-11 12:29 ` [PATCH v3 30/32] arm: " Lorenzo Pieralisi
@ 2017-04-11 12:29 ` Lorenzo Pieralisi
  2017-04-11 12:29 ` [PATCH v3 32/32] lib: implement Devres ioremap_nopost() interface Lorenzo Pieralisi
  2017-04-11 13:38 ` [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Benjamin Herrenschmidt
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:29 UTC (permalink / raw)
  To: linux-arm-kernel

The offset parameter in Devres devm_ioremap_* functions kerneldoc
entries is erroneously defined as BUS offset whereas it is actually
a resource address.

Since it is actually misleading, fix the Devres devm_ioremap_* offset
parameter kerneldoc entry by replacing BUS offset with a more
suitable description (ie Resource address).

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Tejun Heo <tj@kernel.org>
---
 lib/devres.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/devres.c b/lib/devres.c
index cb1464c..78eca71 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -17,7 +17,7 @@ static int devm_ioremap_match(struct device *dev, void *res, void *match_data)
 /**
  * devm_ioremap - Managed ioremap()
  * @dev: Generic device to remap IO address for
- * @offset: BUS offset to map
+ * @offset: Resource address to map
  * @size: Size of map
  *
  * Managed ioremap().  Map is automatically unmapped on driver detach.
@@ -45,7 +45,7 @@ EXPORT_SYMBOL(devm_ioremap);
 /**
  * devm_ioremap_nocache - Managed ioremap_nocache()
  * @dev: Generic device to remap IO address for
- * @offset: BUS offset to map
+ * @offset: Resource address to map
  * @size: Size of map
  *
  * Managed ioremap_nocache().  Map is automatically unmapped on driver
@@ -74,7 +74,7 @@ EXPORT_SYMBOL(devm_ioremap_nocache);
 /**
  * devm_ioremap_wc - Managed ioremap_wc()
  * @dev: Generic device to remap IO address for
- * @offset: BUS offset to map
+ * @offset: Resource address to map
  * @size: Size of map
  *
  * Managed ioremap_wc().  Map is automatically unmapped on driver detach.
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 32/32] lib: implement Devres ioremap_nopost() interface
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (30 preceding siblings ...)
  2017-04-11 12:29 ` [PATCH v3 31/32] lib: fix Devres devm_ioremap_* offset parameter kerneldoc description Lorenzo Pieralisi
@ 2017-04-11 12:29 ` Lorenzo Pieralisi
  2017-04-11 13:38 ` [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Benjamin Herrenschmidt
  32 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 12:29 UTC (permalink / raw)
  To: linux-arm-kernel

The introduction of the ioremap_nopost() interface allows
kernel drivers to map memory through a dedicated kernel
interface providing non-posted writes semantics.

Introduce two new functions in the Devres kernel layer and Devres
documentation:

- devm_ioremap_nopost()
- devm_ioremap_nopost_resource()

so that drivers can make use of devm_* interface to map memory
regions requiring non-posted writes memory attributes.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Tejun Heo <tj@kernel.org>
---
 Documentation/driver-model/devres.txt |  3 ++
 include/linux/device.h                |  2 +
 include/linux/io.h                    |  2 +
 lib/devres.c                          | 78 +++++++++++++++++++++++++++++++++++
 4 files changed, 85 insertions(+)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index bf34d5b..9991a66 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -294,7 +294,10 @@ IOMAP
   devm_ioremap()
   devm_ioremap_nocache()
   devm_ioremap_wc()
+  devm_ioremap_nopost()
   devm_ioremap_resource() : checks resource, requests memory region, ioremaps
+  devm_ioremap_nopost_resource() : do devm_ioremap_resource() with nopost
+				   memory attributes
   devm_iounmap()
   pcim_iomap()
   pcim_iomap_regions()	: do request_region() and iomap() on multiple BARs
diff --git a/include/linux/device.h b/include/linux/device.h
index 9ef518a..1dce865 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -689,6 +689,8 @@ extern unsigned long devm_get_free_pages(struct device *dev,
 extern void devm_free_pages(struct device *dev, unsigned long addr);
 
 void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res);
+void __iomem *devm_ioremap_nopost_resource(struct device *dev,
+					   struct resource *res);
 
 /* allows to add/remove a custom action to devres stack */
 int devm_add_action(struct device *dev, void (*action)(void *), void *data);
diff --git a/include/linux/io.h b/include/linux/io.h
index 82ef36e..e34d799 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -79,6 +79,8 @@ void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
 				   resource_size_t size);
 void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
 				   resource_size_t size);
+void __iomem *devm_ioremap_nopost(struct device *dev, resource_size_t offset,
+				   resource_size_t size);
 void devm_iounmap(struct device *dev, void __iomem *addr);
 int check_signature(const volatile void __iomem *io_addr,
 			const unsigned char *signature, int length);
diff --git a/lib/devres.c b/lib/devres.c
index 78eca71..d16bd76 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -100,6 +100,34 @@ void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
 EXPORT_SYMBOL(devm_ioremap_wc);
 
 /**
+ * devm_ioremap_nopost - Managed ioremap_nopost()
+ * @dev: Generic device to remap IO address for
+ * @offset: Resource address to map
+ * @size: Size of map
+ *
+ * Managed ioremap_nopost().  Map is automatically unmapped on driver detach.
+ */
+void __iomem *devm_ioremap_nopost(struct device *dev, resource_size_t offset,
+			      resource_size_t size)
+{
+	void __iomem **ptr, *addr;
+
+	ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return NULL;
+
+	addr = ioremap_nopost(offset, size);
+	if (addr) {
+		*ptr = addr;
+		devres_add(dev, ptr);
+	} else
+		devres_free(ptr);
+
+	return addr;
+}
+EXPORT_SYMBOL(devm_ioremap_nopost);
+
+/**
  * devm_iounmap - Managed iounmap()
  * @dev: Generic device to unmap for
  * @addr: Address to unmap
@@ -163,6 +191,56 @@ void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res)
 }
 EXPORT_SYMBOL(devm_ioremap_resource);
 
+/**
+ * devm_ioremap_nopost_resource() - devm_ioremap_resource() nopost version
+ * @dev: generic device to handle the resource for
+ * @res: resource to be handled
+ *
+ * Checks that a resource is a valid memory region, requests the memory
+ * region and ioremaps it with ioremap_nopost() interface.
+ * All operations are managed and will be undone on driver detach.
+ *
+ * Returns a pointer to the remapped memory or an ERR_PTR() encoded error code
+ * on failure. Usage example:
+ *
+ *	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ *	base = devm_ioremap_nopost_resource(&pdev->dev, res);
+ *	if (IS_ERR(base))
+ *		return PTR_ERR(base);
+ */
+void __iomem *devm_ioremap_nopost_resource(struct device *dev,
+					   struct resource *res)
+{
+	resource_size_t size;
+	const char *name;
+	void __iomem *dest_ptr;
+
+	BUG_ON(!dev);
+
+	if (!res || resource_type(res) != IORESOURCE_MEM) {
+		dev_err(dev, "invalid resource\n");
+		return IOMEM_ERR_PTR(-EINVAL);
+	}
+
+	size = resource_size(res);
+	name = res->name ?: dev_name(dev);
+
+	if (!devm_request_mem_region(dev, res->start, size, name)) {
+		dev_err(dev, "can't request region for resource %pR\n", res);
+		return IOMEM_ERR_PTR(-EBUSY);
+	}
+
+	dest_ptr = devm_ioremap_nopost(dev, res->start, size);
+	if (!dest_ptr) {
+		dev_err(dev, "ioremap failed for resource %pR\n", res);
+		devm_release_mem_region(dev, res->start, size);
+		dest_ptr = IOMEM_ERR_PTR(-ENOMEM);
+	}
+
+	return dest_ptr;
+}
+EXPORT_SYMBOL(devm_ioremap_nopost_resource);
+
 #ifdef CONFIG_HAS_IOPORT_MAP
 /*
  * Generic iomap devres
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 08/32] cris: include default ioremap_nopost() implementation
  2017-04-11 12:28 ` [PATCH v3 08/32] cris: " Lorenzo Pieralisi
@ 2017-04-11 13:15   ` Jesper Nilsson
  0 siblings, 0 replies; 63+ messages in thread
From: Jesper Nilsson @ 2017-04-11 13:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 11, 2017 at 01:28:48PM +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.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Niklas Cassel <nks@flawful.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>

For the CRIS-part:

Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson at axis.com

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
                   ` (31 preceding siblings ...)
  2017-04-11 12:29 ` [PATCH v3 32/32] lib: implement Devres ioremap_nopost() interface Lorenzo Pieralisi
@ 2017-04-11 13:38 ` Benjamin Herrenschmidt
  2017-04-11 14:08   ` Lorenzo Pieralisi
  32 siblings, 1 reply; 63+ messages in thread
From: Benjamin Herrenschmidt @ 2017-04-11 13:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2017-04-11 at 13:28 +0100, Lorenzo Pieralisi wrote:
> This patch series[1] is a v3 of a previous version:
> 
> v2: https://lkml.org/lkml/2017/3/27/220

I am not a fan of this at All.

That whole concept of "ioremap_nopost" is simply not applicable to the
majority of architectures and certainly not in a way that can apply to
arbitrary mappings.

It's also very wrong to provide a "default" operation whose semantics
are weaker than what it's supposed to implement. Very wrong actually.
People will use it assuming the non-posted behaviour and things will
break in subtle way when it cannot be provided.

What exactly are you trying to fix here ?

If a given PCIe host bridge (architecture specific) require a special
sauce to provide the illusion of non-posting, then implement this in
the actual root complex code.

BTW. I'm pretty sure we "accidentally" made config writes posted at
least to the PHB on a number of powerpc systems forever and we *never*
had a problem because of it ;)

> v2 -> v3:
> 	- Created a default ioremap_nopost() implementation in a
> separate
> 	??asm-generic header and patched all arches to make use of it
> 	- Removed PCI drivers patches from the series to simplify the
> 	??review, they will be posted separately once the
> ioremap_nopost()
> 	??interface is settled
> 	- Fixed devm_ioremap_* BUS offset comments and implemented
> 	??nopost interface on top of it
> 	- Added collected tags
> 
> v1: https://lkml.org/lkml/2017/2/27/228
> 
> v1 -> v2:
> 	- Changed pci_remap_cfgspace() to more generic ioremap_nopost()
> 	??interface
> 	- Added pgprot_nonposted
> 	- Fixed build errors on arches not relying on asm-generic
> headers
> 	- Added PCI versatile host controller driver patch
> 	- Added missing config space remapping to hisilicon host
> controller
> 
> ---------------------
> Original cover letter
> ---------------------
> 
> PCI local bus specifications (Rev3.0, 3.2.5 "Transaction Ordering
> and Posting") strictly require PCI configuration and I/O Address
> space
> write transactions to be non-posted.
> 
> Current crop of DT/ACPI PCI host controllers drivers relies on
> the ioremap interface to map ECAM and ECAM-derivative PCI config
> regions and pci_remap_iospace() to create a VMA for mapping
> PCI host bridge I/O Address space transactions to CPU virtual address
> space.
> 
> On some platforms (ie ARM/ARM64) ioremap fails to comply with the PCI
> configuration non-posted write transactions requirement, because it
> provides a memory mapping that issues "bufferable" or, in PCI terms
> "posted" write transactions. Likewise, the current
> pci_remap_iospace()
> implementation maps the physical address range that the PCI
> translates
> to I/O space cycles to virtual address space through pgprot_device()
> attributes that on eg ARM64 provides a memory mapping issuing
> posted writes transactions, which is not PCI specifications
> compliant.
> 
> This patch series[1] addresses both issues in one go:
> 
> - It updates the pci_remap_iospace() function to use a page mapping
> ? that guarantees non-posted write transactions for I/O space
> addresses
> - It adds a kernel API to remap PCI config space resources, so that
> ? architecture can override it with a mapping implementation that
> ? guarantees PCI specifications compliancy wrt non-posted write
> ? configuration transactions
> - It updates all PCI host controller implementations (and the generic
> ? ECAM layer) to use the newly introduced mapping interface
> 
> Tested on Juno ECAM based interface (DT/ACPI).
> 
> Non-ECAM PCI host controller drivers patches need checking to make
> sure that:
> 
> - I patched the correct resource region mapping for config space
> - There are not any other ways to ensure posted-write completion
> ? in the respective pci_ops that make the relevant patch unnecessary
> 
> [1]
> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git
> pci/config-io-mappings-fix-v3
> 
> Lorenzo Pieralisi (32):
> ? PCI: remove __weak tag from pci_remap_iospace()
> ? asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute
> ? PCI: fix pci_remap_iospace() remap attribute
> ? asm-generic: add ioremap_nopost() remap interface
> ? alpha: include default ioremap_nopost() implementation
> ? avr32: include default ioremap_nopost() implementation
> ? arc: include default ioremap_nopost() implementation
> ? cris: include default ioremap_nopost() implementation
> ? frv: include default ioremap_nopost() implementation
> ? hexagon: include default ioremap_nopost() implementation
> ? ia64: include default ioremap_nopost() implementation
> ? m32r: include default ioremap_nopost() implementation
> ? m68k: include default ioremap_nopost() implementation
> ? metag: include default ioremap_nopost() implementation
> ? microblaze: include default ioremap_nopost() implementation
> ? mips: include default ioremap_nopost() implementation
> ? mn10300: include default ioremap_nopost() implementation
> ? nios2: include default ioremap_nopost() implementation
> ? openrisc: include default ioremap_nopost() implementation
> ? parisc: include default ioremap_nopost() implementation
> ? powerpc: include default ioremap_nopost() implementation
> ? s390: include default ioremap_nopost() implementation
> ? sh: include default ioremap_nopost() implementation
> ? sparc: include default ioremap_nopost() implementation
> ? tile: include default ioremap_nopost() implementation
> ? unicore32: include default ioremap_nopost() implementation
> ? x86: include default ioremap_nopost() implementation
> ? xtensa: include default ioremap_nopost() implementation
> ? arm64: implement ioremap_nopost() interface
> ? arm: implement ioremap_nopost() interface
> ? lib: fix Devres devm_ioremap_* offset parameter kerneldoc
> description
> ? lib: implement Devres ioremap_nopost() interface
> 
> ?Documentation/driver-model/devres.txt |??3 ++
> ?arch/alpha/include/asm/io.h???????????|??1 +
> ?arch/arc/include/asm/io.h?????????????|??1 +
> ?arch/arm/include/asm/io.h?????????????|??9 ++++
> ?arch/arm/mm/ioremap.c?????????????????|??7 +++
> ?arch/arm/mm/nommu.c???????????????????|??9 ++++
> ?arch/arm64/include/asm/io.h???????????| 12 +++++
> ?arch/avr32/include/asm/io.h???????????|??1 +
> ?arch/cris/include/asm/io.h????????????|??1 +
> ?arch/frv/include/asm/io.h?????????????|??1 +
> ?arch/hexagon/include/asm/io.h?????????|??2 +
> ?arch/ia64/include/asm/io.h????????????|??1 +
> ?arch/m32r/include/asm/io.h????????????|??1 +
> ?arch/m68k/include/asm/io.h????????????|??1 +
> ?arch/metag/include/asm/io.h???????????|??2 +
> ?arch/microblaze/include/asm/io.h??????|??1 +
> ?arch/mips/include/asm/io.h????????????|??1 +
> ?arch/mn10300/include/asm/io.h?????????|??1 +
> ?arch/nios2/include/asm/io.h???????????|??1 +
> ?arch/openrisc/include/asm/io.h????????|??2 +
> ?arch/parisc/include/asm/io.h??????????|??1 +
> ?arch/powerpc/include/asm/io.h?????????|??1 +
> ?arch/s390/include/asm/io.h????????????|??1 +
> ?arch/sh/include/asm/io.h??????????????|??1 +
> ?arch/sparc/include/asm/io.h???????????|??1 +
> ?arch/tile/include/asm/io.h????????????|??1 +
> ?arch/unicore32/include/asm/io.h???????|??1 +
> ?arch/x86/include/asm/io.h?????????????|??1 +
> ?arch/xtensa/include/asm/io.h??????????|??1 +
> ?drivers/pci/pci.c?????????????????????|??4 +-
> ?include/asm-generic/ioremap-nopost.h??|??9 ++++
> ?include/asm-generic/pgtable.h?????????|??4 ++
> ?include/linux/device.h????????????????|??2 +
> ?include/linux/io.h????????????????????|??2 +
> ?lib/devres.c??????????????????????????| 84
> +++++++++++++++++++++++++++++++++--
> ?35 files changed, 167 insertions(+), 5 deletions(-)
> ?create mode 100644 include/asm-generic/ioremap-nopost.h
> 

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 21/32] powerpc: include default ioremap_nopost() implementation
  2017-04-11 12:29 ` [PATCH v3 21/32] powerpc: " Lorenzo Pieralisi
@ 2017-04-11 13:38   ` Benjamin Herrenschmidt
  2017-04-11 14:24     ` Lorenzo Pieralisi
  0 siblings, 1 reply; 63+ messages in thread
From: Benjamin Herrenschmidt @ 2017-04-11 13:38 UTC (permalink / raw)
  To: linux-arm-kernel

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.

> 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);
> ?

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 04/32] asm-generic: add ioremap_nopost() remap interface
  2017-04-11 12:28 ` [PATCH v3 04/32] asm-generic: add ioremap_nopost() remap interface Lorenzo Pieralisi
@ 2017-04-11 13:39   ` Benjamin Herrenschmidt
  2017-04-11 14:31     ` Lorenzo Pieralisi
  2017-04-12 11:20     ` Russell King - ARM Linux
  0 siblings, 2 replies; 63+ messages in thread
From: Benjamin Herrenschmidt @ 2017-04-11 13:39 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2017-04-11 at 13:28 +0100, Lorenzo Pieralisi wrote:
> +static inline void __iomem *ioremap_nopost(phys_addr_t offset,
> size_t size)
> +{
> +???????return ioremap_nocache(offset, size);
> +}
> +

No this is wrong as I explained.

This is a semantic that simply *cannot* be generically provided accross
architectures as a mapping attribute.

The solution to your problem lies elsewhere.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 06/32] avr32: include default ioremap_nopost() implementation
  2017-04-11 12:28 ` [PATCH v3 06/32] avr32: " Lorenzo Pieralisi
@ 2017-04-11 13:55   ` Nicolas Ferre
  0 siblings, 0 replies; 63+ messages in thread
From: Nicolas Ferre @ 2017-04-11 13:55 UTC (permalink / raw)
  To: linux-arm-kernel

Le 11/04/2017 ? 14:28, Lorenzo Pieralisi a ?crit :
> 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: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>
> Cc: Haavard Skinnemoen <hskinnemoen@gmail.com>
> ---
>  arch/avr32/include/asm/io.h | 1 +
>  1 file changed, 1 insertion(+)

You probably need to remove this one as the avr32 architecture will be
removed in kernel 4.12:

https://lkml.org/lkml/2017/3/27/422

Best regards,

> diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
> index f855646..b2ff522 100644
> --- a/arch/avr32/include/asm/io.h
> +++ b/arch/avr32/include/asm/io.h
> @@ -298,6 +298,7 @@ extern void __iounmap(void __iomem *addr);
>  #define ioremap_wc ioremap_nocache
>  #define ioremap_wt ioremap_nocache
>  #define ioremap_uc ioremap_nocache
> +#include <asm-generic/ioremap-nopost.h>
>  
>  #define cached(addr) P1SEGADDR(addr)
>  #define uncached(addr) P2SEGADDR(addr)
> 


-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-11 13:38 ` [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Benjamin Herrenschmidt
@ 2017-04-11 14:08   ` Lorenzo Pieralisi
  2017-04-11 23:12     ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 14:08 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 11, 2017 at 11:38:26PM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2017-04-11 at 13:28 +0100, Lorenzo Pieralisi wrote:
> > This patch series[1] is a v3 of a previous version:
> > 
> > v2: https://lkml.org/lkml/2017/3/27/220
> 
> I am not a fan of this at All.
> 
> That whole concept of "ioremap_nopost" is simply not applicable to the
> majority of architectures and certainly not in a way that can apply to
> arbitrary mappings.
> 
> It's also very wrong to provide a "default" operation whose semantics
> are weaker than what it's supposed to implement. Very wrong actually.
> People will use it assuming the non-posted behaviour and things will
> break in subtle way when it cannot be provided.

Well, what's very wrong for you it is not very wrong for others
(it is just v3, that's fine, see thread below).

I can easily make ioremap_nopost() mirror ioremap_uc() (ie return
NULL unless overriden so that basically you can't use in on an arch
that can't provide its semantics) but then that becomes very wrong
for other reviewers.

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

> What exactly are you trying to fix here ?

I wrote in the commit logs and cover letter what I am fixing here.

Anyway:

"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."

On ARM64:

"This rule is reinforced by the ARM v8 architecture reference manual
(issue A.k, Early Write Acknowledgment) that explicitly recommends
that No Early Write Acknowledgment attribute should be used to map
PCI configuration (write) transactions."

> If a given PCIe host bridge (architecture specific) require a special
> sauce to provide the illusion of non-posting, then implement this in
> the actual root complex code.

> 
> BTW. I'm pretty sure we "accidentally" made config writes posted at
> least to the PHB on a number of powerpc systems forever and we *never*
> had a problem because of it ;)

Ok so we should ignore the PCIe specifications and ARM v8 reference
manual waiting for a kernel bug to appear ? Is that what you suggest
doing ?

Lorenzo

> > v2 -> v3:
> > 	- Created a default ioremap_nopost() implementation in a
> > separate
> > 	??asm-generic header and patched all arches to make use of it
> > 	- Removed PCI drivers patches from the series to simplify the
> > 	??review, they will be posted separately once the
> > ioremap_nopost()
> > 	??interface is settled
> > 	- Fixed devm_ioremap_* BUS offset comments and implemented
> > 	??nopost interface on top of it
> > 	- Added collected tags
> > 
> > v1: https://lkml.org/lkml/2017/2/27/228
> > 
> > v1 -> v2:
> > 	- Changed pci_remap_cfgspace() to more generic ioremap_nopost()
> > 	??interface
> > 	- Added pgprot_nonposted
> > 	- Fixed build errors on arches not relying on asm-generic
> > headers
> > 	- Added PCI versatile host controller driver patch
> > 	- Added missing config space remapping to hisilicon host
> > controller
> > 
> > ---------------------
> > Original cover letter
> > ---------------------
> > 
> > PCI local bus specifications (Rev3.0, 3.2.5 "Transaction Ordering
> > and Posting") strictly require PCI configuration and I/O Address
> > space
> > write transactions to be non-posted.
> > 
> > Current crop of DT/ACPI PCI host controllers drivers relies on
> > the ioremap interface to map ECAM and ECAM-derivative PCI config
> > regions and pci_remap_iospace() to create a VMA for mapping
> > PCI host bridge I/O Address space transactions to CPU virtual address
> > space.
> > 
> > On some platforms (ie ARM/ARM64) ioremap fails to comply with the PCI
> > configuration non-posted write transactions requirement, because it
> > provides a memory mapping that issues "bufferable" or, in PCI terms
> > "posted" write transactions. Likewise, the current
> > pci_remap_iospace()
> > implementation maps the physical address range that the PCI
> > translates
> > to I/O space cycles to virtual address space through pgprot_device()
> > attributes that on eg ARM64 provides a memory mapping issuing
> > posted writes transactions, which is not PCI specifications
> > compliant.
> > 
> > This patch series[1] addresses both issues in one go:
> > 
> > - It updates the pci_remap_iospace() function to use a page mapping
> > ? that guarantees non-posted write transactions for I/O space
> > addresses
> > - It adds a kernel API to remap PCI config space resources, so that
> > ? architecture can override it with a mapping implementation that
> > ? guarantees PCI specifications compliancy wrt non-posted write
> > ? configuration transactions
> > - It updates all PCI host controller implementations (and the generic
> > ? ECAM layer) to use the newly introduced mapping interface
> > 
> > Tested on Juno ECAM based interface (DT/ACPI).
> > 
> > Non-ECAM PCI host controller drivers patches need checking to make
> > sure that:
> > 
> > - I patched the correct resource region mapping for config space
> > - There are not any other ways to ensure posted-write completion
> > ? in the respective pci_ops that make the relevant patch unnecessary
> > 
> > [1]
> > git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git
> > pci/config-io-mappings-fix-v3
> > 
> > Lorenzo Pieralisi (32):
> > ? PCI: remove __weak tag from pci_remap_iospace()
> > ? asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute
> > ? PCI: fix pci_remap_iospace() remap attribute
> > ? asm-generic: add ioremap_nopost() remap interface
> > ? alpha: include default ioremap_nopost() implementation
> > ? avr32: include default ioremap_nopost() implementation
> > ? arc: include default ioremap_nopost() implementation
> > ? cris: include default ioremap_nopost() implementation
> > ? frv: include default ioremap_nopost() implementation
> > ? hexagon: include default ioremap_nopost() implementation
> > ? ia64: include default ioremap_nopost() implementation
> > ? m32r: include default ioremap_nopost() implementation
> > ? m68k: include default ioremap_nopost() implementation
> > ? metag: include default ioremap_nopost() implementation
> > ? microblaze: include default ioremap_nopost() implementation
> > ? mips: include default ioremap_nopost() implementation
> > ? mn10300: include default ioremap_nopost() implementation
> > ? nios2: include default ioremap_nopost() implementation
> > ? openrisc: include default ioremap_nopost() implementation
> > ? parisc: include default ioremap_nopost() implementation
> > ? powerpc: include default ioremap_nopost() implementation
> > ? s390: include default ioremap_nopost() implementation
> > ? sh: include default ioremap_nopost() implementation
> > ? sparc: include default ioremap_nopost() implementation
> > ? tile: include default ioremap_nopost() implementation
> > ? unicore32: include default ioremap_nopost() implementation
> > ? x86: include default ioremap_nopost() implementation
> > ? xtensa: include default ioremap_nopost() implementation
> > ? arm64: implement ioremap_nopost() interface
> > ? arm: implement ioremap_nopost() interface
> > ? lib: fix Devres devm_ioremap_* offset parameter kerneldoc
> > description
> > ? lib: implement Devres ioremap_nopost() interface
> > 
> > ?Documentation/driver-model/devres.txt |??3 ++
> > ?arch/alpha/include/asm/io.h???????????|??1 +
> > ?arch/arc/include/asm/io.h?????????????|??1 +
> > ?arch/arm/include/asm/io.h?????????????|??9 ++++
> > ?arch/arm/mm/ioremap.c?????????????????|??7 +++
> > ?arch/arm/mm/nommu.c???????????????????|??9 ++++
> > ?arch/arm64/include/asm/io.h???????????| 12 +++++
> > ?arch/avr32/include/asm/io.h???????????|??1 +
> > ?arch/cris/include/asm/io.h????????????|??1 +
> > ?arch/frv/include/asm/io.h?????????????|??1 +
> > ?arch/hexagon/include/asm/io.h?????????|??2 +
> > ?arch/ia64/include/asm/io.h????????????|??1 +
> > ?arch/m32r/include/asm/io.h????????????|??1 +
> > ?arch/m68k/include/asm/io.h????????????|??1 +
> > ?arch/metag/include/asm/io.h???????????|??2 +
> > ?arch/microblaze/include/asm/io.h??????|??1 +
> > ?arch/mips/include/asm/io.h????????????|??1 +
> > ?arch/mn10300/include/asm/io.h?????????|??1 +
> > ?arch/nios2/include/asm/io.h???????????|??1 +
> > ?arch/openrisc/include/asm/io.h????????|??2 +
> > ?arch/parisc/include/asm/io.h??????????|??1 +
> > ?arch/powerpc/include/asm/io.h?????????|??1 +
> > ?arch/s390/include/asm/io.h????????????|??1 +
> > ?arch/sh/include/asm/io.h??????????????|??1 +
> > ?arch/sparc/include/asm/io.h???????????|??1 +
> > ?arch/tile/include/asm/io.h????????????|??1 +
> > ?arch/unicore32/include/asm/io.h???????|??1 +
> > ?arch/x86/include/asm/io.h?????????????|??1 +
> > ?arch/xtensa/include/asm/io.h??????????|??1 +
> > ?drivers/pci/pci.c?????????????????????|??4 +-
> > ?include/asm-generic/ioremap-nopost.h??|??9 ++++
> > ?include/asm-generic/pgtable.h?????????|??4 ++
> > ?include/linux/device.h????????????????|??2 +
> > ?include/linux/io.h????????????????????|??2 +
> > ?lib/devres.c??????????????????????????| 84
> > +++++++++++++++++++++++++++++++++--
> > ?35 files changed, 167 insertions(+), 5 deletions(-)
> > ?create mode 100644 include/asm-generic/ioremap-nopost.h
> > 

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 21/32] powerpc: include default ioremap_nopost() implementation
  2017-04-11 13:38   ` Benjamin Herrenschmidt
@ 2017-04-11 14:24     ` Lorenzo Pieralisi
  2017-04-11 23:15       ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 14:24 UTC (permalink / raw)
  To: linux-arm-kernel

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);
> > ?

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 04/32] asm-generic: add ioremap_nopost() remap interface
  2017-04-11 13:39   ` Benjamin Herrenschmidt
@ 2017-04-11 14:31     ` Lorenzo Pieralisi
  2017-04-11 23:14       ` Benjamin Herrenschmidt
  2017-04-12 11:20     ` Russell King - ARM Linux
  1 sibling, 1 reply; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-11 14:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 11, 2017 at 11:39:43PM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2017-04-11 at 13:28 +0100, Lorenzo Pieralisi wrote:
> > +static inline void __iomem *ioremap_nopost(phys_addr_t offset,
> > size_t size)
> > +{
> > +???????return ioremap_nocache(offset, size);
> > +}
> > +
> 
> No this is wrong as I explained.
> 
> This is a semantic that simply *cannot* be generically provided accross
> architectures as a mapping attribute.

I agree that a default implementation does not make much sense. The
only solution to this, if we want the ioremap_nopost to be made available
to generic code (and drivers - ie DT PCI host bridge drivers on ARM/ARM64
are not arch code), is to make the ioremap_nopost() call return NULL
unless overriden by arch code that can provide its semantics.

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-11 14:08   ` Lorenzo Pieralisi
@ 2017-04-11 23:12     ` Benjamin Herrenschmidt
  2017-04-12  9:44       ` Lorenzo Pieralisi
  2017-04-12 11:31       ` Russell King - ARM Linux
  0 siblings, 2 replies; 63+ messages in thread
From: Benjamin Herrenschmidt @ 2017-04-11 23:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2017-04-11 at 15:08 +0100, Lorenzo Pieralisi wrote:
> On Tue, Apr 11, 2017 at 11:38:26PM +1000, Benjamin Herrenschmidt wrote:
> > On Tue, 2017-04-11 at 13:28 +0100, Lorenzo Pieralisi wrote:
> > > This patch series[1] is a v3 of a previous version:
> > > 
> > > v2: https://lkml.org/lkml/2017/3/27/220
> > 
> > I am not a fan of this at All.
> > 
> > That whole concept of "ioremap_nopost" is simply not applicable to the
> > majority of architectures and certainly not in a way that can apply to
> > arbitrary mappings.
> > 
> > It's also very wrong to provide a "default" operation whose semantics
> > are weaker than what it's supposed to implement. Very wrong actually.
> > People will use it assuming the non-posted behaviour and things will
> > break in subtle way when it cannot be provided.
> 
> Well, what's very wrong for you it is not very wrong for others
> (it is just v3, that's fine, see thread below).

Maybe, but I don't see in what universe it is ok to have something
defined for the stronger ordering semantics it provide silently
fallback to weaker semantics.

> I can easily make ioremap_nopost() mirror ioremap_uc() (ie return
> NULL unless overriden so that basically you can't use in on an arch
> that can't provide its semantics) but then that becomes very wrong
> for other reviewers.

Those reviewers are WRONG :-)

> https://lkml.org/lkml/2017/4/6/396
> 
> > What exactly are you trying to fix here ?
> 
> I wrote in the commit logs and cover letter what I am fixing here.

Right right, what *actual bug you have observed* are you trying to fix
?

I'm pretty such close to all non-x86 archs had that "problem" since the
dawn of time and it has never hurt anybody.

That said, I don't think it makes sense to "solve" it by creating a
"generic" mapping semantic that is basically impossible to implement on
most architectures out there (and cannot be emulated).

This is a problem to be solved by the bridge itself. If ARM has a
mapping attribute to make stores non-posted, keep this an ARM specific
attribute at this stage I'd say.

> Anyway:
> 
> "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."
> 
> On ARM64:
> 
> "This rule is reinforced by the ARM v8 architecture reference manual
> (issue A.k, Early Write Acknowledgment) that explicitly recommends
> that No Early Write Acknowledgment attribute should be used to map
> PCI configuration (write) transactions."
> 
> > If a given PCIe host bridge (architecture specific) require a special
> > sauce to provide the illusion of non-posting, then implement this in
> > the actual root complex code.
> > 
> > BTW. I'm pretty sure we "accidentally" made config writes posted at
> > least to the PHB on a number of powerpc systems forever and we *never*
> > had a problem because of it ;)
> 
> Ok so we should ignore the PCIe specifications and ARM v8 reference
> manual waiting for a kernel bug to appear ? Is that what you suggest
> doing ?
> 
> Lorenzo
> 
> > > v2 -> v3:
> > > 	- Created a default ioremap_nopost() implementation in a
> > > separate
> > > 	??asm-generic header and patched all arches to make use of it
> > > 	- Removed PCI drivers patches from the series to simplify the
> > > 	??review, they will be posted separately once the
> > > ioremap_nopost()
> > > 	??interface is settled
> > > 	- Fixed devm_ioremap_* BUS offset comments and implemented
> > > 	??nopost interface on top of it
> > > 	- Added collected tags
> > > 
> > > v1: https://lkml.org/lkml/2017/2/27/228
> > > 
> > > v1 -> v2:
> > > 	- Changed pci_remap_cfgspace() to more generic ioremap_nopost()
> > > 	??interface
> > > 	- Added pgprot_nonposted
> > > 	- Fixed build errors on arches not relying on asm-generic
> > > headers
> > > 	- Added PCI versatile host controller driver patch
> > > 	- Added missing config space remapping to hisilicon host
> > > controller
> > > 
> > > ---------------------
> > > Original cover letter
> > > ---------------------
> > > 
> > > PCI local bus specifications (Rev3.0, 3.2.5 "Transaction Ordering
> > > and Posting") strictly require PCI configuration and I/O Address
> > > space
> > > write transactions to be non-posted.
> > > 
> > > Current crop of DT/ACPI PCI host controllers drivers relies on
> > > the ioremap interface to map ECAM and ECAM-derivative PCI config
> > > regions and pci_remap_iospace() to create a VMA for mapping
> > > PCI host bridge I/O Address space transactions to CPU virtual address
> > > space.
> > > 
> > > On some platforms (ie ARM/ARM64) ioremap fails to comply with the PCI
> > > configuration non-posted write transactions requirement, because it
> > > provides a memory mapping that issues "bufferable" or, in PCI terms
> > > "posted" write transactions. Likewise, the current
> > > pci_remap_iospace()
> > > implementation maps the physical address range that the PCI
> > > translates
> > > to I/O space cycles to virtual address space through pgprot_device()
> > > attributes that on eg ARM64 provides a memory mapping issuing
> > > posted writes transactions, which is not PCI specifications
> > > compliant.
> > > 
> > > This patch series[1] addresses both issues in one go:
> > > 
> > > - It updates the pci_remap_iospace() function to use a page mapping
> > > ? that guarantees non-posted write transactions for I/O space
> > > addresses
> > > - It adds a kernel API to remap PCI config space resources, so that
> > > ? architecture can override it with a mapping implementation that
> > > ? guarantees PCI specifications compliancy wrt non-posted write
> > > ? configuration transactions
> > > - It updates all PCI host controller implementations (and the generic
> > > ? ECAM layer) to use the newly introduced mapping interface
> > > 
> > > Tested on Juno ECAM based interface (DT/ACPI).
> > > 
> > > Non-ECAM PCI host controller drivers patches need checking to make
> > > sure that:
> > > 
> > > - I patched the correct resource region mapping for config space
> > > - There are not any other ways to ensure posted-write completion
> > > ? in the respective pci_ops that make the relevant patch unnecessary
> > > 
> > > [1]
> > > git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git
> > > pci/config-io-mappings-fix-v3
> > > 
> > > Lorenzo Pieralisi (32):
> > > ? PCI: remove __weak tag from pci_remap_iospace()
> > > ? asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute
> > > ? PCI: fix pci_remap_iospace() remap attribute
> > > ? asm-generic: add ioremap_nopost() remap interface
> > > ? alpha: include default ioremap_nopost() implementation
> > > ? avr32: include default ioremap_nopost() implementation
> > > ? arc: include default ioremap_nopost() implementation
> > > ? cris: include default ioremap_nopost() implementation
> > > ? frv: include default ioremap_nopost() implementation
> > > ? hexagon: include default ioremap_nopost() implementation
> > > ? ia64: include default ioremap_nopost() implementation
> > > ? m32r: include default ioremap_nopost() implementation
> > > ? m68k: include default ioremap_nopost() implementation
> > > ? metag: include default ioremap_nopost() implementation
> > > ? microblaze: include default ioremap_nopost() implementation
> > > ? mips: include default ioremap_nopost() implementation
> > > ? mn10300: include default ioremap_nopost() implementation
> > > ? nios2: include default ioremap_nopost() implementation
> > > ? openrisc: include default ioremap_nopost() implementation
> > > ? parisc: include default ioremap_nopost() implementation
> > > ? powerpc: include default ioremap_nopost() implementation
> > > ? s390: include default ioremap_nopost() implementation
> > > ? sh: include default ioremap_nopost() implementation
> > > ? sparc: include default ioremap_nopost() implementation
> > > ? tile: include default ioremap_nopost() implementation
> > > ? unicore32: include default ioremap_nopost() implementation
> > > ? x86: include default ioremap_nopost() implementation
> > > ? xtensa: include default ioremap_nopost() implementation
> > > ? arm64: implement ioremap_nopost() interface
> > > ? arm: implement ioremap_nopost() interface
> > > ? lib: fix Devres devm_ioremap_* offset parameter kerneldoc
> > > description
> > > ? lib: implement Devres ioremap_nopost() interface
> > > 
> > > ?Documentation/driver-model/devres.txt |??3 ++
> > > ?arch/alpha/include/asm/io.h???????????|??1 +
> > > ?arch/arc/include/asm/io.h?????????????|??1 +
> > > ?arch/arm/include/asm/io.h?????????????|??9 ++++
> > > ?arch/arm/mm/ioremap.c?????????????????|??7 +++
> > > ?arch/arm/mm/nommu.c???????????????????|??9 ++++
> > > ?arch/arm64/include/asm/io.h???????????| 12 +++++
> > > ?arch/avr32/include/asm/io.h???????????|??1 +
> > > ?arch/cris/include/asm/io.h????????????|??1 +
> > > ?arch/frv/include/asm/io.h?????????????|??1 +
> > > ?arch/hexagon/include/asm/io.h?????????|??2 +
> > > ?arch/ia64/include/asm/io.h????????????|??1 +
> > > ?arch/m32r/include/asm/io.h????????????|??1 +
> > > ?arch/m68k/include/asm/io.h????????????|??1 +
> > > ?arch/metag/include/asm/io.h???????????|??2 +
> > > ?arch/microblaze/include/asm/io.h??????|??1 +
> > > ?arch/mips/include/asm/io.h????????????|??1 +
> > > ?arch/mn10300/include/asm/io.h?????????|??1 +
> > > ?arch/nios2/include/asm/io.h???????????|??1 +
> > > ?arch/openrisc/include/asm/io.h????????|??2 +
> > > ?arch/parisc/include/asm/io.h??????????|??1 +
> > > ?arch/powerpc/include/asm/io.h?????????|??1 +
> > > ?arch/s390/include/asm/io.h????????????|??1 +
> > > ?arch/sh/include/asm/io.h??????????????|??1 +
> > > ?arch/sparc/include/asm/io.h???????????|??1 +
> > > ?arch/tile/include/asm/io.h????????????|??1 +
> > > ?arch/unicore32/include/asm/io.h???????|??1 +
> > > ?arch/x86/include/asm/io.h?????????????|??1 +
> > > ?arch/xtensa/include/asm/io.h??????????|??1 +
> > > ?drivers/pci/pci.c?????????????????????|??4 +-
> > > ?include/asm-generic/ioremap-nopost.h??|??9 ++++
> > > ?include/asm-generic/pgtable.h?????????|??4 ++
> > > ?include/linux/device.h????????????????|??2 +
> > > ?include/linux/io.h????????????????????|??2 +
> > > ?lib/devres.c??????????????????????????| 84
> > > +++++++++++++++++++++++++++++++++--
> > > ?35 files changed, 167 insertions(+), 5 deletions(-)
> > > ?create mode 100644 include/asm-generic/ioremap-nopost.h
> > > 

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 04/32] asm-generic: add ioremap_nopost() remap interface
  2017-04-11 14:31     ` Lorenzo Pieralisi
@ 2017-04-11 23:14       ` Benjamin Herrenschmidt
  2017-04-12 10:00         ` Lorenzo Pieralisi
  0 siblings, 1 reply; 63+ messages in thread
From: Benjamin Herrenschmidt @ 2017-04-11 23:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2017-04-11 at 15:31 +0100, Lorenzo Pieralisi wrote:
> > This is a semantic that simply *cannot* be generically provided accross
> > architectures as a mapping attribute.
> 
> I agree that a default implementation does not make much sense. The
> only solution to this, if we want the ioremap_nopost to be made available
> to generic code (and drivers - ie DT PCI host bridge drivers on ARM/ARM64
> are not arch code), is to make the ioremap_nopost() call return NULL
> unless overriden by arch code that can provide its semantics.

That would be a better option.

You might be able to implement a fallback, for example by having the
config ops do a read back from the bridge.

Ben.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 21/32] powerpc: include default ioremap_nopost() implementation
  2017-04-11 14:24     ` Lorenzo Pieralisi
@ 2017-04-11 23:15       ` Benjamin Herrenschmidt
  2017-04-13  3:35         ` Michael Ellerman
  0 siblings, 1 reply; 63+ messages in thread
From: Benjamin Herrenschmidt @ 2017-04-11 23:15 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-11 23:12     ` Benjamin Herrenschmidt
@ 2017-04-12  9:44       ` Lorenzo Pieralisi
  2017-04-12 13:48         ` Benjamin Herrenschmidt
  2017-04-12 11:31       ` Russell King - ARM Linux
  1 sibling, 1 reply; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-12  9:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Apr 12, 2017 at 09:12:51AM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2017-04-11 at 15:08 +0100, Lorenzo Pieralisi wrote:
> > On Tue, Apr 11, 2017 at 11:38:26PM +1000, Benjamin Herrenschmidt wrote:
> > > On Tue, 2017-04-11 at 13:28 +0100, Lorenzo Pieralisi wrote:
> > > > This patch series[1] is a v3 of a previous version:
> > > > 
> > > > v2: https://lkml.org/lkml/2017/3/27/220
> > > 
> > > I am not a fan of this at All.
> > > 
> > > That whole concept of "ioremap_nopost" is simply not applicable to the
> > > majority of architectures and certainly not in a way that can apply to
> > > arbitrary mappings.
> > > 
> > > It's also very wrong to provide a "default" operation whose semantics
> > > are weaker than what it's supposed to implement. Very wrong actually.
> > > People will use it assuming the non-posted behaviour and things will
> > > break in subtle way when it cannot be provided.
> > 
> > Well, what's very wrong for you it is not very wrong for others
> > (it is just v3, that's fine, see thread below).
> 
> Maybe, but I don't see in what universe it is ok to have something
> defined for the stronger ordering semantics it provide silently
> fallback to weaker semantics.

I agree :). The drivers I am patching use ioremap() already that's
why falling back to ioremap_nocache() seemed reasonable, but again
I agree with you here.

> > I can easily make ioremap_nopost() mirror ioremap_uc() (ie return
> > NULL unless overriden so that basically you can't use in on an arch
> > that can't provide its semantics) but then that becomes very wrong
> > for other reviewers.
> 
> Those reviewers are WRONG :-)
> 
> > https://lkml.org/lkml/2017/4/6/396
> > 
> > > What exactly are you trying to fix here ?
> > 
> > I wrote in the commit logs and cover letter what I am fixing here.
> 
> Right right, what *actual bug you have observed* are you trying to fix
> ?

I have not observed any bug and I never claimed that. It started here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2017-January/477353.html

If you prefer I am making ARM/ARM64 PCI host bridge drivers
specifications compliant, given that it is architecturally
required.

> I'm pretty such close to all non-x86 archs had that "problem" since the
> dawn of time and it has never hurt anybody.

Good but I still do not see why I would not make this PCI/ARM architecture
compliant.

> That said, I don't think it makes sense to "solve" it by creating a
> "generic" mapping semantic that is basically impossible to implement on
> most architectures out there (and cannot be emulated).
> 
> This is a problem to be solved by the bridge itself. If ARM has a
> mapping attribute to make stores non-posted, keep this an ARM specific
> attribute at this stage I'd say.

I can't. Some PCI host bridge drivers (DT) are shared between ARM/ARM64
and live in drivers (and are also reused on some arches eg microblaze)
and they use ioremap() to map config space.

So the idea behind this series was to add an interface (that is
overriden on ARM/ARM64), it started as a PCI specific interface (v1) and
evolved to ioremap_nopost() (v2).

Thanks,
Lorenzo

> > Anyway:
> > 
> > "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."
> > 
> > On ARM64:
> > 
> > "This rule is reinforced by the ARM v8 architecture reference manual
> > (issue A.k, Early Write Acknowledgment) that explicitly recommends
> > that No Early Write Acknowledgment attribute should be used to map
> > PCI configuration (write) transactions."
> > 
> > > If a given PCIe host bridge (architecture specific) require a special
> > > sauce to provide the illusion of non-posting, then implement this in
> > > the actual root complex code.
> > > 
> > > BTW. I'm pretty sure we "accidentally" made config writes posted at
> > > least to the PHB on a number of powerpc systems forever and we *never*
> > > had a problem because of it ;)
> > 
> > Ok so we should ignore the PCIe specifications and ARM v8 reference
> > manual waiting for a kernel bug to appear ? Is that what you suggest
> > doing ?
> > 
> > Lorenzo
> > 
> > > > v2 -> v3:
> > > > 	- Created a default ioremap_nopost() implementation in a
> > > > separate
> > > > 	??asm-generic header and patched all arches to make use of it
> > > > 	- Removed PCI drivers patches from the series to simplify the
> > > > 	??review, they will be posted separately once the
> > > > ioremap_nopost()
> > > > 	??interface is settled
> > > > 	- Fixed devm_ioremap_* BUS offset comments and implemented
> > > > 	??nopost interface on top of it
> > > > 	- Added collected tags
> > > > 
> > > > v1: https://lkml.org/lkml/2017/2/27/228
> > > > 
> > > > v1 -> v2:
> > > > 	- Changed pci_remap_cfgspace() to more generic ioremap_nopost()
> > > > 	??interface
> > > > 	- Added pgprot_nonposted
> > > > 	- Fixed build errors on arches not relying on asm-generic
> > > > headers
> > > > 	- Added PCI versatile host controller driver patch
> > > > 	- Added missing config space remapping to hisilicon host
> > > > controller
> > > > 
> > > > ---------------------
> > > > Original cover letter
> > > > ---------------------
> > > > 
> > > > PCI local bus specifications (Rev3.0, 3.2.5 "Transaction Ordering
> > > > and Posting") strictly require PCI configuration and I/O Address
> > > > space
> > > > write transactions to be non-posted.
> > > > 
> > > > Current crop of DT/ACPI PCI host controllers drivers relies on
> > > > the ioremap interface to map ECAM and ECAM-derivative PCI config
> > > > regions and pci_remap_iospace() to create a VMA for mapping
> > > > PCI host bridge I/O Address space transactions to CPU virtual address
> > > > space.
> > > > 
> > > > On some platforms (ie ARM/ARM64) ioremap fails to comply with the PCI
> > > > configuration non-posted write transactions requirement, because it
> > > > provides a memory mapping that issues "bufferable" or, in PCI terms
> > > > "posted" write transactions. Likewise, the current
> > > > pci_remap_iospace()
> > > > implementation maps the physical address range that the PCI
> > > > translates
> > > > to I/O space cycles to virtual address space through pgprot_device()
> > > > attributes that on eg ARM64 provides a memory mapping issuing
> > > > posted writes transactions, which is not PCI specifications
> > > > compliant.
> > > > 
> > > > This patch series[1] addresses both issues in one go:
> > > > 
> > > > - It updates the pci_remap_iospace() function to use a page mapping
> > > > ? that guarantees non-posted write transactions for I/O space
> > > > addresses
> > > > - It adds a kernel API to remap PCI config space resources, so that
> > > > ? architecture can override it with a mapping implementation that
> > > > ? guarantees PCI specifications compliancy wrt non-posted write
> > > > ? configuration transactions
> > > > - It updates all PCI host controller implementations (and the generic
> > > > ? ECAM layer) to use the newly introduced mapping interface
> > > > 
> > > > Tested on Juno ECAM based interface (DT/ACPI).
> > > > 
> > > > Non-ECAM PCI host controller drivers patches need checking to make
> > > > sure that:
> > > > 
> > > > - I patched the correct resource region mapping for config space
> > > > - There are not any other ways to ensure posted-write completion
> > > > ? in the respective pci_ops that make the relevant patch unnecessary
> > > > 
> > > > [1]
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git
> > > > pci/config-io-mappings-fix-v3
> > > > 
> > > > Lorenzo Pieralisi (32):
> > > > ? PCI: remove __weak tag from pci_remap_iospace()
> > > > ? asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute
> > > > ? PCI: fix pci_remap_iospace() remap attribute
> > > > ? asm-generic: add ioremap_nopost() remap interface
> > > > ? alpha: include default ioremap_nopost() implementation
> > > > ? avr32: include default ioremap_nopost() implementation
> > > > ? arc: include default ioremap_nopost() implementation
> > > > ? cris: include default ioremap_nopost() implementation
> > > > ? frv: include default ioremap_nopost() implementation
> > > > ? hexagon: include default ioremap_nopost() implementation
> > > > ? ia64: include default ioremap_nopost() implementation
> > > > ? m32r: include default ioremap_nopost() implementation
> > > > ? m68k: include default ioremap_nopost() implementation
> > > > ? metag: include default ioremap_nopost() implementation
> > > > ? microblaze: include default ioremap_nopost() implementation
> > > > ? mips: include default ioremap_nopost() implementation
> > > > ? mn10300: include default ioremap_nopost() implementation
> > > > ? nios2: include default ioremap_nopost() implementation
> > > > ? openrisc: include default ioremap_nopost() implementation
> > > > ? parisc: include default ioremap_nopost() implementation
> > > > ? powerpc: include default ioremap_nopost() implementation
> > > > ? s390: include default ioremap_nopost() implementation
> > > > ? sh: include default ioremap_nopost() implementation
> > > > ? sparc: include default ioremap_nopost() implementation
> > > > ? tile: include default ioremap_nopost() implementation
> > > > ? unicore32: include default ioremap_nopost() implementation
> > > > ? x86: include default ioremap_nopost() implementation
> > > > ? xtensa: include default ioremap_nopost() implementation
> > > > ? arm64: implement ioremap_nopost() interface
> > > > ? arm: implement ioremap_nopost() interface
> > > > ? lib: fix Devres devm_ioremap_* offset parameter kerneldoc
> > > > description
> > > > ? lib: implement Devres ioremap_nopost() interface
> > > > 
> > > > ?Documentation/driver-model/devres.txt |??3 ++
> > > > ?arch/alpha/include/asm/io.h???????????|??1 +
> > > > ?arch/arc/include/asm/io.h?????????????|??1 +
> > > > ?arch/arm/include/asm/io.h?????????????|??9 ++++
> > > > ?arch/arm/mm/ioremap.c?????????????????|??7 +++
> > > > ?arch/arm/mm/nommu.c???????????????????|??9 ++++
> > > > ?arch/arm64/include/asm/io.h???????????| 12 +++++
> > > > ?arch/avr32/include/asm/io.h???????????|??1 +
> > > > ?arch/cris/include/asm/io.h????????????|??1 +
> > > > ?arch/frv/include/asm/io.h?????????????|??1 +
> > > > ?arch/hexagon/include/asm/io.h?????????|??2 +
> > > > ?arch/ia64/include/asm/io.h????????????|??1 +
> > > > ?arch/m32r/include/asm/io.h????????????|??1 +
> > > > ?arch/m68k/include/asm/io.h????????????|??1 +
> > > > ?arch/metag/include/asm/io.h???????????|??2 +
> > > > ?arch/microblaze/include/asm/io.h??????|??1 +
> > > > ?arch/mips/include/asm/io.h????????????|??1 +
> > > > ?arch/mn10300/include/asm/io.h?????????|??1 +
> > > > ?arch/nios2/include/asm/io.h???????????|??1 +
> > > > ?arch/openrisc/include/asm/io.h????????|??2 +
> > > > ?arch/parisc/include/asm/io.h??????????|??1 +
> > > > ?arch/powerpc/include/asm/io.h?????????|??1 +
> > > > ?arch/s390/include/asm/io.h????????????|??1 +
> > > > ?arch/sh/include/asm/io.h??????????????|??1 +
> > > > ?arch/sparc/include/asm/io.h???????????|??1 +
> > > > ?arch/tile/include/asm/io.h????????????|??1 +
> > > > ?arch/unicore32/include/asm/io.h???????|??1 +
> > > > ?arch/x86/include/asm/io.h?????????????|??1 +
> > > > ?arch/xtensa/include/asm/io.h??????????|??1 +
> > > > ?drivers/pci/pci.c?????????????????????|??4 +-
> > > > ?include/asm-generic/ioremap-nopost.h??|??9 ++++
> > > > ?include/asm-generic/pgtable.h?????????|??4 ++
> > > > ?include/linux/device.h????????????????|??2 +
> > > > ?include/linux/io.h????????????????????|??2 +
> > > > ?lib/devres.c??????????????????????????| 84
> > > > +++++++++++++++++++++++++++++++++--
> > > > ?35 files changed, 167 insertions(+), 5 deletions(-)
> > > > ?create mode 100644 include/asm-generic/ioremap-nopost.h
> > > > 

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 04/32] asm-generic: add ioremap_nopost() remap interface
  2017-04-11 23:14       ` Benjamin Herrenschmidt
@ 2017-04-12 10:00         ` Lorenzo Pieralisi
  0 siblings, 0 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-12 10:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Apr 12, 2017 at 09:14:07AM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2017-04-11 at 15:31 +0100, Lorenzo Pieralisi wrote:
> > > This is a semantic that simply *cannot* be generically provided accross
> > > architectures as a mapping attribute.
> > 
> > I agree that a default implementation does not make much sense. The
> > only solution to this, if we want the ioremap_nopost to be made available
> > to generic code (and drivers - ie DT PCI host bridge drivers on ARM/ARM64
> > are not arch code), is to make the ioremap_nopost() call return NULL
> > unless overriden by arch code that can provide its semantics.
> 
> That would be a better option.

I think that's what I will implement which basically means I will
replace the default:

static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
{
	return ioremap_nocache(offset, size);
}

with

static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
{
	return NULL;
}

If an arch can override ioremap_nopost() with sensible mapping
attributes (or whatever make it enforceable) it does, if it can't
any ioremap_nopost() usage will result in a mapping failure, if you
see any other viable solution please shout.

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 04/32] asm-generic: add ioremap_nopost() remap interface
  2017-04-11 13:39   ` Benjamin Herrenschmidt
  2017-04-11 14:31     ` Lorenzo Pieralisi
@ 2017-04-12 11:20     ` Russell King - ARM Linux
  2017-04-18 15:49       ` Lorenzo Pieralisi
  1 sibling, 1 reply; 63+ messages in thread
From: Russell King - ARM Linux @ 2017-04-12 11:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 11, 2017 at 11:39:43PM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2017-04-11 at 13:28 +0100, Lorenzo Pieralisi wrote:
> > +static inline void __iomem *ioremap_nopost(phys_addr_t offset,
> > size_t size)
> > +{
> > +???????return ioremap_nocache(offset, size);
> > +}
> > +
> 
> No this is wrong as I explained.
> 
> This is a semantic that simply *cannot* be generically provided accross
> architectures as a mapping attribute.
> 
> The solution to your problem lies elsewhere.

I disagree.  Sure, it may not be supportable across all architectures,
but we're not introducing something brand new here.

What we're trying to do is to fix some _existing_ drivers that are
already using ioremap() to map the PCI IO and configuration spaces.
Maybe it would help if those drivers were part of this patch set,
rather than the patch set just introducing a whole new architecture
interface without really showing where the problem drivers are.

The issue here is that if we make this new ioremap_nopost() fail by
returning NULL if an architecture does not provide an implementation,
then these drivers are going to start failing on such architectures.

It is only safe to do that where we know for certain that the drivers
are not used - but I don't think that's the case here (it's difficult
to tell, because no drivers are updated in this series, so we don't
know which are going to be affected.)

So, the question is:

- is it better to provide a default implementation which provides the
  functionality that existing drivers are _already_ using, albiet not
  entirely correctly.

or:

- is it better to break drivers on architectures when they're converted
  to fix up this issue.

What, I suppose, we could do is not bother with a default implementation,
but instead litter drivers with:

+#ifdef ioremap_post
+	base = ioremap_post(...);
+#else
	base = ioremap(...);
+#endif

which gets around your objection - not providing a default that's weaker
than required, but also not breaking the drivers.  The problem is that
we end up littering drivers with ifdefs.

However, I'm willing to bet that the architectures that you're saying
will not be able to support the weaker semantic won't have any need to
use these drivers, or if they do, the drivers will need the method of
accessing stuff like PCI IO and configuration spaces radically altered.

So, maybe the best solution is to not provide any kind of default
implementation, add a HAVE_IOREMAP_POST Kconfig symbol, have architectures
select that when they do provide ioremap_post(), and make the drivers
depend on that (so we don't end up trying to build these drivers on
architectures where they can never work.)  Down side to that is reduced
build coverage.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-11 23:12     ` Benjamin Herrenschmidt
  2017-04-12  9:44       ` Lorenzo Pieralisi
@ 2017-04-12 11:31       ` Russell King - ARM Linux
  2017-04-12 13:51         ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 63+ messages in thread
From: Russell King - ARM Linux @ 2017-04-12 11:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Apr 12, 2017 at 09:12:51AM +1000, Benjamin Herrenschmidt wrote:
> This is a problem to be solved by the bridge itself. If ARM has a
> mapping attribute to make stores non-posted, keep this an ARM specific
> attribute at this stage I'd say.

So what you seem to be saying is to place drivers not in drivers/pci/host
but in arch/arm, and a duplicate copy in arch/arm64.

No.  We're way past that by popular concensus.  We're not going back to
doing what we did in the 2000s.  Drivers go in the drivers subtree,
period.  That means we require architecture interfaces that provide
access to architecture specific details that may not be suitable for
all architectures to support.

In the case of something brand new, then I agree with you that the
default implementation should fail if it's not supportable on all
architectures.  However, when we have existing drivers using an
interface that doesn't provide the semantics they already require,
then it makes no sense to effectively break these drivers on a range
of existing architectures.

The question really is - what's the best way to solve the problem with
existing drivers without breaking them.  I suspect that, sadly, the
only realistic way forward here is via the litter-drivers-with-ifdefs
approach since you don't like providing a default implementation that
is compatible with what these drivers are already doing.

Drivers, such as:

drivers/pci/host/pci-mvebu.c
drivers/pci/host/pci-versatile.c
drivers/pci/host/pci-xgene.c

to name a few.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-12  9:44       ` Lorenzo Pieralisi
@ 2017-04-12 13:48         ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 63+ messages in thread
From: Benjamin Herrenschmidt @ 2017-04-12 13:48 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 2017-04-12 at 10:44 +0100, Lorenzo Pieralisi wrote:
> > This is a problem to be solved by the bridge itself. If ARM has a
> > mapping attribute to make stores non-posted, keep this an ARM specific
> > attribute at this stage I'd say.
> 
> I can't. Some PCI host bridge drivers (DT) are shared between ARM/ARM64
> and live in drivers (and are also reused on some arches eg microblaze)
> and they use ioremap() to map config space.
> 
> So the idea behind this series was to add an interface (that is
> overriden on ARM/ARM64), it started as a PCI specific interface (v1) and
> evolved to ioremap_nopost() (v2).

Then provide some kind of HAVE_* or config option indicating that this
mapping attribute exist so the bridge code at least has the option of
implementing an "alternative" mechanism when it doesn't rather than
silently being wrong :-)

(Or just return NULL, that works too)

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-12 11:31       ` Russell King - ARM Linux
@ 2017-04-12 13:51         ` Benjamin Herrenschmidt
  2017-04-12 14:16           ` Russell King - ARM Linux
  0 siblings, 1 reply; 63+ messages in thread
From: Benjamin Herrenschmidt @ 2017-04-12 13:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 2017-04-12 at 12:31 +0100, Russell King - ARM Linux wrote:
> default implementation should fail if it's not supportable on all
> architectures.? However, when we have existing drivers using an
> interface that doesn't provide the semantics they already require,
> then it makes no sense to effectively break these drivers on a range
> of existing architectures.
> 
> The question really is - what's the best way to solve the problem
> with
> existing drivers without breaking them.? I suspect that, sadly, the
> only realistic way forward here is via the litter-drivers-with-ifdefs
> approach since you don't like providing a default implementation that
> is compatible with what these drivers are already doing.

Then make ioremap_nopost return NULL when the arch doesn't have 
the right semantic.

The driver than can *chose* to either silently fallback to ioremap,
which has served us well for a long time despite being theorically in
violation of the spec, or do funny things like read back some register
after every config write to ensure ordering etc...

I much prefer that approach than having some generic ioremap function
that exposes a semantic that silently provides a weaker one on some
architecture.

At least we make the failure explicit, and the driver can take
alternate (possibly sub-optimal) action if it chooses to do so.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-12 13:51         ` Benjamin Herrenschmidt
@ 2017-04-12 14:16           ` Russell King - ARM Linux
  2017-04-12 14:41             ` Lorenzo Pieralisi
  0 siblings, 1 reply; 63+ messages in thread
From: Russell King - ARM Linux @ 2017-04-12 14:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Apr 12, 2017 at 11:51:59PM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2017-04-12 at 12:31 +0100, Russell King - ARM Linux wrote:
> > default implementation should fail if it's not supportable on all
> > architectures.? However, when we have existing drivers using an
> > interface that doesn't provide the semantics they already require,
> > then it makes no sense to effectively break these drivers on a range
> > of existing architectures.
> > 
> > The question really is - what's the best way to solve the problem
> > with
> > existing drivers without breaking them.? I suspect that, sadly, the
> > only realistic way forward here is via the litter-drivers-with-ifdefs
> > approach since you don't like providing a default implementation that
> > is compatible with what these drivers are already doing.
> 
> Then make ioremap_nopost return NULL when the arch doesn't have 
> the right semantic.
> 
> The driver than can *chose* to either silently fallback to ioremap,
> which has served us well for a long time despite being theorically in
> violation of the spec, or do funny things like read back some register
> after every config write to ensure ordering etc...
> 
> I much prefer that approach than having some generic ioremap function
> that exposes a semantic that silently provides a weaker one on some
> architecture.
> 
> At least we make the failure explicit, and the driver can take
> alternate (possibly sub-optimal) action if it chooses to do so.

The same points apply to things like pgprot_writecombine(),
pgprot_noncached(), pgprot_device().  Then there's also pgprot_nonposted()
that this series also introduces.

If ioremap_nopost() is not possible on an architecture, then
pgprot_nonposted() won't be possible either - but you've made no
mention of that so far.

Just like the proposed ioremap_nopost(), pgprot_nonposted() is given a
default implementation that uses pgprot_noncached().  Maybe we should
also make pci_remap_iospace() fail if pgprot_nonposted() is not defined
by the architecture?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-12 14:16           ` Russell King - ARM Linux
@ 2017-04-12 14:41             ` Lorenzo Pieralisi
  2017-04-12 22:30               ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-12 14:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Apr 12, 2017 at 03:16:55PM +0100, Russell King - ARM Linux wrote:
> On Wed, Apr 12, 2017 at 11:51:59PM +1000, Benjamin Herrenschmidt wrote:
> > On Wed, 2017-04-12 at 12:31 +0100, Russell King - ARM Linux wrote:
> > > default implementation should fail if it's not supportable on all
> > > architectures.? However, when we have existing drivers using an
> > > interface that doesn't provide the semantics they already require,
> > > then it makes no sense to effectively break these drivers on a range
> > > of existing architectures.
> > > 
> > > The question really is - what's the best way to solve the problem
> > > with
> > > existing drivers without breaking them.? I suspect that, sadly, the
> > > only realistic way forward here is via the litter-drivers-with-ifdefs
> > > approach since you don't like providing a default implementation that
> > > is compatible with what these drivers are already doing.
> > 
> > Then make ioremap_nopost return NULL when the arch doesn't have 
> > the right semantic.
> > 
> > The driver than can *chose* to either silently fallback to ioremap,
> > which has served us well for a long time despite being theorically in
> > violation of the spec, or do funny things like read back some register
> > after every config write to ensure ordering etc...
> > 
> > I much prefer that approach than having some generic ioremap function
> > that exposes a semantic that silently provides a weaker one on some
> > architecture.
> > 
> > At least we make the failure explicit, and the driver can take
> > alternate (possibly sub-optimal) action if it chooses to do so.
> 
> The same points apply to things like pgprot_writecombine(),
> pgprot_noncached(), pgprot_device().  Then there's also pgprot_nonposted()
> that this series also introduces.
> 
> If ioremap_nopost() is not possible on an architecture, then
> pgprot_nonposted() won't be possible either - but you've made no
> mention of that so far.
> 
> Just like the proposed ioremap_nopost(), pgprot_nonposted() is given a
> default implementation that uses pgprot_noncached().  Maybe we should
> also make pci_remap_iospace() fail if pgprot_nonposted() is not defined
> by the architecture?

Yes, I was about to mention that and you are right, I should deal with
that too unfortunately. BTW, I have not posted the drivers to make the
review easier (ie it would add 20 more patches to an already massive
patch series - that will be trimmed when the asm-generic include is
removed from arches according to this discussion).

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-12 14:41             ` Lorenzo Pieralisi
@ 2017-04-12 22:30               ` Benjamin Herrenschmidt
  2017-04-12 22:45                 ` Russell King - ARM Linux
  0 siblings, 1 reply; 63+ messages in thread
From: Benjamin Herrenschmidt @ 2017-04-12 22:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 2017-04-12 at 15:41 +0100, Lorenzo Pieralisi wrote:
> > > At least we make the failure explicit, and the driver can take
> > > alternate (possibly sub-optimal) action if it chooses to do so.
> > 
> > The same points apply to things like pgprot_writecombine(),
> > pgprot_noncached(), pgprot_device().? Then there's also pgprot_nonposted()
> > that this series also introduces.

No. pgprot_writecombine() silently falling back to simply non-cached is
ok as we aren't "weakening" the ordering rules silently here. Something
that is correct with writecombine will also work without. It's just an
optimisation. Not correctness.

Things like noncached() must of course be honored, and I don't think
we have a case anywhere where it isn't.

My point with nopost() is that it's never ok to silently downgrade it.
Code written with the assumption that there is no posting will be
*incorrect* if posting happens. We do live with that "bug" today indeed
but once we have that accessors we might start growing more code that
relies on the specific attribute that things aren't posted and will be
wrong on all the archs providing the default implementation.

This is why I insist that pgprot_nopost() if it exists globally, should
return NULL when the semantic cannot be provided. That way there is a
clear line in the sand. If the driver choses to operate with posted
non-cached anyway, then make it an explicit driver choice.

> > If ioremap_nopost() is not possible on an architecture, then
> > pgprot_nonposted() won't be possible either - but you've made no
> > mention of that so far.

Right. It's not on most in fact.

> > Just like the proposed ioremap_nopost(), pgprot_nonposted() is given a
> > default implementation that uses pgprot_noncached().? Maybe we should
> > also make pci_remap_iospace() fail if pgprot_nonposted() is not defined
> > by the architecture?

Or we *document* that mmap of IO space can result in something that is
partially non-posted.

> Yes, I was about to mention that and you are right, I should deal with
> that too unfortunately. BTW, I have not posted the drivers to make the
> review easier (ie it would add 20 more patches to an already massive
> patch series - that will be trimmed when the asm-generic include is
> removed from arches according to this discussion).
> 
> Thanks,
> Lorenzo

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-12 22:30               ` Benjamin Herrenschmidt
@ 2017-04-12 22:45                 ` Russell King - ARM Linux
  2017-04-13  0:53                   ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 63+ messages in thread
From: Russell King - ARM Linux @ 2017-04-12 22:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Apr 13, 2017 at 08:30:40AM +1000, Benjamin Herrenschmidt wrote:
> My point with nopost() is that it's never ok to silently downgrade it.
> Code written with the assumption that there is no posting will be
> *incorrect* if posting happens. We do live with that "bug" today indeed
> but once we have that accessors we might start growing more code that
> relies on the specific attribute that things aren't posted and will be
> wrong on all the archs providing the default implementation.
> 
> This is why I insist that pgprot_nopost() if it exists globally, should
> return NULL when the semantic cannot be provided.

Now you're not talking sense.  pgprot_nopost() does _not_ return a pointer.
You're talking here as if you're still talking about ioremap_nopost().
So, I think you're confused.

> > > Just like the proposed ioremap_nopost(), pgprot_nonposted() is given a
> > > default implementation that uses pgprot_noncached().? Maybe we should
> > > also make pci_remap_iospace() fail if pgprot_nonposted() is not defined
> > > by the architecture?
> 
> Or we *document* that mmap of IO space can result in something that is
> partially non-posted.

Oh, so we _can_ provide an interface that has weaker semantics than it
should provided we document it.

It's insane to have different behaviours from these two interfaces, yet
you seem to have said exactly that in your reply.

It's actually worse than that - what you've just said is that it's okay
for userspace to map IO space with weaker semantics than the PCI
specification states, but it's not okay for kernel space to do that.
Especially as userspace can't know what semantics its going to end up
with, this seems to be a very strange stance to take.

I'd say that if we can't offer the no-posting behaviour that PCI
specifies, then we shouldn't be exposing IO mappings to userspace.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-12 22:45                 ` Russell King - ARM Linux
@ 2017-04-13  0:53                   ` Benjamin Herrenschmidt
  2017-04-18  8:57                     ` Lorenzo Pieralisi
  0 siblings, 1 reply; 63+ messages in thread
From: Benjamin Herrenschmidt @ 2017-04-13  0:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 2017-04-12 at 23:45 +0100, Russell King - ARM Linux wrote:
> On Thu, Apr 13, 2017 at 08:30:40AM +1000, Benjamin Herrenschmidt wrote:
> > My point with nopost() is that it's never ok to silently downgrade it.
> > Code written with the assumption that there is no posting will be
> > *incorrect* if posting happens. We do live with that "bug" today indeed
> > but once we have that accessors we might start growing more code that
> > relies on the specific attribute that things aren't posted and will be
> > wrong on all the archs providing the default implementation.
> > 
> > This is why I insist that pgprot_nopost() if it exists globally, should
> > return NULL when the semantic cannot be provided.
> 
> Now you're not talking sense.??pgprot_nopost() does _not_ return a pointer.
> You're talking here as if you're still talking about ioremap_nopost().
> So, I think you're confused.

Nah, just "typo", I meant ioremap_nopost.

> > > > Just like the proposed ioremap_nopost(), pgprot_nonposted() is given a
> > > > default implementation that uses pgprot_noncached().? Maybe we should
> > > > also make pci_remap_iospace() fail if pgprot_nonposted() is not defined
> > > > by the architecture?
> > 
> > Or we *document* that mmap of IO space can result in something that is
> > partially non-posted.
> 
> Oh, so we _can_ provide an interface that has weaker semantics than it
> should provided we document it.
> 
> It's insane to have different behaviours from these two interfaces, yet
> you seem to have said exactly that in your reply.
>
> It's actually worse than that - what you've just said is that it's okay
> for userspace to map IO space with weaker semantics than the PCI
> specification states, but it's not okay for kernel space to do that.

That is not what I'm saying. What I'm saying is that it's not ok to
provide a generic mapping attribute that silently happens to be weaker
than documented on some architectures.

The PCI part is orthogonal. How do you handle PCI in absence of that
attribute is a separate problem (which is probably a matter of just
documenting things).

BTW. Is config space also non-posted on Intel with mmconfig ? I didn't
think they could do non posted MMIO stores, but maybe I'm wrong.

> Especially as userspace can't know what semantics its going to end up
> with, this seems to be a very strange stance to take.

That's why we document that the userspace interface for *PCI* is
relaxed.

> I'd say that if we can't offer the no-posting behaviour that PCI
> specifies, then we shouldn't be exposing IO mappings to userspace.

I strongly disagree. We've been doing it for decades and it works
fine in pretty much all cases.

Note also that some platforms (including some powerpc afaik) do provide
the non-posted behaviour, simply not as a mapping attribute. Internal
fabrics aren't necessarily doing posted writes and some bridges will
hold the response for non-posted requests.

Anyway, I don't object to trying to improve compliance with the spec
on arch that have such a mapping attribute. But I do object to having
a generic mapping attribute (that isn't fundamentally a PCI thing) that
silently downgrades to something weaker.

Ben.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 21/32] powerpc: include default ioremap_nopost() implementation
  2017-04-11 23:15       ` Benjamin Herrenschmidt
@ 2017-04-13  3:35         ` Michael Ellerman
  0 siblings, 0 replies; 63+ messages in thread
From: Michael Ellerman @ 2017-04-13  3:35 UTC (permalink / raw)
  To: linux-arm-kernel

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> 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 ?

No ... idea.

Looks like it was added in:

commit b41e5fffe8b81fc939067d8c1c195cc79115d5a3
Author:     Emil Medve <Emilian.Medve@Freescale.com>
AuthorDate: Sat May 3 06:34:04 2008 +1000
Commit:     Paul Mackerras <paulus@samba.org>
CommitDate: Mon May 5 16:47:14 2008 +1000

    [POWERPC] devres: Add devm_ioremap_prot()
    
    We provide an ioremap_flags, so this provides a corresponding
    devm_ioremap_prot.  The slight name difference is at Ben
    Herrenschmidt's request as he plans on changing ioremap_flags to
    ioremap_prot in the future.
    
    Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
    Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
    Acked-by: Tejun Heo <htejun@gmail.com>
    Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Paul Mackerras <paulus@samba.org>

diff --git a/include/asm-powerpc/io.h b/include/asm-powerpc/io.h
index afae0697e8ce..e0062d73db1c 100644
--- a/include/asm-powerpc/io.h
+++ b/include/asm-powerpc/io.h
@@ -18,6 +18,9 @@ extern int check_legacy_ioport(unsigned long base_port);
 #define _PNPWRP		0xa79
 #define PNPBIOS_BASE	0xf000
 
+#include <linux/device.h>
+#include <linux/io.h>
+
 #include <linux/compiler.h>
 #include <asm/page.h>
 #include <asm/byteorder.h>
@@ -744,6 +747,9 @@ static inline void * bus_to_virt(unsigned long address)
 
 #define clrsetbits_8(addr, clear, set) clrsetbits(8, addr, clear, set)
 
+void __iomem *devm_ioremap_prot(struct device *dev, resource_size_t offset,
+				size_t size, unsigned long flags);
+
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_POWERPC_IO_H */


I'll try removing it and see what breaks.

cheers

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-13  0:53                   ` Benjamin Herrenschmidt
@ 2017-04-18  8:57                     ` Lorenzo Pieralisi
  2017-04-18 10:36                       ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-18  8:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Apr 13, 2017 at 10:53:00AM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2017-04-12 at 23:45 +0100, Russell King - ARM Linux wrote:
> > On Thu, Apr 13, 2017 at 08:30:40AM +1000, Benjamin Herrenschmidt wrote:
> > > My point with nopost() is that it's never ok to silently downgrade it.
> > > Code written with the assumption that there is no posting will be
> > > *incorrect* if posting happens. We do live with that "bug" today indeed
> > > but once we have that accessors we might start growing more code that
> > > relies on the specific attribute that things aren't posted and will be
> > > wrong on all the archs providing the default implementation.
> > > 
> > > This is why I insist that pgprot_nopost() if it exists globally, should
> > > return NULL when the semantic cannot be provided.
> > 
> > Now you're not talking sense.??pgprot_nopost() does _not_ return a pointer.
> > You're talking here as if you're still talking about ioremap_nopost().
> > So, I think you're confused.
> 
> Nah, just "typo", I meant ioremap_nopost.
> 
> > > > > Just like the proposed ioremap_nopost(), pgprot_nonposted() is given a
> > > > > default implementation that uses pgprot_noncached().? Maybe we should
> > > > > also make pci_remap_iospace() fail if pgprot_nonposted() is not defined
> > > > > by the architecture?
> > > 
> > > Or we *document* that mmap of IO space can result in something that is
> > > partially non-posted.
> > 
> > Oh, so we _can_ provide an interface that has weaker semantics than it
> > should provided we document it.
> > 
> > It's insane to have different behaviours from these two interfaces, yet
> > you seem to have said exactly that in your reply.
> >
> > It's actually worse than that - what you've just said is that it's okay
> > for userspace to map IO space with weaker semantics than the PCI
> > specification states, but it's not okay for kernel space to do that.
> 
> That is not what I'm saying. What I'm saying is that it's not ok to
> provide a generic mapping attribute that silently happens to be weaker
> than documented on some architectures.
> 
> The PCI part is orthogonal. How do you handle PCI in absence of that
> attribute is a separate problem (which is probably a matter of just
> documenting things).

I can add a defined(pgprot_nonposted) to pci_remap_iospace() if that's
not too ugly (I suspect Bjorn is thrilled about it :)), that plus
the Kconfig option for ioremap_nopost() should complete this series.

int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
{
#if defined(PCI_IOBASE) && defined(CONFIG_MMU) && defined(pgprot_nonposted)
	unsigned long vaddr = (unsigned long)PCI_IOBASE + res->start;

	if (!(res->flags & IORESOURCE_IO))
		return -EINVAL;

	if (res->end > IO_SPACE_LIMIT)
		return -EINVAL
	return ioremap_page_range(vaddr, vaddr + resource_size(res), phys_addr,
				  pgprot_nonposted(PAGE_KERNEL));
#else
	/* this architecture does not have memory mapped I/O space,
	   so this function should never be called */
	WARN_ONCE(1, "This architecture does not support memory mapped I/O\n");
	return -ENODEV;
#endif
}

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-18  8:57                     ` Lorenzo Pieralisi
@ 2017-04-18 10:36                       ` Benjamin Herrenschmidt
  2017-04-18 11:03                         ` Lorenzo Pieralisi
  0 siblings, 1 reply; 63+ messages in thread
From: Benjamin Herrenschmidt @ 2017-04-18 10:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2017-04-18 at 09:57 +0100, Lorenzo Pieralisi wrote:
> I can add a defined(pgprot_nonposted) to pci_remap_iospace() if that's
> not too ugly (I suspect Bjorn is thrilled about it :)), that plus
> the Kconfig option for ioremap_nopost() should complete this series.
> 
> int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
> {
> #if defined(PCI_IOBASE) && defined(CONFIG_MMU) && defined(pgprot_nonposted)
> ????????unsigned long vaddr = (unsigned long)PCI_IOBASE + res->start;
> 
> ????????if (!(res->flags & IORESOURCE_IO))
> ????????????????return -EINVAL;
> 
> ????????if (res->end > IO_SPACE_LIMIT)
> ????????????????return -EINVAL
> ????????return ioremap_page_range(vaddr, vaddr + resource_size(res), phys_addr,
> ????????????????????????????????? pgprot_nonposted(PAGE_KERNEL));
> #else
> ????????/* this architecture does not have memory mapped I/O space,
> ?????????? so this function should never be called */
> ????????WARN_ONCE(1, "This architecture does not support memory mapped I/O\n");
> ????????return -ENODEV;
> #endif

The above would effectively disable mmap'ing of IO space for any
architecture that doesn't have pgprot_nonposted... so everybody except
ARM. Thus breaking a number of systems that have been working fine for
years.

I fail to see the point....

I think you are giving the whole non-posted stuff way more importance
than it deserves. It's originally a kludge Intel did to PCI because it
well with their synchronous IO space, which was itself a remnant of
pre-history that should have long died.

In the specific case of PCI (again I'm not talking about the general
case of pgprot/ioremap_nonposted), we have routinely been "violating"
that rule, at least on the CPU -> PCI Bridge path (the PCI bridge
itself tends to respect it though I've seen exceptions) for decades
without any adverse effect.

I don't think there's much code (if any) out there which actually
relies on the non-posted characteristics of IO space.

I don't care *that* much mind you, we dropped IO space on PCI with
POWER8, but it would break stuff on existing older machines such as
PowerMacs for no good reason.

I'd rather we document that mmap'ing IO space via sysfs doesn't fully
respect the "non-posted" semantics of IO and be done with it.

Is there any other practical use of non-posted mappings ? Config space
I suppose, though here mostly PCI host bridges handle it by doing a
read back in the config ops...

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-18 10:36                       ` Benjamin Herrenschmidt
@ 2017-04-18 11:03                         ` Lorenzo Pieralisi
  2017-04-18 22:38                           ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-18 11:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 18, 2017 at 08:36:48PM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2017-04-18 at 09:57 +0100, Lorenzo Pieralisi wrote:
> > I can add a defined(pgprot_nonposted) to pci_remap_iospace() if that's
> > not too ugly (I suspect Bjorn is thrilled about it :)), that plus
> > the Kconfig option for ioremap_nopost() should complete this series.
> > 
> > int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
> > {
> > #if defined(PCI_IOBASE) && defined(CONFIG_MMU) && defined(pgprot_nonposted)
> > ????????unsigned long vaddr = (unsigned long)PCI_IOBASE + res->start;
> > 
> > ????????if (!(res->flags & IORESOURCE_IO))
> > ????????????????return -EINVAL;
> > 
> > ????????if (res->end > IO_SPACE_LIMIT)
> > ????????????????return -EINVAL
> > ????????return ioremap_page_range(vaddr, vaddr + resource_size(res), phys_addr,
> > ????????????????????????????????? pgprot_nonposted(PAGE_KERNEL));
> > #else
> > ????????/* this architecture does not have memory mapped I/O space,
> > ?????????? so this function should never be called */
> > ????????WARN_ONCE(1, "This architecture does not support memory mapped I/O\n");
> > ????????return -ENODEV;
> > #endif
> 
> The above would effectively disable mmap'ing of IO space for any
> architecture that doesn't have pgprot_nonposted... so everybody except
> ARM. Thus breaking a number of systems that have been working fine for
> years.

pci_remap_iospace() is used on ARM/ARM64 only AFAICT I do not understand
what I would actually break (and I am not sure at all how well PCI IO
space is tested on ARM/ARM64 machines anyway).

> I fail to see the point....
> 
> I think you are giving the whole non-posted stuff way more importance
> than it deserves. It's originally a kludge Intel did to PCI because it
> well with their synchronous IO space, which was itself a remnant of
> pre-history that should have long died.
> 
> In the specific case of PCI (again I'm not talking about the general
> case of pgprot/ioremap_nonposted), we have routinely been "violating"
> that rule, at least on the CPU -> PCI Bridge path (the PCI bridge
> itself tends to respect it though I've seen exceptions) for decades
> without any adverse effect.
> 
> I don't think there's much code (if any) out there which actually
> relies on the non-posted characteristics of IO space.
> 
> I don't care *that* much mind you, we dropped IO space on PCI with
> POWER8, but it would break stuff on existing older machines such as
> PowerMacs for no good reason.

Again, the change above should not break anything.

> respect the "non-posted" semantics of IO and be done with it.

I can do that too (or leave IO space alone, I do not care either).

> Is there any other practical use of non-posted mappings ? Config space
> I suppose, though here mostly PCI host bridges handle it by doing a
> read back in the config ops...

I started by adding a pci_remap_cfgspace() interface. Bjorn did not
like it to be PCI specific so I made it a generic one. I am not aware
of any other non-posted writes ioremap requirements apart from config
space.

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 04/32] asm-generic: add ioremap_nopost() remap interface
  2017-04-12 11:20     ` Russell King - ARM Linux
@ 2017-04-18 15:49       ` Lorenzo Pieralisi
  2017-04-18 16:31         ` Bjorn Helgaas
  2017-04-18 22:43         ` Benjamin Herrenschmidt
  0 siblings, 2 replies; 63+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-18 15:49 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Apr 12, 2017 at 12:20:22PM +0100, Russell King - ARM Linux wrote:
> On Tue, Apr 11, 2017 at 11:39:43PM +1000, Benjamin Herrenschmidt wrote:
> > On Tue, 2017-04-11 at 13:28 +0100, Lorenzo Pieralisi wrote:
> > > +static inline void __iomem *ioremap_nopost(phys_addr_t offset,
> > > size_t size)
> > > +{
> > > +???????return ioremap_nocache(offset, size);
> > > +}
> > > +
> > 
> > No this is wrong as I explained.
> > 
> > This is a semantic that simply *cannot* be generically provided accross
> > architectures as a mapping attribute.
> > 
> > The solution to your problem lies elsewhere.
> 
> I disagree.  Sure, it may not be supportable across all architectures,
> but we're not introducing something brand new here.
> 
> What we're trying to do is to fix some _existing_ drivers that are
> already using ioremap() to map the PCI IO and configuration spaces.
> Maybe it would help if those drivers were part of this patch set,
> rather than the patch set just introducing a whole new architecture
> interface without really showing where the problem drivers are.
> 
> The issue here is that if we make this new ioremap_nopost() fail by
> returning NULL if an architecture does not provide an implementation,
> then these drivers are going to start failing on such architectures.
> 
> It is only safe to do that where we know for certain that the drivers
> are not used - but I don't think that's the case here (it's difficult
> to tell, because no drivers are updated in this series, so we don't
> know which are going to be affected.)
> 
> So, the question is:
> 
> - is it better to provide a default implementation which provides the
>   functionality that existing drivers are _already_ using, albiet not
>   entirely correctly.
> 
> or:
> 
> - is it better to break drivers on architectures when they're converted
>   to fix up this issue.
> 
> What, I suppose, we could do is not bother with a default implementation,
> but instead litter drivers with:
> 
> +#ifdef ioremap_post
> +	base = ioremap_post(...);
> +#else
> 	base = ioremap(...);
> +#endif
> 
> which gets around your objection - not providing a default that's weaker
> than required, but also not breaking the drivers.  The problem is that
> we end up littering drivers with ifdefs.
> 
> However, I'm willing to bet that the architectures that you're saying
> will not be able to support the weaker semantic won't have any need to
> use these drivers, or if they do, the drivers will need the method of
> accessing stuff like PCI IO and configuration spaces radically altered.
> 
> So, maybe the best solution is to not provide any kind of default
> implementation, add a HAVE_IOREMAP_POST Kconfig symbol, have architectures
> select that when they do provide ioremap_post(), and make the drivers
> depend on that (so we don't end up trying to build these drivers on
> architectures where they can never work.)  Down side to that is reduced
> build coverage.

I can do that yes, which already means I have to know if eg microblaze
(drivers/pci/host/pcie-xilinx.c) can provide a mapping with nonposted
writes semantics, otherwise it is a dead-end.

Another option would be going back to what v1 did, namely, to implement
a pci_remap_cfgspace() interface (it is the _nopost() suffix that stirred
debate - nobody would object to having a default pci_remap_cfgspace()
implementation that defaults to ioremap_nocache(), I know Bjorn does not
like it to be PCI specific, just adding an option on the table to make
progress).

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 04/32] asm-generic: add ioremap_nopost() remap interface
  2017-04-18 15:49       ` Lorenzo Pieralisi
@ 2017-04-18 16:31         ` Bjorn Helgaas
  2017-04-18 22:43         ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 63+ messages in thread
From: Bjorn Helgaas @ 2017-04-18 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 18, 2017 at 10:49 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Wed, Apr 12, 2017 at 12:20:22PM +0100, Russell King - ARM Linux wrote:
>> On Tue, Apr 11, 2017 at 11:39:43PM +1000, Benjamin Herrenschmidt wrote:
>> > On Tue, 2017-04-11 at 13:28 +0100, Lorenzo Pieralisi wrote:
>> > > +static inline void __iomem *ioremap_nopost(phys_addr_t offset,
>> > > size_t size)
>> > > +{
>> > > +       return ioremap_nocache(offset, size);
>> > > +}
>> > > +
>> >
>> > No this is wrong as I explained.
>> >
>> > This is a semantic that simply *cannot* be generically provided accross
>> > architectures as a mapping attribute.
>> >
>> > The solution to your problem lies elsewhere.
>>
>> I disagree.  Sure, it may not be supportable across all architectures,
>> but we're not introducing something brand new here.
>>
>> What we're trying to do is to fix some _existing_ drivers that are
>> already using ioremap() to map the PCI IO and configuration spaces.
>> Maybe it would help if those drivers were part of this patch set,
>> rather than the patch set just introducing a whole new architecture
>> interface without really showing where the problem drivers are.
>>
>> The issue here is that if we make this new ioremap_nopost() fail by
>> returning NULL if an architecture does not provide an implementation,
>> then these drivers are going to start failing on such architectures.
>>
>> It is only safe to do that where we know for certain that the drivers
>> are not used - but I don't think that's the case here (it's difficult
>> to tell, because no drivers are updated in this series, so we don't
>> know which are going to be affected.)
>>
>> So, the question is:
>>
>> - is it better to provide a default implementation which provides the
>>   functionality that existing drivers are _already_ using, albiet not
>>   entirely correctly.
>>
>> or:
>>
>> - is it better to break drivers on architectures when they're converted
>>   to fix up this issue.
>>
>> What, I suppose, we could do is not bother with a default implementation,
>> but instead litter drivers with:
>>
>> +#ifdef ioremap_post
>> +     base = ioremap_post(...);
>> +#else
>>       base = ioremap(...);
>> +#endif
>>
>> which gets around your objection - not providing a default that's weaker
>> than required, but also not breaking the drivers.  The problem is that
>> we end up littering drivers with ifdefs.
>>
>> However, I'm willing to bet that the architectures that you're saying
>> will not be able to support the weaker semantic won't have any need to
>> use these drivers, or if they do, the drivers will need the method of
>> accessing stuff like PCI IO and configuration spaces radically altered.
>>
>> So, maybe the best solution is to not provide any kind of default
>> implementation, add a HAVE_IOREMAP_POST Kconfig symbol, have architectures
>> select that when they do provide ioremap_post(), and make the drivers
>> depend on that (so we don't end up trying to build these drivers on
>> architectures where they can never work.)  Down side to that is reduced
>> build coverage.
>
> I can do that yes, which already means I have to know if eg microblaze
> (drivers/pci/host/pcie-xilinx.c) can provide a mapping with nonposted
> writes semantics, otherwise it is a dead-end.
>
> Another option would be going back to what v1 did, namely, to implement
> a pci_remap_cfgspace() interface (it is the _nopost() suffix that stirred
> debate - nobody would object to having a default pci_remap_cfgspace()
> implementation that defaults to ioremap_nocache(), I know Bjorn does not
> like it to be PCI specific, just adding an option on the table to make
> progress).

The reason I objected to pci_remap_cfgspace() was that I thought
ioremap_nopost() was implementable across arches.  That turned out not
to be true, so I'm fine with calling it pci_remap_cfgspace().  Maybe
it's worth a note in the default implementation that arches should
override it with a non-postable version if they can.

Bjorn

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
  2017-04-18 11:03                         ` Lorenzo Pieralisi
@ 2017-04-18 22:38                           ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 63+ messages in thread
From: Benjamin Herrenschmidt @ 2017-04-18 22:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2017-04-18 at 12:03 +0100, Lorenzo Pieralisi wrote:
> > The above would effectively disable mmap'ing of IO space for any
> > architecture that doesn't have pgprot_nonposted... so everybody
> > except
> > ARM. Thus breaking a number of systems that have been working fine
> > for
> > years.
> 
> pci_remap_iospace() is used on ARM/ARM64 only AFAICT I do not
> understand
> what I would actually break (and I am not sure at all how well PCI IO
> space is tested on ARM/ARM64 machines anyway).

My bad, I incorrectly assumed you were hacking the generic sysfs PCI
mapping.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 04/32] asm-generic: add ioremap_nopost() remap interface
  2017-04-18 15:49       ` Lorenzo Pieralisi
  2017-04-18 16:31         ` Bjorn Helgaas
@ 2017-04-18 22:43         ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 63+ messages in thread
From: Benjamin Herrenschmidt @ 2017-04-18 22:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2017-04-18 at 16:49 +0100, Lorenzo Pieralisi wrote:
> I can do that yes, which already means I have to know if eg microblaze
> (drivers/pci/host/pcie-xilinx.c) can provide a mapping with nonposted
> writes semantics, otherwise it is a dead-end.
> 
> Another option would be going back to what v1 did, namely, to implement
> a pci_remap_cfgspace() interface (it is the _nopost() suffix that stirred
> debate - nobody would object to having a default pci_remap_cfgspace()
> implementation that defaults to ioremap_nocache(), I know Bjorn does not
> like it to be PCI specific, just adding an option on the table to make
> progress).

Well, it boils down again to the fact that a mapping attribute isn't
sufficient.

Let's say I'm microblaze and I can't do non-posted mapping. Then the
Host Bridge driver needs to *know* that so it can implement a different
workaround, such as reading back from some bridge register after every
config write which ensures the previous write reached its destination
for example, or whatever other IP specific mechanism.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 63+ messages in thread

end of thread, other threads:[~2017-04-18 22:43 UTC | newest]

Thread overview: 63+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-11 12:28 [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 01/32] PCI: remove __weak tag from pci_remap_iospace() Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 02/32] asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 03/32] PCI: fix pci_remap_iospace() " Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 04/32] asm-generic: add ioremap_nopost() remap interface Lorenzo Pieralisi
2017-04-11 13:39   ` Benjamin Herrenschmidt
2017-04-11 14:31     ` Lorenzo Pieralisi
2017-04-11 23:14       ` Benjamin Herrenschmidt
2017-04-12 10:00         ` Lorenzo Pieralisi
2017-04-12 11:20     ` Russell King - ARM Linux
2017-04-18 15:49       ` Lorenzo Pieralisi
2017-04-18 16:31         ` Bjorn Helgaas
2017-04-18 22:43         ` Benjamin Herrenschmidt
2017-04-11 12:28 ` [PATCH v3 05/32] alpha: include default ioremap_nopost() implementation Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 06/32] avr32: " Lorenzo Pieralisi
2017-04-11 13:55   ` Nicolas Ferre
2017-04-11 12:28 ` [PATCH v3 07/32] arc: " Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 08/32] cris: " Lorenzo Pieralisi
2017-04-11 13:15   ` Jesper Nilsson
2017-04-11 12:28 ` [PATCH v3 09/32] frv: " Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 10/32] hexagon: " Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 11/32] ia64: " Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 12/32] m32r: " Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 13/32] m68k: " Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 14/32] metag: " Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 15/32] microblaze: " Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 16/32] mips: " Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 17/32] mn10300: " Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 18/32] nios2: " Lorenzo Pieralisi
2017-04-11 12:28 ` [PATCH v3 19/32] openrisc: " Lorenzo Pieralisi
2017-04-11 12:29 ` [PATCH v3 20/32] parisc: " Lorenzo Pieralisi
2017-04-11 12:29 ` [PATCH v3 21/32] powerpc: " Lorenzo Pieralisi
2017-04-11 13:38   ` Benjamin Herrenschmidt
2017-04-11 14:24     ` Lorenzo Pieralisi
2017-04-11 23:15       ` Benjamin Herrenschmidt
2017-04-13  3:35         ` Michael Ellerman
2017-04-11 12:29 ` [PATCH v3 22/32] s390: " Lorenzo Pieralisi
2017-04-11 12:29 ` [PATCH v3 23/32] sh: " Lorenzo Pieralisi
2017-04-11 12:29 ` [PATCH v3 24/32] sparc: " Lorenzo Pieralisi
2017-04-11 12:29 ` [PATCH v3 25/32] tile: " Lorenzo Pieralisi
2017-04-11 12:29 ` [PATCH v3 26/32] unicore32: " Lorenzo Pieralisi
2017-04-11 12:29 ` [PATCH v3 27/32] x86: " Lorenzo Pieralisi
2017-04-11 12:29 ` [PATCH v3 28/32] xtensa: " Lorenzo Pieralisi
2017-04-11 12:29 ` [PATCH v3 29/32] arm64: implement ioremap_nopost() interface Lorenzo Pieralisi
2017-04-11 12:29 ` [PATCH v3 30/32] arm: " Lorenzo Pieralisi
2017-04-11 12:29 ` [PATCH v3 31/32] lib: fix Devres devm_ioremap_* offset parameter kerneldoc description Lorenzo Pieralisi
2017-04-11 12:29 ` [PATCH v3 32/32] lib: implement Devres ioremap_nopost() interface Lorenzo Pieralisi
2017-04-11 13:38 ` [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings Benjamin Herrenschmidt
2017-04-11 14:08   ` Lorenzo Pieralisi
2017-04-11 23:12     ` Benjamin Herrenschmidt
2017-04-12  9:44       ` Lorenzo Pieralisi
2017-04-12 13:48         ` Benjamin Herrenschmidt
2017-04-12 11:31       ` Russell King - ARM Linux
2017-04-12 13:51         ` Benjamin Herrenschmidt
2017-04-12 14:16           ` Russell King - ARM Linux
2017-04-12 14:41             ` Lorenzo Pieralisi
2017-04-12 22:30               ` Benjamin Herrenschmidt
2017-04-12 22:45                 ` Russell King - ARM Linux
2017-04-13  0:53                   ` Benjamin Herrenschmidt
2017-04-18  8:57                     ` Lorenzo Pieralisi
2017-04-18 10:36                       ` Benjamin Herrenschmidt
2017-04-18 11:03                         ` Lorenzo Pieralisi
2017-04-18 22:38                           ` Benjamin Herrenschmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).