MT8173 IOMMU SUPPORT

This patch add mediatek iommu dts binding document.

Signed-off-by Yong Wu <[email protected]>

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

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt new file +0 -0

new file mode 100644
index 0000000..f2cc7c0

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt new file +8 -0

+              EMI (External Memory Interface)
+               |
+              m4u (Multimedia Memory Management Unit)
+               |
+              smi (Smart Multimedia Interface)
+               |
+        +---------------+-------
+        |               |

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt new file +5 -0

+
+/* larb0 */
+#define        M4U_PORT_DISP_OVL0              M4U_LARB0_PORT(0)
+#define        M4U_PORT_DISP_RDMA0             M4U_LARB0_PORT(1)
+#define        M4U_PORT_DISP_WDMA0             M4U_LARB0_PORT(2)

Hi,

Please see my comments inline.

Quoted 16 lines · Yong Wu

This patch add mediatek iommu dts binding document.

Signed-off-by Yong Wu <[email protected]>

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

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt new file +0 -0

new file mode 100644
index 0000000..f2cc7c0

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt line 3

Quoted 3 lines

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt new file +3 -0

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

.

Quoted 8 lines

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt new file +8 -0

+              EMI (External Memory Interface)
+               |
+              m4u (Multimedia Memory Management Unit)
+               |
+              smi (Smart Multimedia Interface)
+               |
+        +---------------+-------
+        |               |

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt new file +10 -0

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

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt line 29

Quoted 9 lines

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt new file +8 -0

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

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt line 34

Quoted 5 lines

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt new file +5 -0

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

[snip]

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt

Quoted 13 lines

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt new file +13 -0

+#define M4U_LARB0_PORT_NR      8
+#define M4U_LARB1_PORT_NR      10
+#define M4U_LARB2_PORT_NR      21
+#define M4U_LARB3_PORT_NR      15
+#define M4U_LARB4_PORT_NR      6
+#define M4U_LARB5_PORT_NR      9
+
+#define M4U_LARB0_PORT(n)      (n)
+#define M4U_LARB1_PORT(n)      ((n) + M4U_LARB0_PORT_NR + M4U_LARB0_PORT(0))
+#define M4U_LARB2_PORT(n)      ((n) + M4U_LARB1_PORT_NR + M4U_LARB1_PORT(0))
+#define M4U_LARB3_PORT(n)      ((n) + M4U_LARB2_PORT_NR + M4U_LARB2_PORT(0))
+#define M4U_LARB4_PORT(n)      ((n) + M4U_LARB3_PORT_NR + M4U_LARB3_PORT(0))
+#define M4U_LARB5_PORT(n)      ((n) + M4U_LARB4_PORT_NR + M4U_LARB4_PORT(0))

This looks like some artificial indexing. Does it have any correspondence with actual hardware configuration?

Quoted 6 lines

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt new file +5 -0

+
+/* larb0 */
+#define        M4U_PORT_DISP_OVL0              M4U_LARB0_PORT(0)
+#define        M4U_PORT_DISP_RDMA0             M4U_LARB0_PORT(1)
+#define        M4U_PORT_DISP_WDMA0             M4U_LARB0_PORT(2)

[snip]

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt

Quoted 3 lines

Documentation/devicetree/bindings/iommu/mediatek,iommu.txt new file +3 -0

+#define        M4U_PORT_VENC_CUR_CHROMA_SET2   M4U_LARB5_PORT(6)
+#define        M4U_PORT_VENC_RD_COMA_SET2      M4U_LARB5_PORT(7)
+#define        M4U_PORT_VENC_SV_COMA_SET2      M4U_LARB5_PORT(8)

This convinces me even more that this binding actually needs #iommu-cells to be 2 and the indexing system I described above.

Best regards,
Tomasz

This patch add smi binding document.

Signed-off-by Yong Wu <[email protected]>

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

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt new file +4 -0

new file mode 100644
index 0000000..c06c5b6
@@ -0,0 +1,24 @@
+SMI(Smart Multimedia Interface) Local Arbiter
+
+The hardware block diagram please check bindings/iommu/mediatek,iommu.txt
+

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt new file +10 -0

+
+Example:
+	larb0:larb at 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";
+	};

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt new file +3 -0

new file mode 100644
index 0000000..c2389b4
@@ -0,0 +1,22 @@
+SMI(Smart Multimedia Interface)
+
+The hardware block diagram please check bindings/iommu/mediatek,iommu.txt

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt new file +9 -0

+
+Example:
+	smi_common:smi at 14022000 {
+		compatible = "mediatek,mt8173-smi";
+		reg = <0 0x14022000 0 0x1000>;
+		clocks = <&mmsys MM_SMI_COMMON>,
+			 <&mmsys MM_SMI_COMMON>;
+		clock-names = "apb", "smi";
+	};

This patch add smi binding document.

Signed-off-by Yong Wu <[email protected]>

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

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt new file +0 -0

new file mode 100644
index 0000000..c06c5b6

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt new file +10 -0

+
+Example:
+       larb0:larb at 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";
+       };

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt new file +0 -0

new file mode 100644
index 0000000..c2389b4

Hi,

Please see my comments inline.

Quoted 16 lines · Yong Wu

This patch add smi binding document.

Signed-off-by Yong Wu <[email protected]>

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

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt new file +0 -0

new file mode 100644
index 0000000..c06c5b6

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt line 7

Quoted 7 lines

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt new file +7 -0

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

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt line 8

Quoted 1 line

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt new file +1 -0

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

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt line 13

Quoted 6 lines

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt new file +5 -0

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

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt line 14

Quoted 1 line

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt new file +1 -0

+- 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" : <here comes the description of APB clock>,
- "smi" : <here comes the description of SMI clock>.

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt line 23

Quoted 9 lines

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt new file +9 -0

+
+Example:
+       larb0:larb at 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";

Quoted 7 lines

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt new file +1 -0

+       };

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt new file +0 -0

new file mode 100644
index 0000000..c2389b4

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt line 7

Quoted 7 lines

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt new file +7 -0

@@ -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/

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt line 13

Quoted 6 lines

Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt new file +6 -0

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

This patch is for ARM Short Descriptor Format.It has 2-levels pagetable and the allocator supports 4K/64K/1M/16M.

Signed-off-by Yong Wu <[email protected]>

---
  drivers/iommu/Kconfig                |   7 +
  drivers/iommu/Makefile               |   1 +
  drivers/iommu/io-pgtable-arm-short.c | 490 +++++++++++++++++++++++++++++++++++
  drivers/iommu/io-pgtable.c           |   4 +
  drivers/iommu/io-pgtable.h           |   6 +
  5 files changed, 508 insertions(+)

create mode 100644 drivers/iommu/io-pgtable-arm-short.c

drivers/iommu/Kconfig +7 -0

index 1ae4e54..3d2eac6 100644
@@ -39,6 +39,13 @@ config IOMMU_IO_PGTABLE_LPAE_SELFTEST
 
           If unsure, say N here.
 
+config IOMMU_IO_PGTABLE_SHORT
+       bool "ARMv7/v8 Short Descriptor Format"
+       select IOMMU_IO_PGTABLE
+       help
+         Enable support for the ARM Short descriptor pagetable format.
+         It has 2-levels pagetable and The allocator supports 4K/64K/1M/16M.
+
  endmenu
 
  config IOMMU_IOVA

drivers/iommu/Makefile +1 -0

index 080ffab..815b3c8 100644
@@ -3,6 +3,7 @@ obj-$(CONFIG_IOMMU_API) += iommu-traces.o
  obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
  obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
  obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
+obj-$(CONFIG_IOMMU_IO_PGTABLE_SHORT) += io-pgtable-arm-short.o
  obj-$(CONFIG_IOMMU_IOVA) += iova.o
  obj-$(CONFIG_OF_IOMMU) += of_iommu.o
  obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o msm_iommu_dev.o

drivers/iommu/io-pgtable-arm-short.c new file +9 -0

