[PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings

[PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings

[Lorenzo Pieralisi]

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

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
	- Rebased against v4.11-rc4

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

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

Cc: Pratyush Anand <pratyush.anand@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Mingkai Hu <mingkai.hu@freescale.com>
Cc: Tanmay Inamdar <tinamdar@apm.com>
Cc: Murali Karicheri <m-karicheri2@ti.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Wenrui Li <wenrui.li@rock-chips.com>
Cc: Shawn Lin <shawn.lin@rock-chips.com>
Cc: Minghuan Lian <minghuan.Lian@freescale.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Jon Mason <jonmason@broadcom.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Gabriele Paoloni <gabriele.paoloni@huawei.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Joao Pinto <Joao.Pinto@synopsys.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Haavard Skinnemoen <hskinnemoen@gmail.com>
Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
Cc: Zhou Wang <wangzhou1@hisilicon.com>
Cc: Roy Zang <tie-fei.zang@freescale.com>

Lorenzo Pieralisi (22):
  PCI: remove __weak tag from pci_remap_iospace()
  asm-generic/io.h: add ioremap_nopost remap interface
  asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute
  PCI: fix pci_remap_iospace() remap attribute
  ARM64: implement ioremap_nopost() interface
  ARM: implement ioremap_nopost() interface
  PCI: ECAM: use ioremap_nopost() to map config region
  lib: implement Devres ioremap_nopost() interface
  PCI: xilinx: update PCI config space remap function
  PCI: xilinx-nwl: update PCI config space remap function
  PCI: spear13xx: update PCI config space remap function
  PCI: rockchip: update PCI config space remap function
  PCI: qcom: update PCI config space remap function
  PCI: iproc-platform: update PCI config space remap function
  PCI: hisi: update PCI config space remap function
  PCI: designware: update PCI config space remap function
  PCI: armada8k: update PCI config space remap function
  PCI: xgene: update PCI config space remap function
  PCI: tegra: update PCI config space remap function
  PCI: layerscape: update PCI config space remap function
  PCI: keystone-dw: update PCI config space remap function
  PCI: versatile: update PCI config space remap function

 Documentation/driver-model/devres.txt  |  3 ++
 arch/alpha/include/asm/io.h            |  1 +
 arch/arm/include/asm/io.h              | 10 +++++
 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/frv/include/asm/io.h              |  1 +
 arch/ia64/include/asm/io.h             |  1 +
 arch/x86/include/asm/io.h              |  1 +
 drivers/pci/dwc/pci-keystone-dw.c      |  2 +-
 drivers/pci/dwc/pci-layerscape.c       |  2 +-
 drivers/pci/dwc/pcie-armada8k.c        |  2 +-
 drivers/pci/dwc/pcie-designware-host.c | 12 +++---
 drivers/pci/dwc/pcie-hisi.c            |  7 ++-
 drivers/pci/dwc/pcie-qcom.c            |  2 +-
 drivers/pci/dwc/pcie-spear13xx.c       |  2 +-
 drivers/pci/ecam.c                     |  5 ++-
 drivers/pci/host/pci-tegra.c           |  4 +-
 drivers/pci/host/pci-versatile.c       |  3 +-
 drivers/pci/host/pci-xgene.c           |  4 +-
 drivers/pci/host/pcie-iproc-platform.c |  3 +-
 drivers/pci/host/pcie-rockchip.c       |  2 +-
 drivers/pci/host/pcie-xilinx-nwl.c     |  2 +-
 drivers/pci/host/pcie-xilinx.c         |  2 +-
 drivers/pci/pci.c                      |  4 +-
 include/asm-generic/io.h               |  4 ++
 include/asm-generic/pgtable.h          |  4 ++
 include/linux/device.h                 |  2 +
 include/linux/io.h                     |  2 +
 lib/devres.c                           | 78 ++++++++++++++++++++++++++++++++++
 31 files changed, 166 insertions(+), 28 deletions(-)

-- 
2.10.0

[PATCH v2 01/22] PCI: remove __weak tag from pci_remap_iospace()

[Lorenzo Pieralisi]

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

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Lorenzo Pieralisi]

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 that
defaults to ioremap_nocache() (which should provide a sane default
behaviour) but still allowing architectures on which ioremap_nocache()
results in posted write transactions to override the function
call with an arch specific implementation that complies with
the PCI specifications for configuration transactions.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Haavard Skinnemoen <hskinnemoen@gmail.com>
---
 arch/alpha/include/asm/io.h | 1 +
 arch/avr32/include/asm/io.h | 1 +
 arch/frv/include/asm/io.h   | 1 +
 arch/ia64/include/asm/io.h  | 1 +
 arch/x86/include/asm/io.h   | 1 +
 include/asm-generic/io.h    | 4 ++++
 6 files changed, 9 insertions(+)

diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
index ff40491..27379ea 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
+#define ioremap_nopost ioremap_nocache
 
 static inline void iounmap(volatile void __iomem *addr)
 {
diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
index f855646..3f1ced8 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
+#define ioremap_nopost ioremap_nocache
 
 #define cached(addr) P1SEGADDR(addr)
 #define uncached(addr) P2SEGADDR(addr)
diff --git a/arch/frv/include/asm/io.h b/arch/frv/include/asm/io.h
index 8062fc7..302fb8c 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
+#define ioremap_nopost ioremap_nocache
 
 extern void iounmap(void volatile __iomem *addr);
 
diff --git a/arch/ia64/include/asm/io.h b/arch/ia64/include/asm/io.h
index 5de673a..70a4985 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
+#define ioremap_nopost ioremap_nocache
 
 
 /*
diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 7afb0e2..50b292f 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
 extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
 extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
 #define ioremap_uc ioremap_uc
+#define ioremap_nopost ioremap_nocache
 
 extern void __iomem *ioremap_cache(resource_size_t offset, unsigned long size);
 extern void __iomem *ioremap_prot(resource_size_t offset, unsigned long size, unsigned long prot_val);
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 7ef015e..0e81938 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
 #endif /* CONFIG_GENERIC_IOMAP */
 #endif /* CONFIG_HAS_IOPORT_MAP */
 
+#ifndef ioremap_nopost
+#define ioremap_nopost ioremap_nocache
+#endif
+
 #ifndef xlate_dev_kmem_ptr
 #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
 static inline void *xlate_dev_kmem_ptr(void *addr)
-- 
2.10.0

[PATCH v2 03/22] asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute

[Lorenzo Pieralisi]

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

[PATCH v2 04/22] PCI: fix pci_remap_iospace() remap attribute

[Lorenzo Pieralisi]

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

[PATCH v2 05/22] ARM64: implement ioremap_nopost() interface

[Lorenzo Pieralisi]

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.

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

[PATCH v2 06/22] ARM: implement ioremap_nopost() interface

[Lorenzo Pieralisi]

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 | 10 ++++++++++
 arch/arm/mm/ioremap.c     |  7 +++++++
 arch/arm/mm/nommu.c       |  9 +++++++++
 3 files changed, 26 insertions(+)

diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 42871fb..49913d1 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,9 @@ 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);
+#define ioremap_nopost ioremap_nopost
+
 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

[PATCH v2 07/22] PCI: ECAM: use ioremap_nopost() to map config region

[Lorenzo Pieralisi]

Current ECAM kernel implementation uses ioremap() to map the ECAM
configuration space memory region; this is not safe in that on some
architectures the ioremap interface provides mappings that allow posted
write transactions. This, as highlighted in the PCIe specifications
(4.0 - Rev0.3, "Ordering Considerations for the Enhanced Configuration
Address Mechanism"), can create ordering issues for software because
posted writes transactions on the CPU host bus are non posted in the
PCI express fabric.

Update the ioremap() interface to use ioremap_nopost() whose
mapping attributes guarantee that non-posted writes transactions
are issued for memory writes within the ECAM memory mapped address
region.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/ecam.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
index 2fee61b..70d722a 100644
--- a/drivers/pci/ecam.c
+++ b/drivers/pci/ecam.c
@@ -84,12 +84,13 @@ struct pci_config_window *pci_ecam_create(struct device *dev,
 		if (!cfg->winp)
 			goto err_exit_malloc;
 		for (i = 0; i < bus_range; i++) {
-			cfg->winp[i] = ioremap(cfgres->start + i * bsz, bsz);
+			cfg->winp[i] = ioremap_nopost(cfgres->start + i * bsz,
+						      bsz);
 			if (!cfg->winp[i])
 				goto err_exit_iomap;
 		}
 	} else {
-		cfg->win = ioremap(cfgres->start, bus_range * bsz);
+		cfg->win = ioremap_nopost(cfgres->start, bus_range * bsz);
 		if (!cfg->win)
 			goto err_exit_iomap;
 	}
-- 
2.10.0

[PATCH v2 08/22] lib: implement Devres ioremap_nopost() interface

[Lorenzo Pieralisi]

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 cb1464c..4e05660 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: BUS offset 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

[PATCH v2 09/22] PCI: xilinx: update PCI config space remap function

[Lorenzo Pieralisi]

PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
Cc: Michal Simek <michal.simek@xilinx.com>
---
 drivers/pci/host/pcie-xilinx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index 7f030f5..e96f4a7 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -606,7 +606,7 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)
 		return err;
 	}
 
-	port->reg_base = devm_ioremap_resource(dev, &regs);
+	port->reg_base = devm_ioremap_nopost_resource(dev, &regs);
 	if (IS_ERR(port->reg_base))
 		return PTR_ERR(port->reg_base);
 
-- 
2.10.0

[PATCH v2 10/22] PCI: xilinx-nwl: update PCI config space remap function

[Lorenzo Pieralisi]

PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
Cc: Michal Simek <michal.simek@xilinx.com>
---
 drivers/pci/host/pcie-xilinx-nwl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/pcie-xilinx-nwl.c b/drivers/pci/host/pcie-xilinx-nwl.c
index 4c3e0ab..cc042b1 100644
--- a/drivers/pci/host/pcie-xilinx-nwl.c
+++ b/drivers/pci/host/pcie-xilinx-nwl.c
@@ -761,7 +761,7 @@ static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
 	pcie->phys_pcie_reg_base = res->start;
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
-	pcie->ecam_base = devm_ioremap_resource(dev, res);
+	pcie->ecam_base = devm_ioremap_nopost_resource(dev, res);
 	if (IS_ERR(pcie->ecam_base))
 		return PTR_ERR(pcie->ecam_base);
 	pcie->phys_ecam_base = res->start;
-- 
2.10.0

[PATCH v2 11/22] PCI: spear13xx: update PCI config space remap function

[Lorenzo Pieralisi]

