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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 21 Jan 2021 15:42:30 +0800
From:   Chunyan Zhang <zhang.lyra@...il.com>
To:     Robin Murphy <robin.murphy@....com>
Cc:     DTML <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Chunyan Zhang <chunyan.zhang@...soc.com>,
        Sheng Xu <sheng.xu@...soc.com>,
        iommu@...ts.linux-foundation.org, Rob Herring <robh+dt@...nel.org>,
        Kevin Tang <kevin.tang@...soc.com>,
        Baolin Wang <baolin.wang7@...il.com>,
        Orson Zhai <orsonzhai@...il.com>
Subject: Re: [RFC PATCH V2 2/2] iommu: add Unisoc iommu basic driver

On Wed, 20 Jan 2021 at 20:29, Robin Murphy <robin.murphy@....com> 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.