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@amd.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