PCI configuration space should be mapped with a memory region type that
generate on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Pratyush Anand <pratyush.anand@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/dwc/pcie-spear13xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/dwc/pcie-spear13xx.c b/drivers/pci/dwc/pcie-spear13xx.c
index eaa4ea8..0fe2b2b 100644
--- a/drivers/pci/dwc/pcie-spear13xx.c
+++ b/drivers/pci/dwc/pcie-spear13xx.c
@@ -273,7 +273,7 @@ static int spear13xx_pcie_probe(struct platform_device *pdev)
 	}
 
 	dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
-	pci->dbi_base = devm_ioremap_resource(dev, dbi_base);
+	pci->dbi_base = devm_ioremap_nopost_resource(dev, dbi_base);
 	if (IS_ERR(pci->dbi_base)) {
 		dev_err(dev, "couldn't remap dbi base %p\n", dbi_base);
 		ret = PTR_ERR(pci->dbi_base);
-- 
2.10.0

[PATCH v2 12/22] PCI: rockchip: update PCI config space remap function

[Lorenzo Pieralisi]

PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Wenrui Li <wenrui.li@rock-chips.com>
Cc: Shawn Lin <shawn.lin@rock-chips.com>
---
 drivers/pci/host/pcie-rockchip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 26ddd35..a078131 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -822,7 +822,7 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
 	regs = platform_get_resource_byname(pdev,
 					    IORESOURCE_MEM,
 					    "axi-base");
-	rockchip->reg_base = devm_ioremap_resource(dev, regs);
+	rockchip->reg_base = devm_ioremap_nopost_resource(dev, regs);
 	if (IS_ERR(rockchip->reg_base))
 		return PTR_ERR(rockchip->reg_base);
 
-- 
2.10.0

[PATCH v2 13/22] PCI: qcom: update PCI config space remap function

[Lorenzo Pieralisi]

PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
---
 drivers/pci/dwc/pcie-qcom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/dwc/pcie-qcom.c b/drivers/pci/dwc/pcie-qcom.c
index 67eb7f5..393e379 100644
--- a/drivers/pci/dwc/pcie-qcom.c
+++ b/drivers/pci/dwc/pcie-qcom.c
@@ -700,7 +700,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pcie->parf);
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
-	pci->dbi_base = devm_ioremap_resource(dev, res);
+	pci->dbi_base = devm_ioremap_nopost_resource(dev, res);
 	if (IS_ERR(pci->dbi_base))
 		return PTR_ERR(pci->dbi_base);
 
-- 
2.10.0

[PATCH v2 14/22] PCI: iproc-platform: update PCI config space remap function

[Lorenzo Pieralisi]

PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Jon Mason <jonmason@broadcom.com>
---
 drivers/pci/host/pcie-iproc-platform.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pcie-iproc-platform.c b/drivers/pci/host/pcie-iproc-platform.c
index f4909bb..b13d4a2 100644
--- a/drivers/pci/host/pcie-iproc-platform.c
+++ b/drivers/pci/host/pcie-iproc-platform.c
@@ -67,7 +67,8 @@ static int iproc_pcie_pltfm_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	pcie->base = devm_ioremap(dev, reg.start, resource_size(&reg));
+	pcie->base = devm_ioremap_nopost(dev, reg.start,
+					 resource_size(&reg));
 	if (!pcie->base) {
 		dev_err(dev, "unable to map controller registers\n");
 		return -ENOMEM;
-- 
2.10.0

[PATCH v2 15/22] PCI: hisi: update PCI config space remap function

[Lorenzo Pieralisi]

PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Zhou Wang <wangzhou1@hisilicon.com>
---
 drivers/pci/dwc/pcie-hisi.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/dwc/pcie-hisi.c b/drivers/pci/dwc/pcie-hisi.c
index fd66a31..66a91b2 100644
--- a/drivers/pci/dwc/pcie-hisi.c
+++ b/drivers/pci/dwc/pcie-hisi.c
@@ -99,7 +99,7 @@ static int hisi_pcie_init(struct pci_config_window *cfg)
 		return -ENOMEM;
 	}
 
-	reg_base = devm_ioremap(dev, res->start, resource_size(res));
+	reg_base = devm_ioremap_nopost(dev, res->start, resource_size(res));
 	if (!reg_base)
 		return -ENOMEM;
 
@@ -296,10 +296,9 @@ static int hisi_pcie_probe(struct platform_device *pdev)
 	}
 
 	reg = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbi");
-	pci->dbi_base = devm_ioremap_resource(dev, reg);
+	pci->dbi_base = devm_ioremap_nopost_resource(dev, reg);
 	if (IS_ERR(pci->dbi_base))
 		return PTR_ERR(pci->dbi_base);
-
 	platform_set_drvdata(pdev, hisi_pcie);
 
 	ret = hisi_add_pcie_port(hisi_pcie, pdev);
@@ -360,7 +359,7 @@ static int hisi_pcie_platform_init(struct pci_config_window *cfg)
 		return -EINVAL;
 	}
 
-	reg_base = devm_ioremap(dev, res->start, resource_size(res));
+	reg_base = devm_ioremap_nopost(dev, res->start, resource_size(res));
 	if (!reg_base)
 		return -ENOMEM;
 
-- 
2.10.0

[PATCH v2 16/22] PCI: designware: update PCI config space remap function

[Lorenzo Pieralisi]

PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Joao Pinto <Joao.Pinto@synopsys.com>
---
 drivers/pci/dwc/pcie-designware-host.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index 5ba3349..d2dd228 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -338,8 +338,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	}
 
 	if (!pci->dbi_base) {
-		pci->dbi_base = devm_ioremap(dev, pp->cfg->start,
-					resource_size(pp->cfg));
+		pci->dbi_base = devm_ioremap_nopost(dev, pp->cfg->start,
+						    resource_size(pp->cfg));
 		if (!pci->dbi_base) {
 			dev_err(dev, "error with ioremap\n");
 			ret = -ENOMEM;
@@ -350,8 +350,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	pp->mem_base = pp->mem->start;
 
 	if (!pp->va_cfg0_base) {
-		pp->va_cfg0_base = devm_ioremap(dev, pp->cfg0_base,
-						pp->cfg0_size);
+		pp->va_cfg0_base = devm_ioremap_nopost(dev, pp->cfg0_base,
+						       pp->cfg0_size);
 		if (!pp->va_cfg0_base) {
 			dev_err(dev, "error with ioremap in function\n");
 			ret = -ENOMEM;
@@ -360,8 +360,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	}
 
 	if (!pp->va_cfg1_base) {
-		pp->va_cfg1_base = devm_ioremap(dev, pp->cfg1_base,
-						pp->cfg1_size);
+		pp->va_cfg1_base = devm_ioremap_nopost(dev, pp->cfg1_base,
+						       pp->cfg1_size);
 		if (!pp->va_cfg1_base) {
 			dev_err(dev, "error with ioremap\n");
 			ret = -ENOMEM;
-- 
2.10.0

[PATCH v2 17/22] PCI: armada8k: update PCI config space remap function

[Lorenzo Pieralisi]

PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/pci/dwc/pcie-armada8k.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/dwc/pcie-armada8k.c b/drivers/pci/dwc/pcie-armada8k.c
index f110e3b..6bc7c7f 100644
--- a/drivers/pci/dwc/pcie-armada8k.c
+++ b/drivers/pci/dwc/pcie-armada8k.c
@@ -230,7 +230,7 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 
 	/* Get the dw-pcie unit configuration/control registers base. */
 	base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctrl");
-	pci->dbi_base = devm_ioremap_resource(dev, base);
+	pci->dbi_base = devm_ioremap_nopost_resource(dev, base);
 	if (IS_ERR(pci->dbi_base)) {
 		dev_err(dev, "couldn't remap regs base %p\n", base);
 		ret = PTR_ERR(pci->dbi_base);
-- 
2.10.0

[PATCH v2 18/22] PCI: xgene: update PCI config space remap function

[Lorenzo Pieralisi]

PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Tanmay Inamdar <tinamdar@apm.com>
---
 drivers/pci/host/pci-xgene.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 1a61087..df8e13c 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -248,7 +248,7 @@ static int xgene_pcie_ecam_init(struct pci_config_window *cfg, u32 ipversion)
 		dev_err(dev, "can't get CSR resource\n");
 		return ret;
 	}
-	port->csr_base = devm_ioremap_resource(dev, &csr);
+	port->csr_base = devm_ioremap_nopost_resource(dev, &csr);
 	if (IS_ERR(port->csr_base))
 		return PTR_ERR(port->csr_base);
 
@@ -359,7 +359,7 @@ static int xgene_pcie_map_reg(struct xgene_pcie_port *port,
 	struct resource *res;
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "csr");
-	port->csr_base = devm_ioremap_resource(dev, res);
+	port->csr_base = devm_ioremap_nopost_resource(dev, res);
 	if (IS_ERR(port->csr_base))
 		return PTR_ERR(port->csr_base);
 
-- 
2.10.0

[PATCH v2 19/22] PCI: tegra: update PCI config space remap function

[Lorenzo Pieralisi]

PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use correct memory mapping attributes to map config space
regions to enforce configuration space non-posted writes behaviour.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
---
 drivers/pci/host/pci-tegra.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index ed8a93f..ec72c3a 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -380,7 +380,7 @@ static struct tegra_pcie_bus *tegra_pcie_bus_alloc(struct tegra_pcie *pcie,
 						   unsigned int busnr)
 {
 	struct device *dev = pcie->dev;
-	pgprot_t prot = pgprot_device(PAGE_KERNEL);
+	pgprot_t prot = pgprot_nonposted(PAGE_KERNEL);
 	phys_addr_t cs = pcie->cs->start;
 	struct tegra_pcie_bus *bus;
 	unsigned int i;
@@ -1962,7 +1962,7 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
 		rp->pcie = pcie;
 		rp->np = port;
 
-		rp->base = devm_ioremap_resource(dev, &rp->regs);
+		rp->base = devm_ioremap_nopost_resource(dev, &rp->regs);
 		if (IS_ERR(rp->base))
 			return PTR_ERR(rp->base);
 
-- 
2.10.0

[PATCH v2 20/22] PCI: layerscape: update PCI config space remap function

[Lorenzo Pieralisi]

PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Mingkai Hu <mingkai.hu@freescale.com>
Cc: Minghuan Lian <minghuan.Lian@freescale.com>
Cc: Roy Zang <tie-fei.zang@freescale.com>
---
 drivers/pci/dwc/pci-layerscape.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/dwc/pci-layerscape.c b/drivers/pci/dwc/pci-layerscape.c
index c32e392..a1bd86a 100644
--- a/drivers/pci/dwc/pci-layerscape.c
+++ b/drivers/pci/dwc/pci-layerscape.c
@@ -283,7 +283,7 @@ static int __init ls_pcie_probe(struct platform_device *pdev)
 	pcie->pci = pci;
 
 	dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
-	pci->dbi_base = devm_ioremap_resource(dev, dbi_base);
+	pci->dbi_base = devm_ioremap_nopost_resource(dev, dbi_base);
 	if (IS_ERR(pci->dbi_base))
 		return PTR_ERR(pci->dbi_base);
 
-- 
2.10.0

[PATCH v2 21/22] PCI: keystone-dw: update PCI config space remap function

[Lorenzo Pieralisi]

PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/pci/dwc/pci-keystone-dw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/dwc/pci-keystone-dw.c b/drivers/pci/dwc/pci-keystone-dw.c
index 6b396f6..cba65ca 100644
--- a/drivers/pci/dwc/pci-keystone-dw.c
+++ b/drivers/pci/dwc/pci-keystone-dw.c
@@ -543,7 +543,7 @@ int __init ks_dw_pcie_host_init(struct keystone_pcie *ks_pcie,
 
 	/* Index 0 is the config reg. space address */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	pci->dbi_base = devm_ioremap_resource(dev, res);
+	pci->dbi_base = devm_ioremap_nopost_resource(dev, res);
 	if (IS_ERR(pci->dbi_base))
 		return PTR_ERR(pci->dbi_base);
 
-- 
2.10.0

[PATCH v2 22/22] PCI: versatile: update PCI config space remap function

[Lorenzo Pieralisi]

PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Rob Herring <robh@kernel.org>
---
 drivers/pci/host/pci-versatile.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-versatile.c b/drivers/pci/host/pci-versatile.c
index 5ebee7d..60fd396 100644
--- a/drivers/pci/host/pci-versatile.c
+++ b/drivers/pci/host/pci-versatile.c
@@ -138,7 +138,8 @@ static int versatile_pci_probe(struct platform_device *pdev)
 		return PTR_ERR(versatile_cfg_base[0]);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
-	versatile_cfg_base[1] = devm_ioremap_resource(&pdev->dev, res);
+	versatile_cfg_base[1] = devm_ioremap_nopost_resource(&pdev->dev,
+							     res);
 	if (IS_ERR(versatile_cfg_base[1]))
 		return PTR_ERR(versatile_cfg_base[1]);
 
-- 
2.10.0

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Bjorn Helgaas]

On Mon, Mar 27, 2017 at 10:49:30AM +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.
> 
> 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 that
> defaults to ioremap_nocache() (which should provide a sane default
> behaviour) but still allowing architectures on which ioremap_nocache()
> results in posted write transactions to override the function
> call with an arch specific implementation that complies with
> the PCI specifications for configuration transactions.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Haavard Skinnemoen <hskinnemoen@gmail.com>
> ---
>  arch/alpha/include/asm/io.h | 1 +
>  arch/avr32/include/asm/io.h | 1 +
>  arch/frv/include/asm/io.h   | 1 +
>  arch/ia64/include/asm/io.h  | 1 +
>  arch/x86/include/asm/io.h   | 1 +
>  include/asm-generic/io.h    | 4 ++++
>  6 files changed, 9 insertions(+)
> 
> diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
> index ff40491..27379ea 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
> +#define ioremap_nopost ioremap_nocache
>  
>  static inline void iounmap(volatile void __iomem *addr)
>  {
> diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
> index f855646..3f1ced8 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
> +#define ioremap_nopost ioremap_nocache
>  
>  #define cached(addr) P1SEGADDR(addr)
>  #define uncached(addr) P2SEGADDR(addr)
> diff --git a/arch/frv/include/asm/io.h b/arch/frv/include/asm/io.h
> index 8062fc7..302fb8c 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
> +#define ioremap_nopost ioremap_nocache
>  
>  extern void iounmap(void volatile __iomem *addr);
>  
> diff --git a/arch/ia64/include/asm/io.h b/arch/ia64/include/asm/io.h
> index 5de673a..70a4985 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
> +#define ioremap_nopost ioremap_nocache
>  
>  
>  /*
> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> index 7afb0e2..50b292f 100644
> --- a/arch/x86/include/asm/io.h
> +++ b/arch/x86/include/asm/io.h
> @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
>  extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
>  extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
>  #define ioremap_uc ioremap_uc
> +#define ioremap_nopost ioremap_nocache

These are all the same as the default from asm-generic.h.  Do we really
need these definitions in alpha, avr32, frv, ia64, x86?

>  extern void __iomem *ioremap_cache(resource_size_t offset, unsigned long size);
>  extern void __iomem *ioremap_prot(resource_size_t offset, unsigned long size, unsigned long prot_val);
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index 7ef015e..0e81938 100644
> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
>  #endif /* CONFIG_GENERIC_IOMAP */
>  #endif /* CONFIG_HAS_IOPORT_MAP */
>  
> +#ifndef ioremap_nopost
> +#define ioremap_nopost ioremap_nocache
> +#endif
> +
>  #ifndef xlate_dev_kmem_ptr
>  #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
>  static inline void *xlate_dev_kmem_ptr(void *addr)
> -- 
> 2.10.0
> 

[PATCH v2 08/22] lib: implement Devres ioremap_nopost() interface

[Bjorn Helgaas]

On Mon, Mar 27, 2017 at 10:49:36AM +0100, Lorenzo Pieralisi wrote:
> 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 cb1464c..4e05660 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: BUS offset to map

I know this comment was just copied from other devm_ioremap*()
interfaces, but I think it's slightly misleading to call this the "BUS
offset".  That suggests this is a bus address, but it is not.  Maybe
it should be "resource address" or similar?

I propose a separate patch to update the existing comments, followed
by this patch to add this new one.

> + * @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
> 

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Lorenzo Pieralisi]

On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> On Mon, Mar 27, 2017 at 10:49:30AM +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.
> > 
> > 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 that
> > defaults to ioremap_nocache() (which should provide a sane default
> > behaviour) but still allowing architectures on which ioremap_nocache()
> > results in posted write transactions to override the function
> > call with an arch specific implementation that complies with
> > the PCI specifications for configuration transactions.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Richard Henderson <rth@twiddle.net>
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: Tony Luck <tony.luck@intel.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Haavard Skinnemoen <hskinnemoen@gmail.com>
> > ---
> >  arch/alpha/include/asm/io.h | 1 +
> >  arch/avr32/include/asm/io.h | 1 +
> >  arch/frv/include/asm/io.h   | 1 +
> >  arch/ia64/include/asm/io.h  | 1 +
> >  arch/x86/include/asm/io.h   | 1 +
> >  include/asm-generic/io.h    | 4 ++++
> >  6 files changed, 9 insertions(+)
> > 
> > diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
> > index ff40491..27379ea 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
> > +#define ioremap_nopost ioremap_nocache
> >  
> >  static inline void iounmap(volatile void __iomem *addr)
> >  {
> > diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
> > index f855646..3f1ced8 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
> > +#define ioremap_nopost ioremap_nocache
> >  
> >  #define cached(addr) P1SEGADDR(addr)
> >  #define uncached(addr) P2SEGADDR(addr)
> > diff --git a/arch/frv/include/asm/io.h b/arch/frv/include/asm/io.h
> > index 8062fc7..302fb8c 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
> > +#define ioremap_nopost ioremap_nocache
> >  
> >  extern void iounmap(void volatile __iomem *addr);
> >  
> > diff --git a/arch/ia64/include/asm/io.h b/arch/ia64/include/asm/io.h
> > index 5de673a..70a4985 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
> > +#define ioremap_nopost ioremap_nocache
> >  
> >  
> >  /*
> > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > index 7afb0e2..50b292f 100644
> > --- a/arch/x86/include/asm/io.h
> > +++ b/arch/x86/include/asm/io.h
> > @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
> >  extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
> >  extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
> >  #define ioremap_uc ioremap_uc
> > +#define ioremap_nopost ioremap_nocache
> 
> These are all the same as the default from asm-generic.h.  Do we really
> need these definitions in alpha, avr32, frv, ia64, x86?

Those arches do not include asm-generic.h (I suppose for a good reason)
but a definition is needed anyway if we want code using ioremap_nopost()
to be unconditional. This is one way of doing it, there are others not
sure they are any better, I am open to suggestions.

Thanks,
Lorenzo

> >  extern void __iomem *ioremap_cache(resource_size_t offset, unsigned long size);
> >  extern void __iomem *ioremap_prot(resource_size_t offset, unsigned long size, unsigned long prot_val);
> > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > index 7ef015e..0e81938 100644
> > --- a/include/asm-generic/io.h
> > +++ b/include/asm-generic/io.h
> > @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
> >  #endif /* CONFIG_GENERIC_IOMAP */
> >  #endif /* CONFIG_HAS_IOPORT_MAP */
> >  
> > +#ifndef ioremap_nopost
> > +#define ioremap_nopost ioremap_nocache
> > +#endif
> > +
> >  #ifndef xlate_dev_kmem_ptr
> >  #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
> >  static inline void *xlate_dev_kmem_ptr(void *addr)
> > -- 
> > 2.10.0
> > 

[PATCH v2 08/22] lib: implement Devres ioremap_nopost() interface

[Lorenzo Pieralisi]

On Mon, Mar 27, 2017 at 08:41:29PM -0500, Bjorn Helgaas wrote:
> On Mon, Mar 27, 2017 at 10:49:36AM +0100, Lorenzo Pieralisi wrote:
> > 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 cb1464c..4e05660 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: BUS offset to map
> 
> I know this comment was just copied from other devm_ioremap*()
> interfaces, but I think it's slightly misleading to call this the "BUS
> offset".  That suggests this is a bus address, but it is not.  Maybe
> it should be "resource address" or similar?
> 
> I propose a separate patch to update the existing comments, followed
> by this patch to add this new one.

I agree with you, maybe Tejun has more context on why that parameter
is called BUS offset ? Anyway, I can easily do what you suggest above,
just asking Tejun if there is something I am missing.

Tejun ?

Thanks !
Lorenzo

> 
> > + * @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
> > 

[PATCH v2 08/22] lib: implement Devres ioremap_nopost() interface

[Tejun Heo]

Hello,

On Tue, Mar 28, 2017 at 03:50:03PM +0100, Lorenzo Pieralisi wrote:
> > I know this comment was just copied from other devm_ioremap*()
> > interfaces, but I think it's slightly misleading to call this the "BUS
> > offset".  That suggests this is a bus address, but it is not.  Maybe
> > it should be "resource address" or similar?
> > 
> > I propose a separate patch to update the existing comments, followed
> > by this patch to add this new one.
> 
> I agree with you, maybe Tejun has more context on why that parameter
> is called BUS offset ? Anyway, I can easily do what you suggest above,
> just asking Tejun if there is something I am missing.

I'm pretty sure I copied it from somewhere without thinking much about
it.  Please feel free to change.

Thanks.

-- 
tejun

[PATCH v2 05/22] ARM64: implement ioremap_nopost() interface

[Will Deacon]

On Mon, Mar 27, 2017 at 10:49:33AM +0100, Lorenzo Pieralisi wrote:
> 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.
> 
> 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(+)

Acked-by: Will Deacon <will.deacon@arm.com>

Let me know if you need this taken via the arm64 tree.

Will

[PATCH v2 07/22] PCI: ECAM: use ioremap_nopost() to map config region

[Will Deacon]

On Mon, Mar 27, 2017 at 10:49:35AM +0100, Lorenzo Pieralisi wrote:
> Current ECAM kernel implementation uses ioremap() to map the ECAM
> configuration space memory region; this is not safe in that on some
> architectures the ioremap interface provides mappings that allow posted
> write transactions. This, as highlighted in the PCIe specifications
> (4.0 - Rev0.3, "Ordering Considerations for the Enhanced Configuration
> Address Mechanism"), can create ordering issues for software because
> posted writes transactions on the CPU host bus are non posted in the
> PCI express fabric.
> 
> Update the ioremap() interface to use ioremap_nopost() whose
> mapping attributes guarantee that non-posted writes transactions
> are issued for memory writes within the ECAM memory mapped address
> region.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/pci/ecam.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Acked-by: Will Deacon <will.deacon@arm.com>

Will

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Bjorn Helgaas]

On Tue, Mar 28, 2017 at 03:45:43PM +0100, Lorenzo Pieralisi wrote:
> On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> > On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:

> > >  #define ioremap_uc ioremap_uc
> > > +#define ioremap_nopost ioremap_nocache
> > 
> > These are all the same as the default from asm-generic.h.  Do we really
> > need these definitions in alpha, avr32, frv, ia64, x86?
> 
> Those arches do not include asm-generic.h (I suppose for a good reason)

OK, that explains it.  I'm not at all sure there's a good reason for
those arches not using asm-generic.h, but I haven't looked at the code
to see.

> but a definition is needed anyway if we want code using ioremap_nopost()
> to be unconditional. This is one way of doing it, there are others not
> sure they are any better, I am open to suggestions.

[PATCH v2 06/22] ARM: implement ioremap_nopost() interface

[Lorenzo Pieralisi]

Hi Russell,

On Mon, Mar 27, 2017 at 10:49:34AM +0100, Lorenzo Pieralisi wrote:
> 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 | 10 ++++++++++
>  arch/arm/mm/ioremap.c     |  7 +++++++
>  arch/arm/mm/nommu.c       |  9 +++++++++
>  3 files changed, 26 insertions(+)

I have not added your ACK to this patch since I slightly tweaked
it to adapt it to ioremap_nopost() interface instead of a PCI
specific one. Furthermore, I mechanically added a ioremap_nopost()
version for nommu too in the process, would be good to have a look
if I did that properly please.

Can I add your ACK on this patch ? If you spot any issues please
do let me know.

Thank you !
Lorenzo

> diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
> index 42871fb..49913d1 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,9 @@ 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);
> +#define ioremap_nopost ioremap_nopost
> +
>  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
> 

[PATCH v2 06/22] ARM: implement ioremap_nopost() interface

[Lorenzo Pieralisi]

On Fri, Mar 31, 2017 at 12:08:13PM +0100, Lorenzo Pieralisi wrote:
> Hi Russell,
> 
> On Mon, Mar 27, 2017 at 10:49:34AM +0100, Lorenzo Pieralisi wrote:
> > 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 | 10 ++++++++++
> >  arch/arm/mm/ioremap.c     |  7 +++++++
> >  arch/arm/mm/nommu.c       |  9 +++++++++
> >  3 files changed, 26 insertions(+)
> 
> I have not added your ACK to this patch since I slightly tweaked
> it to adapt it to ioremap_nopost() interface instead of a PCI
> specific one. Furthermore, I mechanically added a ioremap_nopost()
> version for nommu too in the process, would be good to have a look
> if I did that properly please.
> 
> Can I add your ACK on this patch ? If you spot any issues please
> do let me know.

Hi Russell,

please let me know if your ACK on this patch stands as I have to respin
it shortly for hopefully final round of review.

Thanks !
Lorenzo

> Thank you !
> Lorenzo
> 
> > diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
> > index 42871fb..49913d1 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,9 @@ 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);
> > +#define ioremap_nopost ioremap_nopost
> > +
> >  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
> > 

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Russell King - ARM Linux]

On Tue, Mar 28, 2017 at 03:45:43PM +0100, Lorenzo Pieralisi wrote:
> On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> > On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:
> > > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > > index 7afb0e2..50b292f 100644
> > > --- a/arch/x86/include/asm/io.h
> > > +++ b/arch/x86/include/asm/io.h
> > > @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
> > >  extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
> > >  extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
> > >  #define ioremap_uc ioremap_uc
> > > +#define ioremap_nopost ioremap_nocache
> > 
> > These are all the same as the default from asm-generic.h.  Do we really
> > need these definitions in alpha, avr32, frv, ia64, x86?
> 
> Those arches do not include asm-generic.h (I suppose for a good reason)
> but a definition is needed anyway if we want code using ioremap_nopost()
> to be unconditional. This is one way of doing it, there are others not
> sure they are any better, I am open to suggestions.

We do have linux/io.h, which should be included in preference to asm/io.h.
linux/io.h has existed for years, but I still see (from time to time)
patches adding drivers that (imho incorrectly) use asm/io.h.

Also, this:

> > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > > index 7ef015e..0e81938 100644
> > > --- a/include/asm-generic/io.h
> > > +++ b/include/asm-generic/io.h
> > > @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
> > >  #endif /* CONFIG_GENERIC_IOMAP */
> > >  #endif /* CONFIG_HAS_IOPORT_MAP */
> > >  
> > > +#ifndef ioremap_nopost
> > > +#define ioremap_nopost ioremap_nocache
> > > +#endif
> > > +
> > >  #ifndef xlate_dev_kmem_ptr
> > >  #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
> > >  static inline void *xlate_dev_kmem_ptr(void *addr)

could well be located in linux/io.h, which would make it available
everywhere.

I'd suggest one change to this though:

#ifndef ioremap_nopost
static inline void __iomem *ioremap_nopost(resource_size_t offset,
					   unsigned long size)
{
	return ioremap_nocache(offset, size);
}
#endif

This way, if someone forgets to define ioremap_nopost() in their
asm/io.h but provides a definition or extern prototype for
ioremap_nopost(), we end up with a compile error to highlight the
error, rather than the error being hidden by the preprocessor.

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

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Lorenzo Pieralisi]

On Wed, Apr 05, 2017 at 11:58:41AM +0100, Russell King - ARM Linux wrote:
> On Tue, Mar 28, 2017 at 03:45:43PM +0100, Lorenzo Pieralisi wrote:
> > On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> > > On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:
> > > > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > > > index 7afb0e2..50b292f 100644
> > > > --- a/arch/x86/include/asm/io.h
> > > > +++ b/arch/x86/include/asm/io.h
> > > > @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
> > > >  extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
> > > >  extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
> > > >  #define ioremap_uc ioremap_uc
> > > > +#define ioremap_nopost ioremap_nocache
> > > 
> > > These are all the same as the default from asm-generic.h.  Do we really
> > > need these definitions in alpha, avr32, frv, ia64, x86?
> > 
> > Those arches do not include asm-generic.h (I suppose for a good reason)
> > but a definition is needed anyway if we want code using ioremap_nopost()
> > to be unconditional. This is one way of doing it, there are others not
> > sure they are any better, I am open to suggestions.
> 
> We do have linux/io.h, which should be included in preference to asm/io.h.
> linux/io.h has existed for years, but I still see (from time to time)
> patches adding drivers that (imho incorrectly) use asm/io.h.
> 
> Also, this:
> 
> > > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > > > index 7ef015e..0e81938 100644
> > > > --- a/include/asm-generic/io.h
> > > > +++ b/include/asm-generic/io.h
> > > > @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
> > > >  #endif /* CONFIG_GENERIC_IOMAP */
> > > >  #endif /* CONFIG_HAS_IOPORT_MAP */
> > > >  
> > > > +#ifndef ioremap_nopost
> > > > +#define ioremap_nopost ioremap_nocache
> > > > +#endif
> > > > +
> > > >  #ifndef xlate_dev_kmem_ptr
> > > >  #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
> > > >  static inline void *xlate_dev_kmem_ptr(void *addr)
> 
> could well be located in linux/io.h, which would make it available
> everywhere.
> 
> I'd suggest one change to this though:
> 
> #ifndef ioremap_nopost
> static inline void __iomem *ioremap_nopost(resource_size_t offset,
> 					   unsigned long size)
> {
> 	return ioremap_nocache(offset, size);
> }
> #endif
> 
> This way, if someone forgets to define ioremap_nopost() in their
> asm/io.h but provides a definition or extern prototype for
> ioremap_nopost(), we end up with a compile error to highlight the
> error, rather than the error being hidden by the preprocessor.

Yes, I could add ioremap_nopost() to linux/io.h I did not do it because
linux/io.h, for whatever reason, does not seem to contain ioremap_*
prototypes; this does not mean we should not add it there but that
would look odd (with all others ioremap_* in asm/io.h), all I am
saying. This stuff requires some clean-up regardless, for the records.

As for the static inline, I did that and that did not make the
kbuild robot happy at all on some arches:

eg: openrisc

>> include/asm-generic/io.h:922:9: error: implicit declaration of
>> function 'ioremap_nocache' [-Werror=implicit-function-declaration]
     return ioremap_nocache(offset, size);

I will have another stab at it since what you put forward makes sense,
I just have to find a way that works across arches.

Thanks !
Lorenzo

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Lorenzo Pieralisi]

On Wed, Apr 05, 2017 at 01:38:12PM +0100, Lorenzo Pieralisi wrote:
> On Wed, Apr 05, 2017 at 11:58:41AM +0100, Russell King - ARM Linux wrote:
> > On Tue, Mar 28, 2017 at 03:45:43PM +0100, Lorenzo Pieralisi wrote:
> > > On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> > > > On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:
> > > > > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > > > > index 7afb0e2..50b292f 100644
> > > > > --- a/arch/x86/include/asm/io.h
> > > > > +++ b/arch/x86/include/asm/io.h
> > > > > @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
> > > > >  extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
> > > > >  extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
> > > > >  #define ioremap_uc ioremap_uc
> > > > > +#define ioremap_nopost ioremap_nocache
> > > > 
> > > > These are all the same as the default from asm-generic.h.  Do we really
> > > > need these definitions in alpha, avr32, frv, ia64, x86?
> > > 
> > > Those arches do not include asm-generic.h (I suppose for a good reason)
> > > but a definition is needed anyway if we want code using ioremap_nopost()
> > > to be unconditional. This is one way of doing it, there are others not
> > > sure they are any better, I am open to suggestions.
> > 
> > We do have linux/io.h, which should be included in preference to asm/io.h.
> > linux/io.h has existed for years, but I still see (from time to time)
> > patches adding drivers that (imho incorrectly) use asm/io.h.
> > 
> > Also, this:
> > 
> > > > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > > > > index 7ef015e..0e81938 100644
> > > > > --- a/include/asm-generic/io.h
> > > > > +++ b/include/asm-generic/io.h
> > > > > @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
> > > > >  #endif /* CONFIG_GENERIC_IOMAP */
> > > > >  #endif /* CONFIG_HAS_IOPORT_MAP */
> > > > >  
> > > > > +#ifndef ioremap_nopost
> > > > > +#define ioremap_nopost ioremap_nocache
> > > > > +#endif
> > > > > +
> > > > >  #ifndef xlate_dev_kmem_ptr
> > > > >  #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
> > > > >  static inline void *xlate_dev_kmem_ptr(void *addr)
> > 
> > could well be located in linux/io.h, which would make it available
> > everywhere.
> > 
> > I'd suggest one change to this though:
> > 
> > #ifndef ioremap_nopost
> > static inline void __iomem *ioremap_nopost(resource_size_t offset,
> > 					   unsigned long size)
> > {
> > 	return ioremap_nocache(offset, size);
> > }
> > #endif
> > 
> > This way, if someone forgets to define ioremap_nopost() in their
> > asm/io.h but provides a definition or extern prototype for
> > ioremap_nopost(), we end up with a compile error to highlight the
> > error, rather than the error being hidden by the preprocessor.
> 
> Yes, I could add ioremap_nopost() to linux/io.h I did not do it because
> linux/io.h, for whatever reason, does not seem to contain ioremap_*
> prototypes; this does not mean we should not add it there but that
> would look odd (with all others ioremap_* in asm/io.h), all I am
> saying. This stuff requires some clean-up regardless, for the records.
> 
> As for the static inline, I did that and that did not make the
> kbuild robot happy at all on some arches:
> 
> eg: openrisc
> 
> >> include/asm-generic/io.h:922:9: error: implicit declaration of
> >> function 'ioremap_nocache' [-Werror=implicit-function-declaration]
>      return ioremap_nocache(offset, size);

Indeed, the static inline ioremap_nocache() fallback does not work
on all arches (whether I add the fallback in linux/io.h or
asm-generic/io.h is irrelevant), I bump into issues such as the one
reported above.

I could make it:

#ifndef ioremap_nopost
static inline void __iomem *ioremap_nopost(resource_size_t offset,
					   unsigned long size)
{
	return NULL;
}
#endif

which would force arches to define ioremap_nopost() (if they need it).

This is what I *assume* was done for ioremap_uc() in asm-generic/io.h,
but honestly history is hard to follow here.

Thoughts ?

Thanks,
Lorenzo

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Russell King - ARM Linux]

On Thu, Apr 06, 2017 at 11:26:36AM +0100, Lorenzo Pieralisi wrote:
> On Wed, Apr 05, 2017 at 01:38:12PM +0100, Lorenzo Pieralisi wrote:
> > eg: openrisc
> > 
> > >> include/asm-generic/io.h:922:9: error: implicit declaration of
> > >> function 'ioremap_nocache' [-Werror=implicit-function-declaration]
> >      return ioremap_nocache(offset, size);
> 
> Indeed, the static inline ioremap_nocache() fallback does not work
> on all arches (whether I add the fallback in linux/io.h or
> asm-generic/io.h is irrelevant), I bump into issues such as the one
> reported above.

>From what I can see:

(a) openrisc does define ioremap_nocache() in its asm/io.h
(b) these do not:

$ grep -L 'ioremap_nocache' arch/*/include/asm/io.h
arch/blackfin/include/asm/io.h
arch/h8300/include/asm/io.h
arch/m68k/include/asm/io.h
arch/score/include/asm/io.h
arch/sparc/include/asm/io.h

Out of those, blackfin, h8300 and score do not define it, whereas m68k
and sparc do in other headers included by asm/io.h.  So it looks like
we have three problem architectures that don't define an ioremap_nocache().

PCI on blackfin depends on BROKEN, so it's not selectable.  From what I
can tell, h8300 and score do not allow PCI to be enabled (but maybe its
buried elsewhere in their Kconfig files, I didn't check.)

So, I think a way around this is to make ioremap_nopost() conditional
on PCI in linux/io.h for the time being - if its scope wants to be
enlarged, then these three architectures would need to have either an
ioremap_nopost() implementation added, or an ioremap_nocache()
implementation.

> I could make it:
> 
> #ifndef ioremap_nopost
> static inline void __iomem *ioremap_nopost(resource_size_t offset,
> 					   unsigned long size)
> {
> 	return NULL;
> }
> #endif
> 
> which would force arches to define ioremap_nopost() (if they need it).
> 
> This is what I *assume* was done for ioremap_uc() in asm-generic/io.h,
> but honestly history is hard to follow here.

If we are going to consider doing that, the correct question we need to be
asking is whether anything will break as a result of this - is there any
existing arch using the code as it stands that will end up being broken
when we switch PCI to use ioremap_nopost(), and we end up using this NULL-
returning default?

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

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Luis R. Rodriguez]

On Thu, Apr 06, 2017 at 11:26:36AM +0100, Lorenzo Pieralisi wrote:
> On Wed, Apr 05, 2017 at 01:38:12PM +0100, Lorenzo Pieralisi wrote:
> > On Wed, Apr 05, 2017 at 11:58:41AM +0100, Russell King - ARM Linux wrote:
> > > On Tue, Mar 28, 2017 at 03:45:43PM +0100, Lorenzo Pieralisi wrote:
> > > > On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> > > > > On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:
> > > > > > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > > > > > index 7afb0e2..50b292f 100644
> > > > > > --- a/arch/x86/include/asm/io.h
> > > > > > +++ b/arch/x86/include/asm/io.h
> > > > > > @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
> > > > > >  extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
> > > > > >  extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
> > > > > >  #define ioremap_uc ioremap_uc
> > > > > > +#define ioremap_nopost ioremap_nocache
> > > > > 
> > > > > These are all the same as the default from asm-generic.h.  Do we really
> > > > > need these definitions in alpha, avr32, frv, ia64, x86?
> > > > 
> > > > Those arches do not include asm-generic.h (I suppose for a good reason)
> > > > but a definition is needed anyway if we want code using ioremap_nopost()
> > > > to be unconditional. This is one way of doing it, there are others not
> > > > sure they are any better, I am open to suggestions.
> > > 
> > > We do have linux/io.h, which should be included in preference to asm/io.h.
> > > linux/io.h has existed for years, but I still see (from time to time)
> > > patches adding drivers that (imho incorrectly) use asm/io.h.
> > > 
> > > Also, this:
> > > 
> > > > > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > > > > > index 7ef015e..0e81938 100644
> > > > > > --- a/include/asm-generic/io.h
> > > > > > +++ b/include/asm-generic/io.h
> > > > > > @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
> > > > > >  #endif /* CONFIG_GENERIC_IOMAP */
> > > > > >  #endif /* CONFIG_HAS_IOPORT_MAP */
> > > > > >  
> > > > > > +#ifndef ioremap_nopost
> > > > > > +#define ioremap_nopost ioremap_nocache
> > > > > > +#endif
> > > > > > +
> > > > > >  #ifndef xlate_dev_kmem_ptr
> > > > > >  #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
> > > > > >  static inline void *xlate_dev_kmem_ptr(void *addr)
> > > 
> > > could well be located in linux/io.h, which would make it available
> > > everywhere.
> > > 
> > > I'd suggest one change to this though:
> > > 
> > > #ifndef ioremap_nopost
> > > static inline void __iomem *ioremap_nopost(resource_size_t offset,
> > > 					   unsigned long size)
> > > {
> > > 	return ioremap_nocache(offset, size);
> > > }
> > > #endif
> > > 
> > > This way, if someone forgets to define ioremap_nopost() in their
> > > asm/io.h but provides a definition or extern prototype for
> > > ioremap_nopost(), we end up with a compile error to highlight the
> > > error, rather than the error being hidden by the preprocessor.
> > 
> > Yes, I could add ioremap_nopost() to linux/io.h I did not do it because
> > linux/io.h, for whatever reason, does not seem to contain ioremap_*
> > prototypes; this does not mean we should not add it there but that
> > would look odd (with all others ioremap_* in asm/io.h), all I am
> > saying. This stuff requires some clean-up regardless, for the records.
> > 
> > As for the static inline, I did that and that did not make the
> > kbuild robot happy at all on some arches:
> > 
> > eg: openrisc
> > 
> > >> include/asm-generic/io.h:922:9: error: implicit declaration of
> > >> function 'ioremap_nocache' [-Werror=implicit-function-declaration]
> >      return ioremap_nocache(offset, size);
> 
> Indeed, the static inline ioremap_nocache() fallback does not work
> on all arches (whether I add the fallback in linux/io.h or
> asm-generic/io.h is irrelevant), I bump into issues such as the one
> reported above.

Its also not *safe* to assume on behalf of all architectures a new ioremap
call is both a good idea and proper.

> I could make it:
> 
> #ifndef ioremap_nopost
> static inline void __iomem *ioremap_nopost(resource_size_t offset,
> 					   unsigned long size)
> {
> 	return NULL;
> }
> #endif
> 
> which would force arches to define ioremap_nopost() (if they need it).

Yes, please do this.

> This is what I *assume* was done for ioremap_uc() in asm-generic/io.h,
> but honestly history is hard to follow here.
> 
> Thoughts ?

Hard to follow ?

I think commit 8c7ea50c010b2 ("x86/mm, asm-generic: Add IOMMU ioremap_uc()
variant default") makes it very clear that historically we have made bad
assumptions and these assumptions are not safe, so the only correct thing we
can do to not stall development is to do what you did above, and each
architecture then can add support for its proper mapping. Its up to each
architecture maintainer to be attentive and review these patches, if they don't
get to it, this will just not work for the new driver that needs it, such is
life, its better than having incorrect defaults spread for all architectures.

The old strategy was very sloppy. Its why I tried to be as clear as possible on
both the commit log and headers about this. If the commit log and headers were
not clear, please help clarify this more somehow in your patches.

  Luis

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Lorenzo Pieralisi]

On Thu, Apr 06, 2017 at 12:53:12PM +0200, Luis R. Rodriguez wrote:
> On Thu, Apr 06, 2017 at 11:26:36AM +0100, Lorenzo Pieralisi wrote:
> > On Wed, Apr 05, 2017 at 01:38:12PM +0100, Lorenzo Pieralisi wrote:
> > > On Wed, Apr 05, 2017 at 11:58:41AM +0100, Russell King - ARM Linux wrote:
> > > > On Tue, Mar 28, 2017 at 03:45:43PM +0100, Lorenzo Pieralisi wrote:
> > > > > On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> > > > > > On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:
> > > > > > > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > > > > > > index 7afb0e2..50b292f 100644
> > > > > > > --- a/arch/x86/include/asm/io.h
> > > > > > > +++ b/arch/x86/include/asm/io.h
> > > > > > > @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
> > > > > > >  extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
> > > > > > >  extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
> > > > > > >  #define ioremap_uc ioremap_uc
> > > > > > > +#define ioremap_nopost ioremap_nocache
> > > > > > 
> > > > > > These are all the same as the default from asm-generic.h.  Do we really
> > > > > > need these definitions in alpha, avr32, frv, ia64, x86?
> > > > > 
> > > > > Those arches do not include asm-generic.h (I suppose for a good reason)
> > > > > but a definition is needed anyway if we want code using ioremap_nopost()
> > > > > to be unconditional. This is one way of doing it, there are others not
> > > > > sure they are any better, I am open to suggestions.
> > > > 
> > > > We do have linux/io.h, which should be included in preference to asm/io.h.
> > > > linux/io.h has existed for years, but I still see (from time to time)
> > > > patches adding drivers that (imho incorrectly) use asm/io.h.
> > > > 
> > > > Also, this:
> > > > 
> > > > > > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > > > > > > index 7ef015e..0e81938 100644
> > > > > > > --- a/include/asm-generic/io.h
> > > > > > > +++ b/include/asm-generic/io.h
> > > > > > > @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
> > > > > > >  #endif /* CONFIG_GENERIC_IOMAP */
> > > > > > >  #endif /* CONFIG_HAS_IOPORT_MAP */
> > > > > > >  
> > > > > > > +#ifndef ioremap_nopost
> > > > > > > +#define ioremap_nopost ioremap_nocache
> > > > > > > +#endif
> > > > > > > +
> > > > > > >  #ifndef xlate_dev_kmem_ptr
> > > > > > >  #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
> > > > > > >  static inline void *xlate_dev_kmem_ptr(void *addr)
> > > > 
> > > > could well be located in linux/io.h, which would make it available
> > > > everywhere.
> > > > 
> > > > I'd suggest one change to this though:
> > > > 
> > > > #ifndef ioremap_nopost
> > > > static inline void __iomem *ioremap_nopost(resource_size_t offset,
> > > > 					   unsigned long size)
> > > > {
> > > > 	return ioremap_nocache(offset, size);
> > > > }
> > > > #endif
> > > > 
> > > > This way, if someone forgets to define ioremap_nopost() in their
> > > > asm/io.h but provides a definition or extern prototype for
> > > > ioremap_nopost(), we end up with a compile error to highlight the
> > > > error, rather than the error being hidden by the preprocessor.
> > > 
> > > Yes, I could add ioremap_nopost() to linux/io.h I did not do it because
> > > linux/io.h, for whatever reason, does not seem to contain ioremap_*
> > > prototypes; this does not mean we should not add it there but that
> > > would look odd (with all others ioremap_* in asm/io.h), all I am
> > > saying. This stuff requires some clean-up regardless, for the records.
> > > 
> > > As for the static inline, I did that and that did not make the
> > > kbuild robot happy at all on some arches:
> > > 
> > > eg: openrisc
> > > 
> > > >> include/asm-generic/io.h:922:9: error: implicit declaration of
> > > >> function 'ioremap_nocache' [-Werror=implicit-function-declaration]
> > >      return ioremap_nocache(offset, size);
> > 
> > Indeed, the static inline ioremap_nocache() fallback does not work
> > on all arches (whether I add the fallback in linux/io.h or
> > asm-generic/io.h is irrelevant), I bump into issues such as the one
> > reported above.
> 
> Its also not *safe* to assume on behalf of all architectures a new ioremap
> call is both a good idea and proper.
> 
> > I could make it:
> > 
> > #ifndef ioremap_nopost
> > static inline void __iomem *ioremap_nopost(resource_size_t offset,
> > 					   unsigned long size)
> > {
> > 	return NULL;
> > }
> > #endif
> > 
> > which would force arches to define ioremap_nopost() (if they need it).
> 
> Yes, please do this.

Ok but where ? linux/io.h or asm-generic/io.h ? As I replied to Russell,
having it in linux/io.h is a bit odd given that it would be the only
ioremap_* implementation declared there, I think we need more
consistency here instead of deviating again from what you did.

> > This is what I *assume* was done for ioremap_uc() in asm-generic/io.h,
> > but honestly history is hard to follow here.
> > 
> > Thoughts ?
> 
> Hard to follow ?

I was referring to the whole asm-generic/io.h history.

> I think commit 8c7ea50c010b2 ("x86/mm, asm-generic: Add IOMMU ioremap_uc()
> variant default") makes it very clear that historically we have made bad
> assumptions and these assumptions are not safe, so the only correct thing we
> can do to not stall development is to do what you did above, and each
> architecture then can add support for its proper mapping. Its up to each
> architecture maintainer to be attentive and review these patches, if they don't
> get to it, this will just not work for the new driver that needs it, such is
> life, its better than having incorrect defaults spread for all architectures.
> 
> The old strategy was very sloppy. Its why I tried to be as clear as possible on
> both the commit log and headers about this. If the commit log and headers were
> not clear, please help clarify this more somehow in your patches.

I see your point and I can replicate (probably I will have to patch
microblaze too since some of the PCI drivers patches in this series
that I updated to use ioremap_nopost() will run on it too) what you
did for ioremap_uc() for ioremap_nopost(), I am ok with that, I need
to know if we all are.

Thanks,
Lorenzo

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Luis R. Rodriguez]

On Thu, Apr 06, 2017 at 12:38:45PM +0100, Lorenzo Pieralisi wrote:
> On Thu, Apr 06, 2017 at 12:53:12PM +0200, Luis R. Rodriguez wrote:
> > On Thu, Apr 06, 2017 at 11:26:36AM +0100, Lorenzo Pieralisi wrote:
> > > On Wed, Apr 05, 2017 at 01:38:12PM +0100, Lorenzo Pieralisi wrote:
> > > > On Wed, Apr 05, 2017 at 11:58:41AM +0100, Russell King - ARM Linux wrote:
> > > > > On Tue, Mar 28, 2017 at 03:45:43PM +0100, Lorenzo Pieralisi wrote:
> > > > > > On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> > > > > > > On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:
> > > > > > > > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > > > > > > > index 7afb0e2..50b292f 100644
> > > > > > > > --- a/arch/x86/include/asm/io.h
> > > > > > > > +++ b/arch/x86/include/asm/io.h
> > > > > > > > @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
> > > > > > > >  extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
> > > > > > > >  extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
> > > > > > > >  #define ioremap_uc ioremap_uc
> > > > > > > > +#define ioremap_nopost ioremap_nocache
> > > > > > > 
> > > > > > > These are all the same as the default from asm-generic.h.  Do we really
> > > > > > > need these definitions in alpha, avr32, frv, ia64, x86?
> > > > > > 
> > > > > > Those arches do not include asm-generic.h (I suppose for a good reason)
> > > > > > but a definition is needed anyway if we want code using ioremap_nopost()
> > > > > > to be unconditional. This is one way of doing it, there are others not
> > > > > > sure they are any better, I am open to suggestions.
> > > > > 
> > > > > We do have linux/io.h, which should be included in preference to asm/io.h.
> > > > > linux/io.h has existed for years, but I still see (from time to time)
> > > > > patches adding drivers that (imho incorrectly) use asm/io.h.
> > > > > 
> > > > > Also, this:
> > > > > 
> > > > > > > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > > > > > > > index 7ef015e..0e81938 100644
> > > > > > > > --- a/include/asm-generic/io.h
> > > > > > > > +++ b/include/asm-generic/io.h
> > > > > > > > @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
> > > > > > > >  #endif /* CONFIG_GENERIC_IOMAP */
> > > > > > > >  #endif /* CONFIG_HAS_IOPORT_MAP */
> > > > > > > >  
> > > > > > > > +#ifndef ioremap_nopost
> > > > > > > > +#define ioremap_nopost ioremap_nocache
> > > > > > > > +#endif
> > > > > > > > +
> > > > > > > >  #ifndef xlate_dev_kmem_ptr
> > > > > > > >  #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
> > > > > > > >  static inline void *xlate_dev_kmem_ptr(void *addr)
> > > > > 
> > > > > could well be located in linux/io.h, which would make it available
> > > > > everywhere.
> > > > > 
> > > > > I'd suggest one change to this though:
> > > > > 
> > > > > #ifndef ioremap_nopost
> > > > > static inline void __iomem *ioremap_nopost(resource_size_t offset,
> > > > > 					   unsigned long size)
> > > > > {
> > > > > 	return ioremap_nocache(offset, size);
> > > > > }
> > > > > #endif
> > > > > 
> > > > > This way, if someone forgets to define ioremap_nopost() in their
> > > > > asm/io.h but provides a definition or extern prototype for
> > > > > ioremap_nopost(), we end up with a compile error to highlight the
> > > > > error, rather than the error being hidden by the preprocessor.
> > > > 
> > > > Yes, I could add ioremap_nopost() to linux/io.h I did not do it because
> > > > linux/io.h, for whatever reason, does not seem to contain ioremap_*
> > > > prototypes; this does not mean we should not add it there but that
> > > > would look odd (with all others ioremap_* in asm/io.h), all I am
> > > > saying. This stuff requires some clean-up regardless, for the records.
> > > > 
> > > > As for the static inline, I did that and that did not make the
> > > > kbuild robot happy at all on some arches:
> > > > 
> > > > eg: openrisc
> > > > 
> > > > >> include/asm-generic/io.h:922:9: error: implicit declaration of
> > > > >> function 'ioremap_nocache' [-Werror=implicit-function-declaration]
> > > >      return ioremap_nocache(offset, size);
> > > 
> > > Indeed, the static inline ioremap_nocache() fallback does not work
> > > on all arches (whether I add the fallback in linux/io.h or
> > > asm-generic/io.h is irrelevant), I bump into issues such as the one
> > > reported above.
> > 
> > Its also not *safe* to assume on behalf of all architectures a new ioremap
> > call is both a good idea and proper.
> > 
> > > I could make it:
> > > 
> > > #ifndef ioremap_nopost
> > > static inline void __iomem *ioremap_nopost(resource_size_t offset,
> > > 					   unsigned long size)
> > > {
> > > 	return NULL;
> > > }
> > > #endif
> > > 
> > > which would force arches to define ioremap_nopost() (if they need it).
> > 
> > Yes, please do this.
> 
> Ok but where ? linux/io.h or asm-generic/io.h ? As I replied to Russell,
> having it in linux/io.h is a bit odd given that it would be the only
> ioremap_* implementation declared there, I think we need more
> consistency here instead of deviating again from what you did.

asm-generic/io.h is the right place for generics which let you override.

> > > This is what I *assume* was done for ioremap_uc() in asm-generic/io.h,
> > > but honestly history is hard to follow here.
> > > 
> > > Thoughts ?
> > 
> > Hard to follow ?
> 
> I was referring to the whole asm-generic/io.h history.
> 
> > I think commit 8c7ea50c010b2 ("x86/mm, asm-generic: Add IOMMU ioremap_uc()
> > variant default") makes it very clear that historically we have made bad
> > assumptions and these assumptions are not safe, so the only correct thing we
> > can do to not stall development is to do what you did above, and each
> > architecture then can add support for its proper mapping. Its up to each
> > architecture maintainer to be attentive and review these patches, if they don't
> > get to it, this will just not work for the new driver that needs it, such is
> > life, its better than having incorrect defaults spread for all architectures.
> > 
> > The old strategy was very sloppy. Its why I tried to be as clear as possible on
> > both the commit log and headers about this. If the commit log and headers were
> > not clear, please help clarify this more somehow in your patches.
> 
> I see your point and I can replicate (probably I will have to patch
> microblaze too since some of the PCI drivers patches in this series
> that I updated to use ioremap_nopost() will run on it too) 

Cool right so if you add support for the archs for the drivers that you know
use this new ioremap then there is no regressions, and then only if a new
driver gets added and an arch needs this they will also need this, but that
is precisely the sort of requirement thought process we want.

> what you
> did for ioremap_uc() for ioremap_nopost(), I am ok with that, I need
> to know if we all are.

Sure. And again, if consensus is built (again) on that strategy please
update the docs to ensure that this is even clearer for the next person.
I thought it was rather clear now but it does not seem so.

  Luis

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Russell King - ARM Linux]

On Thu, Apr 06, 2017 at 12:53:12PM +0200, Luis R. Rodriguez wrote:
> On Thu, Apr 06, 2017 at 11:26:36AM +0100, Lorenzo Pieralisi wrote:
> > Indeed, the static inline ioremap_nocache() fallback does not work
> > on all arches (whether I add the fallback in linux/io.h or
> > asm-generic/io.h is irrelevant), I bump into issues such as the one
> > reported above.
> 
> Its also not *safe* to assume on behalf of all architectures a new ioremap
> call is both a good idea and proper.

You may be right in general, but not in this case.

Currently, many drivers use plain ioremap() to map this resource.  We
are replacing that existing call - which is known to work in the majority
of cases - with a new call to cater for different semantics required by
an architecture.

Doing a replace of these ioremap() calls with ioremap_nopost() in this
situation, and then having ioremap_nopost() fail is a recipe for causing
lots and lots of regressions.

The only sane approach is to have ioremap_post() default to modelling the
_existing_ behaviour everywhere that it is used.

Requiring it to fail until architecture folk trip over the failure is
totally insane, and I very strongly disagree with you on this.

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

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Luis R. Rodriguez]