new file mode 100644
index 0000000..cc286ce5
@@ -0,0 +1,490 @@
+/*
+ * Copyright (c) 2014-2015 MediaTek Inc.
+ * Author: Yong Wu <[email protected]>
+ *
+ * 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,

drivers/iommu/io-pgtable-arm-short.c new file +7 -0

+
+#include "io-pgtable.h"
+
+typedef u32 arm_short_iopte;
+
+struct arm_short_io_pgtable {
+       struct io_pgtable       iop;

drivers/iommu/io-pgtable-arm-short.c new file +8 -0

+
+#define io_pgtable_short_ops_to_data(x)                                \
+       io_pgtable_short_to_data(io_pgtable_ops_to_pgtable(x))
+
+#define ARM_SHORT_MAX_ADDR_BITS                        32
+
+#define ARM_SHORT_PGDIR_SHIFT                  20
+#define ARM_SHORT_PAGE_SHIFT                   12

drivers/iommu/io-pgtable-arm-short.c new file +44 -0

+#define ARM_SHORT_F_PGD_TYPE_IS_SECTION(pgd)           \
+       (((pgd) & ARM_SHORT_F_PGD_TYPE_SECTION_MSK)     \
+               == ARM_SHORT_F_PGD_TYPE_SECTION)
+#define ARM_SHORT_F_PGD_TYPE_IS_SUPERSECTION(pgd)      \
+       (((pgd) & ARM_SHORT_F_PGD_TYPE_SECTION_MSK)     \
+               == ARM_SHORT_F_PGD_TYPE_SUPERSECTION)
+
+#define ARM_SHORT_F_PGD_B_BIT                  BIT(2)
+#define ARM_SHORT_F_PGD_C_BIT                  BIT(3)
+#define ARM_SHORT_F_PGD_IMPLE_BIT              BIT(9)
+#define ARM_SHORT_F_PGD_S_BIT                  BIT(16)
+#define ARM_SHORT_F_PGD_NG_BIT                 BIT(17)
+#define ARM_SHORT_F_PGD_NS_BIT_PAGE            BIT(3)
+#define ARM_SHORT_F_PGD_NS_BIT_SECTION         BIT(19)
+
+#define ARM_SHORT_F_PGD_PA_PAGETABLE_MSK       0xfffffc00
+#define ARM_SHORT_F_PGD_PA_SECTION_MSK         0xfff00000
+#define ARM_SHORT_F_PGD_PA_SUPERSECTION_MSK    0xff000000
+
+/* 2 level pagetable */
+#define ARM_SHORT_F_PTE_TYPE_GET(val)          ((val) & 0x3)
+#define ARM_SHORT_F_PTE_TYPE_LARGE             BIT(0)
+#define ARM_SHORT_F_PTE_TYPE_SMALL             BIT(1)
+#define ARM_SHORT_F_PTE_B_BIT                  BIT(2)
+#define ARM_SHORT_F_PTE_C_BIT                  BIT(3)
+#define ARM_SHORT_F_PTE_IMPLE_BIT              BIT(9)
+#define ARM_SHORT_F_PTE_S_BIT                  BIT(10)
+#define ARM_SHORT_F_PTE_PA_LARGE_MSK            0xffff0000
+#define ARM_SHORT_F_PTE_PA_SMALL_MSK            0xfffff000
+
+#define ARM_SHORT_PGD_IDX(a)                   ((a) >> ARM_SHORT_PGDIR_SHIFT)
+#define ARM_SHORT_PTE_IDX(a)                   \
+       (((a) >> ARM_SHORT_PAGE_SHIFT) & 0xff)
+#define ARM_SHORT_GET_PTE_VA(pgd)              \
+       (phys_to_virt((unsigned long)pgd & ARM_SHORT_F_PGD_PA_PAGETABLE_MSK))
+
+static arm_short_iopte *
+arm_short_get_pte_in_pgd(arm_short_iopte curpgd, unsigned int iova)
+{
+       arm_short_iopte *pte;
+
+       pte = ARM_SHORT_GET_PTE_VA(curpgd);
+       pte += ARM_SHORT_PTE_IDX(iova);
+       return pte;

drivers/iommu/io-pgtable-arm-short.c new file +50 -0

+{
+       arm_short_iopte *pte;
+       int i;
+
+       pte = ARM_SHORT_GET_PTE_VA(*pgd);
+
+       for (i = 0; i < ARM_SHORT_PTRS_PER_PTE; i++) {
+               if (pte[i] != 0)
+                       return 1;
+       }
+
+       /* Free PTE */
+       kmem_cache_free(data->ptekmem, pte);
+       *pgd = 0;
+
+       return 0;
+}
+
+static phys_addr_t arm_short_iova_to_phys(struct io_pgtable_ops *ops,
+                                         unsigned long iova)
+{
+       struct arm_short_io_pgtable *data = io_pgtable_short_ops_to_data(ops);
+       arm_short_iopte *pte, *pgd = data->pgd;
+       phys_addr_t pa = 0;
+
+       pgd += ARM_SHORT_PGD_IDX(iova);
+
+       if (ARM_SHORT_F_PGD_TYPE_IS_PAGE(*pgd)) {
+               u8 pte_type;
+
+               pte = arm_short_get_pte_in_pgd(*pgd, iova);
+               pte_type = ARM_SHORT_F_PTE_TYPE_GET(*pte);
+
+               if (pte_type == ARM_SHORT_F_PTE_TYPE_LARGE) {
+                       pa = (*pte) & ARM_SHORT_F_PTE_PA_LARGE_MSK;
+                       pa |= iova & (~ARM_SHORT_F_PTE_PA_LARGE_MSK);
+               } else if (pte_type == ARM_SHORT_F_PTE_TYPE_SMALL) {
+                       pa = (*pte) & ARM_SHORT_F_PTE_PA_SMALL_MSK;
+                       pa |= iova & (~ARM_SHORT_F_PTE_PA_SMALL_MSK);
+               }
+       } else {
+               if (ARM_SHORT_F_PGD_TYPE_IS_SECTION(*pgd)) {
+                       pa = (*pgd) & ARM_SHORT_F_PGD_PA_SECTION_MSK;
+                       pa |= iova & (~ARM_SHORT_F_PGD_PA_SECTION_MSK);
+               } else if (ARM_SHORT_F_PGD_TYPE_IS_SUPERSECTION(*pgd)) {
+                       pa = (*pgd) & ARM_SHORT_F_PGD_PA_SUPERSECTION_MSK;
+                       pa |= iova & (~ARM_SHORT_F_PGD_PA_SUPERSECTION_MSK);
+               }
+       }
+

drivers/iommu/io-pgtable-arm-short.c new file +5 -0

+       const struct iommu_gather_ops *tlb = data->iop.cfg.tlb;
+       void *cookie = data->iop.cookie;
+       int ret;
+
+       do {

drivers/iommu/io-pgtable-arm-short.c new file +19 -0

+                               (ARM_SHORT_PTRS_PER_PTE - pte_offset));
+
+                       pte = arm_short_get_pte_in_pgd(*pgd, iova);
+
+                       memset(pte, 0, num_to_clean * sizeof(arm_short_iopte));
+
+                       ret = _arm_short_check_free_pte(data, pgd);
+                       if (ret == 1)/* pte is not freed, need to flush pte */
+                               tlb->flush_pgtable(
+                                       pte,
+                                       num_to_clean * sizeof(arm_short_iopte),
+                                       cookie);
+                       else
+                               tlb->flush_pgtable(pgd, sizeof(arm_short_iopte),
+                                                  cookie);
+
+                       iova += num_to_clean << PAGE_SHIFT;
+               } else if (ARM_SHORT_F_PGD_TYPE_IS_SECTION(*pgd)) {
+                       *pgd = 0;

drivers/iommu/io-pgtable-arm-short.c new file +5 -0

+{
+       arm_short_iopte pteprot;
+
+       pteprot = ARM_SHORT_F_PTE_S_BIT;
+

drivers/iommu/io-pgtable-arm-short.c new file +19 -0

+{
+       arm_short_iopte pgdprot;
+
+       pgdprot = ARM_SHORT_F_PGD_S_BIT;
+       pgdprot |= super ? ARM_SHORT_F_PGD_TYPE_SUPERSECTION :
+                               ARM_SHORT_F_PGD_TYPE_SECTION;
+       if (prot & IOMMU_CACHE)
+               pgdprot |= ARM_SHORT_F_PGD_C_BIT | ARM_SHORT_F_PGD_B_BIT;
+
+       return pgdprot;
+}
+
+static int _arm_short_map_page(struct arm_short_io_pgtable *data,
+                              unsigned int iova, phys_addr_t pa,
+                              unsigned int prot, bool largepage)
+{
+       arm_short_iopte *pgd = data->pgd;
+       arm_short_iopte *pte;
+       arm_short_iopte pgdprot, pteprot;

drivers/iommu/io-pgtable-arm-short.c new file +12 -0

+               ptenew = true;
+               *pgd = virt_to_phys(pte_new_va) | pgdprot;
+               kmemleak_ignore(pte_new_va);
+               data->iop.cfg.tlb->flush_pgtable(pgd, sizeof(arm_short_iopte),
+                                                cookie);
+       } else {
+               /* Someone else may have allocated for this pgd */
+               if (((*pgd) & (~ARM_SHORT_F_PGD_PA_PAGETABLE_MSK)) != pgdprot) {
+                       pr_err("The prot of old pgd is not Right!iova=0x%x pgd=0x%x pgprot=0x%x\n",
+                              iova, (*pgd), pgdprot);
+                       return -EEXIST;
+               }

drivers/iommu/io-pgtable-arm-short.c new file +6 -0

+
+       for (i = 0; i < ptenum; i++) {
+               if (pte[i]) {
+                       pr_err("The To-Map pte exists!(iova=0x%x pte=0x%x i=%d)\n",
+                              iova, pte[i], i);
+                       goto err_out;

drivers/iommu/io-pgtable-arm-short.c new file +30 -0

+               pte[i] = 0;
+       if (ptenew)
+               kmem_cache_free(data->ptekmem, pte_new_va);
+       return -EEXIST;
+}
+
+static int _arm_short_map_section(struct arm_short_io_pgtable *data,
+                                 unsigned int iova, phys_addr_t pa,
+                                 int prot, bool supersection)
+{
+       arm_short_iopte pgprot;
+       arm_short_iopte mask = supersection ?
+                               ARM_SHORT_F_PGD_PA_SUPERSECTION_MSK :
+                               ARM_SHORT_F_PGD_PA_SECTION_MSK;
+       arm_short_iopte *pgd = data->pgd;
+       int i;
+       unsigned int pgdnum = supersection ? 16 : 1;
+
+       if ((iova | pa) & (~mask)) {
+               pr_err("IOVA|PA Not Aligned(iova=0x%x pa=0x%pa type=%s)\n",
+                      iova, &pa, supersection ? "supersection" : "section");
+               return -EINVAL;
+       }
+
+       pgprot = (arm_short_iopte)pa;
+
+       if (data->iop.cfg.quirks & IO_PGTABLE_QUIRK_ARM_NS)
+               pgprot |= ARM_SHORT_F_PGD_NS_BIT_SECTION;
+
+       pgprot |= __arm_short_pgd_port(prot, supersection);

drivers/iommu/io-pgtable-arm-short.c new file +27 -0

+                              iova, pgd[i], i);
+                       goto err_out;
+               }
+               pgd[i] = pgprot;
+       }
+       data->iop.cfg.tlb->flush_pgtable(pgd,
+                                        pgdnum * sizeof(arm_short_iopte),
+                                        data->iop.cookie);
+       return 0;
+
+ err_out:
+       for (i--; i >= 0; i--)
+               pgd[i] = 0;
+       return -EEXIST;
+}
+
+static int arm_short_map(struct io_pgtable_ops *ops, unsigned long iova,
+                        phys_addr_t paddr, size_t size, int prot)
+{
+       struct arm_short_io_pgtable *data = io_pgtable_short_ops_to_data(ops);
+       const struct iommu_gather_ops *tlb = data->iop.cfg.tlb;
+       int ret;
+
+       if (!(prot & (IOMMU_READ | IOMMU_WRITE)))
+               return -EINVAL;
+
+       if (size == SZ_4K) {/* most case */

drivers/iommu/io-pgtable-arm-short.c new file +14 -0

+       tlb->tlb_add_flush(iova, size, true, data->iop.cookie);
+       return ret;
+}
+
+static struct io_pgtable *
+arm_short_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie)
+{
+       struct arm_short_io_pgtable *data;
+
+       if (cfg->ias != 32)
+               return NULL;
+
+       if (cfg->oas > ARM_SHORT_MAX_ADDR_BITS)
+               return NULL;

drivers/iommu/io-pgtable-arm-short.c new file +52 -0

+       if (!data->pgd)
+               goto out_free_data;
+
+       cfg->tlb->flush_pgtable(data->pgd, data->pgd_size, cookie);
+
+       /* kmem for pte */
+       data->ptekmem = kmem_cache_create("short-descriptor-pte",
+                                          ARM_SHORT_BYTES_PER_PTE,
+                                          ARM_SHORT_BYTES_PER_PTE,
+                                          0, NULL);
+
+       if (IS_ERR_OR_NULL(data->ptekmem)) {
+               pr_err("Failed to Create cached mem for PTE %ld\n",
+                      PTR_ERR(data->ptekmem));
+               goto out_free_pte;
+       }
+
+       /* TTBRs */
+       cfg->arm_short_cfg.ttbr[0] = virt_to_phys(data->pgd);
+       cfg->arm_short_cfg.ttbr[1] = 0;
+
+       cfg->arm_short_cfg.tcr = 0;
+
+       data->iop.ops = (struct io_pgtable_ops) {
+               .map            = arm_short_map,
+               .unmap          = arm_short_unmap,
+               .iova_to_phys   = arm_short_iova_to_phys,
+       };
+
+       return &data->iop;
+
+out_free_pte:
+       free_pages_exact(data->pgd, data->pgd_size);
+out_free_data:
+       kfree(data);
+       return NULL;
+}
+
+static void arm_short_free_pgtable(struct io_pgtable *iop)
+{
+       struct arm_short_io_pgtable *data = io_pgtable_short_to_data(iop);
+
+       kmem_cache_destroy(data->ptekmem);
+       free_pages_exact(data->pgd, data->pgd_size);
+       kfree(data);
+}
+
+struct io_pgtable_init_fns io_pgtable_arm_short_init_fns = {
+       .alloc  = arm_short_alloc_pgtable,
+       .free   = arm_short_free_pgtable,
+};
+

drivers/iommu/io-pgtable.c +4 -0

index 6436fe2..14a9b3a 100644
@@ -28,6 +28,7 @@ extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s1_init_fns;
  extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s2_init_fns;
  extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns;
  extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns;
+extern struct io_pgtable_init_fns io_pgtable_arm_short_init_fns;
 
  static const struct io_pgtable_init_fns *
  io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] =

@@ -38,6 +39,9 @@ io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] =
         [ARM_64_LPAE_S1] = &io_pgtable_arm_64_lpae_s1_init_fns,
         [ARM_64_LPAE_S2] = &io_pgtable_arm_64_lpae_s2_init_fns,
  #endif
