Re: [PATCH 4/8] PCI: host: brcmstb: add dma-ranges for inbound traffic

2017-10-25 Thread Jim Quinlan
On Wed, Oct 25, 2017 at 5:46 AM, David Laight  wrote:
> From: Jim QuinlanPCIE_IPROC_MSI
>> Sent: 24 October 2017 19:16
>> The Broadcom STB PCIe host controller is intimately related to the
>> memory subsystem.  This close relationship adds complexity to how cpu
>> system memory is mapped to PCIe memory.  Ideally, this mapping is an
>> identity mapping, or an identity mapping off by a constant.  Not so in
>> this case.
>>
>> Consider the Broadcom reference board BCM97445LCC_4X8 which has 6 GB
>> of system memory.  Here is how the PCIe controller maps the
>> system memory to PCIe memory:
>>
>>   memc0-a@[03fff] <=> pci@[03fff]
>>   memc0-b@[1...13fff] <=> pci@[ 40007fff]
>>   memc1-a@[ 40007fff] <=> pci@[ 8000bfff]
>>   memc1-b@[3...33fff] <=> pci@[ c000]
>>   memc2-a@[ 8000bfff] <=> pci@[1...13fff]
>>   memc2-b@[c...c3fff] <=> pci@[14000...17fff]
>
> I presume the first column is the 'cpu physical address'
> and the second the 'pci' address?
>
Yes.  I probably made this more difficult to read because I ordered
the rows by PCI addresses.

> ...
>
> Isn't this just the same as having an iommu that converts 'bus'
> addresses into 'physical' ones?

Pretty much, but for PCIe devices only.  This could be done by somehow
overriding the arch specific phys_to_dma() and dma_to_phys() calls.

>
> A simple table lookup of the high address bits will do the
> conversion.
True, but this table could be passed something like ARM_MAPPING_ERROR,
which may be out the table (the driver is not privy to
ARM_MAPPING_ERROR's definition).

Thanks,
Jim

>
> David
>


RE: [PATCH 4/8] PCI: host: brcmstb: add dma-ranges for inbound traffic

2017-10-25 Thread David Laight
From: Jim Quinlan
> Sent: 24 October 2017 19:16
> The Broadcom STB PCIe host controller is intimately related to the
> memory subsystem.  This close relationship adds complexity to how cpu
> system memory is mapped to PCIe memory.  Ideally, this mapping is an
> identity mapping, or an identity mapping off by a constant.  Not so in
> this case.
> 
> Consider the Broadcom reference board BCM97445LCC_4X8 which has 6 GB
> of system memory.  Here is how the PCIe controller maps the
> system memory to PCIe memory:
> 
>   memc0-a@[03fff] <=> pci@[03fff]
>   memc0-b@[1...13fff] <=> pci@[ 40007fff]
>   memc1-a@[ 40007fff] <=> pci@[ 8000bfff]
>   memc1-b@[3...33fff] <=> pci@[ c000]
>   memc2-a@[ 8000bfff] <=> pci@[1...13fff]
>   memc2-b@[c...c3fff] <=> pci@[14000...17fff]

I presume the first column is the 'cpu physical address'
and the second the 'pci' address?

...

Isn't this just the same as having an iommu that converts 'bus'
addresses into 'physical' ones?

A simple table lookup of the high address bits will do the
conversion.

David



[PATCH 4/8] PCI: host: brcmstb: add dma-ranges for inbound traffic

2017-10-24 Thread Jim Quinlan
The Broadcom STB PCIe host controller is intimately related to the
memory subsystem.  This close relationship adds complexity to how cpu
system memory is mapped to PCIe memory.  Ideally, this mapping is an
identity mapping, or an identity mapping off by a constant.  Not so in
this case.

Consider the Broadcom reference board BCM97445LCC_4X8 which has 6 GB
of system memory.  Here is how the PCIe controller maps the
system memory to PCIe memory:

  memc0-a@[03fff] <=> pci@[03fff]
  memc0-b@[1...13fff] <=> pci@[ 40007fff]
  memc1-a@[ 40007fff] <=> pci@[ 8000bfff]
  memc1-b@[3...33fff] <=> pci@[ c000]
  memc2-a@[ 8000bfff] <=> pci@[1...13fff]
  memc2-b@[c...c3fff] <=> pci@[14000...17fff]

Although there are some "gaps" that can be added between the
individual mappings by software, the permutation of memory regions for
the most part is fixed by HW.  The solution of having something close
to an identity mapping is not possible.

The idea behind this HW design is that the same PCIe module can
act as an RC or EP, and if it acts as an EP it concatenates all
of system memory into a BAR so anything can be accessed.  Unfortunately,
when the PCIe block is in the role of an RC it also presents this
"BAR" to downstream PCIe devices, rather than offering an identity map
between its system memory and PCIe space.

Suppose that an endpoint driver allocs some DMA memory.  Suppose this
memory is located at 0x6000_, which is in the middle of memc1-a.
The driver wants a dma_addr_t value that it can pass on to the EP to
use.  Without doing any custom mapping, the EP will use this value for
DMA: the driver will get a dma_addr_t equal to 0x6000_.  But this
won't work; the device needs a dma_addr_t that reflects the PCIe space
address, namely 0xa000_.

So, essentially the solution to this problem must modify the
dma_addr_t returned by the DMA routines routines.  There are two
ways (I know of) of doing this:

(a) overriding/redefining the dma_to_phys() and phys_to_dma() calls
that are used by the dma_ops routines.  This is the approach of

arch/mips/cavium-octeon/dma-octeon.c

In ARM and ARM64 these two routines are defiend in asm/dma-mapping.h
as static inline functions.

(b) Subscribe to a notifier that notifies when a device is added to a
bus.  When this happens, set_dma_ops() can be called for the device.
This method is mentioned in:

http://lxr.free-electrons.com/source/drivers/of/platform.c?v=3.16#L152

where it says as a comment

"In case if platform code need to use own special DMA
configuration, it can use Platform bus notifier and
handle BUS_NOTIFY_ADD_DEVICE event to fix up DMA
configuration."

Solution (b) is what this commit does.  It uses its own set of
dma_ops which are wrappers around the arch_dma_ops.  The
wrappers translate the dma addresses before/after invoking
the arch_dma_ops, as appropriate.

Signed-off-by: Jim Quinlan 
---
 drivers/pci/host/Makefile  |   3 +-
 drivers/pci/host/pci-brcmstb-dma.c | 317 +
 drivers/pci/host/pci-brcmstb.c | 137 ++--
 drivers/pci/host/pci-brcmstb.h |   6 +
 4 files changed, 449 insertions(+), 14 deletions(-)
 create mode 100644 drivers/pci/host/pci-brcmstb-dma.c

diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 4398d2c..c283321 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -21,7 +21,8 @@ obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
 obj-$(CONFIG_PCIE_MEDIATEK) += pcie-mediatek.o
 obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-tango.o
 obj-$(CONFIG_VMD) += vmd.o
-obj-$(CONFIG_PCI_BRCMSTB) += pci-brcmstb.o
+obj-$(CONFIG_PCI_BRCMSTB) += brcmstb-pci.o
+brcmstb-pci-objs := pci-brcmstb.o pci-brcmstb-dma.o
 
 # The following drivers are for devices that use the generic ACPI
 # pci_root.c driver but don't support standard ECAM config access.
diff --git a/drivers/pci/host/pci-brcmstb-dma.c 
b/drivers/pci/host/pci-brcmstb-dma.c
new file mode 100644
index 000..02c4061
--- /dev/null
+++ b/drivers/pci/host/pci-brcmstb-dma.c
@@ -0,0 +1,317 @@
+/*
+ * Copyright (C) 2015-2017 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "pci-brcmstb.h"
+
+static const struct dma_map_ops *arch_dma_ops;
+static const struct dma_map_ops *brcm_dma_ops_ptr;
+
+static dma_addr_t brcm_to_