On Tue, Aug 6, 2019 at 8:50 AM Christoph Hellwig hch@lst.de wrote:

On Tue, Aug 06, 2019 at 07:11:41AM -0700, Rob Clark wrote:

...

Agreed that drm_cflush_* isn't a great API. In this particular case (IIUC), I need wb+inv so that there aren't dirty cache lines that drop out to memory later, and so that I don't get a cache hit on uncached/wc mmap'ing.

So what is the use case here? Allocate pages using the page allocator (or CMA for that matter), and then mmaping them to userspace and never touching them again from the kernel?

Currently, it is pages coming from tmpfs. Ideally we want pages that are swappable when unpinned.

CPU mappings are *mostly* just mapping to userspace. There are a few exceptions that are vmap'd (fbcon, and ringbuffer).

(Eventually I'd like to support pages passed in from userspace.. but that is down the road.)

...

Tying it in w/ iommu seems a bit weird to me.. but maybe that is just me, I'm certainly willing to consider proposals or to try things and see how they work out.

This was just my through as the fit seems easy. But maybe you'll need to explain your use case(s) a bit more so that we can figure out what a good high level API is.

Tying it to iommu_map/unmap would be awkward, as we could need to setup cpu mmap before it ends up mapped to iommu. And the plan to support per-process pagetables involved creating an iommu_domain per userspace gl context.. some buffers would end up mapped into multiple contexts/iommu_domains.

If the cache operation was detached from iommu_map/unmap, then it would seem weird to be part of the iommu API.

I guess I'm not entirely sure what you had in mind, but this is why iommu seemed to me like a bad fit.

...

Exposing the arch_sync_* API and using that directly (bypassing drm_cflush_*) actually seems pretty reasonable and pragmatic. I did have one doubt, as phys_to_virt() is only valid for kernel direct mapped memory (AFAIU), what happens for pages that are not in kernel linear map? Maybe it is ok to ignore those pages, since they won't have an aliased mapping?

They could have an aliased mapping in vmalloc/vmap space for example, if you created one. We have the flush_kernel_vmap_range / invalidate_kernel_vmap_range APIs for those, that are implement on architectures with VIVT caches.

If I only have to worry about aliased mappings that I create myself, that doesn't seem too bad.. I could track that myself easily enough.

BR, -R