Re: [RFC 2/3] iommu/samsung: Introduce Exynos sysmmu-v8 driver

[Marek Szyprowski <m.szyprowski@samsung.com>]

On 22.06.2022 11:14, Robin Murphy wrote:
> On 2022-06-21 20:57, Sam Protsenko wrote:
>> Hi Marek,
>>
>> On Fri, 21 Jan 2022 at 14:31, Marek Szyprowski 
>> <m.szyprowski@samsung.com> wrote:
>>
>> [snip]
>>
>>>
>>> Well, for starting point the existing exynos-iommu driver is really
>>> enough. I've played a bit with newer Exyos SoCs some time ago. If I
>>> remember right, if you limit the iommu functionality to the essential
>>> things like mapping pages to IO-virtual space, the hardware differences
>>> between SYSMMU v5 (already supported by the exynos-iommu driver) and v7
>>> are just a matter of changing a one register during the initialization
>>> and different bits the page fault reason decoding. You must of course
>>> rely on the DMA-mapping framework and its implementation based on
>>> mainline DMA-IOMMU helpers. All the code for custom iommu group(s)
>>> handling or extended fault management are not needed for the initial
>>> version.
>>>
>>
>> Thanks for the advice! Just implemented some testing driver, which
>> uses "Emulated Translation" registers available on SysMMU v7. That's
>> one way to verify the IOMMU driver with no actual users of it. It
>> works fine with vendor SysMMU driver I ported to mainline earlier, and
>> now I'm trying to use it with exynos-sysmmu driver (existing
>> upstream). If you're curious -- I can share the testing driver
>> somewhere on GitHub.
>>
>> I believe the register you mentioned is PT_BASE one, so I used
>> REG_V7_FLPT_BASE_VM = 0x800C instead of REG_V5_PT_BASE_PFN. But I
>> didn't manage to get that far, unfortunately, as
>> exynos_iommu_domain_alloc() function fails in my case, with BUG_ON()
>> at this line:
>>
>>      /* For mapping page table entries we rely on dma == phys */
>>      BUG_ON(handle != virt_to_phys(domain->pgtable));
>>
>> One possible explanation for this BUG is that "dma-ranges" property is
>> not provided in DTS (which seems to be the case right now for all
>> users of "samsung,exynos-sysmmu" driver). Because of that the SWIOTLB
>> is used for dma_map_single() call (in exynos_iommu_domain_alloc()
>> function), which in turn leads to that BUG. At least that's what
>> happens in my case. The call chain looks like this:
>>
>>      exynos_iommu_domain_alloc()
>>          v
>>      dma_map_single()
>>          v
>>      dma_map_single_attrs()
>>          v
>>      dma_map_page_attrs()
>>          v
>>      dma_direct_map_page()  // dma_capable() == false
>>          v
>>      swiotlb_map()
>>          v
>>      swiotlb_tbl_map_single()
>>
>> And the last call of course always returns the address different than
>> the address for allocated pgtable. E.g. in my case I see this:
>>
>>      handle = 0x00000000fbfff000
>>      virt_to_phys(domain->pgtable) = 0x0000000880d0c000
>>
>> Do you know what might be the reason for that? I just wonder how the
>> SysMMU driver work for all existing Exynos platforms right now. I feel
>> I might be missing something, like some DMA option should be enabled
>> so that SWIOTLB is not used, or something like that. Please let me
>> know if you have any idea on possible cause. The vendor's SysMMU
>> driver is kinda different in that regard, as it doesn't use
>> dma_map_single(), so I don't see such issue there.
>
> If this SysMMU version is capable of addressing more than 32 bits, 
> then exynos_iommu_probe_device() should set its DMA masks appropriately.

Well, Sam's question was about the Exynos SYSMMU own platform device, 
not the device for which Exynos SYSMMU manages the IO virtual address 
space.

Simply add something like

dma_set_mask(dev, DMA_BIT_MASK(36));

to the beginning of the exynos_sysmmu_probe(). This will disable SWIOTLB 
and switch to DMA-direct for the Exynos SYSMMU platform device.


> (as a side note since I looked, the use of PAGE_SIZE/PAGE_SHIFT in the 
> driver looks wrong, since I can't imagine that the hardware knows 
> whether Linux is using 4KB, 16KB or 64KB and adjusts itself 
> accordingly...)

Right, this has to be cleaned up. This driver was never used on systems 
with kernel configuration for non-4KB pages.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland