Re: [PATCH v2 1/6] dt-bindings: iommu: Add binding for mediatek IOMMU

[Yong Wu]

Hi Tomasz,
Thanks very much for your suggestion!.
please help check my comment.
On Mon, 2015-05-25 at 15:31 +0900, Tomasz Figa wrote:
> Hi,
> 
> Please see my comments inline.
> 
> On Fri, May 15, 2015 at 6:43 PM, Yong Wu  wrote:
> > This patch add mediatek iommu dts binding document.
> >
> > Signed-off-by: Yong Wu 
> > ---
> >  .../devicetree/bindings/iommu/mediatek,iommu.txt   |  51 ++
> >  include/dt-bindings/iommu/mt8173-iommu-port.h  | 112 
> > +
> >  2 files changed, 163 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> >  create mode 100644 include/dt-bindings/iommu/mt8173-iommu-port.h
> >
> > diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt 
> > b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> > new file mode 100644
> > index 000..f2cc7c0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> > @@ -0,0 +1,51 @@
> > +/**/
> > +/*Mediatek IOMMU Hardware Block Diagram   */
> > +/**/
> 
> nit: Could you follow one of existing styles of DT binding
> documentation? You might be able to use [1] as an example.
> 
> [1] 
> http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/iommu/arm,smmu.txt
How about below:

* Mediatek IOMMU Architecture Implementation

  Mediatek Socs may contain a implementation of Multimedia Memory
Management Unit(M4U),which use ARM Short-descriptor translation table
to achieve address translation.
  
  The IOMMU Hardware Block Diagram, please check below:
> .
> 
> > +  EMI (External Memory Interface)
> > +   |
> > +  m4u (Multimedia Memory Management Unit)
> > +   |
> > +  smi (Smart Multimedia Interface)
> > +   |
> > ++---+---
> > +|   |
> > +|   |
> > +vdec larb   disp larb  ... SoCs have different smi local 
> > arbiter(larb).
> > +|   |
> > +|   |
> > +   ++++-+-+
> > +   |||| | |...
> > +   |||| | |...
> > +   |||| | |...
> > +  MC   PP   VLD  OVL0 RDMA0 WDMA0  ... There are different ports in each 
> > larb.
> > +
> 
> This diagram is still quite meaningless without proper description of
> all the functionality off all the blocks and interfaces between them.
How about it if like this:
 //
As above, The Multimedia HW will go through SMI and M4U while it
access EMI. SMI is a brige between m4u and the Multimedia HW. It contain
smi local arbiter and smi common. It will control whether the Multimedia
HW should go though the m4u for translation or bypass it and talk
directly with EMI.And also SMI will help control the clocks for each
local arbiter.
About smi local arbiter(larb), Normally we specify a local
arbiter(larb) for each multimedia HW like display, video decode, and
camera. And there are different ports in each larb. Take a example,
There are some ports like MC, PP, VLD, AVC_MV, PRED_RD, PRED_WR in the
video local arbiter, all the ports are according to the video HW.
  //==
> 
> > +Required properties:
> > +- compatible : must be "mediatek,mt8173-m4u".
> > +- reg : m4u register base and size.
> > +- interrupts : the interrupt of m4u.
> > +- clocks : must contain one entry for each clock-names.
> > +- clock-names : must be "bclk", It is the block clock of m4u.
> > +- larb-portes-nr : must contain the number of the portes for each 
> > larb(local
> > +   arbiter). The number is defined in 
> > dt-binding/iommu/mt8173-iommu-port.h.
> 
> typo: Should it be "ports" instead of "portes"?
> 
> Anyway, shouldn't this be a property of larb nodes? I.e. the larb0
> node would have a port-count property set to M4U_LARB0_PORT_NR.
> 
> > +- larb : must contain the local arbiters of the current platform. Refer to
> > +   bindings/soc/mediatek/mediatek,smi.txt. It must sort according to 
> > the
> > +   local arbiter index, like larb0, larb1, larb2...
> > +- iommu-cells : must be 1. Specifies the client PortID as defined in
> > +   dt-binding/iommu/mt8173-iommu-port.h
> 
> Looking at the driver, wouldn't a 2 cell system be more appropriate
> here? With first cell indexing the larbs and second the ports within
> that larb.
Thanks very much. This is very helpful!
if we use 2, we can enter smi driver to enable/disable iommu easily. and
the code will be easier to read.

Then How about the descriptor if I write like this:

//
iommu-cells : must be 2.There are 2 cell needed to enable/disable iommu.
The first is local arbiter index(larbid), and the second is port
index(portid) within local arbiter. Specifies the larbid and portid as
defined in dt-binding/iommu/mt8173-iommu-port.h

[PATCH] iommu/amd: Handle integer overflow in dma_ops_area_alloc

[Joerg Roedel]

From: Joerg Roedel 

Handle this case to make sure boundary_size does not become
0 and trigger a BUG_ON later.

Signed-off-by: Joerg Roedel 
---
 drivers/iommu/amd_iommu.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index e43d489..f2d12c0 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1704,14 +1704,16 @@ static unsigned long dma_ops_area_alloc(struct device 
*dev,
unsigned long next_bit = dom->next_address % APERTURE_RANGE_SIZE;
int max_index = dom->aperture_size >> APERTURE_RANGE_SHIFT;
int i = start >> APERTURE_RANGE_SHIFT;
-   unsigned long boundary_size;
+   unsigned long boundary_size, mask;
unsigned long address = -1;
unsigned long limit;
 
next_bit >>= PAGE_SHIFT;
 
-   boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
-   PAGE_SIZE) >> PAGE_SHIFT;
+   mask = dma_get_seg_boundary(dev);
+
+   boundary_size = mask + 1 ? ALIGN(mask + 1, PAGE_SIZE) >> PAGE_SHIFT :
+  1UL << (BITS_PER_LONG - PAGE_SHIFT);
 
for (;i < max_index; ++i) {
unsigned long offset = dom->aperture[i]->offset >> PAGE_SHIFT;
-- 
1.9.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 5/6] iommu/mediatek: Add mt8173 IOMMU driver

[Yong Wu]

Hi Tomasz,
Thanks. please help check my comments.
The others I will change in next version.

