Message-ID: <YrY0cQY1BTL5H7Xp@phenom.ffwll.local>
Date:   Sat, 25 Jun 2022 00:02:25 +0200
From:   Daniel Vetter <daniel@...ll.ch>
To:     Christian König <christian.koenig@....com>
Cc:     Daniel Stone <daniel@...ishbar.org>,
        Pekka Paalanen <ppaalanen@...il.com>,
        "Sharma, Shashank" <Shashank.Sharma@....com>,
        lkml <linux-kernel@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Nicolas Dufresne <nicolas@...fresne.ca>,
        linaro-mm-sig@...ts.linaro.org,
        Sumit Semwal <sumit.semwal@...aro.org>,
        linux-media <linux-media@...r.kernel.org>
Subject: Re: [Linaro-mm-sig] Re: DMA-buf and uncached system memory

On Thu, Jun 23, 2022 at 01:32:18PM +0200, Christian König wrote:
> Am 23.06.22 um 13:27 schrieb Daniel Stone:
> > Hi Christian,
> > 
> > On Thu, 23 Jun 2022 at 12:11, Christian König <christian.koenig@....com> wrote:
> > > > In fact DMA-buf sharing works fine on most of those SoCs because
> > > > everyone just assumes that all the accelerators don't snoop, so the
> > > > memory shared via DMA-buf is mostly CPU uncached. It only falls apart
> > > > for uses like the UVC cameras, where the shared buffer ends up being
> > > > CPU cached.
> > > Well then the existing DMA-buf framework is not what you want to use for
> > > this.
> > > 
> > > > Non-coherent without explicit domain transfer points is just not going
> > > > to work. So why can't we solve the issue for DMA-buf in the same way as
> > > > the DMA API already solved it years ago: by adding the equivalent of
> > > > the dma_sync calls that do cache maintenance when necessary? On x86 (or
> > > > any system where things are mostly coherent) you could still no-op them
> > > > for the common case and only trigger cache cleaning if the importer
> > > > explicitly says that is going to do a non-snooping access.
> > > Because DMA-buf is a framework for buffer sharing between cache coherent
> > > devices which don't signal transitions.
> > > 
> > > We intentionally didn't implemented any of the dma_sync_* functions
> > > because that would break the intended use case.
> > > 
> > > You can of course use DMA-buf in an incoherent environment, but then you
> > > can't expect that this works all the time.
> > > 
> > > This is documented behavior and so far we have bluntly rejected any of
> > > the complains that it doesn't work on most ARM SoCs and I don't really
> > > see a way to do this differently.
> > For some strange reason, 'let's do buffer sharing but make sure it
> > doesn't work on Arm' wasn't exactly the intention of the groups who
> > came together under the Linaro umbrella to create dmabuf.
> > 
> > If it's really your belief that dmabuf requires universal snooping, I
> > recommend you send the patch to update the documentation, as well as
> > to remove DRIVER_PRIME from, realistically, most non-PCIE drivers.
> 
> Well, to be honest I think that would indeed be necessary.
> 
> What we have created are essentially two different worlds, one for PCI
> devices and one for the rest.
> 
> This was indeed not the intention, but it's a fact that basically all
> DMA-buf based PCI drivers assume coherent access.

dma-buf does not require universal snooping.

It does defacto require that all device access is coherent with all other
device access, and consistent with the exporters notion of how cpu
coherency is achieved. Not that coherent does not mean snooping, as long
as all devices do unsnooped access and the exporter either does wc/uc or
flushes caches that's perfectly fine, and how all the arm soc dma-buf
sharing works.

We did originally have the wording in there that you have to map/unamp
around every device access, but that got dropped because no one was doing
that anyway.

Now where this totally breaks down is how we make this work, because the
idea was that dma_buf_attach validates this all. Where this means all the
hilarious reasons buffer sharing might not work:
- wrong coherency mode (cpu cached or not)
- not contiguous (we do check that, but only once we get the sg from
  dma_buf_attachment_map, which strictly speaking is a bit too late but
  most drivers do attach&map as one step so not that bad in practice)
- whether the dma api will throw in bounce buffers or not
- random shit like "oh this is in the wrong memory bank", which I think
  never landed in upstream

p2p connectivity is about the only one that gets this right, yay. And the
only reason we can even get it right is because all the information is
exposed to drivers fully.

The issue is that the device dma api refuses to share this information
because it would "leak". Which sucks, because we have defacto build every
single cross-device use-case of dma-buf on the assumption we can check
this (up to gl/vk specs), but oh well.

So in practice this gets sorted out by endless piles of hacks to make
individual use-cases work.

Oh and: This is definitely not limited to arm socs. x86 socs with intel
at least have exactly all the same issues, and they get solved by adding
various shitty hacks to the involved drivers (like i915+amdgpu). Luckily
the intel camera driver isn't in upstream yet, since that would break a
bunch of the hacks since suddently there will be now 2 cpu cache
incoherent devices in an x86 system.

Ideally someone fixes this, but I'm not hopeful.

I recommend pouring more drinks.

What is definitely not correct is claiming that dma-buf wasn't meant for
this. We discussed cache coherency issues endless in budapest 12 or so
years ago, I was there. It's just that the reality of the current
implementation is falling short, and every time someone tries to fix it we
get shouted down by dma api maintainers for looking behind their current.

tldr; You have to magically know to not use cpu cached allocators on these
machines.

Aside: This is also why vgem alloates wc memory on x86. It's the least
common denominator that works. arm unfortunately doesn't allow you to
allocate wc memory, so there stuff is simply somewhat broken.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Powered by blists - more mailing lists