On Thu, Apr 06, 2017 at 01:11:57PM +0100, Russell King - ARM Linux wrote:
> On Thu, Apr 06, 2017 at 12:53:12PM +0200, Luis R. Rodriguez wrote:
> > On Thu, Apr 06, 2017 at 11:26:36AM +0100, Lorenzo Pieralisi wrote:
> > > Indeed, the static inline ioremap_nocache() fallback does not work
> > > on all arches (whether I add the fallback in linux/io.h or
> > > asm-generic/io.h is irrelevant), I bump into issues such as the one
> > > reported above.
> > 
> > Its also not *safe* to assume on behalf of all architectures a new ioremap
> > call is both a good idea and proper.
> 
> You may be right in general, but not in this case.
> 
> Currently, many drivers use plain ioremap() to map this resource.  We
> are replacing that existing call - which is known to work in the majority
> of cases - with a new call to cater for different semantics required by
> an architecture.
> 
> Doing a replace of these ioremap() calls with ioremap_nopost() in this
> situation, and then having ioremap_nopost() fail is a recipe for causing
> lots and lots of regressions.
> 
> The only sane approach is to have ioremap_post() default to modelling the
> _existing_ behaviour everywhere that it is used.
> 
> Requiring it to fail until architecture folk trip over the failure is
> totally insane, and I very strongly disagree with you on this.