On Mon, 2015-05-25 at 17:29 +0900, Tomasz Figa wrote:
> Hi,
> 
> Please see my comments inline.
> 
> On Fri, May 15, 2015 at 6:43 PM, Yong Wu  wrote:
> [snip]
> > +
> > +struct mtk_iommu_info {
> > +   void __iomem*base;
> > +   int irq;
> > +   struct device   *dev;
> > +   struct device   *larbdev[MTK_IOMMU_LARB_MAX_NR];
> > +   struct clk  *bclk;
> > +   dma_addr_t  protect_base; /* protect memory base */
> > +   unsigned intlarb_nr;  /* local arbiter number */
> > +   unsigned intlarb_portid_base[MTK_IOMMU_LARB_MAX_NR];
> > +   /* the port id base for each local arbiter 
> > */
> 
> It might not be a bad idea to convert this kind of comments into a
> proper kerneldoc comment for the whole struct. Similarly for other
> structs in the driver.
Yes. I will delete larb_portid_base if iommu-cells is 2.
> 
> [snip]
> 
> > +static void mtk_iommu_clear_intr(void __iomem *m4u_base)
> 
> nit: Instead of pasing m4u_base here, could you pass a pointer to the
> mtk_iommu_info here and use its base field? This would be more strict,
> because currently a void pointer permits passing anything to this
> function.
> 
> > +{
> > +   u32 val;
> > +
> > +   val = readl(m4u_base + REG_MMU_INT_CONTROL0);
> > +   val |= F_INT_L2_CLR_BIT;
> > +   writel(val, m4u_base + REG_MMU_INT_CONTROL0);
> > +}
> > +
> > +static void mtk_iommu_tlb_flush_all(void *cookie)
> > +{
> > +   struct mtk_iommu_domain *domain = cookie;
> > +   u32 val;
> > +
> > +   val = F_INVLD_EN1 | F_INVLD_EN0;
> > +   writel(val, domain->imuinfo->base + REG_MMU_INV_SEL);
> 
> nit: Do you need this val variable?
> 
> > +   writel(F_ALL_INVLD, domain->imuinfo->base + REG_MMU_INVALIDATE);
> > +}
> > +
> > +static void mtk_iommu_tlb_add_flush(unsigned long iova, size_t size,
> > +   bool leaf, void *cookie)
> > +{
> > +   struct mtk_iommu_domain *domain = cookie;
> > +   void __iomem *m4u_base = domain->imuinfo->base;
> > +   unsigned int iova_start = iova, iova_end = iova + size - 1;
> > +   int ret;
> > +   u32 val;
> > +
> > +   val = F_INVLD_EN1 | F_INVLD_EN0;
> > +   writel(val, m4u_base + REG_MMU_INV_SEL);
> 
> nit: Does this write need to go through this val variable?
> 
> > +
> > +   writel(iova_start, m4u_base + REG_MMU_INVLD_START_A);
> > +   writel(iova_end, m4u_base + REG_MMU_INVLD_END_A);
> > +   writel(F_MMU_INV_RANGE, m4u_base + REG_MMU_INVALIDATE);
> > +
> > +   ret = readl_poll_timeout_atomic(m4u_base + REG_MMU_CPE_DONE, val,
> > +   val != 0, 10, 100);
> > +   if (ret) {
> > +   dev_warn(domain->imuinfo->dev, "Invalid tlb don't done\n");
> 
> Maybe "Partial TLB flush timed out, falling back to full flush\n"?
> 
> > +   mtk_iommu_tlb_flush_all(cookie);
> > +   }
> > +   writel(0, m4u_base + REG_MMU_CPE_DONE);
> > +}
> > +
> > +static void mtk_iommu_flush_pgtable(void *ptr, size_t size, void *cookie)
> > +{
> > +   /*
> > +* After delete arch_setup_dma_ops,
> > +* This will be replaced with dma_map_page
> > +*/
> > +__dma_flush_range(ptr, ptr + size);
> > +}
> > +
> > +static struct iommu_gather_ops mtk_iommu_gather_ops = {
> > +   .tlb_flush_all = mtk_iommu_tlb_flush_all,
> > +   .tlb_add_flush = mtk_iommu_tlb_add_flush,
> > +   .tlb_sync = mtk_iommu_tlb_flush_all,
> > +   .flush_pgtable = mtk_iommu_flush_pgtable,
> > +};
> > +
> > +static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
> > +{
> > +   struct mtk_iommu_domain *mtkdomain = dev_id;
> > +   struct mtk_iommu_info *piommu = mtkdomain->imuinfo;
> > +   struct device *dev = piommu->dev;
> > +   void __iomem *m4u_base = piommu->base;
> > +   u32 int_state, regval, fault_iova, fault_pa;
> > +   unsigned int fault_larb, fault_port;
> > +   bool layer, write;
> > +
> > +   int_state = readl(m4u_base + REG_MMU_FAULT_ST1);
> > +
> > +   /* read error info from registers */
> > +   fault_iova = readl(m4u_base + REG_MMU_FAULT_VA);
> > +   layer = fault_iova & F_MMU_FAULT_VA_LAYER_BIT;
> > +   write = fault_iova & F_MMU_FAULT_VA_WRITE_BIT;
> > +   fault_iova &= F_MMU_FAULT_VA_MSK;
> > +   fault_pa = readl(m4u_base + REG_MMU_INVLD_PA);
> > +   regval = readl(m4u_base + REG_MMU_INT_ID);
> > +   fault_larb = F_MMU0_INT_ID_LARB_ID(regval);
> > +   fault_port = F_MMU0_INT_ID_PORT_ID(regval);
> > +
> > +   report_iommu_fault(&mtkdomain->domain, dev, fault_iova,
> > +  write ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ);
> > +
> > +   if (int_state & F_INT_TRANSLATION_FAULT) {
> > +   dev_err_ratelimited(
> >

Re: [PATCH v2 2/6] dt-bindings: mediatek: Add smi dts binding

[Yong Wu]

Hi Tomasz,

On Mon, 2015-05-25 at 15:48 +0900, Tomasz Figa wrote:
> Hi,
> 
> Please see my comments inline.
> 
> On Fri, May 15, 2015 at 6:43 PM, Yong Wu  wrote:
> > This patch add smi binding document.
> >
> > Signed-off-by: Yong Wu 
> > ---
> >  .../bindings/soc/mediatek/mediatek,smi-larb.txt| 24 
> > ++
> >  .../bindings/soc/mediatek/mediatek,smi.txt | 22 
> > 
> >  2 files changed, 46 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt
> >  create mode 100644 
> > Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt
> >
> > diff --git 
> > a/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt 
> > b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt
> > new file mode 100644
> > index 000..c06c5b6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt
> > @@ -0,0 +1,24 @@
> > +SMI(Smart Multimedia Interface) Local Arbiter
> > +
> > +The hardware block diagram please check bindings/iommu/mediatek,iommu.txt
> > +
> > +Required properties:
> > +- compatible : must be "mediatek,mt8173-smi-larb"
> > +- reg : the register and size of each local arbiter.
> 
> Shouldn't it be "of this local arbiter"?
> 
> > +- smi : must be "&smi_common". Refer to bindings/soc/mediatek,smi.txt.
> 
> This is incorrect. Device tree source authors are free to use any
> labels for their nodes and documentation should not rely on the fact
> that there is some node with some particular label. This should be
> something like:
> 
> - smi : a phandle to node of XYZ
> 
> with proper description of that node in place of XYZ.
Thanks. how about this?
smi : a phandle to the smi_common node 
> 
> > +- clocks : must contain one entry for each clock-names.
> > +   There are 2 clockes:
> > +   APB clock : Advanced Peripheral Bus clock, It's the clock for 
> > setting
> > +   the register.
> > +   SMI clock : It's the clock for transfer data and command.
> 
> The description of each clock should go to "clock-names" property.
> Please use some of already existing bindings as an example.
> 
> > +- clock-name: must be "apb" and "smi".
> 
> typo: Should be "clock-names".
> 
> I'd recommend describing this property as below:
> 
> - clock-names: must contain 2 entries, as follows:
>   - "apb" : ,
>   - "smi" : .
Thanks. I will follow this.

> > +
> > +Example:
> > +   larb0:larb@14021000 {
> > +   compatible = "mediatek,mt8173-smi-larb";
> > +   reg = <0 0x14021000 0 0x1000>;
> > +   smi = <&smi_common>;
> > +   clocks = <&mmsys MM_SMI_LARB0>,
> > +<&mmsys MM_SMI_LARB0>;
> > +   clock-names = "apb", "smi";
And another question,
Could I add a item(port-count) in larb node. like:
  port-count = M4U_PORT_LARB0_NR;
This item will only check the portid in client dsti is right or not?
   iommu = <&iommu larbid portid>

Then, the port-count is necessary to add? or we could assume all the
input is correct.

> > +   };
> > diff --git 
> > a/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt 
> > b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt
> > new file mode 100644
> > index 000..c2389b4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt
> > @@ -0,0 +1,22 @@
> > +SMI(Smart Multimedia Interface)
> > +
> > +The hardware block diagram please check bindings/iommu/mediatek,iommu.txt
> > +
> > +Required properties:
> > +- compatible : must be "mediatek,mt8173-smi"
> > +- reg : the register and size of the smi.
> 
> nit: s/smi/SMI block/
> 
> > +- clocks : must contain one entry for each clock-names.
> > +   There are 2 clockes:
> > +   APB clock : Advanced Peripheral Bus clock, It's the clock for 
> > setting
> > +   the register.
> > +   SMI clock : It's the clock for transfer data and command.
> > +- clock-name: must be "apb" and "smi".
> 
> See my comment for those properties in the first binding.
> 
> Best regards,
> Tomasz


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/3] iommu/arm-smmu: Add initial driver support for ARM SMMUv3 devices

[leizhen]

On 2015/5/27 0:12, Will Deacon wrote:
> On Mon, May 25, 2015 at 03:07:17AM +0100, leizhen wrote:
>> On 2015/5/21 19:25, Will Deacon wrote:
>>> On Wed, May 13, 2015 at 09:33:19AM +0100, leizhen wrote:
 If SMMU_IDR1.SIDSIZE = 32 really exist(or too big), we need dynamic choose
 Lv2 table size(4K,16K,64K).  Because Lv1 table maybe too big, and can not
 be allocated by current API, a dts configuration should be added, like
 lv1-table-base = <0x0 0x0>, and we use ioremap_cache get VA(maybe ioremap,
 for non-coherent SMMU).

 Oh, I can do it after your patches upstreamed, because this problem maybe
 only I met.
>>>
>>> I'll have a think about this and see what I can come up with for version
>>> 2 of the patch. I'd like to avoid adding additional properties to the DT
>>> until they're actually needed, though.
>>
>> OK. Will you support non-pci devices in patch version 2?
> 
> I don't (yet) plan to support non-PCI devices, for two reasons:
> 
>   (1) The support should be based on top of Laurent's RFC series here:
> 
>   http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/343686.html
> 
>   which needs some review etc.
> 
>   (2) My simulator only has PCI endpoints
> 
> Of course, if you have something that you can test with, then I'm more
> than happy to review patches adding this support providing that they're
> based on the series above.

OK. I'm so glad to do it.

> 
> On the table front, my current v2 patch uses 16k level-2 tables allocated
> lazily (so each one has 256 entries and covers a single PCI bus). I've
> also capped the SIDSIZE to 21 for the moment, which means we get a 64k
> table there.
> 
> Sound reasonable for the time being?

OK. My SIDSIZE is 25, I will also do this.

> 
> Will
> 
> .
> 


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC/PATCH 0/9] IOMMU probe deferral support

[Marek Szyprowski]

Hello,

On 2015-05-15 01:00, Laurent Pinchart wrote:

This patch series attempts to implement support for deferring probe of both
IOMMU drivers and bus master drivers.

The relationship between bus masters and IOMMUs creates a strong ordering
during initialization of devices. As in the general case IOMMUs are hidden
behind the DMA mapping API, IOMMU support relies on the automatic setup of DMA
operations without any direct intervention of bus master drivers.

DMA operations are set up when platform devices are added to the system. This
requires IOMMUs to be available at that time. On systems where ordering of
device add and probe can't be guaranteed (such as, but not limited to,
DT-based systems) this caused incorrect DMA operation setup. This has been
addressed by a patches series [1] that introduced a DT-based early
registration mechanism for IOMMUs.

However, that mechanism fails to address all issues. Various dependencies
exist between IOMMU devices and other devices, in particular on clocks and on
power domains (as mentioned by Marek in [2]). While there are mechanisms to
handle some of them without probe deferral (for instance by using the
OF_DECLARE macros to register clock drivers), generalizing those mechanisms
would essentially recreate a probe ordering mechanism similar to link order
probe ordering and couldn't really scale.

Additionally, IOMMUs could also be present hot-pluggable devices and depend on
resources that are thus hot-plugged. OF_DECLARE wouldn't help in that case.
For all those reasons probe deferral for IOMMUs has been considered as desired
if it can be implemented cleanly. For more in-depth information see [3].

This RFC series is a first attempt at implementing IOMMU probe deferral
support cleanly.

The core idea is to move setup of DMA operations from device add time to
device probe time, implemented in patch 6/9. It could be possible to move
setup of other DMA parameters (namely masks and offset) to probe time as well,
but that change would be more intrusive and has a higher risk of introducing
regressions. For that reason I've decided to keep DMA masks and offset setup
at device add time and thus split DMA configuration in masks and operations
(patch 5/9). This can be revisited if we decide that the DMA mapping API
shouldn't require masks and offset to be set before probe time.

Patch 8/9 then defers probe of bus master drivers when required IOMMUs are not
available yet. This requires knowing when a failed IOMMU lookup should be
considered as permanent or temporary. I've reused the OF_DECLARE_IOMMU for
this purpose, considering that the presence of a driver compatible with the
IOMMU DT node indicates that the failure is temporary and probing of the bus
master device should be deferred.

Note that only IOMMU drivers using the recent .of_xlate() mechanism for
DT-based IOMMU reference can cause probe deferral of bus master devices. The
.add_device() mechanism isn't supported in this case.

As an example I've converted the ipmmu-vmsa driver to the new API in patch 9/9.

At this point many enhancements are possible, but I'd like to receive feedback
on the proposed approach before basing more patches on this series. One
particular point I would like to address (or see being addressed) in the
future is the use of struct iommu_ops with of_iommu_get_ops() and
of_iommu_set_ops(). I believe we should introduct a struct iommu and register
IOMMU instances instead of IOMMU operations. That should bring us one step
closer to removing bus_set_iommu().


I've checked this patchset with my recent updates to Exynos IOMMU driver. It
works fine. Then I removed my exynos_iommu_of_setup() callback and 
relied only
on proper initialization from platform device probe() of all iommu 
controllers.

The driver also worked fine in such case.

The other change I had to make to get everything working was removal of some
hackery in dma-mapping internal structures in Exynos DRM driver, but this is
completely different story.

You can add the following tag for the of/iommu/dma-mapping patches:
Tested-by: Marek Szyprowski 


[1] http://www.spinics.net/lists/arm-kernel/msg382787.html
[2] 
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/323238.html
[3] https://lkml.org/lkml/2015/2/16/345

Laurent Pinchart (9):
   arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops()
   arm: dma-mapping: Support IOMMU mappings spanning the full 32 bits
 range
   of: dma: Move range size workaround to of_dma_get_range()
   of: dma: Make of_dma_deconfigure() public
   of: dma: Split of_configure_dma() into mask and ops configuration
   drivers: platform: Configure dma operations at probe time
   iommu: of: Document the of_iommu_configure() function
   iommu: of: Handle IOMMU lookup failure with deferred probing or error
   iommu/ipmmu-vmsa: Use DT-based instantiation

  arch/arm/include/asm/dma-iommu.h |   2 +-
  arch/arm/mm/dma-mapping.c|  21 +++--
  drivers/base/platform.c  |   

[PATCH 1/4] iommu/iova: Avoid over-allocating when size-aligned

[Robin Murphy]

Currently, allocating a size-aligned IOVA region quietly adjusts the
actual allocation size in the process, returning a rounded-up
power-of-two-sized allocation. This results in mismatched behaviour in
the IOMMU driver if the original size was not a power of two, where the
original size is mapped, but the rounded-up IOVA size is unmapped.

Whilst some IOMMUs will happily unmap already-unmapped pages, others
consider this an error, so fix it by computing the necessary alignment
padding without altering the actual allocation size. Also clean up by
making pad_size unsigned, since negative padding makes no sense.

CC: David Woodhouse 
Signed-off-by: Robin Murphy 
---
 drivers/iommu/intel-iommu.c |  2 ++
 drivers/iommu/iova.c| 23 ++-
 2 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 68d43be..ada1fb9 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2910,6 +2910,8 @@ static struct iova *intel_alloc_iova(struct device *dev,
 
/* Restrict dma_mask to the width that the iommu can handle */
dma_mask = min_t(uint64_t, DOMAIN_MAX_ADDR(domain->gaw), dma_mask);
+   /* Ensure we reserve the whole size-aligned region */
+   nrpages = __roundup_pow_of_two(nrpages);
 
if (!dmar_forcedac && dma_mask > DMA_BIT_MASK(32)) {
/*
diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index 9dd8208..2c4a51b 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -120,19 +120,14 @@ __cached_rbnode_delete_update(struct iova_domain *iovad, 
struct iova *free)
}
 }
 
-/* Computes the padding size required, to make the
- * the start address naturally aligned on its size
+/*
+ * Computes the padding size required, to make the start address
+ * naturally aligned on the power-of-two order of its size
  */
-static int
-iova_get_pad_size(int size, unsigned int limit_pfn)
+static unsigned int
+iova_get_pad_size(unsigned int size, unsigned int limit_pfn)
 {
-   unsigned int pad_size = 0;
-   unsigned int order = ilog2(size);
-
-   if (order)
-   pad_size = (limit_pfn + 1) % (1 << order);
-
-   return pad_size;
+   return (limit_pfn + 1 - size) & (__roundup_pow_of_two(size) - 1);
 }
 
 static int __alloc_and_insert_iova_range(struct iova_domain *iovad,
@@ -264,12 +259,6 @@ alloc_iova(struct iova_domain *iovad, unsigned long size,
if (!new_iova)
return NULL;
 
-   /* If size aligned is set then round the size to
-* to next power of two.
-*/
-   if (size_aligned)
-   size = __roundup_pow_of_two(size);
-
ret = __alloc_and_insert_iova_range(iovad, size, limit_pfn,
new_iova, size_aligned);
 
-- 
1.9.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 2/4] iommu: Implement common IOMMU ops for DMA mapping

[Robin Murphy]

Taking inspiration from the existing arch/arm code, break out some
generic functions to interface the DMA-API to the IOMMU-API. This will
do the bulk of the heavy lifting for IOMMU-backed dma-mapping.

Signed-off-by: Robin Murphy 
---
 drivers/iommu/Kconfig |   7 +
 drivers/iommu/Makefile|   1 +
 drivers/iommu/dma-iommu.c | 560 ++
 include/linux/dma-iommu.h |  94 
 4 files changed, 662 insertions(+)
 create mode 100644 drivers/iommu/dma-iommu.c
 create mode 100644 include/linux/dma-iommu.h

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 1ae4e54..9107b6e 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -48,6 +48,13 @@ config OF_IOMMU
def_bool y
depends on OF && IOMMU_API
 
+# IOMMU-agnostic DMA-mapping layer
+config IOMMU_DMA
+   bool
+   depends on NEED_SG_DMA_LENGTH
+   select IOMMU_API
+   select IOMMU_IOVA
+
 config FSL_PAMU
bool "Freescale IOMMU support"
depends on PPC32
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 080ffab..574b241 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -1,6 +1,7 @@
 obj-$(CONFIG_IOMMU_API) += iommu.o
 obj-$(CONFIG_IOMMU_API) += iommu-traces.o
 obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
+obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
 obj-$(CONFIG_IOMMU_IOVA) += iova.o
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
new file mode 100644
index 000..5b2b065
--- /dev/null
+++ b/drivers/iommu/dma-iommu.c
@@ -0,0 +1,560 @@
+/*
+ * A fairly generic DMA-API to IOMMU-API glue layer.
+ *
+ * Copyright (C) 2014-2015 ARM Ltd.
+ *
+ * based in part on arch/arm/mm/dma-mapping.c:
+ * Copyright (C) 2000-2004 Russell King
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#define pr_fmt(fmt)"%s: " fmt, __func__
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int iommu_dma_init(void)
+{
+   return iommu_iova_cache_init();
+}
+
+struct iommu_dma_domain {
+   struct iommu_domain *domain;
+   struct iova_domain *iovad;
+   struct kref kref;
+};
+
+/**
+ * iommu_dma_create_domain - Create a DMA mapping domain
+ * @ops: iommu_ops representing the IOMMU backing this domain. It is down to
+ *   the IOMMU driver whether a domain may span multiple IOMMU instances
+ * @base: IOVA at which the mappable address space starts
+ * @size: Size of IOVA space
+ *
+ * @base and @size should be exact multiples of IOMMU page granularity to
+ * avoid rounding surprises. If necessary, we reserve the page at address 0
+ * to ensure it is an invalid IOVA.
+ *
+ * Return: Pointer to a domain initialised with the given IOVA range,
+ * or NULL on failure. If successful, the caller holds an initial
+ * reference, which may be released with iommu_dma_release_domain()
+ * once a device is attached.
+ */
+struct iommu_dma_domain *iommu_dma_create_domain(const struct iommu_ops *ops,
+   dma_addr_t base, u64 size)
+{
+   struct iommu_dma_domain *dom;
+   struct iommu_domain *domain;
+   struct iova_domain *iovad;
+   unsigned long order, base_pfn, end_pfn;
+
+   dom = kzalloc(sizeof(*dom), GFP_KERNEL);
+   if (!dom)
+   return NULL;
+   /*
+* HACK(sort of): These domains currently belong to this layer and are
+* opaque from outside it, so they are "unmanaged" by the IOMMU API
+* itself. Once we have default domain support worked out, then we can
+* turn things inside out and put these inside managed IOMMU domains...
+*/
+   domain = ops->domain_alloc(IOMMU_DOMAIN_UNMANAGED);
+   if (!domain)
+   goto out_free_dma_domain;
+
+   domain->ops = ops;
+   domain->type = IOMMU_DOMAIN_UNMANAGED;
+
+   /* Use the smallest supported page size for IOVA granularity */
+   order = __ffs(ops->pgsize_bitmap);
+   base_pfn = max_t(unsigned long, 1, base >> order);
+   end_pfn = (base + size - 1) >> order;
+
+   /* Check the domain allows at least some access to the device... */
+   if (domain->geometry.force_aperture) {
+   if (base > domain->geometry.aperture_end ||
+   base + size <= domain->geometry.aperture_start) {
+   pr_warn("specified DM

[PATCH 3/4] arm64: Add IOMMU dma_ops

[Robin Murphy]

Taking some inspiration from the arch/arm code, implement the
arch-specific side of the DMA mapping ops using the new IOMMU-DMA layer.

Whilst proliferating per-device private IOMMU data via dev->archdata is
less than ideal, it will do the job for now, especially since we can't
easily handle the kind of problematic system topologies in the current
IOMMU API anyway.

Signed-off-by: Robin Murphy 
---
 arch/arm64/include/asm/device.h  |   3 +
 arch/arm64/include/asm/dma-mapping.h |  14 ++
 arch/arm64/mm/dma-mapping.c  | 342 +++
 include/linux/dma-iommu.h|   4 +-
 4 files changed, 361 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h
index 243ef25..510cee1 100644
--- a/arch/arm64/include/asm/device.h
+++ b/arch/arm64/include/asm/device.h
@@ -20,6 +20,9 @@ struct dev_archdata {
struct dma_map_ops *dma_ops;
 #ifdef CONFIG_IOMMU_API
void *iommu;/* private IOMMU data */
+#ifdef CONFIG_IOMMU_DMA
+   struct iommu_dma_domain *dma_domain;
+#endif
 #endif
bool dma_coherent;
 };
diff --git a/arch/arm64/include/asm/dma-mapping.h 
b/arch/arm64/include/asm/dma-mapping.h
index 9437e3d..835a8d1 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -61,6 +61,20 @@ static inline bool is_device_dma_coherent(struct device *dev)
 }
 
 #include 
+#include 
+
+#ifdef CONFIG_IOMMU_DMA
+static inline struct iommu_dma_domain *arch_get_dma_domain(struct device *dev)
+{
+   return dev->archdata.dma_domain;
+}
+
+static inline void arch_set_dma_domain(struct device *dev,
+ struct iommu_dma_domain *dma_domain)
+{
+   dev->archdata.dma_domain = dma_domain;
+}
+#endif
 
 static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
 {
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index b0bd4e5..189477b 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -434,3 +434,345 @@ static int __init dma_debug_do_init(void)
return 0;
 }
 fs_initcall(dma_debug_do_init);
+
+
+#ifdef CONFIG_IOMMU_DMA
+#include 
+#include 
+#include 
+
+/* Thankfully, all cache ops are by VA so we can ignore phys here */
+static void flush_page(const void *virt, phys_addr_t phys)
+{
+   __dma_flush_range(virt, virt + PAGE_SIZE);
+}
+
+static void *__iommu_alloc_attrs(struct device *dev, size_t size,
+dma_addr_t *handle, gfp_t gfp,
+struct dma_attrs *attrs)
+{
+   bool coherent = is_device_dma_coherent(dev);
+   pgprot_t pgprot = coherent ? __pgprot(PROT_NORMAL) :
+__pgprot(PROT_NORMAL_NC);
+   int ioprot;
+   void *addr;
+
+   if (WARN(!dev, "cannot create IOMMU mapping for unknown device\n"))
+   return NULL;
+
+   if (!(gfp & __GFP_WAIT)) {
+   struct page *page;
+
+   addr = __alloc_from_pool(size, &page, gfp);
+   if (!addr)
+   return NULL;
+
+   ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, false);
+   *handle = iommu_dma_map_page(dev, page, 0, size, ioprot, 
coherent);
+   if (iommu_dma_mapping_error(dev, *handle)) {
+   __free_from_pool(addr, size);
+   addr = NULL;
+   }
+   } else {
+   struct page **pages;
+
+   ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
+   pages = iommu_dma_alloc(dev, size, gfp, ioprot, coherent,
+   handle, coherent ? NULL : flush_page);
+   if (!pages)
+   return NULL;
+
+   addr = dma_common_pages_remap(pages, size, VM_USERMAP,
+   __get_dma_pgprot(attrs, pgprot, coherent),
+   __builtin_return_address(0));
+   if (!addr)
+   iommu_dma_free(dev, pages, size, handle);
+   }
+   return addr;
+}
+
+static void __iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr,
+  dma_addr_t handle, struct dma_attrs *attrs)
+{
+   if (__free_from_pool(cpu_addr, size)) {
+   iommu_dma_unmap_page(dev, handle, size, 0, NULL);
+   } else {
+   struct vm_struct *area = find_vm_area(cpu_addr);
+
+   if (WARN_ON(!area || !area->pages))
+   return;
+   iommu_dma_free(dev, area->pages, size, &handle);
+   dma_common_free_remap(cpu_addr, size, VM_USERMAP);
+   }
+}
+
+static int __iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
+ void *cpu_addr, dma_addr_t dma_addr, size_t size,
+ struct dma_attrs *attrs)
+{
+   struct vm_struct *area;

[PATCH 0/4] arm64: IOMMU-backed DMA mapping

[Robin Murphy]

Hi all,

Here's the cleaned-up and nominally ready-to-go IOMMU DMA mapping series,
with (I think) all the outstanding comments[1] addressed. I appreciate
I've probably missed the boat for 4.2, but I'm ever the optimist...

Major changes since RFC:
- Renamed things, shuffled things around, and removed some of the less
  useful abstraction for simplicity and clarity.
- Fixed iommu_dma_alloc to handle compound pages and map buffers as
  scatterlists.
- Filled the API holes like size-aligned allocations and get_sgtable.
- Solved the problem of attching devices to domains before the devices
  are created, which confuses IOMMU drivers like the ARM SMMU.

A branch is available at:
   git://linux-arm.org/linux-rm iommu/dma

In terms of where we go from here, this is more or less what I envisage:
- Get the current functionality merged to unblock arm64 IOMMU work
- Implement proper managed IOMMU domains around this, integrating the
  iova_domain into the iommu_domain itself, and getting rid of the
  iommu_dma_domain wrapper.
- Find all of the ad-hoc "managed domain" implementations in drivers
  currently using the arm_iommu_* functions and convert them to "proper"
  managed domains.
- Finally, with the external users taken care of, bring arch/arm over
  to the common DMA mapping code.

In the meantime, Laurent's proposal for probe deferral[2] offers hope
that the bus notifier dance may only need to be short-lived, and I'm
hoping to spend some more time on DT-based IOMMU group handling for
platform devices, which should hopefully tie in with default domains
to make the add_device callback mostly redundant.

[1]:http://thread.gmane.org/gmane.linux.kernel.iommu/8773
[2]:http://thread.gmane.org/gmane.linux.kernel.iommu/9552

Robin Murphy (4):
  iommu/iova: Avoid over-allocating when size-aligned
  iommu: Implement common IOMMU ops for DMA mapping
  arm64: Add IOMMU dma_ops
  arm64: Hook up IOMMU dma_ops

 arch/arm64/Kconfig   |   1 +
 arch/arm64/include/asm/device.h  |   3 +
 arch/arm64/include/asm/dma-mapping.h |  25 +-
 arch/arm64/mm/dma-mapping.c  | 357 ++
 drivers/iommu/Kconfig|   7 +
 drivers/iommu/Makefile   |   1 +
 drivers/iommu/dma-iommu.c| 560 +++
 drivers/iommu/intel-iommu.c  |   2 +
 drivers/iommu/iova.c |  23 +-
 include/linux/dma-iommu.h|  94 ++
 10 files changed, 1051 insertions(+), 22 deletions(-)
 create mode 100644 drivers/iommu/dma-iommu.c
 create mode 100644 include/linux/dma-iommu.h

-- 
1.9.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 4/4] arm64: Hook up IOMMU dma_ops

[Robin Murphy]

With iommu_dma_ops in place, hook them up to the configuration code, so
IOMMU-fronted devices will get them automatically.

Signed-off-by: Robin Murphy 
---
 arch/arm64/Kconfig   |  1 +
 arch/arm64/include/asm/dma-mapping.h | 11 ++-
 arch/arm64/mm/dma-mapping.c  | 15 +++
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 7796af4..257cd7e 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -70,6 +70,7 @@ config ARM64
select HAVE_PERF_USER_STACK_DUMP
select HAVE_RCU_TABLE_FREE
select HAVE_SYSCALL_TRACEPOINTS
+   select IOMMU_DMA if IOMMU_SUPPORT
select IRQ_DOMAIN
select MODULES_USE_ELF_RELA
select NO_BOOTMEM
diff --git a/arch/arm64/include/asm/dma-mapping.h 
b/arch/arm64/include/asm/dma-mapping.h
index 835a8d1..a87b507 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -45,11 +45,8 @@ static inline struct dma_map_ops *get_dma_ops(struct device 
*dev)
return __generic_dma_ops(dev);
 }
 
-static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 
size,
- struct iommu_ops *iommu, bool coherent)
-{
-   dev->archdata.dma_coherent = coherent;
-}
+void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
+   struct iommu_ops *iommu, bool coherent);
 #define arch_setup_dma_ops arch_setup_dma_ops
 
 /* do not use this function in a driver */
@@ -64,6 +61,10 @@ static inline bool is_device_dma_coherent(struct device *dev)
 #include 
 
 #ifdef CONFIG_IOMMU_DMA
+
+void arch_teardown_dma_ops(struct device *dev);
+#define arch_teardown_dma_ops  arch_teardown_dma_ops
+
 static inline struct iommu_dma_domain *arch_get_dma_domain(struct device *dev)
 {
return dev->archdata.dma_domain;
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 189477b..f9d4184 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -769,6 +769,14 @@ static void __iommu_setup_dma_ops(struct device *dev, u64 
dma_base, u64 size,
mutex_unlock(&iommu_dma_notifier_lock);
 }
 
+void arch_teardown_dma_ops(struct device *dev)
+{
+   if (dev->archdata.dma_domain) {
+   iommu_dma_detach_device(dev);
+   dev->archdata.dma_ops = NULL;
+   }
+}
+
 #else
 
 static void __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
@@ -776,3 +784,10 @@ static void __iommu_setup_dma_ops(struct device *dev, u64 
dma_base, u64 size,
 { }
 
 #endif  /* CONFIG_IOMMU_DMA */
+
+void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
+   struct iommu_ops *iommu, bool coherent)
+{
+   dev->archdata.dma_coherent = coherent;
+   __iommu_setup_dma_ops(dev, dma_base, size, iommu);
+}
-- 
1.9.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 2/2] iommu/arm-smmu: Make force_stage module param read-only in sysfs

[Will Deacon]

Changing force_stage dynamically isn't supported by the driver and it
also doesn't make a whole lot of sense to change it once the SMMU is up
and running.

This patch makes the sysfs entry for the parameter read-only.

Signed-off-by: Will Deacon 
---
 drivers/iommu/arm-smmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index e2a788eea5ed..dce041b1c139 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -246,7 +246,7 @@
 #define FSYNR0_WNR (1 << 4)
 
 static int force_stage;
-module_param_named(force_stage, force_stage, int, S_IRUGO | S_IWUSR);
+module_param_named(force_stage, force_stage, int, S_IRUGO);
 MODULE_PARM_DESC(force_stage,
"Force SMMU mappings to be installed at a particular stage of 
translation. A value of '1' or '2' forces the corresponding stage. All other 
values are ignored (i.e. no stage is forced). Note that selecting a specific 
stage will disable support for nested translation.");
 
-- 
2.1.4

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 1/2] iommu/arm-smmu: Fix ATS1* register writes

[Will Deacon]

From: Robin Murphy 

The ATS1* address translation registers only support being written
atomically - in SMMUv2 where they are 64 bits wide, 32-bit writes to
the lower half are automatically zero-extended, whilst 32-bit writes
to the upper half are ignored. Thus, the current logic of performing
64-bit writes as two 32-bit accesses is wrong.

Since we already limit IOVAs to 32 bits on 32-bit ARM, the lack of a
suitable writeq() implementation there is not an issue, and we only
need a little preprocessor ugliness to safely hide the 64-bit case.

Signed-off-by: Robin Murphy 
Signed-off-by: Will Deacon 
---
 drivers/iommu/arm-smmu.c | 21 ++---
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 66a803b9dd3a..e2a788eea5ed 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -202,8 +202,7 @@
 #define ARM_SMMU_CB_S1_TLBIVAL 0x620
 #define ARM_SMMU_CB_S2_TLBIIPAS2   0x630
 #define ARM_SMMU_CB_S2_TLBIIPAS2L  0x638
-#define ARM_SMMU_CB_ATS1PR_LO  0x800
-#define ARM_SMMU_CB_ATS1PR_HI  0x804
+#define ARM_SMMU_CB_ATS1PR 0x800
 #define ARM_SMMU_CB_ATSR   0x8f0
 
 #define SCTLR_S1_ASIDPNE   (1 << 12)
@@ -1229,18 +1228,18 @@ static phys_addr_t arm_smmu_iova_to_phys_hard(struct 
iommu_domain *domain,
void __iomem *cb_base;
u32 tmp;
u64 phys;
+   unsigned long va;
 
cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
 
-   if (smmu->version == 1) {
-   u32 reg = iova & ~0xfff;
-   writel_relaxed(reg, cb_base + ARM_SMMU_CB_ATS1PR_LO);
-   } else {
-   u32 reg = iova & ~0xfff;
-   writel_relaxed(reg, cb_base + ARM_SMMU_CB_ATS1PR_LO);
-   reg = ((u64)iova & ~0xfff) >> 32;
-   writel_relaxed(reg, cb_base + ARM_SMMU_CB_ATS1PR_HI);
-   }
+   /* ATS1 registers can only be written atomically */
+   va = iova & ~0xfffUL;
+#ifdef CONFIG_64BIT
+   if (smmu->version == ARM_SMMU_V2)
+   writeq_relaxed(va, cb_base + ARM_SMMU_CB_ATS1PR);
+   else
+#endif
+   writel_relaxed(va, cb_base + ARM_SMMU_CB_ATS1PR);
 
if (readl_poll_timeout_atomic(cb_base + ARM_SMMU_CB_ATSR, tmp,
  !(tmp & ATSR_ACTIVE), 5, 50)) {
-- 
2.1.4

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 1/3] Documentation: dt-bindings: Add device-tree binding for ARM SMMUv3 IOMMU

[Will Deacon]

This patch adds device-tree bindings for ARM SMMUv3 IOMMU devices.

Cc: Mark Rutland 
Signed-off-by: Will Deacon 
---
 .../devicetree/bindings/iommu/arm,smmu-v3.txt  | 37 ++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt 
b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
new file mode 100644
index ..c03eec116872
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
@@ -0,0 +1,37 @@
+* ARM SMMUv3 Architecture Implementation
+
+The SMMUv3 architecture is a significant deparature from previous
+revisions, replacing the MMIO register interface with in-memory command
+and event queues and adding support for the ATS and PRI components of
+the PCIe specification.
+
+** SMMUv3 required properties:
+
+- compatible: Should include:
+
+  * "arm,smmu-v3" for any SMMUv3 compliant
+implementation. This entry should be last in the
+compatible list.
+
+- reg   : Base address and size of the SMMU.
+
+- interrupts: Non-secure interrupt list describing the wired
+  interrupt sources corresponding to entries in
+  interrupt-names. If no wired interrupts are
+  present then this property may be omitted.
+
+- interrupt-names   : When the interrupts property is present, should
+  include the following:
+  * "eventq"- Event Queue not empty
+  * "priq"  - PRI Queue not empty
+  * "cmdq-sync" - CMD_SYNC complete
+  * "gerror"- Global Error activated
+
+** SMMUv3 optional properties:
+
+- dma-coherent  : Present if DMA operations made by the SMMU (page
+  table walks, stream table accesses etc) are cache
+  coherent with the CPU.
+
+  NOTE: this only applies to the SMMU itself, not
+  masters connected upstream of the SMMU.
-- 
2.1.4

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 2/3] iommu/arm-smmu: Add initial driver support for ARM SMMUv3 devices

[Will Deacon]

Version three of the ARM SMMU architecture introduces significant
changes and improvements over previous versions of the specification,
necessitating a new driver in the Linux kernel.

The main change to the programming interface is that the majority of the
configuration data has been moved from MMIO registers to in-memory data
structures, with communication between the CPU and the SMMU being
mediated via in-memory circular queues.

This patch adds an initial driver for SMMUv3 to Linux. We currently
support pinned stage-1 (DMA) and stage-2 (KVM VFIO) mappings using the
generic IO-pgtable code.

Cc: Robin Murphy 
Signed-off-by: Will Deacon 
---
 MAINTAINERS |3 +-
 drivers/iommu/Kconfig   |   13 +
 drivers/iommu/Makefile  |1 +
 drivers/iommu/arm-smmu-v3.c | 2670 +++
 4 files changed, 2686 insertions(+), 1 deletion(-)
 create mode 100644 drivers/iommu/arm-smmu-v3.c

diff --git a/MAINTAINERS b/MAINTAINERS
index f8e0afb708b4..7de082ad2488 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1634,11 +1634,12 @@ F:  drivers/i2c/busses/i2c-cadence.c
 F: drivers/mmc/host/sdhci-of-arasan.c
 F: drivers/edac/synopsys_edac.c
 
-ARM SMMU DRIVER
+ARM SMMU DRIVERS
 M: Will Deacon 
 L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers)
 S: Maintained
 F: drivers/iommu/arm-smmu.c
+F: drivers/iommu/arm-smmu-v3.c
 F: drivers/iommu/io-pgtable-arm.c
 
 ARM64 PORT (AARCH64 ARCHITECTURE)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 1ae4e547b419..40f37a2b4a8a 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -339,6 +339,7 @@ config SPAPR_TCE_IOMMU
  Enables bits of IOMMU API required by VFIO. The iommu_ops
  is not implemented as it is not necessary for VFIO.
 
+# ARM IOMMU support
 config ARM_SMMU
bool "ARM Ltd. System MMU (SMMU) Support"
depends on (ARM64 || ARM) && MMU
@@ -352,4 +353,16 @@ config ARM_SMMU
  Say Y here if your SoC includes an IOMMU device implementing
  the ARM SMMU architecture.
 
+config ARM_SMMU_V3
+   bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
+   depends on ARM64 && PCI
+   select IOMMU_API
+   select IOMMU_IO_PGTABLE_LPAE
+   help
+ Support for implementations of the ARM System MMU architecture
+ version 3 providing translation support to a PCIe root complex.
+
+ Say Y here if your system includes an IOMMU device implementing
+ the ARM SMMUv3 architecture.
+
 endif # IOMMU_SUPPORT
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 080ffab4ed1c..c6dcc513d711 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o msm_iommu_dev.o
 obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o
 obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
 obj-$(CONFIG_ARM_SMMU) += arm-smmu.o
+obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
 obj-$(CONFIG_DMAR_TABLE) += dmar.o
 obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o
 obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
new file mode 100644
index ..f14130121298
--- /dev/null
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -0,0 +1,2670 @@
+/*
+ * IOMMU API for ARM architected SMMUv3 implementations.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ *
+ * Copyright (C) 2015 ARM Limited
+ *
+ * Author: Will Deacon 
+ *
+ * This driver is powered by bad coffee and bombay mix.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "io-pgtable.h"
+
+/* MMIO registers */
+#define ARM_SMMU_IDR0  0x0
+#define IDR0_ST_LVL_SHIFT  27
+#define IDR0_ST_LVL_MASK   0x3
+#define IDR0_ST_LVL_2LVL   (1 << IDR0_ST_LVL_SHIFT)
+#define IDR0_STALL_MODEL   (3 << 24)
+#define IDR0_TTENDIAN_SHIFT21
+#define IDR0_TTENDIAN_MASK 0x3
+#define IDR0_TTENDIAN_LE   (2 << IDR0_TTENDIAN_SHIFT)
+#define IDR0_TTENDIAN_BE   (3 << IDR0_TTENDIAN_SHIFT)
+#define IDR0_TTENDIAN_MIXED(0 << IDR0_TTENDIAN_SHIFT)
+#define IDR0_CD2L  (1 << 19)
+#define IDR0_VMID16(1 << 18)
+#define IDR0_PRI   (1 << 16)
+#define IDR0_SEV   (1

