This patch series[1] is a v4 of a previous version:
v3 -> v4:
- Reverted back to pci_remap_cfgspace() interface for lack of
consensus on ioremap_nopost() semantics
- Dropped pci_remap_iospace() rework (owing to lack of adequate
pgprot_* attribute description across arches)
- Updated pci_remap_cfgspace() comments
v3: http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/500838.html
v2 -> v3:
- Created a default ioremap_nopost() implementation in a separate
asm-generic header and patched all arches to make use of it
- Removed PCI drivers patches from the series to simplify the
review, they will be posted separately once the ioremap_nopost()
interface is settled
- Fixed devm_ioremap_* BUS offset comments and implemented
nopost interface on top of it
- Added collected tags
v2: https://lkml.org/lkml/2017/3/27/220
v2 -> v3:
- Fixed devm_ioremap_* BUS offset comments and implemented
nopost interface on top of it
- Added collected tags
v1: https://lkml.org/lkml/2017/2/27/228
v1 -> v2:
- Changed pci_remap_cfgspace() to more generic ioremap_nopost()
interface
- Added pgprot_nonposted
- Fixed build errors on arches not relying on asm-generic headers
- Added PCI versatile host controller driver patch
- Added missing config space remapping to hisilicon host controller
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-v4
Cc: Pratyush Anand <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Jingoo Han <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Mingkai Hu <[email protected]>
Cc: Tanmay Inamdar <[email protected]>
Cc: Murali Karicheri <[email protected]>
Cc: Russell King <[email protected]>
Cc: Bharat Kumar Gogada <[email protected]>
Cc: Ray Jui <[email protected]>
Cc: Wenrui Li <[email protected]>
Cc: Shawn Lin <[email protected]>
Cc: Minghuan Lian <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Jon Mason <[email protected]>
Cc: Gabriele Paoloni <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Cc: Joao Pinto <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: Luis R. Rodriguez <[email protected]>
Cc: Michal Simek <[email protected]>
Cc: Stanimir Varbanov <[email protected]>
Cc: Zhou Wang <[email protected]>
Cc: Roy Zang <[email protected]>
Lorenzo Pieralisi (21):
PCI: remove __weak tag from pci_remap_iospace()
linux/io.h: add PCI config space remap interface
ARM64: implement pci_remap_cfgspace() interface
ARM: implement pci_remap_cfgspace() interface
lib: fix Devres devm_ioremap_* offset parameter kerneldoc description
PCI: implement Devres interface to map PCI config space
PCI: ECAM: use pci_remap_cfgspace() to map config region
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: 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: hisi: 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 | 6 ++-
arch/arm/include/asm/io.h | 10 ++++
arch/arm/mm/ioremap.c | 7 +++
arch/arm/mm/nommu.c | 12 +++++
arch/arm64/include/asm/io.h | 10 ++++
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 | 6 ++-
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 | 84 +++++++++++++++++++++++++++++++++-
include/linux/io.h | 19 ++++++++
include/linux/pci.h | 5 ++
lib/devres.c | 6 +--
24 files changed, 183 insertions(+), 31 deletions(-)
--
2.10.0
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().
Acked-by: Arnd Bergmann <[email protected]>
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
---
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
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 <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Russell King <[email protected]>
Cc: Catalin Marinas <[email protected]>
---
include/linux/io.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/include/linux/io.h b/include/linux/io.h
index 82ef36e..3934aba 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -91,6 +91,25 @@ void devm_memunmap(struct device *dev, void *addr);
void *__devm_memremap_pages(struct device *dev, struct resource *res);
/*
+ * The PCI specifications (Rev 3.0, 3.2.5 "Transaction Ordering and
+ * Posting") mandate non-posted configuration transactions. There is
+ * no ioremap API in the kernel that can guarantee non-posted write
+ * semantics across arches so provide a default implementation for
+ * mapping PCI config space that defaults to ioremap_nocache(); arches
+ * should override it if they have memory mapping implementations that
+ * guarantee non-posted writes semantics to make the memory mapping
+ * compliant with the PCI specification.
+ */
+#ifndef pci_remap_cfgspace
+#define pci_remap_cfgspace pci_remap_cfgspace
+static inline void __iomem *pci_remap_cfgspace(phys_addr_t offset,
+ size_t size)
+{
+ return ioremap_nocache(offset, size);
+}
+#endif
+
+/*
* Some systems do not have legacy ISA devices.
* /dev/port is not a valid interface on these systems.
* So for those archs, <asm/io.h> should define the following symbol.
--
2.10.0
The offset parameter in Devres devm_ioremap_* functions kerneldoc
entries is erroneously defined as BUS offset whereas it is actually
a resource address.
Since it is actually misleading, fix the Devres devm_ioremap_* offset
parameter kerneldoc entry by replacing BUS offset with a more
suitable description (ie Resource address).
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Tejun Heo <[email protected]>
---
lib/devres.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/devres.c b/lib/devres.c
index cb1464c..78eca71 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -17,7 +17,7 @@ static int devm_ioremap_match(struct device *dev, void *res, void *match_data)
/**
* devm_ioremap - Managed ioremap()
* @dev: Generic device to remap IO address for
- * @offset: BUS offset to map
+ * @offset: Resource address to map
* @size: Size of map
*
* Managed ioremap(). Map is automatically unmapped on driver detach.
@@ -45,7 +45,7 @@ EXPORT_SYMBOL(devm_ioremap);
/**
* devm_ioremap_nocache - Managed ioremap_nocache()
* @dev: Generic device to remap IO address for
- * @offset: BUS offset to map
+ * @offset: Resource address to map
* @size: Size of map
*
* Managed ioremap_nocache(). Map is automatically unmapped on driver
@@ -74,7 +74,7 @@ EXPORT_SYMBOL(devm_ioremap_nocache);
/**
* devm_ioremap_wc - Managed ioremap_wc()
* @dev: Generic device to remap IO address for
- * @offset: BUS offset to map
+ * @offset: Resource address to map
* @size: Size of map
*
* Managed ioremap_wc(). Map is automatically unmapped on driver detach.
--
2.10.0
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 pci_remap_cfgspace() 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 <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Russell King <[email protected]>
---
arch/arm/include/asm/io.h | 10 ++++++++++
arch/arm/mm/ioremap.c | 7 +++++++
arch/arm/mm/nommu.c | 12 ++++++++++++
3 files changed, 29 insertions(+)
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 42871fb..74d1b09 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -187,6 +187,16 @@ static inline void pci_ioremap_set_mem_type(int mem_type) {}
extern int pci_ioremap_io(unsigned int offset, phys_addr_t phys_addr);
/*
+ * PCI configuration space mapping function.
+ *
+ * PCI specifications does not allow configuration write
+ * transactions to be posted. Add an arch specific
+ * pci_remap_cfgspace definition that is implemented
+ * through strongly ordered memory mappings.
+ */
+#define pci_remap_cfgspace pci_remap_cfgspace
+void __iomem *pci_remap_cfgspace(resource_size_t res_cookie, size_t size);
+/*
* Now, pick up the machine-defined IO definitions
*/
#ifdef CONFIG_NEED_MACH_IO_H
diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index ff0eed2..fc91205 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -481,6 +481,13 @@ int pci_ioremap_io(unsigned int offset, phys_addr_t phys_addr)
__pgprot(get_mem_type(pci_ioremap_mem_type)->prot_pte));
}
EXPORT_SYMBOL_GPL(pci_ioremap_io);
+
+void __iomem *pci_remap_cfgspace(resource_size_t res_cookie, size_t size)
+{
+ return arch_ioremap_caller(res_cookie, size, MT_UNCACHED,
+ __builtin_return_address(0));
+}
+EXPORT_SYMBOL_GPL(pci_remap_cfgspace);
#endif
/*
diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
index 33a45bd..3b8e728 100644
--- a/arch/arm/mm/nommu.c
+++ b/arch/arm/mm/nommu.c
@@ -436,6 +436,18 @@ void __iomem *ioremap_wc(resource_size_t res_cookie, size_t size)
}
EXPORT_SYMBOL(ioremap_wc);
+#ifdef CONFIG_PCI
+
+#include <asm/mach/map.h>
+
+void __iomem *pci_remap_cfgspace(resource_size_t res_cookie, size_t size)
+{
+ return arch_ioremap_caller(res_cookie, size, MT_UNCACHED,
+ __builtin_return_address(0));
+}
+EXPORT_SYMBOL_GPL(pci_remap_cfgspace);
+#endif
+
void *arch_memremap_wb(phys_addr_t phys_addr, size_t size)
{
return (void *)phys_addr;
--
2.10.0
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_pci_remap_cfg* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Bharat Kumar Gogada <[email protected]>
Cc: Michal Simek <[email protected]>
---
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..2fe2df5 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, ®s);
+ port->reg_base = devm_pci_remap_cfg_resource(dev, ®s);
if (IS_ERR(port->reg_base))
return PTR_ERR(port->reg_base);
--
2.10.0
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_pci_remap_cfg* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Jingoo Han <[email protected]>
Cc: Joao Pinto <[email protected]>
---
drivers/pci/dwc/pcie-designware-host.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index 5ba3349..2b789af 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -338,8 +338,9 @@ 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_pci_remap_cfgspace(dev,
+ pp->cfg->start,
+ resource_size(pp->cfg));
if (!pci->dbi_base) {
dev_err(dev, "error with ioremap\n");
ret = -ENOMEM;
@@ -350,8 +351,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_pci_remap_cfgspace(dev,
+ pp->cfg0_base, pp->cfg0_size);
if (!pp->va_cfg0_base) {
dev_err(dev, "error with ioremap in function\n");
ret = -ENOMEM;
@@ -360,7 +361,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->va_cfg1_base = devm_pci_remap_cfgspace(dev,
+ pp->cfg1_base,
pp->cfg1_size);
if (!pp->va_cfg1_base) {
dev_err(dev, "error with ioremap\n");
--
2.10.0
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 <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Rob Herring <[email protected]>
---
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..85e7736 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_pci_remap_cfg_resource(&pdev->dev,
+ res);
if (IS_ERR(versatile_cfg_base[1]))
return PTR_ERR(versatile_cfg_base[1]);
--
2.10.0
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_pci_remap_cfg* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Signed-off-by: Gabriele Paoloni <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Zhou Wang <[email protected]>
---
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..1606de5 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_pci_remap_cfgspace(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_pci_remap_cfg_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_pci_remap_cfgspace(dev, res->start, resource_size(res));
if (!reg_base)
return -ENOMEM;
--
2.10.0
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_pci_remap_cfg* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Tanmay Inamdar <[email protected]>
---
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..de19898 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_pci_remap_cfg_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_pci_remap_cfg_resource(dev, res);
if (IS_ERR(port->csr_base))
return PTR_ERR(port->csr_base);
--
2.10.0
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_pci_remap_cfg* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Murali Karicheri <[email protected]>
---
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..8bc626e 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_pci_remap_cfg_resource(dev, res);
if (IS_ERR(pci->dbi_base))
return PTR_ERR(pci->dbi_base);
--
2.10.0
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_pci_remap_cfg* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Mingkai Hu <[email protected]>
Cc: Minghuan Lian <[email protected]>
Cc: Roy Zang <[email protected]>
---
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..8f0ee0d 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_pci_remap_cfg_resource(dev, dbi_base);
if (IS_ERR(pci->dbi_base))
return PTR_ERR(pci->dbi_base);
--
2.10.0
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 <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Thierry Reding <[email protected]>
---
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..2618f87 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_noncached(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_pci_remap_cfg_resource(dev, &rp->regs);
if (IS_ERR(rp->base))
return PTR_ERR(rp->base);
--
2.10.0
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_pci_remap_cfg* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
---
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..3ff3130 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_pci_remap_cfg_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
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_pci_remap_cfg* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Ray Jui <[email protected]>
Cc: Jon Mason <[email protected]>
---
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 8c6a327..90d2bdd 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(®));
+ pcie->base = devm_pci_remap_cfgspace(dev, reg.start,
+ resource_size(®));
if (!pcie->base) {
dev_err(dev, "unable to map controller registers\n");
return -ENOMEM;
--
2.10.0
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_pci_remap_cfg* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Stanimir Varbanov <[email protected]>
---
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..5bf23d4 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_pci_remap_cfg_resource(dev, res);
if (IS_ERR(pci->dbi_base))
return PTR_ERR(pci->dbi_base);
--
2.10.0
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_pci_remap_cfg* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Wenrui Li <[email protected]>
Cc: Shawn Lin <[email protected]>
---
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..690a7b5 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_pci_remap_cfg_resource(dev, regs);
if (IS_ERR(rockchip->reg_base))
return PTR_ERR(rockchip->reg_base);
--
2.10.0
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_pci_remap_cfg* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Pratyush Anand <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
---
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..3ae59de 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_pci_remap_cfg_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
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_pci_remap_cfg* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Bharat Kumar Gogada <[email protected]>
Cc: Michal Simek <[email protected]>
---
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..4b16b26 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_pci_remap_cfg_resource(dev, res);
if (IS_ERR(pcie->ecam_base))
return PTR_ERR(pcie->ecam_base);
pcie->phys_ecam_base = res->start;
--
2.10.0
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 pci_remap_cfgspace() 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 <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Jayachandran C <[email protected]>
---
drivers/pci/ecam.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
index 2fee61b..c228a2e 100644
--- a/drivers/pci/ecam.c
+++ b/drivers/pci/ecam.c
@@ -84,12 +84,14 @@ 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] =
+ pci_remap_cfgspace(cfgres->start + i * bsz,
+ bsz);
if (!cfg->winp[i])
goto err_exit_iomap;
}
} else {
- cfg->win = ioremap(cfgres->start, bus_range * bsz);
+ cfg->win = pci_remap_cfgspace(cfgres->start, bus_range * bsz);
if (!cfg->win)
goto err_exit_iomap;
}
--
2.10.0
The introduction of the pci_remap_cfgspace() interface allows
PCI host controller drivers to map PCI config space through a
dedicated kernel interface. Current PCI host controller drivers
use the devm_ioremap_* Devres interfaces to map PCI configuration
space regions so in order to update them to the new
pci_remap_cfgspace() mapping interface a new set of Devres interfaces
should be implemented so that PCI host controller drivers can make
use of them.
Introduce two new functions in the PCI kernel layer and Devres
documentation:
- devm_pci_remap_cfgspace()
- devm_pci_remap_cfg_resource()
so that PCI host controller drivers can make use of them to map
PCI configuration space regions.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
---
Documentation/driver-model/devres.txt | 6 ++-
drivers/pci/pci.c | 82 +++++++++++++++++++++++++++++++++++
include/linux/pci.h | 5 +++
3 files changed, 91 insertions(+), 2 deletions(-)
diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index bf34d5b..e72587f 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -342,8 +342,10 @@ PER-CPU MEM
devm_free_percpu()
PCI
- pcim_enable_device() : after success, all PCI ops become managed
- pcim_pin_device() : keep PCI device enabled after release
+ devm_pci_remap_cfgspace() : ioremap PCI configuration space
+ devm_pci_remap_cfg_resource() : ioremap PCI configuration space resource
+ pcim_enable_device() : after success, all PCI ops become managed
+ pcim_pin_device() : keep PCI device enabled after release
PHY
devm_usb_get_phy()
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index bd98674..4129f94 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3401,6 +3401,88 @@ void pci_unmap_iospace(struct resource *res)
#endif
}
+/**
+ * devm_pci_remap_cfgspace - Managed pci_remap_cfgspace()
+ * @dev: Generic device to remap IO address for
+ * @offset: Resource address to map
+ * @size: Size of map
+ *
+ * Managed pci_remap_cfgspace(). Map is automatically unmapped on driver
+ * detach.
+ */
+void __iomem *devm_pci_remap_cfgspace(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 = pci_remap_cfgspace(offset, size);
+ if (addr) {
+ *ptr = addr;
+ devres_add(dev, ptr);
+ } else
+ devres_free(ptr);
+
+ return addr;
+}
+EXPORT_SYMBOL(devm_pci_remap_cfgspace);
+
+/**
+ * devm_pci_remap_cfg_resource - check, request region and ioremap cfg resource
+ * @dev: generic device to handle the resource for
+ * @res: configuration space resource to be handled
+ *
+ * Checks that a resource is a valid memory region, requests the memory
+ * region and ioremaps with pci_remap_cfgspace() API that ensures the
+ * proper PCI configuration space memory attributes are guaranteed.
+ *
+ * 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_pci_remap_cfg_resource(&pdev->dev, res);
+ * if (IS_ERR(base))
+ * return PTR_ERR(base);
+ */
+void __iomem *devm_pci_remap_cfg_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_pci_remap_cfgspace(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_pci_remap_cfg_resource);
+
static void __pci_set_master(struct pci_dev *dev, bool enable)
{
u16 old_cmd, cmd;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index eb3da1a..70534d6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1199,6 +1199,11 @@ unsigned long pci_address_to_pio(phys_addr_t addr);
phys_addr_t pci_pio_to_address(unsigned long pio);
int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr);
void pci_unmap_iospace(struct resource *res);
+void __iomem *devm_pci_remap_cfgspace(struct device *dev,
+ resource_size_t offset,
+ resource_size_t size);
+void __iomem *devm_pci_remap_cfg_resource(struct device *dev,
+ struct resource *res);
static inline pci_bus_addr_t pci_bus_address(struct pci_dev *pdev, int bar)
{
--
2.10.0
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 pci_remap_cfgspace() 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 <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Catalin Marinas <[email protected]>
---
arch/arm64/include/asm/io.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
index 0c00c87..7a03250 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -173,6 +173,16 @@ extern void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size);
#define iounmap __iounmap
/*
+ * PCI configuration space mapping function.
+ *
+ * PCI specifications disallows posted write configuration transactions.
+ * Add an arch specific pci_remap_cfgspace 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".
+ */
+#define pci_remap_cfgspace(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
On Wed, Apr 19, 2017 at 05:48:52PM +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 pci_remap_cfgspace() 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 <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Catalin Marinas <[email protected]>
Acked-by: Catalin Marinas <[email protected]>
[+ Michael]
On Wed, Apr 19, 2017 at 05:48:51PM +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 <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> ---
> include/linux/io.h | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/include/linux/io.h b/include/linux/io.h
> index 82ef36e..3934aba 100644
> --- a/include/linux/io.h
> +++ b/include/linux/io.h
> @@ -91,6 +91,25 @@ void devm_memunmap(struct device *dev, void *addr);
> void *__devm_memremap_pages(struct device *dev, struct resource *res);
>
> /*
> + * The PCI specifications (Rev 3.0, 3.2.5 "Transaction Ordering and
> + * Posting") mandate non-posted configuration transactions. There is
> + * no ioremap API in the kernel that can guarantee non-posted write
> + * semantics across arches so provide a default implementation for
> + * mapping PCI config space that defaults to ioremap_nocache(); arches
> + * should override it if they have memory mapping implementations that
> + * guarantee non-posted writes semantics to make the memory mapping
> + * compliant with the PCI specification.
> + */
> +#ifndef pci_remap_cfgspace
> +#define pci_remap_cfgspace pci_remap_cfgspace
> +static inline void __iomem *pci_remap_cfgspace(phys_addr_t offset,
> + size_t size)
> +{
> + return ioremap_nocache(offset, size);
> +}
> +#endif
> +
> +/*
As an heads-up, this patch strictly depends on:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/powerpc/include/asm/io.h?id=590c369e7ecc00be736be39ae0c62d1b5d563a51
to go upstream first, otherwise we would break powerpc compilation
(owing to powerpc including linux/io.h before ioremap_nocache() is
defined in arch/powerpc/include/asm/io.h).
If we want to decouple them I must drop the static inline and make
it a #define, it is not ideal but we must be aware of this, I really
want to prevent breakage if we go ahead with this set (and -next can
hide the issue).
Thanks,
Lorenzo
> * Some systems do not have legacy ISA devices.
> * /dev/port is not a valid interface on these systems.
> * So for those archs, <asm/io.h> should define the following symbol.
> --
> 2.10.0
>
On Thu, Apr 20, 2017 at 5:51 AM, Lorenzo Pieralisi
<[email protected]> wrote:
> [+ Michael]
>
> On Wed, Apr 19, 2017 at 05:48:51PM +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 <[email protected]>
>> Cc: Arnd Bergmann <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: Bjorn Helgaas <[email protected]>
>> Cc: Russell King <[email protected]>
>> Cc: Catalin Marinas <[email protected]>
>> ---
>> include/linux/io.h | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>>
>> diff --git a/include/linux/io.h b/include/linux/io.h
>> index 82ef36e..3934aba 100644
>> --- a/include/linux/io.h
>> +++ b/include/linux/io.h
>> @@ -91,6 +91,25 @@ void devm_memunmap(struct device *dev, void *addr);
>> void *__devm_memremap_pages(struct device *dev, struct resource *res);
>>
>> /*
>> + * The PCI specifications (Rev 3.0, 3.2.5 "Transaction Ordering and
>> + * Posting") mandate non-posted configuration transactions. There is
>> + * no ioremap API in the kernel that can guarantee non-posted write
>> + * semantics across arches so provide a default implementation for
>> + * mapping PCI config space that defaults to ioremap_nocache(); arches
>> + * should override it if they have memory mapping implementations that
>> + * guarantee non-posted writes semantics to make the memory mapping
>> + * compliant with the PCI specification.
>> + */
>> +#ifndef pci_remap_cfgspace
>> +#define pci_remap_cfgspace pci_remap_cfgspace
>> +static inline void __iomem *pci_remap_cfgspace(phys_addr_t offset,
>> + size_t size)
>> +{
>> + return ioremap_nocache(offset, size);
>> +}
>> +#endif
>> +
>> +/*
>
> As an heads-up, this patch strictly depends on:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/powerpc/include/asm/io.h?id=590c369e7ecc00be736be39ae0c62d1b5d563a51
>
> to go upstream first, otherwise we would break powerpc compilation
> (owing to powerpc including linux/io.h before ioremap_nocache() is
> defined in arch/powerpc/include/asm/io.h).
>
> If we want to decouple them I must drop the static inline and make
> it a #define, it is not ideal but we must be aware of this, I really
> want to prevent breakage if we go ahead with this set (and -next can
> hide the issue).
It looks like Stephen merges powerpc/next into linux-next before
pci/next, so this will probably be OK there. I'll try to remember to
wait for the powerpc stuff to make it to Linus' tree during the merge
window before I send my pull request.
Bjorn
On Wed, Apr 19, 2017 at 05:48:49PM +0100, Lorenzo Pieralisi wrote:
> This patch series[1] is a v4 of a previous version:
Applied to pci/ioremap for v4.12, thanks, Lorenzo!
> v3 -> v4:
> - Reverted back to pci_remap_cfgspace() interface for lack of
> consensus on ioremap_nopost() semantics
> - Dropped pci_remap_iospace() rework (owing to lack of adequate
> pgprot_* attribute description across arches)
> - Updated pci_remap_cfgspace() comments
>
> v3: http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/500838.html
>
> v2 -> v3:
> - Created a default ioremap_nopost() implementation in a separate
> asm-generic header and patched all arches to make use of it
> - Removed PCI drivers patches from the series to simplify the
> review, they will be posted separately once the ioremap_nopost()
> interface is settled
> - Fixed devm_ioremap_* BUS offset comments and implemented
> nopost interface on top of it
> - Added collected tags
>
> v2: https://lkml.org/lkml/2017/3/27/220
>
> v2 -> v3:
> - Fixed devm_ioremap_* BUS offset comments and implemented
> nopost interface on top of it
> - Added collected tags
>
> v1: https://lkml.org/lkml/2017/2/27/228
>
> v1 -> v2:
> - Changed pci_remap_cfgspace() to more generic ioremap_nopost()
> interface
> - Added pgprot_nonposted
> - Fixed build errors on arches not relying on asm-generic headers
> - Added PCI versatile host controller driver patch
> - Added missing config space remapping to hisilicon host controller
>
> 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-v4
>
> Cc: Pratyush Anand <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Jonathan Corbet <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Jingoo Han <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Mingkai Hu <[email protected]>
> Cc: Tanmay Inamdar <[email protected]>
> Cc: Murali Karicheri <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Bharat Kumar Gogada <[email protected]>
> Cc: Ray Jui <[email protected]>
> Cc: Wenrui Li <[email protected]>
> Cc: Shawn Lin <[email protected]>
> Cc: Minghuan Lian <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Jon Mason <[email protected]>
> Cc: Gabriele Paoloni <[email protected]>
> Cc: Thomas Petazzoni <[email protected]>
> Cc: Joao Pinto <[email protected]>
> Cc: Thierry Reding <[email protected]>
> Cc: Luis R. Rodriguez <[email protected]>
> Cc: Michal Simek <[email protected]>
> Cc: Stanimir Varbanov <[email protected]>
> Cc: Zhou Wang <[email protected]>
> Cc: Roy Zang <[email protected]>
>
> Lorenzo Pieralisi (21):
> PCI: remove __weak tag from pci_remap_iospace()
> linux/io.h: add PCI config space remap interface
> ARM64: implement pci_remap_cfgspace() interface
> ARM: implement pci_remap_cfgspace() interface
> lib: fix Devres devm_ioremap_* offset parameter kerneldoc description
> PCI: implement Devres interface to map PCI config space
> PCI: ECAM: use pci_remap_cfgspace() to map config region
> 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: 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: hisi: 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 | 6 ++-
> arch/arm/include/asm/io.h | 10 ++++
> arch/arm/mm/ioremap.c | 7 +++
> arch/arm/mm/nommu.c | 12 +++++
> arch/arm64/include/asm/io.h | 10 ++++
> 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 | 6 ++-
> 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 | 84 +++++++++++++++++++++++++++++++++-
> include/linux/io.h | 19 ++++++++
> include/linux/pci.h | 5 ++
> lib/devres.c | 6 +--
> 24 files changed, 183 insertions(+), 31 deletions(-)
>
> --
> 2.10.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wednesday, April 19, 2017 12:49 PM, Lorenzo Pieralisi wrote:
>
> 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_pci_remap_cfg* interface to make sure the correct
> memory mappings for PCI configuration space are used.
>
> Signed-off-by: Lorenzo Pieralisi <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Jingoo Han <[email protected]>
Acked-by: Jingoo Han <[email protected]>
Best regards,
Jingoo Han
> Cc: Joao Pinto <[email protected]>
> ---
> drivers/pci/dwc/pcie-designware-host.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/dwc/pcie-designware-host.c
> b/drivers/pci/dwc/pcie-designware-host.c
> index 5ba3349..2b789af 100644
> --- a/drivers/pci/dwc/pcie-designware-host.c
> +++ b/drivers/pci/dwc/pcie-designware-host.c
> @@ -338,8 +338,9 @@ 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_pci_remap_cfgspace(dev,
> + pp->cfg->start,
> + resource_size(pp->cfg));
> if (!pci->dbi_base) {
> dev_err(dev, "error with ioremap\n");
> ret = -ENOMEM;
> @@ -350,8 +351,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_pci_remap_cfgspace(dev,
> + pp->cfg0_base, pp->cfg0_size);
> if (!pp->va_cfg0_base) {
> dev_err(dev, "error with ioremap in function\n");
> ret = -ENOMEM;
> @@ -360,7 +361,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->va_cfg1_base = devm_pci_remap_cfgspace(dev,
> + pp->cfg1_base,
> pp->cfg1_size);
> if (!pp->va_cfg1_base) {
> dev_err(dev, "error with ioremap\n");
> --
> 2.10.0
On 04/19/2017 12:48 PM, Lorenzo Pieralisi wrote:
> 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.
Side note that I've pinged all of the ARM server vendors and asked them
to verify this patch series on their platforms.
Jon.
On Tuesday, April 25, 2017 2:41 AM, Jon Masters wrote:
>
> On 04/19/2017 12:48 PM, Lorenzo Pieralisi wrote:
>
> > 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.
>
> Side note that I've pinged all of the ARM server vendors and asked them
> to verify this patch series on their platforms.
Good! I really want to know the result of these patches on ARM serves.
Please share it with us. Good luck.
Best regards,
Jingoo Han
>
> Jon.
Ack.
Tested-by: Khuong Dinh <[email protected]>
On Tue, Apr 25, 2017 at 9:20 AM, Jingoo Han <[email protected]> wrote:
> On Tuesday, April 25, 2017 2:41 AM, Jon Masters wrote:
>>
>> On 04/19/2017 12:48 PM, Lorenzo Pieralisi wrote:
>>
>> > 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.
>>
>> Side note that I've pinged all of the ARM server vendors and asked them
>> to verify this patch series on their platforms.
>
> Good! I really want to know the result of these patches on ARM serves.
> Please share it with us. Good luck.
>
> Best regards,
> Jingoo Han
>
>>
>> Jon.
>
>
--
CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is
for the sole use of the intended recipient(s) and contains information that
is confidential and proprietary to Applied Micro Circuits Corporation or
its subsidiaries. It is to be used solely for the purpose of furthering the
parties' business relationship. All unauthorized review, use, disclosure or
distribution is prohibited. If you are not the intended recipient, please
contact the sender by reply e-mail and destroy all copies of the original
message.
Tested-by: Dongdong Liu <[email protected]>
I tested the patchset on HiSilicon ARM64 D05 board.It works ok with 82599 netcard.
Thanks,
Dongdong
在 2017/4/25 14:40, Jon Masters 写道:
> On 04/19/2017 12:48 PM, Lorenzo Pieralisi wrote:
>
>> 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.
>
> Side note that I've pinged all of the ARM server vendors and asked them
> to verify this patch series on their platforms.
>
> Jon.
>
> .
>
On Wednesday, April 26, 2017 6:54 AM, Dongdong Liu wrote;
>
> Tested-by: Dongdong Liu <[email protected]>
>
> I tested the patchset on HiSilicon ARM64 D05 board.It works ok with 82599
> netcard.
Thank you for testing these patches. HiSilicon PCIe may use Designware-based
PCIe controller. In my opinion, other Designware-based PCIe controller will
work properly.
To Dongdong Liu, Khuong Dinh, and other people,
If possible, can you check the output of 'lspci -v'?
If you find something different, please share it with us.
Good luck.
Best regards,
Jingoo Han
>
> Thanks,
> Dongdong
> 在 2017/4/25 14:40, Jon Masters 写道:
> > On 04/19/2017 12:48 PM, Lorenzo Pieralisi wrote:
> >
> >> 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.
> >
> > Side note that I've pinged all of the ARM server vendors and asked them
> > to verify this patch series on their platforms.
> >
> > Jon.
> >
> > .
> >
在 2017/4/27 1:24, Jingoo Han 写道:
> On Wednesday, April 26, 2017 6:54 AM, Dongdong Liu wrote;
>>
>> Tested-by: Dongdong Liu <[email protected]>
>>
>> I tested the patchset on HiSilicon ARM64 D05 board.It works ok with 82599
>> netcard.
>
> Thank you for testing these patches. HiSilicon PCIe may use Designware-based
> PCIe controller. In my opinion, other Designware-based PCIe controller will
> work properly.
>
> To Dongdong Liu, Khuong Dinh, and other people,
> If possible, can you check the output of 'lspci -v'?
> If you find something different, please share it with us.
> Good luck.
root@(none)$ ./lspci -v
0002:80:00.0 Class 0604: Device 19e5:1610 (rev 01)
Flags: bus master, fast devsel, latency 0
Memory at a9e00000 (32-bit, non-prefetchable) [size=64K]
Bus: primary=80, secondary=81, subordinate=82, sec-latency=0
I/O behind bridge: 00000000-00001fff
Memory behind bridge: a8800000-a8ffffff
Prefetchable memory behind bridge: 00000000a9000000-00000000a9dfffff
Capabilities: [40] Power Management version 3
Capabilities: [50] MSI: Enable- Count=1/32 Maskable+ 64bit+
Capabilities: [70] Express Root Port (Slot-), MSI 00
Capabilities: [100] Advanced Error Reporting
Capabilities: [158] #19
Capabilities: [178] #17
Kernel driver in use: pcieport
0002:81:00.0 Class 0200: Device 8086:10fb (rev 01)
Flags: bus master, fast devsel, latency 0, IRQ 255
Memory at a9000000 (64-bit, prefetchable) [size=4M]
I/O ports at 1000 [disabled] [size=32]
Memory at a9800000 (64-bit, prefetchable) [size=16K]
Expansion ROM at a8800000 [disabled] [size=4M]
Capabilities: [40] Power Management version 3
Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
Capabilities: [70] MSI-X: Enable+ Count=64 Masked-
Capabilities: [a0] Express Endpoint, MSI 00
Capabilities: [e0] Vital Product Data
Capabilities: [100] Advanced Error Reporting
Capabilities: [140] Device Serial Number 9c-37-f4-ff-ff-7b-5b-a0
Capabilities: [150] Alternative Routing-ID Interpretation (ARI)
Capabilities: [160] Single Root I/O Virtualization (SR-IOV)
Kernel driver in use: ixgbe
0002:81:00.1 Class 0200: Device 8086:10fb (rev 01)
Flags: bus master, fast devsel, latency 0, IRQ 255
Memory at a9400000 (64-bit, prefetchable) [size=4M]
I/O ports at 1020 [disabled] [size=32]
Memory at a9a04000 (64-bit, prefetchable) [size=16K]
Expansion ROM at a8c00000 [disabled] [size=4M]
Capabilities: [40] Power Management version 3
Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
Capabilities: [70] MSI-X: Enable+ Count=64 Masked-
Capabilities: [a0] Express Endpoint, MSI 00
Capabilities: [e0] Vital Product Data
Capabilities: [100] Advanced Error Reporting
Capabilities: [140] Device Serial Number 9c-37-f4-ff-ff-7b-5b-a0
Capabilities: [150] Alternative Routing-ID Interpretation (ARI)
Capabilities: [160] Single Root I/O Virtualization (SR-IOV)
Kernel driver in use: ixgbe
0004:88:00.0 Class 0604: Device 19e5:1610 (rev 01)
Flags: bus master, fast devsel, latency 0
Memory at 8a9000000 (32-bit, non-prefetchable) [size=64K]
Bus: primary=88, secondary=89, subordinate=89, sec-latency=0
Capabilities: [40] Power Management version 3
Capabilities: [50] MSI: Enable- Count=1/32 Maskable+ 64bit+
Capabilities: [70] Express Root Port (Slot-), MSI 00
Capabilities: [100] Advanced Error Reporting
Capabilities: [158] #19
Capabilities: [178] #17
Kernel driver in use: pcieport
Thanks,
Dongdong
>
> Best regards,
> Jingoo Han
>
>>
>> Thanks,
>> Dongdong
>> 在 2017/4/25 14:40, Jon Masters 写道:
>>> On 04/19/2017 12:48 PM, Lorenzo Pieralisi wrote:
>>>
>>>> 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.
>>>
>>> Side note that I've pinged all of the ARM server vendors and asked them
>>> to verify this patch series on their platforms.
>>>
>>> Jon.
>>>
>>> .
>>>
>
> .
>
Hi,
They're same before and after applying the patch.
It was tested with X-Gene 1 and X-Gene 2 with DT (Device Tree) and ACPI boot.
X-Gene 1 - DT :
[root@(none) ~]# lspci -s 01:00.0 -v
01:00.0 Ethernet controller: Intel Corporation 82574L Gigabit Network Connection
Subsystem: Intel Corporation Gigabit CT Desktop Adapter
Flags: bus master, fast devsel, latency 0, IRQ 68
Memory at e1800c0000 (32-bit, non-prefetchable) [size=128K]
Memory at e180000000 (32-bit, non-prefetchable) [size=512K]
I/O ports at 1000 [disabled] [size=32]
Memory at e1800e0000 (32-bit, non-prefetchable) [size=16K]
Expansion ROM at e180080000 [disabled] [size=256K]
Capabilities: [c8] Power Management version 2
Capabilities: [d0] MSI: Enable- Count=1/1 Maskable- 64bit+
Capabilities: [e0] Express Endpoint, MSI 00
Capabilities: [a0] MSI-X: Enable+ Count=5 Masked-
Capabilities: [100] Advanced Error Reporting
Capabilities: [140] Device Serial Number 00-1b-21-ff-ff-4f-68-3d
Kernel driver in use: e1000e
X-Gene 1 - ACPI :
[root@(none) ~]# lspci -s 01:00.0 -v
01:00.0 Ethernet controller: Intel Corporation 82574L Gigabit Network Connection
Subsystem: Intel Corporation Gigabit CT Desktop Adapter
Flags: bus master, fast devsel, latency 0, IRQ 117
Memory at e0400c0000 (32-bit, non-prefetchable) [size=128K]
Memory at e040000000 (32-bit, non-prefetchable) [size=512K]
I/O ports at 1000 [disabled] [size=32]
Memory at e0400e0000 (32-bit, non-prefetchable) [size=16K]
Expansion ROM at e040080000 [disabled] [size=256K]
Capabilities: [c8] Power Management version 2
Capabilities: [d0] MSI: Enable- Count=1/1 Maskable- 64bit+
Capabilities: [e0] Express Endpoint, MSI 00
Capabilities: [a0] MSI-X: Enable- Count=5 Masked-
Capabilities: [100] Advanced Error Reporting
Capabilities: [140] Device Serial Number 00-1b-21-ff-ff-4f-68-3d
Kernel driver in use: e1000e
X-Gene 2 - DT :
[root@(none) ~]# lspci -s 0000:01:00.0 -v
0000:01:00.0 Ethernet controller: Intel Corporation 82572EI Gigabit
Ethernet Controller (Copper) (rev 06)
Subsystem: Intel Corporation PRO/1000 PT Desktop Adapter
Flags: bus master, fast devsel, latency 0, IRQ 49
Memory at c120000000 (32-bit, non-prefetchable) [size=128K]
Memory at c120020000 (32-bit, non-prefetchable) [size=128K]
I/O ports at 1000 [disabled] [size=32]
Expansion ROM at c120040000 [disabled] [size=128K]
Capabilities: [c8] Power Management version 2
Capabilities: [d0] MSI: Enable+ Count=1/1 Maskable- 64bit+
Capabilities: [e0] Express Endpoint, MSI 00
Capabilities: [100] Advanced Error Reporting
Capabilities: [140] Device Serial Number 00-1b-21-ff-ff-1e-84-4e
Kernel driver in use: e1000e
X-Gene 2 - ACPI :
[root@(none) ~]# lspci -s 0000:01:00.0 -v
0000:01:00.0 Ethernet controller: Intel Corporation 82572EI Gigabit
Ethernet Controller (Copper) (rev 06)
Subsystem: Intel Corporation PRO/1000 PT Desktop Adapter
Flags: bus master, fast devsel, latency 0, IRQ 93
Memory at c040000000 (32-bit, non-prefetchable) [size=128K]
Memory at c040020000 (32-bit, non-prefetchable) [size=128K]
I/O ports at 1000 [disabled] [size=32]
Expansion ROM at c040040000 [disabled] [size=128K]
Capabilities: [c8] Power Management version 2
Capabilities: [d0] MSI: Enable+ Count=1/1 Maskable- 64bit+
Capabilities: [e0] Express Endpoint, MSI 00
Capabilities: [100] Advanced Error Reporting
Capabilities: [140] Device Serial Number 00-1b-21-ff-ff-1e-84-4e
Kernel driver in use: e1000e
Best regards,
Khuong Dinh
On Wed, Apr 26, 2017 at 6:46 PM, Dongdong Liu <[email protected]> wrote:
>
>
> 在 2017/4/27 1:24, Jingoo Han 写道:
>>
>> On Wednesday, April 26, 2017 6:54 AM, Dongdong Liu wrote;
>>>
>>>
>>> Tested-by: Dongdong Liu <[email protected]>
>>>
>>> I tested the patchset on HiSilicon ARM64 D05 board.It works ok with 82599
>>> netcard.
>>
>>
>> Thank you for testing these patches. HiSilicon PCIe may use
>> Designware-based
>> PCIe controller. In my opinion, other Designware-based PCIe controller
>> will
>> work properly.
>>
>> To Dongdong Liu, Khuong Dinh, and other people,
>> If possible, can you check the output of 'lspci -v'?
>> If you find something different, please share it with us.
>> Good luck.
>
>
> root@(none)$ ./lspci -v
> 0002:80:00.0 Class 0604: Device 19e5:1610 (rev 01)
> Flags: bus master, fast devsel, latency 0
> Memory at a9e00000 (32-bit, non-prefetchable) [size=64K]
> Bus: primary=80, secondary=81, subordinate=82, sec-latency=0
> I/O behind bridge: 00000000-00001fff
> Memory behind bridge: a8800000-a8ffffff
> Prefetchable memory behind bridge: 00000000a9000000-00000000a9dfffff
> Capabilities: [40] Power Management version 3
> Capabilities: [50] MSI: Enable- Count=1/32 Maskable+ 64bit+
> Capabilities: [70] Express Root Port (Slot-), MSI 00
> Capabilities: [100] Advanced Error Reporting
> Capabilities: [158] #19
> Capabilities: [178] #17
> Kernel driver in use: pcieport
>
> 0002:81:00.0 Class 0200: Device 8086:10fb (rev 01)
> Flags: bus master, fast devsel, latency 0, IRQ 255
> Memory at a9000000 (64-bit, prefetchable) [size=4M]
> I/O ports at 1000 [disabled] [size=32]
> Memory at a9800000 (64-bit, prefetchable) [size=16K]
> Expansion ROM at a8800000 [disabled] [size=4M]
> Capabilities: [40] Power Management version 3
> Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
> Capabilities: [70] MSI-X: Enable+ Count=64 Masked-
> Capabilities: [a0] Express Endpoint, MSI 00
> Capabilities: [e0] Vital Product Data
> Capabilities: [100] Advanced Error Reporting
> Capabilities: [140] Device Serial Number 9c-37-f4-ff-ff-7b-5b-a0
> Capabilities: [150] Alternative Routing-ID Interpretation (ARI)
> Capabilities: [160] Single Root I/O Virtualization (SR-IOV)
> Kernel driver in use: ixgbe
>
> 0002:81:00.1 Class 0200: Device 8086:10fb (rev 01)
> Flags: bus master, fast devsel, latency 0, IRQ 255
> Memory at a9400000 (64-bit, prefetchable) [size=4M]
> I/O ports at 1020 [disabled] [size=32]
> Memory at a9a04000 (64-bit, prefetchable) [size=16K]
> Expansion ROM at a8c00000 [disabled] [size=4M]
> Capabilities: [40] Power Management version 3
> Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
> Capabilities: [70] MSI-X: Enable+ Count=64 Masked-
> Capabilities: [a0] Express Endpoint, MSI 00
> Capabilities: [e0] Vital Product Data
> Capabilities: [100] Advanced Error Reporting
> Capabilities: [140] Device Serial Number 9c-37-f4-ff-ff-7b-5b-a0
> Capabilities: [150] Alternative Routing-ID Interpretation (ARI)
> Capabilities: [160] Single Root I/O Virtualization (SR-IOV)
> Kernel driver in use: ixgbe
>
> 0004:88:00.0 Class 0604: Device 19e5:1610 (rev 01)
> Flags: bus master, fast devsel, latency 0
> Memory at 8a9000000 (32-bit, non-prefetchable) [size=64K]
> Bus: primary=88, secondary=89, subordinate=89, sec-latency=0
> Capabilities: [40] Power Management version 3
> Capabilities: [50] MSI: Enable- Count=1/32 Maskable+ 64bit+
> Capabilities: [70] Express Root Port (Slot-), MSI 00
> Capabilities: [100] Advanced Error Reporting
> Capabilities: [158] #19
> Capabilities: [178] #17
> Kernel driver in use: pcieport
>
> Thanks,
> Dongdong
>>
>>
>> Best regards,
>> Jingoo Han
>>
>>>
>>> Thanks,
>>> Dongdong
>>> 在 2017/4/25 14:40, Jon Masters 写道:
>>>>
>>>> On 04/19/2017 12:48 PM, Lorenzo Pieralisi wrote:
>>>>
>>>>> 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.
>>>>
>>>>
>>>> Side note that I've pinged all of the ARM server vendors and asked them
>>>> to verify this patch series on their platforms.
>>>>
>>>> Jon.
>>>>
>>>> .
>>>>
>>
>> .
>>
>
--
CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is
for the sole use of the intended recipient(s) and contains information that
is confidential and proprietary to Applied Micro Circuits Corporation or
its subsidiaries. It is to be used solely for the purpose of furthering the
parties' business relationship. All unauthorized review, use, disclosure or
distribution is prohibited. If you are not the intended recipient, please
contact the sender by reply e-mail and destroy all copies of the original
message.
On Wed, Apr 19, 2017 at 05:48:54PM +0100, Lorenzo Pieralisi wrote:
> The offset parameter in Devres devm_ioremap_* functions kerneldoc
> entries is erroneously defined as BUS offset whereas it is actually
> a resource address.
>
> Since it is actually misleading, fix the Devres devm_ioremap_* offset
> parameter kerneldoc entry by replacing BUS offset with a more
> suitable description (ie Resource address).
>
> Signed-off-by: Lorenzo Pieralisi <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Tejun Heo <[email protected]>
Acked-by: Tejun Heo <[email protected]>
Thanks.
--
tejun