Ah yes, what if with this modulo rule of thumb:

The litmus test then is if an existing set of calls are changed to
use a new ioremap then all archs that support those drivers where the new
call is being added must be modified to also have a correct corresponding
API call ?

This is more work on the new person introducing the new API, and should require
review still on arch maintainers but it seems like a fair compromise.

Then if an API is *new* though then things can move forward without requiring
all archs to add the respective call.

  Luis

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Russell King - ARM Linux]

On Thu, Apr 06, 2017 at 01:59:07PM +0200, Luis R. Rodriguez wrote:
> On Thu, Apr 06, 2017 at 12:38:45PM +0100, Lorenzo Pieralisi wrote:
> > Ok but where ? linux/io.h or asm-generic/io.h ? As I replied to Russell,
> > having it in linux/io.h is a bit odd given that it would be the only
> > ioremap_* implementation declared there, I think we need more
> > consistency here instead of deviating again from what you did.
> 
> asm-generic/io.h is the right place for generics which let you override.

I disagree for two reasons, and I will refer you to Linus' comments back
in 2005 at http://yarchive.net/comp/linux/generic.html

1) asm-generic/io.h has become an ifdef mess, every single definition in
   there is conditional.  The reason for this has happened is that
   architectures can't pick and choose what they want from a single file
   unless something like that is done.  It would be _far_ better to
   split this file up by functional group - eg, ISA IO accessors,
   io(read|write)*(), read|write*(), and so forth.