[PATCH v2 0/3] iommu/arm=smmu: Add driver for ARM SMMUv3 devices

[Will Deacon]

Hi all,

This is version two of the patches I originally posted here:

  http://www.spinics.net/lists/arm-kernel/msg416881.html

Changes since version one include:

  - Lazy allocation of the level-2 stream table on device-add
  - Limit stream table size based on SIDSIZE (maximum 21-bit atm)
  - Fixed detection of linear stream table
  - Fixed typo in RECINVSID CR2 field
  - Added cmdline option for changing default bypass behaviour to fault
  - Check IDR0.HYP before EL2 TLBI
  - Reworked stream table so that each level-2 entry resolves 256 SIDs
(i.e. a PCI bus)

All feedback welcome,

Will

--->8

Will Deacon (3):
  Documentation: dt-bindings: Add device-tree binding for ARM SMMUv3
IOMMU
  iommu/arm-smmu: Add initial driver support for ARM SMMUv3 devices
  drivers/vfio: Allow type-1 IOMMU instantiation on top of an ARM SMMUv3

 .../devicetree/bindings/iommu/arm,smmu-v3.txt  |   37 +
 MAINTAINERS|3 +-
 drivers/iommu/Kconfig  |   13 +
 drivers/iommu/Makefile |1 +
 drivers/iommu/arm-smmu-v3.c| 2670 
 drivers/vfio/Kconfig   |2 +-
 6 files changed, 2724 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
 create mode 100644 drivers/iommu/arm-smmu-v3.c

