Add minimal viable support for SysMMU v7.x, which can be found in modern
Exynos chips (like Exynos850 or Google's GS101). SysMMU v7.x may
implement VM register set, and those registers should be initialized
properly if present. Usually 8 translation domains are supported via VM
registers (0..7), but only n=0 (default) is used for now.
Existing exynos-iommu driver only supports SysMMU versions up to v5. But
it's pretty much ready for basic usage with SysMMU v7, only small
changes have to be done. As SysMMU version is tested dynamically (by
reading the corresponding register), there is no need to introduce new
compatible string.
The only major change is that SysMMU v7 can have different register
layouts:
- with Virtual Machine support
- without Virtual Machine support
That can be checked by reading the capability registers. In the case if
SysMMU IP-core is VM-capable, the VM registers have to be used, and some
additional initialization is needed. That is the case on E850-96 board,
which non-secure SysMMU (v7.4) implements VM-capable register set.
Another required change to make SysMMU v7 functional (at least the one
that has VM registers), is to enable default VM instance. That should
be added to the code enabling MMU itself. Insights for that change were
taken by comparing the I/O dump (writel() / readl() operations) for the
vendor driver and this upstream driver.
The patch series was tested on E850-96 board. Because at the moment
there are no SysMMU users for that board, the testing was done using so
called "Emulated Translation" registers available on SysMMU v7. That
allows one to initiate the translation from CPU, by writing to those
registers, and then reading the corresponding TLB registers to find out
the translation result. The testing driver can be found in [1] tree.
Thanks to Marek, who did let me know it only takes a slight change of
registers to make this driver work with v7.
[1] https://github.com/joe-skb7/linux/tree/e850-96-mainline-iommu
Changes in v3:
- Merged "Check if SysMMU v7 has VM registers" patch into "Add SysMMU
v7 register set" patch
- Used variant struct instead of arrays for keeping register offsets
- Kept common registers out of variant struct
- See per-patch changes for more details
Changes in v2:
- Addressed all comments on review
- Reworked commit messages correspondingly
- Added new patch: "iommu/exynos: Handle failed registration properly"
- Added new patch: "iommu/exynos: Add SysMMU v7 register sets"
- Added new patch: "iommu/exynos: Reuse SysMMU constants for page size
and order"
Sam Protsenko (6):
iommu/exynos: Reuse SysMMU constants for page size and order
iommu/exynos: Handle failed IOMMU device registration properly
iommu/exynos: Set correct dma mask for SysMMU v5+
iommu/exynos: Abstract non-common registers on different variants
iommu/exynos: Add SysMMU v7 register set
iommu/exynos: Enable default VM instance on SysMMU v7
drivers/iommu/exynos-iommu.c | 178 ++++++++++++++++++++++++++---------
1 file changed, 136 insertions(+), 42 deletions(-)
--
2.30.2
If iommu_device_register() fails in exynos_sysmmu_probe(), the previous
calls have to be cleaned up. In this case, the iommu_device_sysfs_add()
should be cleaned up, by calling its remove counterpart call.
Fixes: d2c302b6e8b1 ("iommu/exynos: Make use of iommu_device_register interface")
Signed-off-by: Sam Protsenko <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Acked-by: Marek Szyprowski <[email protected]>
---
Changes in v3:
- Added Marek's Acked-by tag
- Added Krzysztof's R-b tag
- Added "Fixes" tag, as suggested by Krzysztof
Changes in v2:
- (none) This patch is new and added in v2
drivers/iommu/exynos-iommu.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 8f80aaa35092..c85db9dab851 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -629,7 +629,7 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
ret = iommu_device_register(&data->iommu, &exynos_iommu_ops, dev);
if (ret)
- return ret;
+ goto err_iommu_register;
platform_set_drvdata(pdev, data);
@@ -656,6 +656,10 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
pm_runtime_enable(dev);
return 0;
+
+err_iommu_register:
+ iommu_device_sysfs_remove(&data->iommu);
+ return ret;
}
static int __maybe_unused exynos_sysmmu_suspend(struct device *dev)
--
2.30.2
SysMMU v5+ supports 36 bit physical address space. Set corresponding DMA
mask to avoid falling back to SWTLBIO usage in dma_map_single() because
of failed dma_capable() check.
The original code for this fix was suggested by Marek.
Signed-off-by: Sam Protsenko <[email protected]>
Co-developed-by: Marek Szyprowski <[email protected]>
Signed-off-by: Marek Szyprowski <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
---
Changes in v3:
- Added Krzysztof's Acked-by tag
Changes in v2:
- Handled failed dma_set_mask() call
- Replaced "Originally-by" tag by "Co-developed-by" + SoB tags
drivers/iommu/exynos-iommu.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index c85db9dab851..494f7d7aa9c5 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -646,6 +646,14 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
}
}
+ if (MMU_MAJ_VER(data->version) >= 5) {
+ ret = dma_set_mask(dev, DMA_BIT_MASK(36));
+ if (ret) {
+ dev_err(dev, "Unable to set DMA mask: %d\n", ret);
+ goto err_dma_set_mask;
+ }
+ }
+
/*
* use the first registered sysmmu device for performing
* dma mapping operations on iommu page tables (cpu cache flush)
@@ -657,6 +665,8 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
return 0;
+err_dma_set_mask:
+ iommu_device_unregister(&data->iommu);
err_iommu_register:
iommu_device_sysfs_remove(&data->iommu);
return ret;
--
2.30.2
At the moment the driver supports SysMMU v1..v5 versions. SysMMU v5 has
different register layout than SysMMU v1..v3. Instead of checking the
version each time before reading/writing the registers, let's create
corresponding register structure for each SysMMU version and set the
needed structure on init, checking the SysMMU version one single time.
This way is faster and more elegant.
No behavior changes from the user's point of view, it's only a
refactoring patch.
Signed-off-by: Sam Protsenko <[email protected]>
Acked-by: Marek Szyprowski <[email protected]>
---
Changes in v3:
- Added Marek's Acked-by tag
- Removed abstracting common regs, used plain readl/writel to access
those instead
- Used variant struct instead of array to keep non-common register
offsets
- Removed 0x1 value used as an offset for missing registers
- Merged __sysmmu_hw_info() into __sysmmu_get_version()
- Refactored __sysmmu_tlb_invalidate_entry() for "num_inv == 1" case
- Reworked the commit message w.r.t. all changes
Changes in v2:
- Reworked existing code (SysMMU v1..v5) to use this approach
- Extracted v7 registers to the separate patches
- Replaced MMU_REG() with corresponding SysMMU read/write functions
- Improved the comment for 0x1 offsets triggering an unaligned access
exception
- Removed support for VMID number, as only VMID=0 (default) is used
for now
- Renamed register index names to reflect the old SysMMU version
register names
drivers/iommu/exynos-iommu.c | 100 +++++++++++++++++++++--------------
1 file changed, 60 insertions(+), 40 deletions(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 494f7d7aa9c5..6a0299fe1722 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -148,26 +148,12 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
#define MAKE_MMU_VER(maj, min) ((((maj) & 0xF) << 7) | ((min) & 0x7F))
/* v1.x - v3.x registers */
-#define REG_MMU_FLUSH 0x00C
-#define REG_MMU_FLUSH_ENTRY 0x010
-#define REG_PT_BASE_ADDR 0x014
-#define REG_INT_STATUS 0x018
-#define REG_INT_CLEAR 0x01C
-
#define REG_PAGE_FAULT_ADDR 0x024
#define REG_AW_FAULT_ADDR 0x028
#define REG_AR_FAULT_ADDR 0x02C
#define REG_DEFAULT_SLAVE_ADDR 0x030
/* v5.x registers */
-#define REG_V5_PT_BASE_PFN 0x00C
-#define REG_V5_MMU_FLUSH_ALL 0x010
-#define REG_V5_MMU_FLUSH_ENTRY 0x014
-#define REG_V5_MMU_FLUSH_RANGE 0x018
-#define REG_V5_MMU_FLUSH_START 0x020
-#define REG_V5_MMU_FLUSH_END 0x024
-#define REG_V5_INT_STATUS 0x060
-#define REG_V5_INT_CLEAR 0x064
#define REG_V5_FAULT_AR_VA 0x070
#define REG_V5_FAULT_AW_VA 0x080
@@ -250,6 +236,21 @@ struct exynos_iommu_domain {
struct iommu_domain domain; /* generic domain data structure */
};
+/*
+ * SysMMU version specific data. Contains offsets for the registers which can
+ * be found in different SysMMU variants, but have different offset values.
+ */
+struct sysmmu_variant {
+ u32 pt_base; /* page table base address (physical) */
+ u32 flush_all; /* invalidate all TLB entries */
+ u32 flush_entry; /* invalidate specific TLB entry */
+ u32 flush_range; /* invalidate TLB entries in specified range */
+ u32 flush_start; /* start address of range invalidation */
+ u32 flush_end; /* end address of range invalidation */
+ u32 int_status; /* interrupt status information */
+ u32 int_clear; /* clear the interrupt */
+};
+
/*
* This structure hold all data of a single SYSMMU controller, this includes
* hw resources like registers and clocks, pointers and list nodes to connect
@@ -274,6 +275,30 @@ struct sysmmu_drvdata {
unsigned int version; /* our version */
struct iommu_device iommu; /* IOMMU core handle */
+ const struct sysmmu_variant *variant; /* version specific data */
+};
+
+#define SYSMMU_REG(data, reg) ((data)->sfrbase + (data)->variant->reg)
+
+/* SysMMU v1..v3 */
+static const struct sysmmu_variant sysmmu_v1_variant = {
+ .flush_all = 0x0c,
+ .flush_entry = 0x10,
+ .pt_base = 0x14,
+ .int_status = 0x18,
+ .int_clear = 0x1c,
+};
+
+/* SysMMU v5 */
+static const struct sysmmu_variant sysmmu_v5_variant = {
+ .pt_base = 0x0c,
+ .flush_all = 0x10,
+ .flush_entry = 0x14,
+ .flush_range = 0x18,
+ .flush_start = 0x20,
+ .flush_end = 0x24,
+ .int_status = 0x60,
+ .int_clear = 0x64,
};
static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom)
@@ -304,10 +329,7 @@ static bool sysmmu_block(struct sysmmu_drvdata *data)
static void __sysmmu_tlb_invalidate(struct sysmmu_drvdata *data)
{
- if (MMU_MAJ_VER(data->version) < 5)
- writel(0x1, data->sfrbase + REG_MMU_FLUSH);
- else
- writel(0x1, data->sfrbase + REG_V5_MMU_FLUSH_ALL);
+ writel(0x1, SYSMMU_REG(data, flush_all));
}
static void __sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data,
@@ -315,33 +337,30 @@ static void __sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data,
{
unsigned int i;
- if (MMU_MAJ_VER(data->version) < 5) {
+ if (MMU_MAJ_VER(data->version) < 5 || num_inv == 1) {
for (i = 0; i < num_inv; i++) {
writel((iova & SPAGE_MASK) | 1,
- data->sfrbase + REG_MMU_FLUSH_ENTRY);
+ SYSMMU_REG(data, flush_entry));
iova += SPAGE_SIZE;
}
} else {
- if (num_inv == 1) {
- writel((iova & SPAGE_MASK) | 1,
- data->sfrbase + REG_V5_MMU_FLUSH_ENTRY);
- } else {
- writel((iova & SPAGE_MASK),
- data->sfrbase + REG_V5_MMU_FLUSH_START);
- writel((iova & SPAGE_MASK) + (num_inv - 1) * SPAGE_SIZE,
- data->sfrbase + REG_V5_MMU_FLUSH_END);
- writel(1, data->sfrbase + REG_V5_MMU_FLUSH_RANGE);
- }
+ writel(iova & SPAGE_MASK, SYSMMU_REG(data, flush_start));
+ writel((iova & SPAGE_MASK) + (num_inv - 1) * SPAGE_SIZE,
+ SYSMMU_REG(data, flush_end));
+ writel(0x1, SYSMMU_REG(data, flush_range));
}
}
static void __sysmmu_set_ptbase(struct sysmmu_drvdata *data, phys_addr_t pgd)
{
+ u32 pt_base;
+
if (MMU_MAJ_VER(data->version) < 5)
- writel(pgd, data->sfrbase + REG_PT_BASE_ADDR);
+ pt_base = pgd;
else
- writel(pgd >> SPAGE_ORDER, data->sfrbase + REG_V5_PT_BASE_PFN);
+ pt_base = pgd >> SPAGE_ORDER;
+ writel(pt_base, SYSMMU_REG(data, pt_base));
__sysmmu_tlb_invalidate(data);
}
@@ -378,6 +397,11 @@ static void __sysmmu_get_version(struct sysmmu_drvdata *data)
dev_dbg(data->sysmmu, "hardware version: %d.%d\n",
MMU_MAJ_VER(data->version), MMU_MIN_VER(data->version));
+ if (MMU_MAJ_VER(data->version) < 5)
+ data->variant = &sysmmu_v1_variant;
+ else
+ data->variant = &sysmmu_v5_variant;
+
__sysmmu_disable_clocks(data);
}
@@ -405,19 +429,14 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
const struct sysmmu_fault_info *finfo;
unsigned int i, n, itype;
sysmmu_iova_t fault_addr;
- unsigned short reg_status, reg_clear;
int ret = -ENOSYS;
WARN_ON(!data->active);
if (MMU_MAJ_VER(data->version) < 5) {
- reg_status = REG_INT_STATUS;
- reg_clear = REG_INT_CLEAR;
finfo = sysmmu_faults;
n = ARRAY_SIZE(sysmmu_faults);
} else {
- reg_status = REG_V5_INT_STATUS;
- reg_clear = REG_V5_INT_CLEAR;
finfo = sysmmu_v5_faults;
n = ARRAY_SIZE(sysmmu_v5_faults);
}
@@ -426,7 +445,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
clk_enable(data->clk_master);
- itype = __ffs(readl(data->sfrbase + reg_status));
+ itype = __ffs(readl(SYSMMU_REG(data, int_status)));
for (i = 0; i < n; i++, finfo++)
if (finfo->bit == itype)
break;
@@ -443,7 +462,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
/* fault is not recovered by fault handler */
BUG_ON(ret != 0);
- writel(1 << itype, data->sfrbase + reg_clear);
+ writel(1 << itype, SYSMMU_REG(data, int_clear));
sysmmu_unblock(data);
@@ -622,6 +641,8 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
data->sysmmu = dev;
spin_lock_init(&data->lock);
+ __sysmmu_get_version(data);
+
ret = iommu_device_sysfs_add(&data->iommu, &pdev->dev, NULL,
dev_name(data->sysmmu));
if (ret)
@@ -633,7 +654,6 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, data);
- __sysmmu_get_version(data);
if (PG_ENT_SHIFT < 0) {
if (MMU_MAJ_VER(data->version) < 5) {
PG_ENT_SHIFT = SYSMMU_PG_ENT_SHIFT;
--
2.30.2
SysMMU v7 might have different register layouts (VM capable or non-VM
capable). Virtual Machine registers (if present) implement multiple
translation domains. If VM registers are not present, the driver
shouldn't try to access those.
Check which layout is implemented in current SysMMU module (by reading
the capability registers) and prepare the corresponding variant
structure for further usage.
Signed-off-by: Sam Protsenko <[email protected]>
---
Changes in v3:
- Merged "Check if SysMMU v7 has VM registers" patch into this patch
- Reworked for using variant struct (instead of array)
Changes in v2:
- (none) This patch is new and added in v2
drivers/iommu/exynos-iommu.c | 50 +++++++++++++++++++++++++++++++++---
1 file changed, 47 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 6a0299fe1722..fc9ef3ff0057 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -135,6 +135,9 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
#define CFG_SYSSEL (1 << 22) /* System MMU 3.2 only */
#define CFG_FLPDCACHE (1 << 20) /* System MMU 3.2+ only */
+#define CAPA0_CAPA1_EXIST BIT(11)
+#define CAPA1_VCR_ENABLED BIT(14)
+
/* common registers */
#define REG_MMU_CTRL 0x000
#define REG_MMU_CFG 0x004
@@ -157,6 +160,10 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
#define REG_V5_FAULT_AR_VA 0x070
#define REG_V5_FAULT_AW_VA 0x080
+/* v7.x registers */
+#define REG_V7_CAPA0 0x870
+#define REG_V7_CAPA1 0x874
+
#define has_sysmmu(dev) (dev_iommu_priv_get(dev) != NULL)
static struct device *dma_dev;
@@ -276,6 +283,9 @@ struct sysmmu_drvdata {
struct iommu_device iommu; /* IOMMU core handle */
const struct sysmmu_variant *variant; /* version specific data */
+
+ /* v7 fields */
+ bool has_vcr; /* virtual machine control register */
};
#define SYSMMU_REG(data, reg) ((data)->sfrbase + (data)->variant->reg)
@@ -289,7 +299,7 @@ static const struct sysmmu_variant sysmmu_v1_variant = {
.int_clear = 0x1c,
};
-/* SysMMU v5 */
+/* SysMMU v5 and v7 (non-VM capable) */
static const struct sysmmu_variant sysmmu_v5_variant = {
.pt_base = 0x0c,
.flush_all = 0x10,
@@ -301,6 +311,18 @@ static const struct sysmmu_variant sysmmu_v5_variant = {
.int_clear = 0x64,
};
+/* SysMMU v7: VM capable register set */
+static const struct sysmmu_variant sysmmu_v7_vm_variant = {
+ .pt_base = 0x800c,
+ .flush_all = 0x8010,
+ .flush_entry = 0x8014,
+ .flush_range = 0x8018,
+ .flush_start = 0x8020,
+ .flush_end = 0x8024,
+ .int_status = 0x60,
+ .int_clear = 0x64,
+};
+
static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom)
{
return container_of(dom, struct exynos_iommu_domain, domain);
@@ -380,6 +402,20 @@ static void __sysmmu_disable_clocks(struct sysmmu_drvdata *data)
clk_disable_unprepare(data->clk_master);
}
+static bool __sysmmu_has_capa1(struct sysmmu_drvdata *data)
+{
+ u32 capa0 = readl(data->sfrbase + REG_V7_CAPA0);
+
+ return capa0 & CAPA0_CAPA1_EXIST;
+}
+
+static void __sysmmu_get_vcr(struct sysmmu_drvdata *data)
+{
+ u32 capa1 = readl(data->sfrbase + REG_V7_CAPA1);
+
+ data->has_vcr = capa1 & CAPA1_VCR_ENABLED;
+}
+
static void __sysmmu_get_version(struct sysmmu_drvdata *data)
{
u32 ver;
@@ -397,10 +433,18 @@ static void __sysmmu_get_version(struct sysmmu_drvdata *data)
dev_dbg(data->sysmmu, "hardware version: %d.%d\n",
MMU_MAJ_VER(data->version), MMU_MIN_VER(data->version));
- if (MMU_MAJ_VER(data->version) < 5)
+ if (MMU_MAJ_VER(data->version) < 5) {
data->variant = &sysmmu_v1_variant;
- else
+ } else if (MMU_MAJ_VER(data->version) < 7) {
data->variant = &sysmmu_v5_variant;
+ } else {
+ if (__sysmmu_has_capa1(data))
+ __sysmmu_get_vcr(data);
+ if (data->has_vcr)
+ data->variant = &sysmmu_v7_vm_variant;
+ else
+ data->variant = &sysmmu_v5_variant;
+ }
__sysmmu_disable_clocks(data);
}
--
2.30.2
In order to enable SysMMU v7 with VM register layout, at least the
default VM instance (n=0) must be enabled, in addition to enabling the
SysMMU itself. To do so, add corresponding write to MMU_CTRL_VM[0]
register, before writing to MMU_CTRL register.
Signed-off-by: Sam Protsenko <[email protected]>
Acked-by: Marek Szyprowski <[email protected]>
---
Changes in v3:
- Reworked for using plain writel()
- Added Marek's Acked-by tag
Changes in v2:
- Extracted VM enabling code to the separate function
- Used new SysMMU read/write functions to access the registers
drivers/iommu/exynos-iommu.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index fc9ef3ff0057..8e18984a0c4f 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -135,6 +135,8 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
#define CFG_SYSSEL (1 << 22) /* System MMU 3.2 only */
#define CFG_FLPDCACHE (1 << 20) /* System MMU 3.2+ only */
+#define CTRL_VM_ENABLE BIT(0)
+#define CTRL_VM_FAULT_MODE_STALL BIT(3)
#define CAPA0_CAPA1_EXIST BIT(11)
#define CAPA1_VCR_ENABLED BIT(14)
@@ -163,6 +165,7 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
/* v7.x registers */
#define REG_V7_CAPA0 0x870
#define REG_V7_CAPA1 0x874
+#define REG_V7_CTRL_VM 0x8000
#define has_sysmmu(dev) (dev_iommu_priv_get(dev) != NULL)
@@ -548,6 +551,18 @@ static void __sysmmu_init_config(struct sysmmu_drvdata *data)
writel(cfg, data->sfrbase + REG_MMU_CFG);
}
+static void __sysmmu_enable_vid(struct sysmmu_drvdata *data)
+{
+ u32 ctrl;
+
+ if (MMU_MAJ_VER(data->version) < 7 || !data->has_vcr)
+ return;
+
+ ctrl = readl(data->sfrbase + REG_V7_CTRL_VM);
+ ctrl |= CTRL_VM_ENABLE | CTRL_VM_FAULT_MODE_STALL;
+ writel(ctrl, data->sfrbase + REG_V7_CTRL_VM);
+}
+
static void __sysmmu_enable(struct sysmmu_drvdata *data)
{
unsigned long flags;
@@ -558,6 +573,7 @@ static void __sysmmu_enable(struct sysmmu_drvdata *data)
writel(CTRL_BLOCK, data->sfrbase + REG_MMU_CTRL);
__sysmmu_init_config(data);
__sysmmu_set_ptbase(data, data->pgtable);
+ __sysmmu_enable_vid(data);
writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL);
data->active = true;
spin_unlock_irqrestore(&data->lock, flags);
--
2.30.2
Using SZ_4K in context of SysMMU driver is better than using PAGE_SIZE,
as PAGE_SIZE might have different value on different platforms. Though
it would be even better to use more specific constants, already existing
in SysMMU driver. Make the code more strict by using SPAGE_ORDER and
SPAGE_SIZE constants.
It also makes sense, as __sysmmu_tlb_invalidate_entry() also uses
SPAGE_* constants for further calculations with num_inv param, so it's
logical that num_inv should be previously calculated using also SPAGE_*
values.
Signed-off-by: Sam Protsenko <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Acked-by: Marek Szyprowski <[email protected]>
---
Changes in v3:
- Added Marek's Acked-by tag
- Added Krzysztof's R-b tag
Changes in v2:
- (none) This patch is new and added in v2
drivers/iommu/exynos-iommu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 79729892eb48..8f80aaa35092 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -340,7 +340,7 @@ static void __sysmmu_set_ptbase(struct sysmmu_drvdata *data, phys_addr_t pgd)
if (MMU_MAJ_VER(data->version) < 5)
writel(pgd, data->sfrbase + REG_PT_BASE_ADDR);
else
- writel(pgd / SZ_4K, data->sfrbase + REG_V5_PT_BASE_PFN);
+ writel(pgd >> SPAGE_ORDER, data->sfrbase + REG_V5_PT_BASE_PFN);
__sysmmu_tlb_invalidate(data);
}
@@ -550,7 +550,7 @@ static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data,
* 64KB page can be one of 16 consecutive sets.
*/
if (MMU_MAJ_VER(data->version) == 2)
- num_inv = min_t(unsigned int, size / SZ_4K, 64);
+ num_inv = min_t(unsigned int, size / SPAGE_SIZE, 64);
if (sysmmu_block(data)) {
__sysmmu_tlb_invalidate_entry(data, iova, num_inv);
--
2.30.2
Hi Sam,
On 14.07.2022 18:55, Sam Protsenko wrote:
> SysMMU v7 might have different register layouts (VM capable or non-VM
> capable). Virtual Machine registers (if present) implement multiple
> translation domains. If VM registers are not present, the driver
> shouldn't try to access those.
>
> Check which layout is implemented in current SysMMU module (by reading
> the capability registers) and prepare the corresponding variant
> structure for further usage.
>
> Signed-off-by: Sam Protsenko <[email protected]>
Looks fine.
Acked-by: Marek Szyprowski <[email protected]>
What about the fault related code in the exynos_sysmmu_irq()? Afair v7
has a bit different layout of the status bits.
> ---
> Changes in v3:
> - Merged "Check if SysMMU v7 has VM registers" patch into this patch
> - Reworked for using variant struct (instead of array)
>
> Changes in v2:
> - (none) This patch is new and added in v2
>
> drivers/iommu/exynos-iommu.c | 50 +++++++++++++++++++++++++++++++++---
> 1 file changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 6a0299fe1722..fc9ef3ff0057 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -135,6 +135,9 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
> #define CFG_SYSSEL (1 << 22) /* System MMU 3.2 only */
> #define CFG_FLPDCACHE (1 << 20) /* System MMU 3.2+ only */
>
> +#define CAPA0_CAPA1_EXIST BIT(11)
> +#define CAPA1_VCR_ENABLED BIT(14)
> +
> /* common registers */
> #define REG_MMU_CTRL 0x000
> #define REG_MMU_CFG 0x004
> @@ -157,6 +160,10 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
> #define REG_V5_FAULT_AR_VA 0x070
> #define REG_V5_FAULT_AW_VA 0x080
>
> +/* v7.x registers */
> +#define REG_V7_CAPA0 0x870
> +#define REG_V7_CAPA1 0x874
> +
> #define has_sysmmu(dev) (dev_iommu_priv_get(dev) != NULL)
>
> static struct device *dma_dev;
> @@ -276,6 +283,9 @@ struct sysmmu_drvdata {
>
> struct iommu_device iommu; /* IOMMU core handle */
> const struct sysmmu_variant *variant; /* version specific data */
> +
> + /* v7 fields */
> + bool has_vcr; /* virtual machine control register */
> };
>
> #define SYSMMU_REG(data, reg) ((data)->sfrbase + (data)->variant->reg)
> @@ -289,7 +299,7 @@ static const struct sysmmu_variant sysmmu_v1_variant = {
> .int_clear = 0x1c,
> };
>
> -/* SysMMU v5 */
> +/* SysMMU v5 and v7 (non-VM capable) */
> static const struct sysmmu_variant sysmmu_v5_variant = {
> .pt_base = 0x0c,
> .flush_all = 0x10,
> @@ -301,6 +311,18 @@ static const struct sysmmu_variant sysmmu_v5_variant = {
> .int_clear = 0x64,
> };
>
> +/* SysMMU v7: VM capable register set */
> +static const struct sysmmu_variant sysmmu_v7_vm_variant = {
> + .pt_base = 0x800c,
> + .flush_all = 0x8010,
> + .flush_entry = 0x8014,
> + .flush_range = 0x8018,
> + .flush_start = 0x8020,
> + .flush_end = 0x8024,
> + .int_status = 0x60,
> + .int_clear = 0x64,
> +};
> +
> static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom)
> {
> return container_of(dom, struct exynos_iommu_domain, domain);
> @@ -380,6 +402,20 @@ static void __sysmmu_disable_clocks(struct sysmmu_drvdata *data)
> clk_disable_unprepare(data->clk_master);
> }
>
> +static bool __sysmmu_has_capa1(struct sysmmu_drvdata *data)
> +{
> + u32 capa0 = readl(data->sfrbase + REG_V7_CAPA0);
> +
> + return capa0 & CAPA0_CAPA1_EXIST;
> +}
> +
> +static void __sysmmu_get_vcr(struct sysmmu_drvdata *data)
> +{
> + u32 capa1 = readl(data->sfrbase + REG_V7_CAPA1);
> +
> + data->has_vcr = capa1 & CAPA1_VCR_ENABLED;
> +}
> +
> static void __sysmmu_get_version(struct sysmmu_drvdata *data)
> {
> u32 ver;
> @@ -397,10 +433,18 @@ static void __sysmmu_get_version(struct sysmmu_drvdata *data)
> dev_dbg(data->sysmmu, "hardware version: %d.%d\n",
> MMU_MAJ_VER(data->version), MMU_MIN_VER(data->version));
>
> - if (MMU_MAJ_VER(data->version) < 5)
> + if (MMU_MAJ_VER(data->version) < 5) {
> data->variant = &sysmmu_v1_variant;
> - else
> + } else if (MMU_MAJ_VER(data->version) < 7) {
> data->variant = &sysmmu_v5_variant;
> + } else {
> + if (__sysmmu_has_capa1(data))
> + __sysmmu_get_vcr(data);
> + if (data->has_vcr)
> + data->variant = &sysmmu_v7_vm_variant;
> + else
> + data->variant = &sysmmu_v5_variant;
> + }
>
> __sysmmu_disable_clocks(data);
> }
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On Thu, Jul 14, 2022 at 07:55:44PM +0300, Sam Protsenko wrote:
> Sam Protsenko (6):
> iommu/exynos: Reuse SysMMU constants for page size and order
> iommu/exynos: Handle failed IOMMU device registration properly
> iommu/exynos: Set correct dma mask for SysMMU v5+
> iommu/exynos: Abstract non-common registers on different variants
> iommu/exynos: Add SysMMU v7 register set
> iommu/exynos: Enable default VM instance on SysMMU v7
Applied, thanks.
On 2022-07-14 17:55, Sam Protsenko wrote:
> At the moment the driver supports SysMMU v1..v5 versions. SysMMU v5 has
> different register layout than SysMMU v1..v3. Instead of checking the
> version each time before reading/writing the registers, let's create
> corresponding register structure for each SysMMU version and set the
> needed structure on init, checking the SysMMU version one single time.
> This way is faster and more elegant.
>
> No behavior changes from the user's point of view, it's only a
> refactoring patch.
>
> Signed-off-by: Sam Protsenko <[email protected]>
> Acked-by: Marek Szyprowski <[email protected]>
> ---
> Changes in v3:
> - Added Marek's Acked-by tag
> - Removed abstracting common regs, used plain readl/writel to access
> those instead
> - Used variant struct instead of array to keep non-common register
> offsets
I'm a bit late, but just for the record I think the new approach in this
version looks really good :)
Cheers,
Robin.
> - Removed 0x1 value used as an offset for missing registers
> - Merged __sysmmu_hw_info() into __sysmmu_get_version()
> - Refactored __sysmmu_tlb_invalidate_entry() for "num_inv == 1" case
> - Reworked the commit message w.r.t. all changes
>
> Changes in v2:
> - Reworked existing code (SysMMU v1..v5) to use this approach
> - Extracted v7 registers to the separate patches
> - Replaced MMU_REG() with corresponding SysMMU read/write functions
> - Improved the comment for 0x1 offsets triggering an unaligned access
> exception
> - Removed support for VMID number, as only VMID=0 (default) is used
> for now
> - Renamed register index names to reflect the old SysMMU version
> register names
>
> drivers/iommu/exynos-iommu.c | 100 +++++++++++++++++++++--------------
> 1 file changed, 60 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 494f7d7aa9c5..6a0299fe1722 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -148,26 +148,12 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
> #define MAKE_MMU_VER(maj, min) ((((maj) & 0xF) << 7) | ((min) & 0x7F))
>
> /* v1.x - v3.x registers */
> -#define REG_MMU_FLUSH 0x00C
> -#define REG_MMU_FLUSH_ENTRY 0x010
> -#define REG_PT_BASE_ADDR 0x014
> -#define REG_INT_STATUS 0x018
> -#define REG_INT_CLEAR 0x01C
> -
> #define REG_PAGE_FAULT_ADDR 0x024
> #define REG_AW_FAULT_ADDR 0x028
> #define REG_AR_FAULT_ADDR 0x02C
> #define REG_DEFAULT_SLAVE_ADDR 0x030
>
> /* v5.x registers */
> -#define REG_V5_PT_BASE_PFN 0x00C
> -#define REG_V5_MMU_FLUSH_ALL 0x010
> -#define REG_V5_MMU_FLUSH_ENTRY 0x014
> -#define REG_V5_MMU_FLUSH_RANGE 0x018
> -#define REG_V5_MMU_FLUSH_START 0x020
> -#define REG_V5_MMU_FLUSH_END 0x024
> -#define REG_V5_INT_STATUS 0x060
> -#define REG_V5_INT_CLEAR 0x064
> #define REG_V5_FAULT_AR_VA 0x070
> #define REG_V5_FAULT_AW_VA 0x080
>
> @@ -250,6 +236,21 @@ struct exynos_iommu_domain {
> struct iommu_domain domain; /* generic domain data structure */
> };
>
> +/*
> + * SysMMU version specific data. Contains offsets for the registers which can
> + * be found in different SysMMU variants, but have different offset values.
> + */
> +struct sysmmu_variant {
> + u32 pt_base; /* page table base address (physical) */
> + u32 flush_all; /* invalidate all TLB entries */
> + u32 flush_entry; /* invalidate specific TLB entry */
> + u32 flush_range; /* invalidate TLB entries in specified range */
> + u32 flush_start; /* start address of range invalidation */
> + u32 flush_end; /* end address of range invalidation */
> + u32 int_status; /* interrupt status information */
> + u32 int_clear; /* clear the interrupt */
> +};
> +
> /*
> * This structure hold all data of a single SYSMMU controller, this includes
> * hw resources like registers and clocks, pointers and list nodes to connect
> @@ -274,6 +275,30 @@ struct sysmmu_drvdata {
> unsigned int version; /* our version */
>
> struct iommu_device iommu; /* IOMMU core handle */
> + const struct sysmmu_variant *variant; /* version specific data */
> +};
> +
> +#define SYSMMU_REG(data, reg) ((data)->sfrbase + (data)->variant->reg)
> +
> +/* SysMMU v1..v3 */
> +static const struct sysmmu_variant sysmmu_v1_variant = {
> + .flush_all = 0x0c,
> + .flush_entry = 0x10,
> + .pt_base = 0x14,
> + .int_status = 0x18,
> + .int_clear = 0x1c,
> +};
> +
> +/* SysMMU v5 */
> +static const struct sysmmu_variant sysmmu_v5_variant = {
> + .pt_base = 0x0c,
> + .flush_all = 0x10,
> + .flush_entry = 0x14,
> + .flush_range = 0x18,
> + .flush_start = 0x20,
> + .flush_end = 0x24,
> + .int_status = 0x60,
> + .int_clear = 0x64,
> };
>
> static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom)
> @@ -304,10 +329,7 @@ static bool sysmmu_block(struct sysmmu_drvdata *data)
>
> static void __sysmmu_tlb_invalidate(struct sysmmu_drvdata *data)
> {
> - if (MMU_MAJ_VER(data->version) < 5)
> - writel(0x1, data->sfrbase + REG_MMU_FLUSH);
> - else
> - writel(0x1, data->sfrbase + REG_V5_MMU_FLUSH_ALL);
> + writel(0x1, SYSMMU_REG(data, flush_all));
> }
>
> static void __sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data,
> @@ -315,33 +337,30 @@ static void __sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data,
> {
> unsigned int i;
>
> - if (MMU_MAJ_VER(data->version) < 5) {
> + if (MMU_MAJ_VER(data->version) < 5 || num_inv == 1) {
> for (i = 0; i < num_inv; i++) {
> writel((iova & SPAGE_MASK) | 1,
> - data->sfrbase + REG_MMU_FLUSH_ENTRY);
> + SYSMMU_REG(data, flush_entry));
> iova += SPAGE_SIZE;
> }
> } else {
> - if (num_inv == 1) {
> - writel((iova & SPAGE_MASK) | 1,
> - data->sfrbase + REG_V5_MMU_FLUSH_ENTRY);
> - } else {
> - writel((iova & SPAGE_MASK),
> - data->sfrbase + REG_V5_MMU_FLUSH_START);
> - writel((iova & SPAGE_MASK) + (num_inv - 1) * SPAGE_SIZE,
> - data->sfrbase + REG_V5_MMU_FLUSH_END);
> - writel(1, data->sfrbase + REG_V5_MMU_FLUSH_RANGE);
> - }
> + writel(iova & SPAGE_MASK, SYSMMU_REG(data, flush_start));
> + writel((iova & SPAGE_MASK) + (num_inv - 1) * SPAGE_SIZE,
> + SYSMMU_REG(data, flush_end));
> + writel(0x1, SYSMMU_REG(data, flush_range));
> }
> }
>
> static void __sysmmu_set_ptbase(struct sysmmu_drvdata *data, phys_addr_t pgd)
> {
> + u32 pt_base;
> +
> if (MMU_MAJ_VER(data->version) < 5)
> - writel(pgd, data->sfrbase + REG_PT_BASE_ADDR);
> + pt_base = pgd;
> else
> - writel(pgd >> SPAGE_ORDER, data->sfrbase + REG_V5_PT_BASE_PFN);
> + pt_base = pgd >> SPAGE_ORDER;
>
> + writel(pt_base, SYSMMU_REG(data, pt_base));
> __sysmmu_tlb_invalidate(data);
> }
>
> @@ -378,6 +397,11 @@ static void __sysmmu_get_version(struct sysmmu_drvdata *data)
> dev_dbg(data->sysmmu, "hardware version: %d.%d\n",
> MMU_MAJ_VER(data->version), MMU_MIN_VER(data->version));
>
> + if (MMU_MAJ_VER(data->version) < 5)
> + data->variant = &sysmmu_v1_variant;
> + else
> + data->variant = &sysmmu_v5_variant;
> +
> __sysmmu_disable_clocks(data);
> }
>
> @@ -405,19 +429,14 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
> const struct sysmmu_fault_info *finfo;
> unsigned int i, n, itype;
> sysmmu_iova_t fault_addr;
> - unsigned short reg_status, reg_clear;
> int ret = -ENOSYS;
>
> WARN_ON(!data->active);
>
> if (MMU_MAJ_VER(data->version) < 5) {
> - reg_status = REG_INT_STATUS;
> - reg_clear = REG_INT_CLEAR;
> finfo = sysmmu_faults;
> n = ARRAY_SIZE(sysmmu_faults);
> } else {
> - reg_status = REG_V5_INT_STATUS;
> - reg_clear = REG_V5_INT_CLEAR;
> finfo = sysmmu_v5_faults;
> n = ARRAY_SIZE(sysmmu_v5_faults);
> }
> @@ -426,7 +445,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
>
> clk_enable(data->clk_master);
>
> - itype = __ffs(readl(data->sfrbase + reg_status));
> + itype = __ffs(readl(SYSMMU_REG(data, int_status)));
> for (i = 0; i < n; i++, finfo++)
> if (finfo->bit == itype)
> break;
> @@ -443,7 +462,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
> /* fault is not recovered by fault handler */
> BUG_ON(ret != 0);
>
> - writel(1 << itype, data->sfrbase + reg_clear);
> + writel(1 << itype, SYSMMU_REG(data, int_clear));
>
> sysmmu_unblock(data);
>
> @@ -622,6 +641,8 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
> data->sysmmu = dev;
> spin_lock_init(&data->lock);
>
> + __sysmmu_get_version(data);
> +
> ret = iommu_device_sysfs_add(&data->iommu, &pdev->dev, NULL,
> dev_name(data->sysmmu));
> if (ret)
> @@ -633,7 +654,6 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, data);
>
> - __sysmmu_get_version(data);
> if (PG_ENT_SHIFT < 0) {
> if (MMU_MAJ_VER(data->version) < 5) {
> PG_ENT_SHIFT = SYSMMU_PG_ENT_SHIFT;
On Fri, 15 Jul 2022 at 15:14, Robin Murphy <[email protected]> wrote:
>
> On 2022-07-14 17:55, Sam Protsenko wrote:
> > At the moment the driver supports SysMMU v1..v5 versions. SysMMU v5 has
> > different register layout than SysMMU v1..v3. Instead of checking the
> > version each time before reading/writing the registers, let's create
> > corresponding register structure for each SysMMU version and set the
> > needed structure on init, checking the SysMMU version one single time.
> > This way is faster and more elegant.
> >
> > No behavior changes from the user's point of view, it's only a
> > refactoring patch.
> >
> > Signed-off-by: Sam Protsenko <[email protected]>
> > Acked-by: Marek Szyprowski <[email protected]>
> > ---
> > Changes in v3:
> > - Added Marek's Acked-by tag
> > - Removed abstracting common regs, used plain readl/writel to access
> > those instead
> > - Used variant struct instead of array to keep non-common register
> > offsets
>
> I'm a bit late, but just for the record I think the new approach in this
> version looks really good :)
>
Thank you for the review, Robin!
> Cheers,
> Robin.
>
> > - Removed 0x1 value used as an offset for missing registers
> > - Merged __sysmmu_hw_info() into __sysmmu_get_version()
> > - Refactored __sysmmu_tlb_invalidate_entry() for "num_inv == 1" case
> > - Reworked the commit message w.r.t. all changes
> >
> > Changes in v2:
> > - Reworked existing code (SysMMU v1..v5) to use this approach
> > - Extracted v7 registers to the separate patches
> > - Replaced MMU_REG() with corresponding SysMMU read/write functions
> > - Improved the comment for 0x1 offsets triggering an unaligned access
> > exception
> > - Removed support for VMID number, as only VMID=0 (default) is used
> > for now
> > - Renamed register index names to reflect the old SysMMU version
> > register names
> >
> > drivers/iommu/exynos-iommu.c | 100 +++++++++++++++++++++--------------
> > 1 file changed, 60 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> > index 494f7d7aa9c5..6a0299fe1722 100644
> > --- a/drivers/iommu/exynos-iommu.c
> > +++ b/drivers/iommu/exynos-iommu.c
> > @@ -148,26 +148,12 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
> > #define MAKE_MMU_VER(maj, min) ((((maj) & 0xF) << 7) | ((min) & 0x7F))
> >
> > /* v1.x - v3.x registers */
> > -#define REG_MMU_FLUSH 0x00C
> > -#define REG_MMU_FLUSH_ENTRY 0x010
> > -#define REG_PT_BASE_ADDR 0x014
> > -#define REG_INT_STATUS 0x018
> > -#define REG_INT_CLEAR 0x01C
> > -
> > #define REG_PAGE_FAULT_ADDR 0x024
> > #define REG_AW_FAULT_ADDR 0x028
> > #define REG_AR_FAULT_ADDR 0x02C
> > #define REG_DEFAULT_SLAVE_ADDR 0x030
> >
> > /* v5.x registers */
> > -#define REG_V5_PT_BASE_PFN 0x00C
> > -#define REG_V5_MMU_FLUSH_ALL 0x010
> > -#define REG_V5_MMU_FLUSH_ENTRY 0x014
> > -#define REG_V5_MMU_FLUSH_RANGE 0x018
> > -#define REG_V5_MMU_FLUSH_START 0x020
> > -#define REG_V5_MMU_FLUSH_END 0x024
> > -#define REG_V5_INT_STATUS 0x060
> > -#define REG_V5_INT_CLEAR 0x064
> > #define REG_V5_FAULT_AR_VA 0x070
> > #define REG_V5_FAULT_AW_VA 0x080
> >
> > @@ -250,6 +236,21 @@ struct exynos_iommu_domain {
> > struct iommu_domain domain; /* generic domain data structure */
> > };
> >
> > +/*
> > + * SysMMU version specific data. Contains offsets for the registers which can
> > + * be found in different SysMMU variants, but have different offset values.
> > + */
> > +struct sysmmu_variant {
> > + u32 pt_base; /* page table base address (physical) */
> > + u32 flush_all; /* invalidate all TLB entries */
> > + u32 flush_entry; /* invalidate specific TLB entry */
> > + u32 flush_range; /* invalidate TLB entries in specified range */
> > + u32 flush_start; /* start address of range invalidation */
> > + u32 flush_end; /* end address of range invalidation */
> > + u32 int_status; /* interrupt status information */
> > + u32 int_clear; /* clear the interrupt */
> > +};
> > +
> > /*
> > * This structure hold all data of a single SYSMMU controller, this includes
> > * hw resources like registers and clocks, pointers and list nodes to connect
> > @@ -274,6 +275,30 @@ struct sysmmu_drvdata {
> > unsigned int version; /* our version */
> >
> > struct iommu_device iommu; /* IOMMU core handle */
> > + const struct sysmmu_variant *variant; /* version specific data */
> > +};
> > +
> > +#define SYSMMU_REG(data, reg) ((data)->sfrbase + (data)->variant->reg)
> > +
> > +/* SysMMU v1..v3 */
> > +static const struct sysmmu_variant sysmmu_v1_variant = {
> > + .flush_all = 0x0c,
> > + .flush_entry = 0x10,
> > + .pt_base = 0x14,
> > + .int_status = 0x18,
> > + .int_clear = 0x1c,
> > +};
> > +
> > +/* SysMMU v5 */
> > +static const struct sysmmu_variant sysmmu_v5_variant = {
> > + .pt_base = 0x0c,
> > + .flush_all = 0x10,
> > + .flush_entry = 0x14,
> > + .flush_range = 0x18,
> > + .flush_start = 0x20,
> > + .flush_end = 0x24,
> > + .int_status = 0x60,
> > + .int_clear = 0x64,
> > };
> >
> > static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom)
> > @@ -304,10 +329,7 @@ static bool sysmmu_block(struct sysmmu_drvdata *data)
> >
> > static void __sysmmu_tlb_invalidate(struct sysmmu_drvdata *data)
> > {
> > - if (MMU_MAJ_VER(data->version) < 5)
> > - writel(0x1, data->sfrbase + REG_MMU_FLUSH);
> > - else
> > - writel(0x1, data->sfrbase + REG_V5_MMU_FLUSH_ALL);
> > + writel(0x1, SYSMMU_REG(data, flush_all));
> > }
> >
> > static void __sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data,
> > @@ -315,33 +337,30 @@ static void __sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data,
> > {
> > unsigned int i;
> >
> > - if (MMU_MAJ_VER(data->version) < 5) {
> > + if (MMU_MAJ_VER(data->version) < 5 || num_inv == 1) {
> > for (i = 0; i < num_inv; i++) {
> > writel((iova & SPAGE_MASK) | 1,
> > - data->sfrbase + REG_MMU_FLUSH_ENTRY);
> > + SYSMMU_REG(data, flush_entry));
> > iova += SPAGE_SIZE;
> > }
> > } else {
> > - if (num_inv == 1) {
> > - writel((iova & SPAGE_MASK) | 1,
> > - data->sfrbase + REG_V5_MMU_FLUSH_ENTRY);
> > - } else {
> > - writel((iova & SPAGE_MASK),
> > - data->sfrbase + REG_V5_MMU_FLUSH_START);
> > - writel((iova & SPAGE_MASK) + (num_inv - 1) * SPAGE_SIZE,
> > - data->sfrbase + REG_V5_MMU_FLUSH_END);
> > - writel(1, data->sfrbase + REG_V5_MMU_FLUSH_RANGE);
> > - }
> > + writel(iova & SPAGE_MASK, SYSMMU_REG(data, flush_start));
> > + writel((iova & SPAGE_MASK) + (num_inv - 1) * SPAGE_SIZE,
> > + SYSMMU_REG(data, flush_end));
> > + writel(0x1, SYSMMU_REG(data, flush_range));
> > }
> > }
> >
> > static void __sysmmu_set_ptbase(struct sysmmu_drvdata *data, phys_addr_t pgd)
> > {
> > + u32 pt_base;
> > +
> > if (MMU_MAJ_VER(data->version) < 5)
> > - writel(pgd, data->sfrbase + REG_PT_BASE_ADDR);
> > + pt_base = pgd;
> > else
> > - writel(pgd >> SPAGE_ORDER, data->sfrbase + REG_V5_PT_BASE_PFN);
> > + pt_base = pgd >> SPAGE_ORDER;
> >
> > + writel(pt_base, SYSMMU_REG(data, pt_base));
> > __sysmmu_tlb_invalidate(data);
> > }
> >
> > @@ -378,6 +397,11 @@ static void __sysmmu_get_version(struct sysmmu_drvdata *data)
> > dev_dbg(data->sysmmu, "hardware version: %d.%d\n",
> > MMU_MAJ_VER(data->version), MMU_MIN_VER(data->version));
> >
> > + if (MMU_MAJ_VER(data->version) < 5)
> > + data->variant = &sysmmu_v1_variant;
> > + else
> > + data->variant = &sysmmu_v5_variant;
> > +
> > __sysmmu_disable_clocks(data);
> > }
> >
> > @@ -405,19 +429,14 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
> > const struct sysmmu_fault_info *finfo;
> > unsigned int i, n, itype;
> > sysmmu_iova_t fault_addr;
> > - unsigned short reg_status, reg_clear;
> > int ret = -ENOSYS;
> >
> > WARN_ON(!data->active);
> >
> > if (MMU_MAJ_VER(data->version) < 5) {
> > - reg_status = REG_INT_STATUS;
> > - reg_clear = REG_INT_CLEAR;
> > finfo = sysmmu_faults;
> > n = ARRAY_SIZE(sysmmu_faults);
> > } else {
> > - reg_status = REG_V5_INT_STATUS;
> > - reg_clear = REG_V5_INT_CLEAR;
> > finfo = sysmmu_v5_faults;
> > n = ARRAY_SIZE(sysmmu_v5_faults);
> > }
> > @@ -426,7 +445,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
> >
> > clk_enable(data->clk_master);
> >
> > - itype = __ffs(readl(data->sfrbase + reg_status));
> > + itype = __ffs(readl(SYSMMU_REG(data, int_status)));
> > for (i = 0; i < n; i++, finfo++)
> > if (finfo->bit == itype)
> > break;
> > @@ -443,7 +462,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
> > /* fault is not recovered by fault handler */
> > BUG_ON(ret != 0);
> >
> > - writel(1 << itype, data->sfrbase + reg_clear);
> > + writel(1 << itype, SYSMMU_REG(data, int_clear));
> >
> > sysmmu_unblock(data);
> >
> > @@ -622,6 +641,8 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
> > data->sysmmu = dev;
> > spin_lock_init(&data->lock);
> >
> > + __sysmmu_get_version(data);
> > +
> > ret = iommu_device_sysfs_add(&data->iommu, &pdev->dev, NULL,
> > dev_name(data->sysmmu));
> > if (ret)
> > @@ -633,7 +654,6 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
> >
> > platform_set_drvdata(pdev, data);
> >
> > - __sysmmu_get_version(data);
> > if (PG_ENT_SHIFT < 0) {
> > if (MMU_MAJ_VER(data->version) < 5) {
> > PG_ENT_SHIFT = SYSMMU_PG_ENT_SHIFT;
On Fri, 15 Jul 2022 at 10:28, Marek Szyprowski <[email protected]> wrote:
>
> Hi Sam,
>
> On 14.07.2022 18:55, Sam Protsenko wrote:
> > SysMMU v7 might have different register layouts (VM capable or non-VM
> > capable). Virtual Machine registers (if present) implement multiple
> > translation domains. If VM registers are not present, the driver
> > shouldn't try to access those.
> >
> > Check which layout is implemented in current SysMMU module (by reading
> > the capability registers) and prepare the corresponding variant
> > structure for further usage.
> >
> > Signed-off-by: Sam Protsenko <[email protected]>
>
> Looks fine.
>
> Acked-by: Marek Szyprowski <[email protected]>
>
> What about the fault related code in the exynos_sysmmu_irq()? Afair v7
> has a bit different layout of the status bits.
>
Hi Marek,
Fault handling is exactly what I'm going to look into next. The idea
behind this series was to make SysMMU minimally functional on my
board, so I can proceed with adding more features on top of that
further. Thanks for your patch you shared btw, I'll try to use it.
Vendor's downstream kernel's got the whole separate .c file for that,
will check if anything can be reused from there too. Good thing my
testing driver [2] also allows generating page faults, so I can at
least verify if the added code works properly.
Thanks!
[1] https://github.com/joe-skb7/linux/blob/iommu-exynos850-dev/drivers/iommu/samsung-iommu-fault.c
[2] https://github.com/joe-skb7/linux/blob/e850-96-mainline-iommu/drivers/iommu/samsung-iommu-test.c
> > ---
> > Changes in v3:
> > - Merged "Check if SysMMU v7 has VM registers" patch into this patch
> > - Reworked for using variant struct (instead of array)
> >
> > Changes in v2:
> > - (none) This patch is new and added in v2
> >
> > drivers/iommu/exynos-iommu.c | 50 +++++++++++++++++++++++++++++++++---
> > 1 file changed, 47 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> > index 6a0299fe1722..fc9ef3ff0057 100644
> > --- a/drivers/iommu/exynos-iommu.c
> > +++ b/drivers/iommu/exynos-iommu.c
> > @@ -135,6 +135,9 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
> > #define CFG_SYSSEL (1 << 22) /* System MMU 3.2 only */
> > #define CFG_FLPDCACHE (1 << 20) /* System MMU 3.2+ only */
> >
> > +#define CAPA0_CAPA1_EXIST BIT(11)
> > +#define CAPA1_VCR_ENABLED BIT(14)
> > +
> > /* common registers */
> > #define REG_MMU_CTRL 0x000
> > #define REG_MMU_CFG 0x004
> > @@ -157,6 +160,10 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
> > #define REG_V5_FAULT_AR_VA 0x070
> > #define REG_V5_FAULT_AW_VA 0x080
> >
> > +/* v7.x registers */
> > +#define REG_V7_CAPA0 0x870
> > +#define REG_V7_CAPA1 0x874
> > +
> > #define has_sysmmu(dev) (dev_iommu_priv_get(dev) != NULL)
> >
> > static struct device *dma_dev;
> > @@ -276,6 +283,9 @@ struct sysmmu_drvdata {
> >
> > struct iommu_device iommu; /* IOMMU core handle */
> > const struct sysmmu_variant *variant; /* version specific data */
> > +
> > + /* v7 fields */
> > + bool has_vcr; /* virtual machine control register */
> > };
> >
> > #define SYSMMU_REG(data, reg) ((data)->sfrbase + (data)->variant->reg)
> > @@ -289,7 +299,7 @@ static const struct sysmmu_variant sysmmu_v1_variant = {
> > .int_clear = 0x1c,
> > };
> >
> > -/* SysMMU v5 */
> > +/* SysMMU v5 and v7 (non-VM capable) */
> > static const struct sysmmu_variant sysmmu_v5_variant = {
> > .pt_base = 0x0c,
> > .flush_all = 0x10,
> > @@ -301,6 +311,18 @@ static const struct sysmmu_variant sysmmu_v5_variant = {
> > .int_clear = 0x64,
> > };
> >
> > +/* SysMMU v7: VM capable register set */
> > +static const struct sysmmu_variant sysmmu_v7_vm_variant = {
> > + .pt_base = 0x800c,
> > + .flush_all = 0x8010,
> > + .flush_entry = 0x8014,
> > + .flush_range = 0x8018,
> > + .flush_start = 0x8020,
> > + .flush_end = 0x8024,
> > + .int_status = 0x60,
> > + .int_clear = 0x64,
> > +};
> > +
> > static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom)
> > {
> > return container_of(dom, struct exynos_iommu_domain, domain);
> > @@ -380,6 +402,20 @@ static void __sysmmu_disable_clocks(struct sysmmu_drvdata *data)
> > clk_disable_unprepare(data->clk_master);
> > }
> >
> > +static bool __sysmmu_has_capa1(struct sysmmu_drvdata *data)
> > +{
> > + u32 capa0 = readl(data->sfrbase + REG_V7_CAPA0);
> > +
> > + return capa0 & CAPA0_CAPA1_EXIST;
> > +}
> > +
> > +static void __sysmmu_get_vcr(struct sysmmu_drvdata *data)
> > +{
> > + u32 capa1 = readl(data->sfrbase + REG_V7_CAPA1);
> > +
> > + data->has_vcr = capa1 & CAPA1_VCR_ENABLED;
> > +}
> > +
> > static void __sysmmu_get_version(struct sysmmu_drvdata *data)
> > {
> > u32 ver;
> > @@ -397,10 +433,18 @@ static void __sysmmu_get_version(struct sysmmu_drvdata *data)
> > dev_dbg(data->sysmmu, "hardware version: %d.%d\n",
> > MMU_MAJ_VER(data->version), MMU_MIN_VER(data->version));
> >
> > - if (MMU_MAJ_VER(data->version) < 5)
> > + if (MMU_MAJ_VER(data->version) < 5) {
> > data->variant = &sysmmu_v1_variant;
> > - else
> > + } else if (MMU_MAJ_VER(data->version) < 7) {
> > data->variant = &sysmmu_v5_variant;
> > + } else {
> > + if (__sysmmu_has_capa1(data))
> > + __sysmmu_get_vcr(data);
> > + if (data->has_vcr)
> > + data->variant = &sysmmu_v7_vm_variant;
> > + else
> > + data->variant = &sysmmu_v5_variant;
> > + }
> >
> > __sysmmu_disable_clocks(data);
> > }
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>