+#ifdef CONFIG_IOMMU_IO_PGTABLE_SHORT
+       [ARM_SHORT_DESC] = &io_pgtable_arm_short_init_fns,
+#endif
  };
 
  struct io_pgtable_ops *alloc_io_pgtable_ops(enum io_pgtable_fmt fmt,

drivers/iommu/io-pgtable.h +1 -0

index 10e32f6..47efaab 100644
@@ -9,6 +9,7 @@ enum io_pgtable_fmt {
         ARM_32_LPAE_S2,
         ARM_64_LPAE_S1,
         ARM_64_LPAE_S2,
+       ARM_SHORT_DESC,
         IO_PGTABLE_NUM_FMTS,
  };
 

drivers/iommu/io-pgtable.h +1 -1

+               } arm_short_cfg;
         };
  };
 
--

1.8.1.1.dirty

Oops, seems I'm rather behind on things - I started this review on the RFC, but I'll finish it here...

Quoted 3 lines · Yong Wu

This patch is for ARM Short Descriptor Format.It has 2-levels pagetable and the allocator supports 4K/64K/1M/16M.

From the look of the code, this doesn't fully support partial unmaps
(i.e. splitting block entries), am I right? That's OK for DMA-API use, since that doesn't permit partial unmaps anyway, but I'd say it's worth making it clear that that's still a TODO in order for short-descriptor mappings to fully support arbitrary raw IOMMU API usage.

Quoted 56 lines

Signed-off-by Yong Wu <[email protected]>

---
  drivers/iommu/Kconfig                |   7 +
  drivers/iommu/Makefile               |   1 +
  drivers/iommu/io-pgtable-arm-short.c | 490 +++++++++++++++++++++++++++++++++++
  drivers/iommu/io-pgtable.c           |   4 +
  drivers/iommu/io-pgtable.h           |   6 +
  5 files changed, 508 insertions(+)

create mode 100644 drivers/iommu/io-pgtable-arm-short.c

drivers/iommu/Kconfig +7 -0

index 1ae4e54..3d2eac6 100644
@@ -39,6 +39,13 @@ config IOMMU_IO_PGTABLE_LPAE_SELFTEST
 
           If unsure, say N here.
 
+config IOMMU_IO_PGTABLE_SHORT
+       bool "ARMv7/v8 Short Descriptor Format"
+       select IOMMU_IO_PGTABLE
+       help
+         Enable support for the ARM Short descriptor pagetable format.
+         It has 2-levels pagetable and The allocator supports 4K/64K/1M/16M.
+
  endmenu
 
  config IOMMU_IOVA

drivers/iommu/Makefile +1 -0

index 080ffab..815b3c8 100644
@@ -3,6 +3,7 @@ obj-$(CONFIG_IOMMU_API) += iommu-traces.o
  obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
  obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
  obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
+obj-$(CONFIG_IOMMU_IO_PGTABLE_SHORT) += io-pgtable-arm-short.o
  obj-$(CONFIG_IOMMU_IOVA) += iova.o
  obj-$(CONFIG_OF_IOMMU) += of_iommu.o
  obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o msm_iommu_dev.o

drivers/iommu/io-pgtable-arm-short.c new file +9 -0

new file mode 100644
index 0000000..cc286ce5
@@ -0,0 +1,490 @@
+/*
+ * Copyright (c) 2014-2015 MediaTek Inc.
+ * Author: Yong Wu <[email protected]>
+ *
+ * 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,

drivers/iommu/io-pgtable-arm-short.c line 19

Quoted 10 lines

drivers/iommu/io-pgtable-arm-short.c new file +10 -0

+ * 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.
+ */
+#define pr_fmt(fmt)    "arm-short-desc io-pgtable: "fmt
+
+#include <linux/err.h>
+#include <linux/mm.h>
+#include <linux/iommu.h>
+#include <linux/errno.h>

Alphabetically-sorted includes, please. Also, this list doesn't look particularly correct - e.g. I don't think you're actually using anything from mm.h, but you are relying on stuff from kernel.h, slab.h, gfp.h, etc. being pulled in indirectly.

Quoted 13 lines

drivers/iommu/io-pgtable-arm-short.c new file +13 -0

+
+#include "io-pgtable.h"
+
+typedef u32 arm_short_iopte;
+
+struct arm_short_io_pgtable {
+       struct io_pgtable       iop;
+       struct kmem_cache       *ptekmem;
+       size_t                  pgd_size;
+       void                    *pgd;
+};
+
+#define io_pgtable_short_to_data(x)                            \

drivers/iommu/io-pgtable-arm-short.c line 36

Quoted 4 lines

drivers/iommu/io-pgtable-arm-short.c new file +4 -0

+       container_of((x), struct arm_short_io_pgtable, iop)
+
+#define io_pgtable_ops_to_pgtable(x)                           \
+       container_of((x), struct io_pgtable, ops)

This macro may as well be factored out into io-pgtable.h before duplication spreads any further. I don't see any reason for it not to live alongside the definition of struct io_pgtable, anyway.

Quoted 15 lines

drivers/iommu/io-pgtable-arm-short.c new file +14 -0

+
+#define io_pgtable_short_ops_to_data(x)                                \
+       io_pgtable_short_to_data(io_pgtable_ops_to_pgtable(x))
+
+#define ARM_SHORT_MAX_ADDR_BITS                        32
+
+#define ARM_SHORT_PGDIR_SHIFT                  20
+#define ARM_SHORT_PAGE_SHIFT                   12
+#define ARM_SHORT_PTRS_PER_PTE                 256
+#define ARM_SHORT_BYTES_PER_PTE                        1024
+
+/* 1 level pagetable */
+#define ARM_SHORT_F_PGD_TYPE_PAGE              (0x1)
+#define ARM_SHORT_F_PGD_TYPE_PAGE_MSK          (0x3)

drivers/iommu/io-pgtable-arm-short.c line 54

Quoted 4 lines

drivers/iommu/io-pgtable-arm-short.c new file +4 -0

+#define ARM_SHORT_F_PGD_TYPE_SECTION           (0x2)
+#define ARM_SHORT_F_PGD_TYPE_SUPERSECTION      (0x2 | (1 << 18))
+#define ARM_SHORT_F_PGD_TYPE_SECTION_MSK       (0x3 | (1 << 18))
+#define ARM_SHORT_F_PGD_TYPE_IS_PAGE(pgd)      (((pgd) & 0x3) == 1)

This confused me on first glance looking at the places it's used, because it's not actually referring to a thing which is a page. Maybe ..._IS_TABLE would be a better name?

Quoted 50 lines

drivers/iommu/io-pgtable-arm-short.c new file +49 -0

+#define ARM_SHORT_F_PGD_TYPE_IS_SECTION(pgd)           \
+       (((pgd) & ARM_SHORT_F_PGD_TYPE_SECTION_MSK)     \
+               == ARM_SHORT_F_PGD_TYPE_SECTION)
+#define ARM_SHORT_F_PGD_TYPE_IS_SUPERSECTION(pgd)      \
+       (((pgd) & ARM_SHORT_F_PGD_TYPE_SECTION_MSK)     \
+               == ARM_SHORT_F_PGD_TYPE_SUPERSECTION)
+
+#define ARM_SHORT_F_PGD_B_BIT                  BIT(2)
+#define ARM_SHORT_F_PGD_C_BIT                  BIT(3)
+#define ARM_SHORT_F_PGD_IMPLE_BIT              BIT(9)
+#define ARM_SHORT_F_PGD_S_BIT                  BIT(16)
+#define ARM_SHORT_F_PGD_NG_BIT                 BIT(17)
+#define ARM_SHORT_F_PGD_NS_BIT_PAGE            BIT(3)
+#define ARM_SHORT_F_PGD_NS_BIT_SECTION         BIT(19)
+
+#define ARM_SHORT_F_PGD_PA_PAGETABLE_MSK       0xfffffc00
+#define ARM_SHORT_F_PGD_PA_SECTION_MSK         0xfff00000
+#define ARM_SHORT_F_PGD_PA_SUPERSECTION_MSK    0xff000000
+
+/* 2 level pagetable */
+#define ARM_SHORT_F_PTE_TYPE_GET(val)          ((val) & 0x3)
+#define ARM_SHORT_F_PTE_TYPE_LARGE             BIT(0)
+#define ARM_SHORT_F_PTE_TYPE_SMALL             BIT(1)
+#define ARM_SHORT_F_PTE_B_BIT                  BIT(2)
+#define ARM_SHORT_F_PTE_C_BIT                  BIT(3)
+#define ARM_SHORT_F_PTE_IMPLE_BIT              BIT(9)
+#define ARM_SHORT_F_PTE_S_BIT                  BIT(10)
+#define ARM_SHORT_F_PTE_PA_LARGE_MSK            0xffff0000
+#define ARM_SHORT_F_PTE_PA_SMALL_MSK            0xfffff000
+
+#define ARM_SHORT_PGD_IDX(a)                   ((a) >> ARM_SHORT_PGDIR_SHIFT)
+#define ARM_SHORT_PTE_IDX(a)                   \
+       (((a) >> ARM_SHORT_PAGE_SHIFT) & 0xff)
+#define ARM_SHORT_GET_PTE_VA(pgd)              \
+       (phys_to_virt((unsigned long)pgd & ARM_SHORT_F_PGD_PA_PAGETABLE_MSK))
+
+static arm_short_iopte *
+arm_short_get_pte_in_pgd(arm_short_iopte curpgd, unsigned int iova)
+{
+       arm_short_iopte *pte;
+
+       pte = ARM_SHORT_GET_PTE_VA(curpgd);
+       pte += ARM_SHORT_PTE_IDX(iova);
+       return pte;
+}
+
+static arm_short_iopte *
+arm_short_supersection_start(arm_short_iopte *pgd)
+{

drivers/iommu/io-pgtable-arm-short.c line 108

Quoted 5 lines

drivers/iommu/io-pgtable-arm-short.c new file +5 -0

+       return (arm_short_iopte *)(round_down((unsigned long)pgd, (16 * 4)));
+}
+
+static int _arm_short_check_free_pte(struct arm_short_io_pgtable *data,
+                                    arm_short_iopte *pgd)

Given that this is only returning success/failure, it should probably be bool rather than int.

Quoted 58 lines

drivers/iommu/io-pgtable-arm-short.c new file +57 -0

+{
+       arm_short_iopte *pte;
+       int i;
+
+       pte = ARM_SHORT_GET_PTE_VA(*pgd);
+
+       for (i = 0; i < ARM_SHORT_PTRS_PER_PTE; i++) {
+               if (pte[i] != 0)
+                       return 1;
+       }
+
+       /* Free PTE */
+       kmem_cache_free(data->ptekmem, pte);
+       *pgd = 0;
+
+       return 0;
+}
+
+static phys_addr_t arm_short_iova_to_phys(struct io_pgtable_ops *ops,
+                                         unsigned long iova)
+{
+       struct arm_short_io_pgtable *data = io_pgtable_short_ops_to_data(ops);
+       arm_short_iopte *pte, *pgd = data->pgd;
+       phys_addr_t pa = 0;
+
+       pgd += ARM_SHORT_PGD_IDX(iova);
+
+       if (ARM_SHORT_F_PGD_TYPE_IS_PAGE(*pgd)) {
+               u8 pte_type;
+
+               pte = arm_short_get_pte_in_pgd(*pgd, iova);
+               pte_type = ARM_SHORT_F_PTE_TYPE_GET(*pte);
+
+               if (pte_type == ARM_SHORT_F_PTE_TYPE_LARGE) {
+                       pa = (*pte) & ARM_SHORT_F_PTE_PA_LARGE_MSK;
+                       pa |= iova & (~ARM_SHORT_F_PTE_PA_LARGE_MSK);
+               } else if (pte_type == ARM_SHORT_F_PTE_TYPE_SMALL) {
+                       pa = (*pte) & ARM_SHORT_F_PTE_PA_SMALL_MSK;
+                       pa |= iova & (~ARM_SHORT_F_PTE_PA_SMALL_MSK);
+               }
+       } else {
+               if (ARM_SHORT_F_PGD_TYPE_IS_SECTION(*pgd)) {
+                       pa = (*pgd) & ARM_SHORT_F_PGD_PA_SECTION_MSK;
+                       pa |= iova & (~ARM_SHORT_F_PGD_PA_SECTION_MSK);
+               } else if (ARM_SHORT_F_PGD_TYPE_IS_SUPERSECTION(*pgd)) {
+                       pa = (*pgd) & ARM_SHORT_F_PGD_PA_SUPERSECTION_MSK;
+                       pa |= iova & (~ARM_SHORT_F_PGD_PA_SUPERSECTION_MSK);
+               }
+       }
+
+       return pa;
+}
+
+static int arm_short_unmap(struct io_pgtable_ops *ops, unsigned long iova,
+                          size_t size)
+{
+       struct arm_short_io_pgtable *data = io_pgtable_short_ops_to_data(ops);

drivers/iommu/io-pgtable-arm-short.c line 168

Quoted 3 lines

drivers/iommu/io-pgtable-arm-short.c new file +3 -0

+       arm_short_iopte *pgd;
+       unsigned long iova_start = iova;
+       unsigned long long end_plus_1 = iova + size;

Since everything's at page granularity, working with IOVA PFNs rather than raw addresses might be more convenient, and also sidesteps the 32-bit overflow problem. On 64-bit platforms, we're wasting a whole 95 bits of a long long here ;)

Quoted 11 lines

drivers/iommu/io-pgtable-arm-short.c new file +10 -0

+       const struct iommu_gather_ops *tlb = data->iop.cfg.tlb;
+       void *cookie = data->iop.cookie;
+       int ret;
+
+       do {
+               pgd = (arm_short_iopte *)data->pgd + ARM_SHORT_PGD_IDX(iova);
+
+               if (ARM_SHORT_F_PGD_TYPE_IS_PAGE(*pgd)) {
+                       arm_short_iopte *pte;
+                       unsigned int pte_offset;

drivers/iommu/io-pgtable-arm-short.c line 183

Quoted 5 lines

drivers/iommu/io-pgtable-arm-short.c new file +5 -0

+                       unsigned int num_to_clean;
+
+                       pte_offset = ARM_SHORT_PTE_IDX(iova);
+                       num_to_clean =
+                           min((unsigned int)((end_plus_1 - iova) / PAGE_SIZE),

Shouldn't this be page size for the IOMMU, not the CPU? I'm a bit slow today, but this looks like it might go wrong when PAGE_SIZE > 4K.

Quoted 20 lines

drivers/iommu/io-pgtable-arm-short.c new file +19 -0

+                               (ARM_SHORT_PTRS_PER_PTE - pte_offset));
+
+                       pte = arm_short_get_pte_in_pgd(*pgd, iova);
+
+                       memset(pte, 0, num_to_clean * sizeof(arm_short_iopte));
+
+                       ret = _arm_short_check_free_pte(data, pgd);
+                       if (ret == 1)/* pte is not freed, need to flush pte */
+                               tlb->flush_pgtable(
+                                       pte,
+                                       num_to_clean * sizeof(arm_short_iopte),
+                                       cookie);
+                       else
+                               tlb->flush_pgtable(pgd, sizeof(arm_short_iopte),
+                                                  cookie);
+
+                       iova += num_to_clean << PAGE_SHIFT;
+               } else if (ARM_SHORT_F_PGD_TYPE_IS_SECTION(*pgd)) {
+                       *pgd = 0;

drivers/iommu/io-pgtable-arm-short.c line 212

Quoted 10 lines

drivers/iommu/io-pgtable-arm-short.c new file +10 -0

+
+                       tlb->flush_pgtable(pgd, sizeof(arm_short_iopte),
+                                          cookie);
+                       iova += SZ_1M;
+               } else if (ARM_SHORT_F_PGD_TYPE_IS_SUPERSECTION(*pgd)) {
+                       arm_short_iopte *start;
+
+                       start = arm_short_supersection_start(pgd);
+                       if (unlikely(start != pgd))
+                               pr_warn("%s:suppersection start isn't aligned.iova=0x%lx,pgd=0x%x\n",

Nit: typo in "supersection" here.

drivers/iommu/io-pgtable-arm-short.c line 220

Quoted 8 lines

drivers/iommu/io-pgtable-arm-short.c new file +8 -0

+                                       __func__, iova, *pgd);
+
+                       memset(start, 0, 16 * sizeof(arm_short_iopte));
+
+                       tlb->flush_pgtable(start, 16 * sizeof(arm_short_iopte),
+                                          cookie);
+
+                       iova = (iova + SZ_16M) & (~(SZ_16M - 1));

iova = ALIGN(iova + SZ_16M, SZ_16M);

Except that being unaligned in the first place is an error condition, so it would make more sense to just have "iova += SZ_16M;" here, and put "iova = ALIGN(iova, SZ_16M) after the warning in the error path above.

Since you don't handle splitting block mappings, and you also seem to be missing an equivalent warning for unaligned second-level large pages, it might be better to simply return an error if the requested size and alignment don't exactly match the type of entry found, rather than let the page tables get into an unexpectedly inconsistent state.

drivers/iommu/io-pgtable-arm-short.c line 231

Quoted 12 lines

drivers/iommu/io-pgtable-arm-short.c new file +11 -0

+               } else {
+                       break;
+               }
+       } while (iova < end_plus_1 && iova);
+
+       tlb->tlb_add_flush(iova_start, size, true, cookie);
+
+       return 0;
+}
+
+static arm_short_iopte __arm_short_pte_port(unsigned int prot, bool large)

I assume _port is a typo of _prot

Quoted 5 lines

drivers/iommu/io-pgtable-arm-short.c new file +5 -0

+{
+       arm_short_iopte pteprot;
+
+       pteprot = ARM_SHORT_F_PTE_S_BIT;
+

drivers/iommu/io-pgtable-arm-short.c line 246

Quoted 10 lines

drivers/iommu/io-pgtable-arm-short.c new file +10 -0

+       pteprot |= large ? ARM_SHORT_F_PTE_TYPE_LARGE :
+                               ARM_SHORT_F_PTE_TYPE_SMALL;
+
+       if (prot & IOMMU_CACHE)
+               pteprot |=  ARM_SHORT_F_PTE_B_BIT | ARM_SHORT_F_PTE_C_BIT;
+
+       return pteprot;
+}
+
+static arm_short_iopte __arm_short_pgd_port(int prot, bool super)

Ditto

Quoted 19 lines

drivers/iommu/io-pgtable-arm-short.c new file +19 -0