-- 
2.1.4

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 3/3] drivers/vfio: Allow type-1 IOMMU instantiation on top of an ARM SMMUv3

[Will Deacon]

The ARM SMMUv3 driver is compatible with the notion of a type-1 IOMMU in
VFIO.

This patch allows VFIO_IOMMU_TYPE1 to be selected if ARM_SMMU_V3=y.

Signed-off-by: Will Deacon 
---
 drivers/vfio/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index 7d092ddc8119..454017928ed0 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -21,7 +21,7 @@ config VFIO_VIRQFD
 menuconfig VFIO
tristate "VFIO Non-Privileged userspace driver framework"
depends on IOMMU_API
-   select VFIO_IOMMU_TYPE1 if (X86 || S390 || ARM_SMMU)
+   select VFIO_IOMMU_TYPE1 if (X86 || S390 || ARM_SMMU || ARM_SMMU_V3)
select VFIO_IOMMU_SPAPR_TCE if (PPC_POWERNV || PPC_PSERIES)
select VFIO_SPAPR_EEH if (PPC_POWERNV || PPC_PSERIES)
select ANON_INODES
-- 
2.1.4

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 3/3] drivers/vfio: Allow type-1 IOMMU instantiation on top of an ARM SMMUv3

[Alex Williamson]

On Wed, 2015-05-27 at 17:26 +0100, Will Deacon wrote:
> The ARM SMMUv3 driver is compatible with the notion of a type-1 IOMMU in
> VFIO.
> 
> This patch allows VFIO_IOMMU_TYPE1 to be selected if ARM_SMMU_V3=y.
> 
> Signed-off-by: Will Deacon 

Acked-by: Alex Williamson 

> ---
>  drivers/vfio/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index 7d092ddc8119..454017928ed0 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -21,7 +21,7 @@ config VFIO_VIRQFD
>  menuconfig VFIO
>   tristate "VFIO Non-Privileged userspace driver framework"
>   depends on IOMMU_API
> - select VFIO_IOMMU_TYPE1 if (X86 || S390 || ARM_SMMU)
> + select VFIO_IOMMU_TYPE1 if (X86 || S390 || ARM_SMMU || ARM_SMMU_V3)
>   select VFIO_IOMMU_SPAPR_TCE if (PPC_POWERNV || PPC_PSERIES)
>   select VFIO_SPAPR_EEH if (PPC_POWERNV || PPC_PSERIES)
>   select ANON_INODES



___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC/PATCH 1/9] arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops()

[Will Deacon]

On Fri, May 15, 2015 at 12:00:02AM +0100, Laurent Pinchart wrote:
> The arch_setup_dma_ops() function is in charge of setting dma_ops with a
> call to set_dma_ops(). set_dma_ops() is also called from
> 
> - highbank and mvebu bus notifiers
> - dmabounce (to be replaced with swiotlb)
> - arm_iommu_attach_device
> 
> (arm_iommu_attach_device is itself called from IOMMU and bus master
> device drivers)
> 
> To allow the arch_setup_dma_ops() call to be moved from device add time
> to device probe time we must ensure that dma_ops already setup by any of
> the above callers will not be overriden.
> 
> Aftering replacing dmabounce with swiotlb, converting IOMMU drivers to

s/Aftering/After/

> of_xlate and taking care of highbank and mvebu, the workaround should be
> removed.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  arch/arm/mm/dma-mapping.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 09c5fe3d30c2..7aa5e339a596 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -2117,6 +2117,15 @@ void arch_setup_dma_ops(struct device *dev, u64 
> dma_base, u64 size,
>   struct dma_map_ops *dma_ops;
>  
>   dev->archdata.dma_coherent = coherent;
> +
> + /*
> +  * Don't override the dma_ops if they have already been set. Ideally
> +  * this should be the only location where dma_ops are set, remove this
> +  * check when all other callers of set_dma_ops will have disappeared.
> +  */
> + if (dev->archdata.dma_ops)
> + return;

Maybe printing a warning at the same time will encourage people to migrate
away from the existing behaviour?

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: Master-aware devices and sideband ID data

[Mark Rutland]

On Tue, May 26, 2015 at 11:20:59PM +0100, Chalamarla, Tirumalesh wrote:
> This is some thing we also like to see in ITS and SMMU drivers. 
> > On Mar 24, 2015, at 8:50 AM, Mark Rutland  wrote:
> > 
> > Hi all,
> > 
> > For some devices, identification of particular masters is critical to
> > their operation (e.g. IOMMUs, MSI controllers). The identity of masters
> > is determined from sideband signals on the bus, and it may or may not be
> > possible to probe and/or configure this information. Worse still, these
> > information may be rewritten by intermediate buses, so the
> > information for a given master may differ at different points in the bus
> > hierarchy.
> > 
> > We currently describe this master information for devices attached to
> > IOMMUs, with a master ID being encoded in the iommu-cells. However this
> > only covers the ID between the master and its IOMMU(s), and we don't
> > currently have a mechanism to describe the master IDs as they are seen
> > by devices beyond the IOMMU(s), or in the absence of any IOMMU(s).
> > 
> > The following are example of master IDs:
> > * PCIe Requester IDs (RIDs) (bus:device.function AKA BDF)
> > * SMMU Stream IDs (SIDs)
> > * GICv3 ITS Device IDs (DIDs)
> > 
> > In the case of a system with PCIe, SMMU and GICv3, the master IDs are
> > rewritten in a chain of RID=>SID=>DID, as in the following diagram:
> > 
> > +-+
> > | PCIe master |
> > +-+
> >||
> >|| Requester ID (RID)
> >|| Probeable from RC.
> >\/
> > +---+
> > | PCIe Root Complex |
> > +---+
> >||
> >|| SMMU Stream ID (SID)
> >|| Derived from RID, static non-probeable mapping.
> >\/
> > +--+
> > | SMMU (IOMMU) |
> > +--+
> >||
> >|| ITS Device ID (DID)
> >|| Derived from SID, static non-probeable mapping.
> >\/
> > ++
> > | GICv3 ITS (MSI controller) |
> > ++
> > 
> > In simpler cases, you might have a simpler set of master ID
> > translations, e.g. just a DID:
> > 
> > +-+
> > | Platform device |
> > +-+
> >||
> >|| ITS Device ID (DID)
> >|| Hard-wired on the bus.
> >\/
> > ++
> > | GICv3 ITS (MSI controller) |
> > ++
> > 
> > Ignoring current bindings for the moment, I can see how we can describe
> > this with a generic master id-binding, with master-id-map along the
> > lines of interrupt-map, with a tuple format like:
> > 
> > 
> > For the PCIe example, this would look something like the following (with
> > properties omitted for brevity):
> > 
> > PCI: pci@af00 {
> > ...
> > 
> > /* Requester ID of PCIe endpoints, implicit at runtime */
> > master-id-cells = <1>;
> > 
> > /* RIDS idmapped to SIDS @ SMMU */
> > master-id-map = <0 0x1 &SMMU 0>;
> > }
> > 
> > SMMU: iommu@bf00 {
> > ...
> > 
> > /* SID, derived from RID */
> > master-id-cells = <1>;
> > 
> > /* 
> >  * For some reason the high bit of the ID was negated.
> >  */
> > master-id-map = <0x 0x8000 &ITS 0x0 0x8000>,
> > <0x8000 0x8000 &ITS 0x0 0x>;
> > 
> > };
> > 
> > ITS: its@cf00 {
> > ...
> > 
> > /* DID, derived from SID */
> > master-id-cells = <2>;
> > 
> > /* 
> >  * Master traffic not propagated beyond this point, so no
> >  * master-id-ranges
> >  */
> > };
> 
> I think it is nice to have max IDs supported by masters. so that drivers can 
> check and enforce.  

The set of IDs that we expect to transform (i.e. those masters use)
would be implicit in the first master-id-map from a master. In the PCI
example above, no master is expected to generate an ID outside of the
range 0-0x1 inclusive (and that's all the SMMU would see).

For devices which are not hotpluggable, the nodes for those devices
would describe the specific set of IDs they use.

Generally, between a master and a slave there might not be one
contiguous set of valid IDs as these may be rewritten along the way (as
happens at the SMMU between the PCI root complex and the ITS in the
example above).

Which drivers do you think need this information? What exactly are they
trying to check and enforce?

> > For the simpler case, this would look something like:
> > 
> > DEV: device@af00 {
> > master-id-cells = <1>;
> > master-ids = <0xf>, <0xb>;
> > master-id-map = <0 0xf &ITS 0 0>;
> > };
> > 
> > ITS: its@cf00 {
> > ...
> > 
> > /* DID */
> > master-id-cells = <2>;
> > };
> > 
> Is this is not depending heavily on discover order, how do drivers
> know which device to get which ID. is it implicitly assumed in
> discovery order? 

I'm not sure I follow the question. Could you elaborate?

> what happens to hot pluggable devices.  

It would be necessary to be able to discover the ID assigned to the
device by the hotpluggable bus. For exam

Re: [RFC/PATCH 3/9] of: dma: Move range size workaround to of_dma_get_range()

[Will Deacon]

On Fri, May 15, 2015 at 12:00:04AM +0100, Laurent Pinchart wrote:
> Invalid dma-ranges values should be worked around when retrieving the
> DMA range in of_dma_get_range(), not by all callers of the function.
> This isn't much of a problem now that we have a single caller, but that
> situation will change when moving DMA configuration to device probe
> time.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/of/address.c | 20 ++--
>  drivers/of/device.c  | 15 ---
>  2 files changed, 18 insertions(+), 17 deletions(-)

Looks good to me:

  Acked-by: Will Deacon 

Will

> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 78a7dcbec7d8..75eebd19ebfa 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -924,8 +924,8 @@ EXPORT_SYMBOL(of_io_request_and_map);
>   *   CPU addr (phys_addr_t)  : pna cells
>   *   size: nsize cells
>   *
> - * It returns -ENODEV if "dma-ranges" property was not found
> - * for this device in DT.
> + * Return 0 on success, -ENODEV if the "dma-ranges" property was not found 
> for
> + * this device in DT, or -EINVAL if the CPU address or size is invalid.
>   */
>  int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 
> *size)
>  {
> @@ -986,6 +986,22 @@ int of_dma_get_range(struct device_node *np, u64 
> *dma_addr, u64 *paddr, u64 *siz
>   *dma_addr = dmaaddr;
>  
>   *size = of_read_number(ranges + naddr + pna, nsize);
> + /*
> +  * DT nodes sometimes incorrectly set the size as a mask. Work around
> +  * those incorrect DT by computing the size as mask + 1.
> +  */
> + if (*size & 1) {
> + pr_warn("%s: size 0x%llx for dma-range in node(%s) set as 
> mask\n",
> + __func__, *size, np->full_name);
> + *size = *size + 1;
> + }
> +
> + if (!*size) {
> + pr_err("%s: invalid size zero for dma-range in node(%s)\n",
> +__func__, np->full_name);
> + ret = -EINVAL;
> + goto out;
> + }
>  
>   pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
>*dma_addr, *paddr, *size);
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 20c1332a0018..530aa1ed3e1b 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -109,21 +109,6 @@ void of_dma_configure(struct device *dev, struct 
> device_node *np)
>   size = dev->coherent_dma_mask + 1;
>   } else {
>   offset = PFN_DOWN(paddr - dma_addr);
> -
> - /*
> -  * Add a work around to treat the size as mask + 1 in case
> -  * it is defined in DT as a mask.
> -  */
> - if (size & 1) {
> - dev_warn(dev, "Invalid size 0x%llx for dma-range\n",
> -  size);
> - size = size + 1;
> - }
> -
> - if (!size) {
> - dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
> - return;
> - }
>   dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
>   }
>  
> -- 
> 2.3.6
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH v2 4/7] DMA-API: Add dma_(un)map_resource() documentation

