Re: [RFC PATCH V2 2/2] iommu: add Unisoc iommu basic driver

[Chunyan Zhang]

On Wed, 20 Jan 2021 at 20:29, Robin Murphy <robin.murphy@xxxxxxx> wrote:
>
> On 2021-01-20 11:40, Chunyan Zhang wrote:
> [...]
> >>> +     pgt_base_iova = dom->pgt_va +
> >>> +             ((iova - mdata->iova_start) >> SPRD_IOMMU_PAGE_SHIFT);
> >>> +
> >>> +     spin_lock_irqsave(&dom->pgtlock, flags);
> >>> +     for (i = 0; i < page_num; i++) {
> >>> +             pgt_base_iova[i] = pabase >> SPRD_IOMMU_PAGE_SHIFT;
> >>
> >> Out of curiosity, is the pagetable walker cache-coherent, or is this
> >> currently managing to work by pure chance and natural cache churn?
> >
> > ->iotlb_sync_map() was implemented in this driver, I guess that has
> > done what you say here?
>
> No, sync_map only ensures that the previous (invalid) PTE isn't held in
> the IOMMU's TLB. If pgt_va is a regular page allocation then you're
> writing the new PTE to normal kernel memory, with nothing to guarantee
> that write goes any further than the CPU's L1 cache. Thus either the
> IOMMU has capable of snooping the CPU caches in order to see the updated
> PTE value (rather than refetching the stale value from DRAM), or you're
> just incredibly lucky that by the time the IOMMU *does* go to fetch the
> PTE for that address, that updated cache line has already been evicted
> out to DRAM naturally.
>

Got it, thanks for the detailed explanation.
In order to make clear why this code can work, I made a test, and
found that if I wrote more than 1024 PTEs, the value would be updated
to DRAM immediately, otherwise the cache line seems not updated even
if I wrote 1023 PTEs.

> This is not an issue if you use the proper DMA allocator, since that
> will ensure you get a non-cacheable buffer if you need one.
>

I will switch to use dma_alloc_coherent().

Thanks again.

Chunyan

> Robin.