+{
+       arm_short_iopte pgdprot;
+
+       pgdprot = ARM_SHORT_F_PGD_S_BIT;
+       pgdprot |= super ? ARM_SHORT_F_PGD_TYPE_SUPERSECTION :
+                               ARM_SHORT_F_PGD_TYPE_SECTION;
+       if (prot & IOMMU_CACHE)
+               pgdprot |= ARM_SHORT_F_PGD_C_BIT | ARM_SHORT_F_PGD_B_BIT;
+
+       return pgdprot;
+}
+
+static int _arm_short_map_page(struct arm_short_io_pgtable *data,
+                              unsigned int iova, phys_addr_t pa,
+                              unsigned int prot, bool largepage)
+{
+       arm_short_iopte *pgd = data->pgd;
+       arm_short_iopte *pte;
+       arm_short_iopte pgdprot, pteprot;

drivers/iommu/io-pgtable-arm-short.c line 275

Quoted 10 lines

drivers/iommu/io-pgtable-arm-short.c new file +10 -0

+       arm_short_iopte mask = largepage ? ARM_SHORT_F_PTE_PA_LARGE_MSK :
+                                               ARM_SHORT_F_PTE_PA_SMALL_MSK;
+       int i, ptenum = largepage ? 16 : 1;
+       bool ptenew = false;
+       void *pte_new_va;
+       void *cookie = data->iop.cookie;
+
+       if ((iova | pa) & (~mask)) {
+               pr_err("IOVA|PA Not Aligned(iova=0x%x pa=0x%pa type=%s)\n",
+                      iova, &pa, largepage ? "large page" : "small page");

Nit: you may as well just have largepage ? "large" : "small" here and "...type=%s page..." in the format string.

drivers/iommu/io-pgtable-arm-short.c line 288

Quoted 13 lines

drivers/iommu/io-pgtable-arm-short.c new file +13 -0

+               return -EINVAL;
+       }
+
+       pgdprot = ARM_SHORT_F_PGD_TYPE_PAGE;
+       if (data->iop.cfg.quirks & IO_PGTABLE_QUIRK_ARM_NS)
+               pgdprot |= ARM_SHORT_F_PGD_NS_BIT_PAGE;
+
+       pgd += ARM_SHORT_PGD_IDX(iova);
+
+       if (!(*pgd)) {
+               pte_new_va = kmem_cache_zalloc(data->ptekmem, GFP_KERNEL);
+               if (unlikely(!pte_new_va)) {
+                       pr_err("Failed to alloc pte\n");

The allocator should already print the details of a failure, so I don't think this adds much.

drivers/iommu/io-pgtable-arm-short.c line 299

Quoted 11 lines

drivers/iommu/io-pgtable-arm-short.c new file +11 -0

+                       return -ENOMEM;
+               }
+
+               /* Check pte alignment -- must 1K align */
+               if (unlikely((unsigned long)pte_new_va &
+                            (ARM_SHORT_BYTES_PER_PTE - 1))) {
+                       pr_err("The new pte is not aligned! (va=0x%p)\n",
+                              pte_new_va);
+                       kmem_cache_free(data->ptekmem, (void *)pte_new_va);
+                       return -ENOMEM;
+               }

Can this ever actually happen? Besides, if kmem_cache_alloc isn't honouring the alignment specified in kmem_cache_create, I think the kernel might have bigger problems anyway...

Quoted 18 lines

drivers/iommu/io-pgtable-arm-short.c new file +17 -0

+               ptenew = true;
+               *pgd = virt_to_phys(pte_new_va) | pgdprot;
+               kmemleak_ignore(pte_new_va);
+               data->iop.cfg.tlb->flush_pgtable(pgd, sizeof(arm_short_iopte),
+                                                cookie);
+       } else {
+               /* Someone else may have allocated for this pgd */
+               if (((*pgd) & (~ARM_SHORT_F_PGD_PA_PAGETABLE_MSK)) != pgdprot) {
+                       pr_err("The prot of old pgd is not Right!iova=0x%x pgd=0x%x pgprot=0x%x\n",
+                              iova, (*pgd), pgdprot);
+                       return -EEXIST;
+               }
+       }
+
+       pteprot = (arm_short_iopte)pa;
+       pteprot |= __arm_short_pte_port(prot, largepage);
+

Quoted 17 lines

drivers/iommu/io-pgtable-arm-short.c new file +17 -0

+static int arm_short_map(struct io_pgtable_ops *ops, unsigned long iova,
+                        phys_addr_t paddr, size_t size, int prot)
+{
+       struct arm_short_io_pgtable *data = io_pgtable_short_ops_to_data(ops);
+       const struct iommu_gather_ops *tlb = data->iop.cfg.tlb;
+       int ret;
+
+       if (!(prot & (IOMMU_READ | IOMMU_WRITE)))
+               return -EINVAL;
+
+       if (size == SZ_4K) {/* most case */
+               ret = _arm_short_map_page(data, iova, paddr, prot, false);
+       } else if (size == SZ_64K) {
+               ret = _arm_short_map_page(data, iova, paddr, prot, true);
+       } else if (size == SZ_1M) {
+               ret = _arm_short_map_section(data, iova, paddr, prot, false);
+       } else if (size == SZ_16M) {

drivers/iommu/io-pgtable-arm-short.c line 414

Quoted 4 lines

drivers/iommu/io-pgtable-arm-short.c new file +4 -0

+               ret = _arm_short_map_section(data, iova, paddr, prot, true);
+       } else {
+               ret = -EINVAL;
+       }

I think this might be nicer as a switch statement. You could perhaps move the add_flush beforehand (since it's unconditional here anyway) and get rid of ret altogether.

Alternatively, given that map_page and map_section are so similar, maybe it's worth sorting out the pgprot and pgd/pte pointer for the page/section distinction here, then just passing those to a single function which maps compound/non-compound leaf entries.

Quoted 20 lines

drivers/iommu/io-pgtable-arm-short.c new file +19 -0

+       tlb->tlb_add_flush(iova, size, true, data->iop.cookie);
+       return ret;
+}
+
+static struct io_pgtable *
+arm_short_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie)
+{
+       struct arm_short_io_pgtable *data;
+
+       if (cfg->ias != 32)
+               return NULL;
+
+       if (cfg->oas > ARM_SHORT_MAX_ADDR_BITS)
+               return NULL;
+
+       cfg->pgsize_bitmap &= SZ_4K | SZ_64K | SZ_1M | SZ_16M;
+
+       data = kzalloc(sizeof(*data), GFP_KERNEL);
+       if (!data)

drivers/iommu/io-pgtable-arm-short.c line 438

Quoted 5 lines

drivers/iommu/io-pgtable-arm-short.c new file +5 -0

+               return NULL;
+
+       data->pgd_size = SZ_16K;
+
+       data->pgd = alloc_pages_exact(data->pgd_size, GFP_KERNEL | __GFP_ZERO);

I think this needs __GFP_DMA too, to ensure the pgd lies below the 4GB boundary on arm64/LPAE systems with memory above that.

Quoted 5 lines

drivers/iommu/io-pgtable-arm-short.c new file +1 -0

@@ -62,6 +63,11 @@ struct io_pgtable_cfg {
                         u64     vttbr;
                         u64     vtcr;
                 } arm_lpae_s2_cfg;
+

drivers/iommu/io-pgtable-arm-short.c struct io_pgtable_cfg { · line 69

Quoted 3 lines

drivers/iommu/io-pgtable-arm-short.c new file +3 -0

+               struct {
+                       u64     ttbr[2];
+                       u64     tcr;

The ARM ARM defines these all as 32-bit registers for the short-descriptor format, so I think u32 would be more appropriate - better to let the compiler truncate things and warn about it, than have the hardware do it silently at runtime ;)

Quoted 8 lines

drivers/iommu/io-pgtable-arm-short.c new file +1 -1

+               } arm_short_cfg;
         };
  };
 
--

1.8.1.1.dirty

This patch is for ARM Short Descriptor Format.It has 2-levels pagetable and the allocator supports 4K/64K/1M/16M.

Signed-off-by Yong Wu <[email protected]>

---
 drivers/iommu/Kconfig                |   7 +
 drivers/iommu/Makefile               |   1 +
 drivers/iommu/io-pgtable-arm-short.c | 490 +++++++++++++++++++++++++++++++++++
 drivers/iommu/io-pgtable.c           |   4 +
 drivers/iommu/io-pgtable.h           |   6 +
 5 files changed, 508 insertions(+)
 create mode 100644 drivers/iommu/io-pgtable-arm-short.c

drivers/iommu/Kconfig +1 -0

+
 endmenu
 
 config IOMMU_IOVA

drivers/iommu/io-pgtable-arm-short.c new file +30 -0

new file mode 100644
index 0000000..cc286ce5
@@ -0,0 +1,490 @@
+/*
+ * Copyright (c) 2014-2015 MediaTek Inc.
+ * Author: Yong Wu <[email protected]>
+ *
+ * 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.
+ */
+#define pr_fmt(fmt)    "arm-short-desc io-pgtable: "fmt
+
+#include <linux/err.h>
+#include <linux/mm.h>
+#include <linux/iommu.h>
+#include <linux/errno.h>
+
+#include "io-pgtable.h"
+
+typedef u32 arm_short_iopte;
+
+struct arm_short_io_pgtable {
+       struct io_pgtable       iop;
+       struct kmem_cache       *ptekmem;
+       size_t                  pgd_size;
+       void                    *pgd;
+};

drivers/iommu/io-pgtable-arm-short.c new file +12 -0

+#define ARM_SHORT_F_PGD_TYPE_IS_SECTION(pgd)           \
+       (((pgd) & ARM_SHORT_F_PGD_TYPE_SECTION_MSK)     \
+               == ARM_SHORT_F_PGD_TYPE_SECTION)
+#define ARM_SHORT_F_PGD_TYPE_IS_SUPERSECTION(pgd)      \
+       (((pgd) & ARM_SHORT_F_PGD_TYPE_SECTION_MSK)     \
+               == ARM_SHORT_F_PGD_TYPE_SUPERSECTION)
+
+#define ARM_SHORT_F_PGD_B_BIT                  BIT(2)
+#define ARM_SHORT_F_PGD_C_BIT                  BIT(3)
+#define ARM_SHORT_F_PGD_IMPLE_BIT              BIT(9)
+#define ARM_SHORT_F_PGD_S_BIT                  BIT(16)
+#define ARM_SHORT_F_PGD_NG_BIT                 BIT(17)

drivers/iommu/io-pgtable-arm-short.c new file +8 -0

+#define ARM_SHORT_GET_PTE_VA(pgd)              \
+       (phys_to_virt((unsigned long)pgd & ARM_SHORT_F_PGD_PA_PAGETABLE_MSK))
+
+static arm_short_iopte *
+arm_short_get_pte_in_pgd(arm_short_iopte curpgd, unsigned int iova)
+{
+       arm_short_iopte *pte;
+

drivers/iommu/io-pgtable-arm-short.c new file +31 -0

+
+       return 0;
+}
+
+static int arm_short_unmap(struct io_pgtable_ops *ops, unsigned long iova,
+                          size_t size)
+{
+       struct arm_short_io_pgtable *data = io_pgtable_short_ops_to_data(ops);
+       arm_short_iopte *pgd;
+       unsigned long iova_start = iova;
+       unsigned long long end_plus_1 = iova + size;
+       const struct iommu_gather_ops *tlb = data->iop.cfg.tlb;
+       void *cookie = data->iop.cookie;
+       int ret;
+
+       do {
+               pgd = (arm_short_iopte *)data->pgd + ARM_SHORT_PGD_IDX(iova);
+
+               if (ARM_SHORT_F_PGD_TYPE_IS_PAGE(*pgd)) {
+                       arm_short_iopte *pte;
+                       unsigned int pte_offset;
+                       unsigned int num_to_clean;
+
+                       pte_offset = ARM_SHORT_PTE_IDX(iova);
+                       num_to_clean =
+                           min((unsigned int)((end_plus_1 - iova) / PAGE_SIZE),
+                               (ARM_SHORT_PTRS_PER_PTE - pte_offset));
+
+                       pte = arm_short_get_pte_in_pgd(*pgd, iova);
+
+                       memset(pte, 0, num_to_clean * sizeof(arm_short_iopte));

drivers/iommu/io-pgtable-arm-short.c new file +4 -0

+               } else if (ARM_SHORT_F_PGD_TYPE_IS_SUPERSECTION(*pgd)) {
+                       arm_short_iopte *start;
+
+                       start = arm_short_supersection_start(pgd);

drivers/iommu/io-pgtable-arm-short.c new file +47 -0

+       return pteprot;
+}
+
+static arm_short_iopte __arm_short_pgd_port(int prot, bool super)
+{
+       arm_short_iopte pgdprot;
+
+       pgdprot = ARM_SHORT_F_PGD_S_BIT;
+       pgdprot |= super ? ARM_SHORT_F_PGD_TYPE_SUPERSECTION :
+                               ARM_SHORT_F_PGD_TYPE_SECTION;
+       if (prot & IOMMU_CACHE)
+               pgdprot |= ARM_SHORT_F_PGD_C_BIT | ARM_SHORT_F_PGD_B_BIT;
+
+       return pgdprot;
+}
+
+static int _arm_short_map_page(struct arm_short_io_pgtable *data,
+                              unsigned int iova, phys_addr_t pa,
+                              unsigned int prot, bool largepage)
+{
+       arm_short_iopte *pgd = data->pgd;
+       arm_short_iopte *pte;
+       arm_short_iopte pgdprot, pteprot;
+       arm_short_iopte mask = largepage ? ARM_SHORT_F_PTE_PA_LARGE_MSK :
+                                               ARM_SHORT_F_PTE_PA_SMALL_MSK;
+       int i, ptenum = largepage ? 16 : 1;
+       bool ptenew = false;
+       void *pte_new_va;
+       void *cookie = data->iop.cookie;
+
+       if ((iova | pa) & (~mask)) {
+               pr_err("IOVA|PA Not Aligned(iova=0x%x pa=0x%pa type=%s)\n",
+                      iova, &pa, largepage ? "large page" : "small page");
+               return -EINVAL;
+       }
+
+       pgdprot = ARM_SHORT_F_PGD_TYPE_PAGE;
+       if (data->iop.cfg.quirks & IO_PGTABLE_QUIRK_ARM_NS)
+               pgdprot |= ARM_SHORT_F_PGD_NS_BIT_PAGE;
+
+       pgd += ARM_SHORT_PGD_IDX(iova);
+
+       if (!(*pgd)) {
+               pte_new_va = kmem_cache_zalloc(data->ptekmem, GFP_KERNEL);
+               if (unlikely(!pte_new_va)) {
+                       pr_err("Failed to alloc pte\n");
+                       return -ENOMEM;

drivers/iommu/io-pgtable-arm-short.c new file +11 -0

+                       return -EEXIST;
+               }
+       }
+
+       pteprot = (arm_short_iopte)pa;
+       pteprot |= __arm_short_pte_port(prot, largepage);
+
+       pte = arm_short_get_pte_in_pgd(*pgd, iova);
+
+       pr_debug("iova:0x%x,pte:0x%p(0x%x),prot:0x%x-%s\n",
+                iova, pte, ARM_SHORT_PTE_IDX(iova), pteprot,

drivers/iommu/io-pgtable-arm-short.c new file +42 -0

+
+       data->iop.cfg.tlb->flush_pgtable(pte, ptenum * sizeof(arm_short_iopte),
+                                        cookie);
+       return 0;
+
+ err_out:
+       for (i--; i >= 0; i--)
+               pte[i] = 0;
+       if (ptenew)
+               kmem_cache_free(data->ptekmem, pte_new_va);
+       return -EEXIST;
+}
+
+static int _arm_short_map_section(struct arm_short_io_pgtable *data,
+                                 unsigned int iova, phys_addr_t pa,
+                                 int prot, bool supersection)
+{
+       arm_short_iopte pgprot;
+       arm_short_iopte mask = supersection ?
+                               ARM_SHORT_F_PGD_PA_SUPERSECTION_MSK :
+                               ARM_SHORT_F_PGD_PA_SECTION_MSK;
+       arm_short_iopte *pgd = data->pgd;
+       int i;
+       unsigned int pgdnum = supersection ? 16 : 1;
+
+       if ((iova | pa) & (~mask)) {
+               pr_err("IOVA|PA Not Aligned(iova=0x%x pa=0x%pa type=%s)\n",
+                      iova, &pa, supersection ? "supersection" : "section");
+               return -EINVAL;
+       }
+
+       pgprot = (arm_short_iopte)pa;
+
+       if (data->iop.cfg.quirks & IO_PGTABLE_QUIRK_ARM_NS)
+               pgprot |= ARM_SHORT_F_PGD_NS_BIT_SECTION;
+
+       pgprot |= __arm_short_pgd_port(prot, supersection);
+
+       pgd += ARM_SHORT_PGD_IDX(iova);
+
+       pr_debug("iova:0x%x,pgd:0x%p(0x%p+0x%x),value:0x%x-%s\n",
+                iova, pgd, data->pgd, ARM_SHORT_PGD_IDX(iova),

drivers/iommu/io-pgtable-arm-short.c new file +10 -0

+       data->iop.cfg.tlb->flush_pgtable(pgd,
+                                        pgdnum * sizeof(arm_short_iopte),
+                                        data->iop.cookie);
+       return 0;
+
+ err_out:
+       for (i--; i >= 0; i--)
+               pgd[i] = 0;
+       return -EEXIST;
+}

drivers/iommu/io-pgtable-arm-short.c new file +38 -0

+               goto out_free_pte;
+       }
+
+       /* TTBRs */
+       cfg->arm_short_cfg.ttbr[0] = virt_to_phys(data->pgd);
+       cfg->arm_short_cfg.ttbr[1] = 0;
+
+       cfg->arm_short_cfg.tcr = 0;
+
+       data->iop.ops = (struct io_pgtable_ops) {
+               .map            = arm_short_map,
+               .unmap          = arm_short_unmap,
+               .iova_to_phys   = arm_short_iova_to_phys,
+       };
+
+       return &data->iop;
+
+out_free_pte:
+       free_pages_exact(data->pgd, data->pgd_size);
+out_free_data:
+       kfree(data);
+       return NULL;
+}
+
+static void arm_short_free_pgtable(struct io_pgtable *iop)
+{
+       struct arm_short_io_pgtable *data = io_pgtable_short_to_data(iop);
+
+       kmem_cache_destroy(data->ptekmem);
+       free_pages_exact(data->pgd, data->pgd_size);
+       kfree(data);
+}
+
+struct io_pgtable_init_fns io_pgtable_arm_short_init_fns = {
+       .alloc  = arm_short_alloc_pgtable,
+       .free   = arm_short_free_pgtable,
+};
+

drivers/iommu/io-pgtable.c +4 -0

index 6436fe2..14a9b3a 100644
@@ -28,6 +28,7 @@ extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s1_init_fns;
 extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s2_init_fns;
 extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns;
 extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns;
+extern struct io_pgtable_init_fns io_pgtable_arm_short_init_fns;
 
 static const struct io_pgtable_init_fns *
 io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] =

@@ -38,6 +39,9 @@ io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] =
        [ARM_64_LPAE_S1] = &io_pgtable_arm_64_lpae_s1_init_fns,
        [ARM_64_LPAE_S2] = &io_pgtable_arm_64_lpae_s2_init_fns,
 #endif
+#ifdef CONFIG_IOMMU_IO_PGTABLE_SHORT
+       [ARM_SHORT_DESC] = &io_pgtable_arm_short_init_fns,
+#endif
 };
 
 struct io_pgtable_ops *alloc_io_pgtable_ops(enum io_pgtable_fmt fmt,

drivers/iommu/io-pgtable.h +1 -0

index 10e32f6..47efaab 100644
@@ -9,6 +9,7 @@ enum io_pgtable_fmt {
        ARM_32_LPAE_S2,
        ARM_64_LPAE_S1,
        ARM_64_LPAE_S2,
+       ARM_SHORT_DESC,
        IO_PGTABLE_NUM_FMTS,
 };
 

Hello,

Thanks for the patch, it's good to see another user of the generic IO page-table code. However, I have quite a lot of comments on the code.

Quoted 4 lines · Yong Wu

This patch is for ARM Short Descriptor Format.It has 2-levels pagetable and the allocator supports 4K/64K/1M/16M.

Signed-off-by Yong Wu <[email protected]>

Quoted 8 lines

---
 drivers/iommu/Kconfig                |   7 +
 drivers/iommu/Makefile               |   1 +
 drivers/iommu/io-pgtable-arm-short.c | 490 +++++++++++++++++++++++++++++++++++
 drivers/iommu/io-pgtable.c           |   4 +
 drivers/iommu/io-pgtable.h           |   6 +
 5 files changed, 508 insertions(+)
 create mode 100644 drivers/iommu/io-pgtable-arm-short.c

For some reason, I ended up reviewing this back-to-front (i.e. starting with the init code), so apologies if the comments feel like they were written in reverse.

Quoted 13 lines

drivers/iommu/io-pgtable-arm-short.c new file +13 -0

+typedef u32 arm_short_iopte;
+
+struct arm_short_io_pgtable {
+       struct io_pgtable       iop;
+       struct kmem_cache       *ptekmem;
+       size_t                  pgd_size;
+       void                    *pgd;
+};
+
+#define io_pgtable_short_to_data(x)                            \
+       container_of((x), struct arm_short_io_pgtable, iop)
+
+#define io_pgtable_ops_to_pgtable(x)                           \

drivers/iommu/io-pgtable-arm-short.c line 40

Quoted 5 lines

drivers/iommu/io-pgtable-arm-short.c new file +5 -0

+       container_of((x), struct io_pgtable, ops)
+
+#define io_pgtable_short_ops_to_data(x)                                \
+       io_pgtable_short_to_data(io_pgtable_ops_to_pgtable(x))
+

These are private macros, so I think you can drop the "short" part to, err, keep them short.

drivers/iommu/io-pgtable-arm-short.c line 46

Quoted 6 lines

drivers/iommu/io-pgtable-arm-short.c new file +6 -0

+#define ARM_SHORT_MAX_ADDR_BITS                        32
+
+#define ARM_SHORT_PGDIR_SHIFT                  20
+#define ARM_SHORT_PAGE_SHIFT                   12
+#define ARM_SHORT_PTRS_PER_PTE                 256
+#define ARM_SHORT_BYTES_PER_PTE                        1024

Isn't that ARM_SHORT_PTRS_PER_PTE * sizeof(arm_short_iopte)?

drivers/iommu/io-pgtable-arm-short.c line 49

Quoted 2 lines

drivers/iommu/io-pgtable-arm-short.c new file +2 -0

+/* 1 level pagetable */
+#define ARM_SHORT_F_PGD_TYPE_PAGE              (0x1)

I think you're using PAGE and PGTABLE interchangeably, which is really confusing to read.

Quoted 1 line

+#define ARM_SHORT_F_PGD_TYPE_PAGE_MSK (0x3)

This is the TYPE mask.

drivers/iommu/io-pgtable-arm-short.c line 52

Quoted 2 lines

drivers/iommu/io-pgtable-arm-short.c new file +2 -0

+#define ARM_SHORT_F_PGD_TYPE_SECTION           (0x2)
+#define ARM_SHORT_F_PGD_TYPE_SUPERSECTION      (0x2 | (1 << 18))

Are you sure this is correct? afaict, bit 0 is PXN, so you should actually be using bit 18 to distinguihs sections and supersections.

drivers/iommu/io-pgtable-arm-short.c line 54

Quoted 2 lines

drivers/iommu/io-pgtable-arm-short.c new file +2 -0

+#define ARM_SHORT_F_PGD_TYPE_SECTION_MSK       (0x3 | (1 << 18))
+#define ARM_SHORT_F_PGD_TYPE_IS_PAGE(pgd)      (((pgd) & 0x3) == 1)

Use your TYPE mask here.

Quoted 19 lines

drivers/iommu/io-pgtable-arm-short.c new file +19 -0

+#define ARM_SHORT_F_PGD_TYPE_IS_SECTION(pgd)           \
+       (((pgd) & ARM_SHORT_F_PGD_TYPE_SECTION_MSK)     \
+               == ARM_SHORT_F_PGD_TYPE_SECTION)
+#define ARM_SHORT_F_PGD_TYPE_IS_SUPERSECTION(pgd)      \
+       (((pgd) & ARM_SHORT_F_PGD_TYPE_SECTION_MSK)     \
+               == ARM_SHORT_F_PGD_TYPE_SUPERSECTION)
+
+#define ARM_SHORT_F_PGD_B_BIT                  BIT(2)
+#define ARM_SHORT_F_PGD_C_BIT                  BIT(3)
+#define ARM_SHORT_F_PGD_IMPLE_BIT              BIT(9)
+#define ARM_SHORT_F_PGD_S_BIT                  BIT(16)
+#define ARM_SHORT_F_PGD_NG_BIT                 BIT(17)
+#define ARM_SHORT_F_PGD_NS_BIT_PAGE            BIT(3)
+#define ARM_SHORT_F_PGD_NS_BIT_SECTION         BIT(19)
+
+#define ARM_SHORT_F_PGD_PA_PAGETABLE_MSK       0xfffffc00
+#define ARM_SHORT_F_PGD_PA_SECTION_MSK         0xfff00000
+#define ARM_SHORT_F_PGD_PA_SUPERSECTION_MSK    0xff000000
+

drivers/iommu/io-pgtable-arm-short.c line 117

Quoted 11 lines

drivers/iommu/io-pgtable-arm-short.c new file +11 -0

+static int _arm_short_check_free_pte(struct arm_short_io_pgtable *data,
+                                    arm_short_iopte *pgd)
+{
+       arm_short_iopte *pte;
+       int i;
+
+       pte = ARM_SHORT_GET_PTE_VA(*pgd);
+
+       for (i = 0; i < ARM_SHORT_PTRS_PER_PTE; i++) {
+               if (pte[i] != 0)
+                       return 1;

-EEXIST?

drivers/iommu/io-pgtable-arm-short.c line 122

Quoted 5 lines

drivers/iommu/io-pgtable-arm-short.c new file +5 -0

+       }
+
+       /* Free PTE */
+       kmem_cache_free(data->ptekmem, pte);
+       *pgd = 0;

I don't think this is safe, as there's a window where the page table walker can see the freed pte memory.

Quoted 31 lines

drivers/iommu/io-pgtable-arm-short.c new file +31 -0

+
+       return 0;
+}
+
+static int arm_short_unmap(struct io_pgtable_ops *ops, unsigned long iova,
+                          size_t size)
+{
+       struct arm_short_io_pgtable *data = io_pgtable_short_ops_to_data(ops);
+       arm_short_iopte *pgd;
+       unsigned long iova_start = iova;
+       unsigned long long end_plus_1 = iova + size;
+       const struct iommu_gather_ops *tlb = data->iop.cfg.tlb;
+       void *cookie = data->iop.cookie;
+       int ret;
+
+       do {
+               pgd = (arm_short_iopte *)data->pgd + ARM_SHORT_PGD_IDX(iova);
+
+               if (ARM_SHORT_F_PGD_TYPE_IS_PAGE(*pgd)) {
+                       arm_short_iopte *pte;
+                       unsigned int pte_offset;
+                       unsigned int num_to_clean;
+
+                       pte_offset = ARM_SHORT_PTE_IDX(iova);
+                       num_to_clean =
+                           min((unsigned int)((end_plus_1 - iova) / PAGE_SIZE),
+                               (ARM_SHORT_PTRS_PER_PTE - pte_offset));
+
+                       pte = arm_short_get_pte_in_pgd(*pgd, iova);
+
+                       memset(pte, 0, num_to_clean * sizeof(arm_short_iopte));

drivers/iommu/io-pgtable-arm-short.c line 198

Quoted 10 lines

drivers/iommu/io-pgtable-arm-short.c new file +10 -0

+
+                       ret = _arm_short_check_free_pte(data, pgd);
+                       if (ret == 1)/* pte is not freed, need to flush pte */
+                               tlb->flush_pgtable(
+                                       pte,
+                                       num_to_clean * sizeof(arm_short_iopte),
+                                       cookie);
+                       else
+                               tlb->flush_pgtable(pgd, sizeof(arm_short_iopte),
+                                                  cookie);

Hopefully this can be cleaned up when you remove the outer loop and you can use the size parameter to figure out which level to unmap.

drivers/iommu/io-pgtable-arm-short.c line 206

Quoted 7 lines

drivers/iommu/io-pgtable-arm-short.c new file +7 -0

+                       iova += num_to_clean << PAGE_SHIFT;
+               } else if (ARM_SHORT_F_PGD_TYPE_IS_SECTION(*pgd)) {
+                       *pgd = 0;
+
+                       tlb->flush_pgtable(pgd, sizeof(arm_short_iopte),
+                                          cookie);
+                       iova += SZ_1M;

Again, these sizes can be derived from other page table properties that you have.

Quoted 4 lines

drivers/iommu/io-pgtable-arm-short.c new file +4 -0

+               } else if (ARM_SHORT_F_PGD_TYPE_IS_SUPERSECTION(*pgd)) {
+                       arm_short_iopte *start;
+
+                       start = arm_short_supersection_start(pgd);

drivers/iommu/io-pgtable-arm-short.c line 220

Quoted 10 lines

drivers/iommu/io-pgtable-arm-short.c new file +10 -0

+                       if (unlikely(start != pgd))
+                               pr_warn("%s:suppersection start isn't aligned.iova=0x%lx,pgd=0x%x\n",
+                                       __func__, iova, *pgd);
+
+                       memset(start, 0, 16 * sizeof(arm_short_iopte));
+
+                       tlb->flush_pgtable(start, 16 * sizeof(arm_short_iopte),
+                                          cookie);
+
+                       iova = (iova + SZ_16M) & (~(SZ_16M - 1));

See later, but I think supersections should not be assumed by default.

drivers/iommu/io-pgtable-arm-short.c line 224

Quoted 4 lines

drivers/iommu/io-pgtable-arm-short.c new file +4 -0

+               } else {
+                       break;
+               }
+       } while (iova < end_plus_1 && iova);

I don't think you need this loop -- unmap will be called in page-sized chunks (where page-size refers to units as advertised in your IOMMU's pgsize_bitmap). The tricky part is when somebody unmaps a subset of a previous mapping that ended up using something like a section. You need to handle that here by splitting blocks at level 1 into a table and allocating a level 2.

drivers/iommu/io-pgtable-arm-short.c line 228

Quoted 5 lines

drivers/iommu/io-pgtable-arm-short.c new file +4 -0

+
+       tlb->tlb_add_flush(iova_start, size, true, cookie);
+
+       return 0;

You need to return the size of the region that you managed to unmap, so 0 isn't right here.

drivers/iommu/io-pgtable-arm-short.c line 241

Quoted 13 lines

drivers/iommu/io-pgtable-arm-short.c new file +13 -0

+}
+
+static arm_short_iopte __arm_short_pte_port(unsigned int prot, bool large)
+{
+       arm_short_iopte pteprot;
+
+       pteprot = ARM_SHORT_F_PTE_S_BIT;
+
+       pteprot |= large ? ARM_SHORT_F_PTE_TYPE_LARGE :
+                               ARM_SHORT_F_PTE_TYPE_SMALL;
+
+       if (prot & IOMMU_CACHE)
+               pteprot |=  ARM_SHORT_F_PTE_B_BIT | ARM_SHORT_F_PTE_C_BIT;

Where do you set TEX[0] for write-allocate?

Quoted 52 lines

drivers/iommu/io-pgtable-arm-short.c new file +51 -0

+       return pteprot;
+}
+
+static arm_short_iopte __arm_short_pgd_port(int prot, bool super)
+{
+       arm_short_iopte pgdprot;
+
+       pgdprot = ARM_SHORT_F_PGD_S_BIT;
+       pgdprot |= super ? ARM_SHORT_F_PGD_TYPE_SUPERSECTION :
+                               ARM_SHORT_F_PGD_TYPE_SECTION;
+       if (prot & IOMMU_CACHE)
+               pgdprot |= ARM_SHORT_F_PGD_C_BIT | ARM_SHORT_F_PGD_B_BIT;
+
+       return pgdprot;
+}
+
+static int _arm_short_map_page(struct arm_short_io_pgtable *data,
+                              unsigned int iova, phys_addr_t pa,
+                              unsigned int prot, bool largepage)
+{
+       arm_short_iopte *pgd = data->pgd;
+       arm_short_iopte *pte;
+       arm_short_iopte pgdprot, pteprot;
+       arm_short_iopte mask = largepage ? ARM_SHORT_F_PTE_PA_LARGE_MSK :
+                                               ARM_SHORT_F_PTE_PA_SMALL_MSK;
+       int i, ptenum = largepage ? 16 : 1;
+       bool ptenew = false;
+       void *pte_new_va;
+       void *cookie = data->iop.cookie;
+
+       if ((iova | pa) & (~mask)) {
+               pr_err("IOVA|PA Not Aligned(iova=0x%x pa=0x%pa type=%s)\n",
+                      iova, &pa, largepage ? "large page" : "small page");
+               return -EINVAL;
+       }
+
+       pgdprot = ARM_SHORT_F_PGD_TYPE_PAGE;
+       if (data->iop.cfg.quirks & IO_PGTABLE_QUIRK_ARM_NS)
+               pgdprot |= ARM_SHORT_F_PGD_NS_BIT_PAGE;
+
+       pgd += ARM_SHORT_PGD_IDX(iova);
+
+       if (!(*pgd)) {
+               pte_new_va = kmem_cache_zalloc(data->ptekmem, GFP_KERNEL);
+               if (unlikely(!pte_new_va)) {
+                       pr_err("Failed to alloc pte\n");
+                       return -ENOMEM;
+               }
+
+               /* Check pte alignment -- must 1K align */
+               if (unlikely((unsigned long)pte_new_va &

drivers/iommu/io-pgtable-arm-short.c line 299

Quoted 6 lines

drivers/iommu/io-pgtable-arm-short.c new file +6 -0

+                            (ARM_SHORT_BYTES_PER_PTE - 1))) {
+                       pr_err("The new pte is not aligned! (va=0x%p)\n",
+                              pte_new_va);
+                       kmem_cache_free(data->ptekmem, (void *)pte_new_va);
+                       return -ENOMEM;
+               }

How are you enforcing this alignment?

drivers/iommu/io-pgtable-arm-short.c line 302

Quoted 4 lines

drivers/iommu/io-pgtable-arm-short.c new file +3 -0

+               ptenew = true;
+               *pgd = virt_to_phys(pte_new_va) | pgdprot;
+               kmemleak_ignore(pte_new_va);

Maybe you should be using alloc_pages instead of your kmem_cache (I mention this again later on).

drivers/iommu/io-pgtable-arm-short.c line 309

Quoted 7 lines

drivers/iommu/io-pgtable-arm-short.c new file +7 -0

+               data->iop.cfg.tlb->flush_pgtable(pgd, sizeof(arm_short_iopte),
+                                                cookie);
+       } else {
+               /* Someone else may have allocated for this pgd */
+               if (((*pgd) & (~ARM_SHORT_F_PGD_PA_PAGETABLE_MSK)) != pgdprot) {
+                       pr_err("The prot of old pgd is not Right!iova=0x%x pgd=0x%x pgprot=0x%x\n",
+                              iova, (*pgd), pgdprot);

You can probably just WARN here, as I do in the LPAE code. It shows a bug in the caller of the API.

Quoted 11 lines

drivers/iommu/io-pgtable-arm-short.c new file +10 -0

+                       return -EEXIST;
+               }
+       }
+
+       pteprot = (arm_short_iopte)pa;
+       pteprot |= __arm_short_pte_port(prot, largepage);
+
+       pte = arm_short_get_pte_in_pgd(*pgd, iova);
+
+       pr_debug("iova:0x%x,pte:0x%p(0x%x),prot:0x%x-%s\n",

drivers/iommu/io-pgtable-arm-short.c line 327

Quoted 8 lines

drivers/iommu/io-pgtable-arm-short.c new file +8 -0

+                iova, pte, ARM_SHORT_PTE_IDX(iova), pteprot,
+                largepage ? "large page" : "small page");
+
+       for (i = 0; i < ptenum; i++) {
+               if (pte[i]) {
+                       pr_err("The To-Map pte exists!(iova=0x%x pte=0x%x i=%d)\n",
+                              iova, pte[i], i);
+                       goto err_out;

drivers/iommu/io-pgtable-arm-short.c line 330

Quoted 3 lines

drivers/iommu/io-pgtable-arm-short.c new file +3 -0

+               }
+               pte[i] = pteprot;
+       }

I don't think you need this loop; you should only be given a page size, like with unmap.

Quoted 47 lines

drivers/iommu/io-pgtable-arm-short.c new file +46 -0

+
+       data->iop.cfg.tlb->flush_pgtable(pte, ptenum * sizeof(arm_short_iopte),
+                                        cookie);
+       return 0;
+
+ err_out:
+       for (i--; i >= 0; i--)
+               pte[i] = 0;
+       if (ptenew)
+               kmem_cache_free(data->ptekmem, pte_new_va);
+       return -EEXIST;
+}
+
+static int _arm_short_map_section(struct arm_short_io_pgtable *data,
+                                 unsigned int iova, phys_addr_t pa,
+                                 int prot, bool supersection)
+{
+       arm_short_iopte pgprot;
+       arm_short_iopte mask = supersection ?
+                               ARM_SHORT_F_PGD_PA_SUPERSECTION_MSK :
+                               ARM_SHORT_F_PGD_PA_SECTION_MSK;
+       arm_short_iopte *pgd = data->pgd;
+       int i;
+       unsigned int pgdnum = supersection ? 16 : 1;
+
+       if ((iova | pa) & (~mask)) {
+               pr_err("IOVA|PA Not Aligned(iova=0x%x pa=0x%pa type=%s)\n",
+                      iova, &pa, supersection ? "supersection" : "section");
+               return -EINVAL;
+       }
+
+       pgprot = (arm_short_iopte)pa;
+
+       if (data->iop.cfg.quirks & IO_PGTABLE_QUIRK_ARM_NS)
+               pgprot |= ARM_SHORT_F_PGD_NS_BIT_SECTION;
+
+       pgprot |= __arm_short_pgd_port(prot, supersection);
+
+       pgd += ARM_SHORT_PGD_IDX(iova);
+
+       pr_debug("iova:0x%x,pgd:0x%p(0x%p+0x%x),value:0x%x-%s\n",
+                iova, pgd, data->pgd, ARM_SHORT_PGD_IDX(iova),
+                pgprot, supersection ? "supersection" : "section");
+
+       for (i = 0; i < pgdnum; i++) {
+               if (unlikely(*pgd)) {

drivers/iommu/io-pgtable-arm-short.c line 382

Quoted 6 lines

drivers/iommu/io-pgtable-arm-short.c new file +6 -0

+                       pr_err("The To-Map pdg exists!(iova=0x%x pgd=0x%x i=%d)\n",
+                              iova, pgd[i], i);
+                       goto err_out;
+               }
+               pgd[i] = pgprot;
+       }

Similar comments here.

Quoted 16 lines

drivers/iommu/io-pgtable-arm-short.c new file +15 -0

+       data->iop.cfg.tlb->flush_pgtable(pgd,
+                                        pgdnum * sizeof(arm_short_iopte),
+                                        data->iop.cookie);
+       return 0;
+
+ err_out:
+       for (i--; i >= 0; i--)
+               pgd[i] = 0;
+       return -EEXIST;
+}
+
+static int arm_short_map(struct io_pgtable_ops *ops, unsigned long iova,
+                        phys_addr_t paddr, size_t size, int prot)
+{
+       struct arm_short_io_pgtable *data = io_pgtable_short_ops_to_data(ops);

drivers/iommu/io-pgtable-arm-short.c line 402

Quoted 5 lines

drivers/iommu/io-pgtable-arm-short.c new file +5 -0

+       const struct iommu_gather_ops *tlb = data->iop.cfg.tlb;
+       int ret;
+
+       if (!(prot & (IOMMU_READ | IOMMU_WRITE)))
+               return -EINVAL;

Why? You could have (another) quirk to select the access model and you should be able to implement read+write, read-only no-exec and no-access.

drivers/iommu/io-pgtable-arm-short.c line 414

Quoted 12 lines

drivers/iommu/io-pgtable-arm-short.c new file +11 -0

+       if (size == SZ_4K) {/* most case */
+               ret = _arm_short_map_page(data, iova, paddr, prot, false);
+       } else if (size == SZ_64K) {
+               ret = _arm_short_map_page(data, iova, paddr, prot, true);
+       } else if (size == SZ_1M) {
+               ret = _arm_short_map_section(data, iova, paddr, prot, false);
+       } else if (size == SZ_16M) {
+               ret = _arm_short_map_section(data, iova, paddr, prot, true);
+       } else {
+               ret = -EINVAL;
+       }

Use a switch statement here?

drivers/iommu/io-pgtable-arm-short.c line 425

Quoted 11 lines

drivers/iommu/io-pgtable-arm-short.c new file +11 -0

+       tlb->tlb_add_flush(iova, size, true, data->iop.cookie);
+       return ret;
+}
+
+static struct io_pgtable *
+arm_short_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie)
+{
+       struct arm_short_io_pgtable *data;
+
+       if (cfg->ias != 32)
+               return NULL;

I think you just need to check '>'; VAs smaller than 32-bit can still be translated.

Review

Quoted 2 lines

(quoted diff) +2 -0

+       if (cfg->oas > ARM_SHORT_MAX_ADDR_BITS)
+               return NULL;

What benefit does ARM_SHORT_MAX_ADDR_BITS offer? Why not just '32'?

drivers/iommu/io-pgtable-arm-short.c line 430

Quoted 2 lines

drivers/iommu/io-pgtable-arm-short.c new file +2 -0

+
+       cfg->pgsize_bitmap &= SZ_4K | SZ_64K | SZ_1M | SZ_16M;

We can't support supersections unconditionally. Please add a quirk for this, as it relies on IOMMU support.

drivers/iommu/io-pgtable-arm-short.c line 442

Quoted 11 lines

drivers/iommu/io-pgtable-arm-short.c new file +11 -0

+       data = kzalloc(sizeof(*data), GFP_KERNEL);
+       if (!data)
+               return NULL;
+
+       data->pgd_size = SZ_16K;
+
+       data->pgd = alloc_pages_exact(data->pgd_size, GFP_KERNEL | __GFP_ZERO);
+       if (!data->pgd)
+               goto out_free_data;
+
+       cfg->tlb->flush_pgtable(data->pgd, data->pgd_size, cookie);

We may as well postpone this flush to the end of the function, given that we can still fail at this point.

drivers/iommu/io-pgtable-arm-short.c line 445

Quoted 2 lines

drivers/iommu/io-pgtable-arm-short.c new file +2 -0

+       /* kmem for pte */
+       data->ptekmem = kmem_cache_create("short-descriptor-pte",

A better name would be "io-pgtable-arm-short", however, why can't you just use GFP_ATOMIC in your pte allocations and do away with the cache altogether? Also, what happens if you try to allocate multiple caches with the same name?

drivers/iommu/io-pgtable-arm-short.c line 450

Quoted 6 lines

drivers/iommu/io-pgtable-arm-short.c new file +5 -0

+                                          ARM_SHORT_BYTES_PER_PTE,
+                                          ARM_SHORT_BYTES_PER_PTE,
+                                          0, NULL);
+
+       if (IS_ERR_OR_NULL(data->ptekmem)) {

I think you just need a NULL check here.

drivers/iommu/io-pgtable-arm-short.c line 452

Quoted 2 lines

drivers/iommu/io-pgtable-arm-short.c new file +2 -0

+               pr_err("Failed to Create cached mem for PTE %ld\n",
+                      PTR_ERR(data->ptekmem));

I don't think this error is particularly useful.

Quoted 72 lines

drivers/iommu/io-pgtable-arm-short.c new file +38 -0

+               goto out_free_pte;
+       }
+
+       /* TTBRs */
+       cfg->arm_short_cfg.ttbr[0] = virt_to_phys(data->pgd);
+       cfg->arm_short_cfg.ttbr[1] = 0;
+
+       cfg->arm_short_cfg.tcr = 0;
+
+       data->iop.ops = (struct io_pgtable_ops) {
+               .map            = arm_short_map,
+               .unmap          = arm_short_unmap,
+               .iova_to_phys   = arm_short_iova_to_phys,
+       };
+
+       return &data->iop;
+
+out_free_pte:
+       free_pages_exact(data->pgd, data->pgd_size);
+out_free_data:
+       kfree(data);
+       return NULL;
+}
+
+static void arm_short_free_pgtable(struct io_pgtable *iop)
+{
+       struct arm_short_io_pgtable *data = io_pgtable_short_to_data(iop);
+
+       kmem_cache_destroy(data->ptekmem);
+       free_pages_exact(data->pgd, data->pgd_size);
+       kfree(data);
+}
+
+struct io_pgtable_init_fns io_pgtable_arm_short_init_fns = {
+       .alloc  = arm_short_alloc_pgtable,
+       .free   = arm_short_free_pgtable,
+};
+

