2022-10-25 17:33:23

by Jernej Škrabec

[permalink] [raw]
Subject: [PATCH v2 0/5] iommu/sun50i: Fix various fixes

Testing IOMMU together with video decoder (Cedrus) exposed many bugs in
sun50i-iommu driver. This series addresses all issues so video decoder
works well with IOMMU.

First two patches address recovery issues in interrupt when either page
faults or permission errors were reported. Third patch fixes permission
domain assignment. Fourth patch fixes dma sync size. Sometimes sync also
touched some other buffers and kernel generated warning in dmesg. Fifth
patch fixes issue with syncing PDE and PTE tables. Without it, page
faults were randomly generated even with valid iova addresses.

Please take a look.

Best regards,
Jernej

Changes from v1:
- rebase on top of v6.1-rc1
- replace last patch with new one - Implement iotlb_sync_map instead
of invalidating each page at each allocation

Jernej Skrabec (5):
iommu/sun50i: Fix reset release
iommu/sun50i: Consider all fault sources for reset
iommu/sun50i: Fix R/W permission check
iommu/sun50i: Fix flush size
iommu/sun50i: Implement .iotlb_sync_map

drivers/iommu/sun50i-iommu.c | 88 ++++++++++++++++++++++++++++++++++--
1 file changed, 83 insertions(+), 5 deletions(-)

--
2.38.1



2022-10-25 17:44:53

by Jernej Škrabec

[permalink] [raw]
Subject: [PATCH v2 5/5] iommu/sun50i: Implement .iotlb_sync_map

Allocated iova ranges need to be invalidated immediately or otherwise
they might or might not work when used by master or CPU. This was
discovered when running video decoder conformity test with Cedrus. Some
videos were now and then decoded incorrectly and generated page faults.

According to vendor driver, it's enough to invalidate just start and end
TLB and PTW cache lines. Documentation says that neighbouring lines must
be invalidated too. Finally, when page fault occurs, that iova must be
invalidated the same way, according to documentation.

Fixes: 4100b8c229b3 ("iommu: Add Allwinner H6 IOMMU driver")
Signed-off-by: Jernej Skrabec <[email protected]>
---
Changes from v1:
- new patch

drivers/iommu/sun50i-iommu.c | 73 ++++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)

diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index e62e245060ac..5cb2d44dfb92 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -93,6 +93,8 @@
#define NUM_PT_ENTRIES 256
#define PT_SIZE (NUM_PT_ENTRIES * PT_ENTRY_SIZE)

+#define SPAGE_SIZE 4096
+
struct sun50i_iommu {
struct iommu_device iommu;

@@ -295,6 +297,62 @@ static void sun50i_table_flush(struct sun50i_iommu_domain *sun50i_domain,
dma_sync_single_for_device(iommu->dev, dma, size, DMA_TO_DEVICE);
}

+static void sun50i_iommu_zap_iova(struct sun50i_iommu *iommu,
+ unsigned long iova)
+{
+ u32 reg;
+ int ret;
+
+ iommu_write(iommu, IOMMU_TLB_IVLD_ADDR_REG, iova);
+ iommu_write(iommu, IOMMU_TLB_IVLD_ADDR_MASK_REG, GENMASK(31, 12));
+ iommu_write(iommu, IOMMU_TLB_IVLD_ENABLE_REG,
+ IOMMU_TLB_IVLD_ENABLE_ENABLE);
+
+ ret = readl_poll_timeout_atomic(iommu->base + IOMMU_TLB_IVLD_ENABLE_REG,
+ reg, !reg, 1, 2000);
+ if (ret)
+ dev_warn(iommu->dev, "TLB invalidation timed out!\n");
+}
+
+static void sun50i_iommu_zap_ptw_cache(struct sun50i_iommu *iommu,
+ unsigned long iova)
+{
+ u32 reg;
+ int ret;
+
+ iommu_write(iommu, IOMMU_PC_IVLD_ADDR_REG, iova);
+ iommu_write(iommu, IOMMU_PC_IVLD_ENABLE_REG,
+ IOMMU_PC_IVLD_ENABLE_ENABLE);
+
+ ret = readl_poll_timeout_atomic(iommu->base + IOMMU_PC_IVLD_ENABLE_REG,
+ reg, !reg, 1, 2000);
+ if (ret)
+ dev_warn(iommu->dev, "PTW cache invalidation timed out!\n");
+}
+
+static void sun50i_iommu_zap_range(struct sun50i_iommu *iommu,
+ unsigned long iova, size_t size)
+{
+ assert_spin_locked(&iommu->iommu_lock);
+
+ iommu_write(iommu, IOMMU_AUTO_GATING_REG, 0);
+
+ sun50i_iommu_zap_iova(iommu, iova);
+ sun50i_iommu_zap_iova(iommu, iova + SPAGE_SIZE);
+ if (size > SPAGE_SIZE) {
+ sun50i_iommu_zap_iova(iommu, iova + size);
+ sun50i_iommu_zap_iova(iommu, iova + size + SPAGE_SIZE);
+ }
+ sun50i_iommu_zap_ptw_cache(iommu, iova);
+ sun50i_iommu_zap_ptw_cache(iommu, iova + SZ_1M);
+ if (size > SZ_1M) {
+ sun50i_iommu_zap_ptw_cache(iommu, iova + size);
+ sun50i_iommu_zap_ptw_cache(iommu, iova + size + SZ_1M);
+ }
+
+ iommu_write(iommu, IOMMU_AUTO_GATING_REG, IOMMU_AUTO_GATING_ENABLE);
+}
+
static int sun50i_iommu_flush_all_tlb(struct sun50i_iommu *iommu)
{
u32 reg;
@@ -344,6 +402,18 @@ static void sun50i_iommu_flush_iotlb_all(struct iommu_domain *domain)
spin_unlock_irqrestore(&iommu->iommu_lock, flags);
}

+static void sun50i_iommu_iotlb_sync_map(struct iommu_domain *domain,
+ unsigned long iova, size_t size)
+{
+ struct sun50i_iommu_domain *sun50i_domain = to_sun50i_domain(domain);
+ struct sun50i_iommu *iommu = sun50i_domain->iommu;
+ unsigned long flags;
+
+ spin_lock_irqsave(&iommu->iommu_lock, flags);
+ sun50i_iommu_zap_range(iommu, iova, size);
+ spin_unlock_irqrestore(&iommu->iommu_lock, flags);
+}
+
static void sun50i_iommu_iotlb_sync(struct iommu_domain *domain,
struct iommu_iotlb_gather *gather)
{
@@ -767,6 +837,7 @@ static const struct iommu_ops sun50i_iommu_ops = {
.attach_dev = sun50i_iommu_attach_device,
.detach_dev = sun50i_iommu_detach_device,
.flush_iotlb_all = sun50i_iommu_flush_iotlb_all,
+ .iotlb_sync_map = sun50i_iommu_iotlb_sync_map,
.iotlb_sync = sun50i_iommu_iotlb_sync,
.iova_to_phys = sun50i_iommu_iova_to_phys,
.map = sun50i_iommu_map,
@@ -786,6 +857,8 @@ static void sun50i_iommu_report_fault(struct sun50i_iommu *iommu,
report_iommu_fault(iommu->domain, iommu->dev, iova, prot);
else
dev_err(iommu->dev, "Page fault while iommu not attached to any domain?\n");
+
+ sun50i_iommu_zap_range(iommu, iova, SPAGE_SIZE);
}

static phys_addr_t sun50i_iommu_handle_pt_irq(struct sun50i_iommu *iommu,
--
2.38.1


2022-11-03 15:26:00

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH v2 0/5] iommu/sun50i: Fix various fixes

On Tue, Oct 25, 2022 at 06:54:10PM +0200, Jernej Skrabec wrote:
> Jernej Skrabec (5):
> iommu/sun50i: Fix reset release
> iommu/sun50i: Consider all fault sources for reset
> iommu/sun50i: Fix R/W permission check
> iommu/sun50i: Fix flush size
> iommu/sun50i: Implement .iotlb_sync_map

Applied, thanks.