Re: [PATCH 2/5] iommu/omap: Fix 'no page for' debug message in flush_iotlb_page()
Hi Suman,
On Thursday 13 March 2014 17:16:07 Suman Anna wrote:
On 03/07/2014 06:46 PM, Laurent Pinchart wrote:
The flush_iotlb_page() function prints a debug message when no
corresponding page was found in the TLB. That condition is incorrectly
checked and always resolves to true, given that the for_each_iotlb_cr()
loop is never interrupted and always reaches obj-nr_tlb_entries.
Nice catch.
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
drivers/iommu/omap-iommu.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index bb605c9..cb1e1de 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -376,6 +376,7 @@ static void flush_iotlb_page(struct omap_iommu *obj,
u32 da)
{
int i;
struct cr_regs cr;
+ bool found = false;
pm_runtime_get_sync(obj-dev);
@@ -394,11 +395,12 @@ static void flush_iotlb_page(struct omap_iommu *obj,
u32 da)
__func__, start, da, bytes);
iotlb_load_cr(obj, cr);
iommu_write_reg(obj, 1, MMU_FLUSH_ENTRY);
+ found = true;
The patch is fine as it is, but I think the loop can be breaked when an
entry is found for simplification. I do not expect that we will have
multiple entries with the same da in the TLBs (it is a multi-hit fault
scenario) to continue looping through all entries. The only means for the
multi-hit scenario to happen is user error with programming TLBs directly.
OK. I wasn't sure whether we could have two TLB entries for the same VA. I'll
update the patch accordingly.
This function call seems to be added in general to take care of any
prefetching. I don't have the complete history on the PREFETCH_IOTLB on why
it was added, it doesn't seem to be enabled but this should ideally be left
to the h/w unless you are locking a TLB entry. DSP/Bridge is the only one
that uses locked TLB entries at the moment, but it is not using the OMAP
IOMMU driver yet.
}
}
pm_runtime_put_sync(obj-dev);
- if (i == obj-nr_tlb_entries)
+ if (!found)
dev_dbg(obj-dev, %s: no page for %08x\n, __func__, da);
}
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] iommu/omap: Fix 'no page for' debug message in flush_iotlb_page()
Hi Laurent,
On 03/07/2014 06:46 PM, Laurent Pinchart wrote:
The flush_iotlb_page() function prints a debug message when no
corresponding page was found in the TLB. That condition is incorrectly
checked and always resolves to true, given that the for_each_iotlb_cr()
loop is never interrupted and always reaches obj-nr_tlb_entries.
Nice catch.
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
drivers/iommu/omap-iommu.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index bb605c9..cb1e1de 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -376,6 +376,7 @@ static void flush_iotlb_page(struct omap_iommu *obj, u32 da)
{
int i;
struct cr_regs cr;
+ bool found = false;
pm_runtime_get_sync(obj-dev);
@@ -394,11 +395,12 @@ static void flush_iotlb_page(struct omap_iommu *obj, u32
da)
__func__, start, da, bytes);
iotlb_load_cr(obj, cr);
iommu_write_reg(obj, 1, MMU_FLUSH_ENTRY);
+ found = true;
The patch is fine as it is, but I think the loop can be breaked when an
entry is found for simplification. I do not expect that we will have
multiple entries with the same da in the TLBs (it is a multi-hit fault
scenario) to continue looping through all entries. The only means for
the multi-hit scenario to happen is user error with programming TLBs
directly. This function call seems to be added in general to take care
of any prefetching. I don't have the complete history on the
PREFETCH_IOTLB on why it was added, it doesn't seem to be enabled but
this should ideally be left to the h/w unless you are locking a TLB
entry. DSP/Bridge is the only one that uses locked TLB entries at the
moment, but it is not using the OMAP IOMMU driver yet.
regards
Suman
}
}
pm_runtime_put_sync(obj-dev);
- if (i == obj-nr_tlb_entries)
+ if (!found)
dev_dbg(obj-dev, %s: no page for %08x\n, __func__, da);
}
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] iommu/omap: Fix 'no page for' debug message in flush_iotlb_page()
The flush_iotlb_page() function prints a debug message when no
corresponding page was found in the TLB. That condition is incorrectly
checked and always resolves to true, given that the for_each_iotlb_cr()
loop is never interrupted and always reaches obj-nr_tlb_entries.
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
drivers/iommu/omap-iommu.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index bb605c9..cb1e1de 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -376,6 +376,7 @@ static void flush_iotlb_page(struct omap_iommu *obj, u32 da)
{
int i;
struct cr_regs cr;
+ bool found = false;
pm_runtime_get_sync(obj-dev);
@@ -394,11 +395,12 @@ static void flush_iotlb_page(struct omap_iommu *obj, u32
da)
__func__, start, da, bytes);
iotlb_load_cr(obj, cr);
iommu_write_reg(obj, 1, MMU_FLUSH_ENTRY);
+ found = true;
}
}
pm_runtime_put_sync(obj-dev);
- if (i == obj-nr_tlb_entries)
+ if (!found)
dev_dbg(obj-dev, %s: no page for %08x\n, __func__, da);
}
--
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