drivers/iommu/io-pgtable.c +4 -0

index 6436fe2..14a9b3a 100644
@@ -28,6 +28,7 @@ extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s1_init_fns;
 extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s2_init_fns;
 extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns;
 extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns;
+extern struct io_pgtable_init_fns io_pgtable_arm_short_init_fns;
 
 static const struct io_pgtable_init_fns *
 io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] =

@@ -38,6 +39,9 @@ io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] =
        [ARM_64_LPAE_S1] = &io_pgtable_arm_64_lpae_s1_init_fns,
        [ARM_64_LPAE_S2] = &io_pgtable_arm_64_lpae_s2_init_fns,
 #endif
+#ifdef CONFIG_IOMMU_IO_PGTABLE_SHORT
+       [ARM_SHORT_DESC] = &io_pgtable_arm_short_init_fns,
+#endif
 };
 
 struct io_pgtable_ops *alloc_io_pgtable_ops(enum io_pgtable_fmt fmt,

drivers/iommu/io-pgtable.h +1 -0

index 10e32f6..47efaab 100644
@@ -9,6 +9,7 @@ enum io_pgtable_fmt {
        ARM_32_LPAE_S2,
        ARM_64_LPAE_S1,
        ARM_64_LPAE_S2,
+       ARM_SHORT_DESC,
        IO_PGTABLE_NUM_FMTS,
 };