2) We're at the point where we have various .c files around the kernel
   _directly_ including asm-generic header files, which means the
   use of asm-generic headers is no longer a choice of the architecture.

3) asm-generic/ started out life as the place where example
   implementations of asm/*.h header files were found, and but has
   grown into a place where we dump default implementations.  We had
   a place for default implementations for years, which were the
   linux/*.h headers.  We have now ended up with a mixture of both
   techniques, even for something like io.h, we have the messy
   asm-generic/io.h, the architecture's asm/io.h, and then linux/io.h.
   We have ended up with both linux/io.h and asm-generic/io.h containing
   default definitions.

We've had the rule that where both linux/foo.h and asm/foo.h exist, the
linux/ counterpart is the preferred include file.  That didn't really
happen with asm/io.h due to the number of users that there were, but
when new stuff was added to asm/io.h which we wanted to be generic,
linux/io.h was created to contain that.  This actually pre-dates the
"let's clutter up asm-generic with shared arch stuff" push.

Now, what you say _may_ make sense if we have an
asm-generic/ioremap-nopost.h header which just defines a default
ioremap_nopost.h implementation, and architectures would then be able to
choose whether to include that or not depending on whether they provide
their own implementation.  No ugly ifdefs are necessary with that
approach.

Out of the three choices, I would much rather see the
asm-generic/ioremap-nopost.h approach.  However, the down-side to that
is it means all architectures asm/io.h would need to be touched to
explicitly include that.

What I'm absolutely certain of, though, is that making asm-generic/io.h
even worse by adding yet more conditional stuff to it is not a sane way
forward.

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

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Lorenzo Pieralisi]

On Thu, Apr 06, 2017 at 02:07:27PM +0100, Russell King - ARM Linux wrote:
> On Thu, Apr 06, 2017 at 01:59:07PM +0200, Luis R. Rodriguez wrote:
> > On Thu, Apr 06, 2017 at 12:38:45PM +0100, Lorenzo Pieralisi wrote:
> > > Ok but where ? linux/io.h or asm-generic/io.h ? As I replied to Russell,
> > > having it in linux/io.h is a bit odd given that it would be the only
> > > ioremap_* implementation declared there, I think we need more
> > > consistency here instead of deviating again from what you did.
> > 
> > asm-generic/io.h is the right place for generics which let you override.
> 
> I disagree for two reasons, and I will refer you to Linus' comments back
> in 2005 at http://yarchive.net/comp/linux/generic.html
> 
> 1) asm-generic/io.h has become an ifdef mess, every single definition in
>    there is conditional.  The reason for this has happened is that
>    architectures can't pick and choose what they want from a single file
>    unless something like that is done.  It would be _far_ better to
>    split this file up by functional group - eg, ISA IO accessors,
>    io(read|write)*(), read|write*(), and so forth.
> 
> 2) We're at the point where we have various .c files around the kernel
>    _directly_ including asm-generic header files, which means the
>    use of asm-generic headers is no longer a choice of the architecture.
> 
> 3) asm-generic/ started out life as the place where example
>    implementations of asm/*.h header files were found, and but has
>    grown into a place where we dump default implementations.  We had
>    a place for default implementations for years, which were the
>    linux/*.h headers.  We have now ended up with a mixture of both
>    techniques, even for something like io.h, we have the messy
>    asm-generic/io.h, the architecture's asm/io.h, and then linux/io.h.
>    We have ended up with both linux/io.h and asm-generic/io.h containing
>    default definitions.
> 
> We've had the rule that where both linux/foo.h and asm/foo.h exist, the
> linux/ counterpart is the preferred include file.  That didn't really
> happen with asm/io.h due to the number of users that there were, but
> when new stuff was added to asm/io.h which we wanted to be generic,
> linux/io.h was created to contain that.  This actually pre-dates the
> "let's clutter up asm-generic with shared arch stuff" push.
> 
> Now, what you say _may_ make sense if we have an
> asm-generic/ioremap-nopost.h header which just defines a default
> ioremap_nopost.h implementation, and architectures would then be able to
> choose whether to include that or not depending on whether they provide
> their own implementation.  No ugly ifdefs are necessary with that
> approach.
> 
> Out of the three choices, I would much rather see the
> asm-generic/ioremap-nopost.h approach.  However, the down-side to that
> is it means all architectures asm/io.h would need to be touched to
> explicitly include that.
> 
> What I'm absolutely certain of, though, is that making asm-generic/io.h
> even worse by adding yet more conditional stuff to it is not a sane way
> forward.

Ok, so:

(1) I can do asm-generic/ioremap-nopost.h, which I assume you want to
    contain something like

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

Funny bit is that it has to be included by asm*/io.h files _after_
ioremap_nocache has been #defined (that's the reason my approach was
failing miserably even on arches like eg powerpc (see [1] below) that
does have ioremap_nocache), not sure it is going to be very nice to have
an include in the middle of asm*/io.h include files (and I have to patch
all arches which I can do).

Or

(2) I add:

#ifndef ioremap_nopost
static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
{
	return NULL; <-(making it return ioremap_nocache() does not
			work, see [1] for the reason)
}
#endif

in linux/io.h

(3) ioremap_nopost follows Luis' ioremap_uc approach

(1)-(2) bypass completely what Luis did for ioremap_uc(), which implies
that we have yet another way of implementing ioremap_*.

I would like to get this patchset done so if you have an opinion it
is time to state it.

Thanks,
Lorenzo

[1] powerpc build report:

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/config-io-mappings-fix-v3
head:   283a324b549a662346c95c05b08983dd5b83354b
commit: e66b493fe93226c02b1a33355f79db7ed6efe718 [2/23] linux/io.h: add ioremap_nopost remap interface
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout e66b493fe93226c02b1a33355f79db7ed6efe718
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc

All errors (new ones prefixed by >>):

   In file included from arch/powerpc/include/asm/io.h:28:0,
                    from arch/powerpc/oprofile/op_model_cell.c:29:
   include/linux/io.h: In function 'ioremap_nopost':
   include/linux/io.h:169:9: error: implicit declaration of function 'ioremap_nocache' [-Werror=implicit-function-declaration]
     return ioremap_nocache(offset, size);
            ^~~~~~~~~~~~~~~