[William Davis]

> -Original Message-
> From: Mark Hounschell [mailto:ma...@compro.net]
> Sent: Wednesday, May 20, 2015 3:07 PM
> To: William Davis; Bjorn Helgaas
> Cc: j...@8bytes.org; iommu@lists.linux-foundation.org; 
> linux-...@vger.kernel.org; Terence Ripperda;
> John Hubbard; jgli...@redhat.com; konrad.w...@oracle.com; Jonathan Corbet; 
> David S. Miller
> Subject: Re: [PATCH v2 4/7] DMA-API: Add dma_(un)map_resource() documentation
> 
> >>
> >> I currently just do
> >>
> >> page = virt_to_page(__va(bus_address));
> >>
> >> then just use the normal API. Works for writes anyway.
> >>
> >
> > What platform is this on? I don't understand how that could work for
> > peer-to-peer. As I understand it, there are no 'struct page's for MMIO
> > regions, and you could actually end up with a very different page as a
> > result of that unchecked translation (e.g., a "valid" struct page, but
> > not in the peer's MMIO range at all).
> >
> 
> This is an x86-64 AMD 990FX chip set. Works fine. Every time. I do have
> the peers memory that is being written to mmap'd by the task that is
> doing this, but this works.
> 
> Mark
> 

I spent some time looking into this, and I was surprised to find it worked
for me too. At least, it seems like it works, but it is a dangerous
accident that it did, and so I think the patch is still necessary.

In the IOMMU mapping paths, the IOMMU drivers (at least on x86) only
really use the struct page * to get the physical address that it
references, and I think that's usually done exclusively with pointer
arithmetic these days. So you are getting a pointer to a struct page that
actually doesn't exist, because it's either in a hole or off the end of
the memory map.

You can verify this by calling dump_page() on the struct page * returned
by this calculation. On my Intel machine, this BUGs, unable to handle the
paging request when looking at the struct page fields. If I just use the
pointer without dereferencing it, the IOMMU paths can get the correct
physical address without any issues, but this is by accident and not by
design, nor is it portable, so we shouldn't rely on this technique.

Thanks,
Will

--
nvpublic
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC/PATCH 3/9] of: dma: Move range size workaround to of_dma_get_range()

[Rob Herring]

On Thu, May 14, 2015 at 6:00 PM, Laurent Pinchart
 wrote:
> Invalid dma-ranges values should be worked around when retrieving the
> DMA range in of_dma_get_range(), not by all callers of the function.
> This isn't much of a problem now that we have a single caller, but that
> situation will change when moving DMA configuration to device probe
> time.
>
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/of/address.c | 20 ++--
>  drivers/of/device.c  | 15 ---

Please use get_maintainers.pl. Anyway, looks fine to me:

Acked-by: Rob Herring 

Rob

>  2 files changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 78a7dcbec7d8..75eebd19ebfa 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -924,8 +924,8 @@ EXPORT_SYMBOL(of_io_request_and_map);
>   * CPU addr (phys_addr_t)  : pna cells
>   * size: nsize cells
>   *
> - * It returns -ENODEV if "dma-ranges" property was not found
> - * for this device in DT.
> + * Return 0 on success, -ENODEV if the "dma-ranges" property was not found 
> for
> + * this device in DT, or -EINVAL if the CPU address or size is invalid.
>   */
>  int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 
> *size)
>  {
> @@ -986,6 +986,22 @@ int of_dma_get_range(struct device_node *np, u64 
> *dma_addr, u64 *paddr, u64 *siz
> *dma_addr = dmaaddr;
>
> *size = of_read_number(ranges + naddr + pna, nsize);
> +   /*
> +* DT nodes sometimes incorrectly set the size as a mask. Work around
> +* those incorrect DT by computing the size as mask + 1.
> +*/
> +   if (*size & 1) {
> +   pr_warn("%s: size 0x%llx for dma-range in node(%s) set as 
> mask\n",
> +   __func__, *size, np->full_name);
> +   *size = *size + 1;
> +   }
> +
> +   if (!*size) {
> +   pr_err("%s: invalid size zero for dma-range in node(%s)\n",
> +  __func__, np->full_name);
> +   ret = -EINVAL;
> +   goto out;
> +   }
>
> pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
>  *dma_addr, *paddr, *size);
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 20c1332a0018..530aa1ed3e1b 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -109,21 +109,6 @@ void of_dma_configure(struct device *dev, struct 
> device_node *np)
> size = dev->coherent_dma_mask + 1;
> } else {
> offset = PFN_DOWN(paddr - dma_addr);
> -
> -   /*
> -* Add a work around to treat the size as mask + 1 in case
> -* it is defined in DT as a mask.
> -*/
> -   if (size & 1) {
> -   dev_warn(dev, "Invalid size 0x%llx for dma-range\n",
> -size);
> -   size = size + 1;
> -   }
> -
> -   if (!size) {
> -   dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
> -   return;
> -   }
> dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
> }
>
> --
> 2.3.6
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu