On Friday 14 March 2014 12:38 PM, Laurent Pinchart wrote: > Hi Santosh, > > On Friday 14 March 2014 12:15:11 Santosh Shilimkar wrote: >> + Russell, Arnd >> >> On Thursday 13 March 2014 10:47 PM, Anna, Suman wrote: >>> On 03/07/2014 06:46 PM, Laurent Pinchart wrote: >>>> The page table entries must be cleaned from the cache before being >>>> accessed by the IOMMU. Instead of implementing cache management manually >>>> (and ignoring L2 cache), use clean_dcache_area() to make sure the >>>> entries are visible to the device. >>> >>> Thanks for fixing this, this has been long pending. There have been some >>> previous attempts at this as well by Ramesh Gupta, with the last thread >>> (a long time now) being >>> http://marc.info/?t=134752035400001&r=1&w=2 >>> >>> Santosh, >>> Can you please take a look at this patch and provide your comments? >>> >>> regards >>> Suman >>> >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> >>>> --- >>>> >>>> drivers/iommu/omap-iommu.c | 41 ++++++++++----------------------------- >>>> 1 file changed, 10 insertions(+), 31 deletions(-) >>>> >>>> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c >>>> index a893eca..bb605c9 100644 >>>> --- a/drivers/iommu/omap-iommu.c >>>> +++ b/drivers/iommu/omap-iommu.c >>>> @@ -500,24 +500,9 @@ EXPORT_SYMBOL_GPL(omap_foreach_iommu_device); >>>> /* >>>> * H/W pagetable operations >>>> */ >>>> -static void flush_iopgd_range(u32 *first, u32 *last) >>>> +static void flush_pgtable(void *addr, size_t size) >>>> { >>>> - /* FIXME: L2 cache should be taken care of if it exists */ >>>> - do { >>>> - asm("mcr p15, 0, %0, c7, c10, 1 @ flush_pgd" >>>> - : : "r" (first)); >>>> - first += L1_CACHE_BYTES / sizeof(*first); >>>> - } while (first <= last); >>>> -} >>>> - >>>> -static void flush_iopte_range(u32 *first, u32 *last) >>>> -{ >>>> - /* FIXME: L2 cache should be taken care of if it exists */ >>>> - do { >>>> - asm("mcr p15, 0, %0, c7, c10, 1 @ flush_pte" >>>> - : : "r" (first)); >>>> - first += L1_CACHE_BYTES / sizeof(*first); >>>> - } while (first <= last); >>>> + clean_dcache_area(addr, size); >> >> I remember NAKing this approach in past and my stand remains same. >> The cache APIs which you are trying to use here are not suppose >> to be used outside. > > Please note that the omap-iommu driver already uses clean_dcache_area(). > That's where I got the idea :-) > So that fall through the cracks then ;-) >> I think the right way to fix this is to make use of streaming APIs. >> If needed, IOMMU can have its own dma_ops for special case >> handling if any. > > I can replace clean_dcache_area() with dma_map_page() as done by the arm-smmu > driver. If that's fine I'll modify this patch accordingly in v2. > That will be definitely a better option than the APIs used. Those streaming APIs will take care of L2 cache as well if present. Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