>> include/linux/io.h:169:9: error: return makes pointer from integer without a cast [-Werror=int-conversion]
     return ioremap_nocache(offset, size);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: all warnings being treated as errors

vim +169 include/linux/io.h

   163  }
   164  #endif
   165 
   166  #ifndef ioremap_nopost
   167  static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
   168  {
 > 169           return ioremap_nocache(offset, size);
   170  }
   171  #endif
   172 

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Russell King - ARM Linux]

On Thu, Apr 06, 2017 at 05:21:56PM +0100, Lorenzo Pieralisi wrote:
> Ok, so:
> 
> (1) I can do asm-generic/ioremap-nopost.h, which I assume you want to
>     contain something like
> 
> static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
> {
> 	return ioremap_nocache(offset, size);
> }
> 
> Funny bit is that it has to be included by asm*/io.h files _after_
> ioremap_nocache has been #defined (that's the reason my approach was
> failing miserably even on arches like eg powerpc (see [1] below) that
> does have ioremap_nocache),

PowerPC does have ioremap_nocache() though:

/**
 * ioremap     -   map bus memory into CPU space
...
 * * ioremap_nocache is identical to ioremap
extern void __iomem *ioremap(phys_addr_t address, unsigned long size);
#define ioremap_nocache(addr, size)     ioremap((addr), (size))

and this include file is included very early on in linux/io.h.  I don't
see anything that conditionalises it on anything except __KERNEL__.  So,
the report from 0-day really doesn't make any sense to me.

Do we know how we're ending up in linux/io.h line 169 without having
picked up the ioremap_nocache() definition provided by PowerPC's
asm/io.h ?

> not sure it is going to be very nice to have
> an include in the middle of asm*/io.h include files (and I have to patch
> all arches which I can do).

You mean like we already have to do with this asm-generic/io.h thing in
the ARM io.h header file, because we need to define all the accessors
first, to prevent the asm-generic/io.h thing defining them for us?
Given how asm-generic has headed in this regard, having include files
at all sorts of strange locations within the architecture asm/*.h
header files has become quite normal, unfortunately.

> (2) I add:
> 
> #ifndef ioremap_nopost
> static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
> {
> 	return NULL; <-(making it return ioremap_nocache() does not
> 			work, see [1] for the reason)
> }
> #endif
> 
> in linux/io.h

... which breaks the kernel if ioremap_nopost is missing from the arch
header, and one of the drivers that you're modifying to use this new
ioremap variant happens to be built and used on such an architecture.

> (3) ioremap_nopost follows Luis' ioremap_uc approach

Same problem as (2), as I understand correctly.

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

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Lorenzo Pieralisi]

On Thu, Apr 06, 2017 at 05:40:16PM +0100, Russell King - ARM Linux wrote:
> On Thu, Apr 06, 2017 at 05:21:56PM +0100, Lorenzo Pieralisi wrote:
> > Ok, so:
> > 
> > (1) I can do asm-generic/ioremap-nopost.h, which I assume you want to
> >     contain something like
> > 
> > static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
> > {
> > 	return ioremap_nocache(offset, size);
> > }
> > 
> > Funny bit is that it has to be included by asm*/io.h files _after_
> > ioremap_nocache has been #defined (that's the reason my approach was
> > failing miserably even on arches like eg powerpc (see [1] below) that
> > does have ioremap_nocache),
> 
> PowerPC does have ioremap_nocache() though:
> 
> /**
>  * ioremap     -   map bus memory into CPU space
> ...
>  * * ioremap_nocache is identical to ioremap
> extern void __iomem *ioremap(phys_addr_t address, unsigned long size);
> #define ioremap_nocache(addr, size)     ioremap((addr), (size))
> 
> and this include file is included very early on in linux/io.h.  I don't
> see anything that conditionalises it on anything except __KERNEL__.  So,
> the report from 0-day really doesn't make any sense to me.
> 
> Do we know how we're ending up in linux/io.h line 169 without having
> picked up the ioremap_nocache() definition provided by PowerPC's
> asm/io.h ?

I will debug it further but I *think* it is because:

eg arch/powerpc/oprofile/op_model_cell.c includes <asm/io.h>

and <asm/io.h> includes <linux/io.h> before ioremap_nocache is defined

> > not sure it is going to be very nice to have
> > an include in the middle of asm*/io.h include files (and I have to patch
> > all arches which I can do).
> 
> You mean like we already have to do with this asm-generic/io.h thing in
> the ARM io.h header file, because we need to define all the accessors
> first, to prevent the asm-generic/io.h thing defining them for us?
> Given how asm-generic has headed in this regard, having include files
> at all sorts of strange locations within the architecture asm/*.h
> header files has become quite normal, unfortunately.

Yes we won't make it any nicer that's for certain, my worry is that
it would end up being even harder to read.

> > (2) I add:
> > 
> > #ifndef ioremap_nopost
> > static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
> > {
> > 	return NULL; <-(making it return ioremap_nocache() does not
> > 			work, see [1] for the reason)
> > }
> > #endif
> > 
> > in linux/io.h
> 
> ... which breaks the kernel if ioremap_nopost is missing from the arch
> header, and one of the drivers that you're modifying to use this new
> ioremap variant happens to be built and used on such an architecture.

Yes agree.

> > (3) ioremap_nopost follows Luis' ioremap_uc approach
> 
> Same problem as (2), as I understand correctly.

Agreed. We have to find the lesser evil, that's it.

Thanks !
Lorenzo

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Russell King - ARM Linux]

On Thu, Apr 06, 2017 at 06:09:51PM +0100, Lorenzo Pieralisi wrote:
> On Thu, Apr 06, 2017 at 05:40:16PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Apr 06, 2017 at 05:21:56PM +0100, Lorenzo Pieralisi wrote:
> > > Ok, so:
> > > 
> > > (1) I can do asm-generic/ioremap-nopost.h, which I assume you want to
> > >     contain something like
> > > 
> > > static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
> > > {
> > > 	return ioremap_nocache(offset, size);
> > > }
> > > 
> > > Funny bit is that it has to be included by asm*/io.h files _after_
> > > ioremap_nocache has been #defined (that's the reason my approach was
> > > failing miserably even on arches like eg powerpc (see [1] below) that
> > > does have ioremap_nocache),
> > 
> > PowerPC does have ioremap_nocache() though:
> > 
> > /**
> >  * ioremap     -   map bus memory into CPU space
> > ...
> >  * * ioremap_nocache is identical to ioremap
> > extern void __iomem *ioremap(phys_addr_t address, unsigned long size);
> > #define ioremap_nocache(addr, size)     ioremap((addr), (size))
> > 
> > and this include file is included very early on in linux/io.h.  I don't
> > see anything that conditionalises it on anything except __KERNEL__.  So,
> > the report from 0-day really doesn't make any sense to me.
> > 
> > Do we know how we're ending up in linux/io.h line 169 without having
> > picked up the ioremap_nocache() definition provided by PowerPC's
> > asm/io.h ?
> 
> I will debug it further but I *think* it is because:
> 
> eg arch/powerpc/oprofile/op_model_cell.c includes <asm/io.h>
> 
> and <asm/io.h> includes <linux/io.h> before ioremap_nocache is defined

Oh, that's just very wrong.  asm/foo.h should never include linux/foo.h
especially when linux/foo.h already includes asm/foo.h.  I think we
need PowerPC folk to fix this.

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

[PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Lorenzo Pieralisi]

On Thu, Apr 06, 2017 at 11:47:37AM +0100, Russell King - ARM Linux wrote:
> On Thu, Apr 06, 2017 at 11:26:36AM +0100, Lorenzo Pieralisi wrote:
> > On Wed, Apr 05, 2017 at 01:38:12PM +0100, Lorenzo Pieralisi wrote:
> > > eg: openrisc
> > > 
> > > >> include/asm-generic/io.h:922:9: error: implicit declaration of
> > > >> function 'ioremap_nocache' [-Werror=implicit-function-declaration]
> > >      return ioremap_nocache(offset, size);
> > 
> > Indeed, the static inline ioremap_nocache() fallback does not work
> > on all arches (whether I add the fallback in linux/io.h or
> > asm-generic/io.h is irrelevant), I bump into issues such as the one
> > reported above.
> 
> From what I can see:
> 
> (a) openrisc does define ioremap_nocache() in its asm/io.h
> (b) these do not:
> 
> $ grep -L 'ioremap_nocache' arch/*/include/asm/io.h
> arch/blackfin/include/asm/io.h
> arch/h8300/include/asm/io.h
> arch/m68k/include/asm/io.h
> arch/score/include/asm/io.h
> arch/sparc/include/asm/io.h
> 
> Out of those, blackfin, h8300 and score do not define it, whereas m68k
> and sparc do in other headers included by asm/io.h.  So it looks like
> we have three problem architectures that don't define an ioremap_nocache().
> 
> PCI on blackfin depends on BROKEN, so it's not selectable.  From what I
> can tell, h8300 and score do not allow PCI to be enabled (but maybe its
> buried elsewhere in their Kconfig files, I didn't check.)
> 
> So, I think a way around this is to make ioremap_nopost() conditional
> on PCI in linux/io.h for the time being - if its scope wants to be
> enlarged, then these three architectures would need to have either an
> ioremap_nopost() implementation added, or an ioremap_nocache()
> implementation.

Ok, I implemented asm-generic/ioremap-nopost.h, I do not think we need
a CONFIG_PCI guard (kbuild has not barfed at it, yet), given that
blackfin and h8300 are !MMU and score selects NO_IOMEM.

Patch below is all arches squashed into one commit, I probably have
to split it one per arch which will make this a 50-patch series in
total:

https://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git/commit/?h=pci/config-io-mappings-fix-v3&id=acc0be820c809101e02ab7cb170f802db53af934

If I hear no complaints I will split patch above one per arch and
repost the series shortly (even though I think I should make two series
out of it, one asm-generic/ioremap-nopost.h boilerplate and second
ARM/ARM64 implementation + PCI drivers).

Lorenzo