Hi Daniel,
thanks for jumping in here.
And yes, you are absolutely right we need to get this fixed and not yell
at each other that we have a different understanding of things.
Your proposal sounds sane to me, but I wouldn't call it slots. Rather
something like "use cases" since we can have multiple fences for each
category I think.
And I see at four here:
1. Internal kernel memory management. Everybody needs to wait for this,
it's equal to bo->moving.
2. Writers for implicit sync, implicit sync readers should wait for them.
3. Readers for implicit sync, implicit sync writers should wait for them.
4. Things like TLB flushes and page table updates, no implicit sync but
memory management must take them into account before moving/freeing
backing store.
Happy weekend and hopefully not so much heat guys.
Cheers,
Christian.
Am 18.06.21 um 20:20 schrieb Daniel Stone:
Sorry for the mobile reply, but V4L2 is absolutely not write-only; there has
never been an intersection of V4L2 supporting dmabuf and not supporting reads.
I see your point about the heritage of dma_resv but it’s a red herring. It
doesn’t matter who’s right, or who was first, or where the code was extracted
from.
It’s well defined that amdgpu defines resv to be one thing, that every other
non-TTM user defines it to be something very different, and that the other TTM
users define it to be something in the middle.
We’ll never get to anything workable if we keep arguing who’s right. Everyone
is wrong, because dma_resv doesn’t globally mean anything.
It seems clear that there are three classes of synchronisation barrier (not
using the ‘f’ word here), in descending exclusion order:
- memory management barriers (amdgpu exclusive fence / ttm_bo->moving)
- implicit synchronisation write barriers (everyone else’s exclusive fences,
amdgpu’s shared fences)
- implicit synchronisation read barriers (everyone else’s shared fences,
also amdgpu’s shared fences sometimes)
I don’t see a world in which these three uses can be reduced to two slots. What
also isn’t clear to me though, is how the memory-management barriers can
exclude all other access in the original proposal with purely userspace CS.
Retaining the three separate modes also seems like a hard requirement to not
completely break userspace, but then I don’t see how three separate slots would
work if they need to be temporally ordered. amdgpu fixed this by redefining the
meaning of the two slots, others fixed this by not doing one of the three modes.
So how do we square the circle without encoding a DAG into the kernel? Do the
two slots need to become a single list which is ordered by time + ‘weight’ and
flattened whenever modified? Something else?
Have a great weekend.
-d
On 18 Jun 2021, at 5:43 pm, Christian König <christian.koe...@amd.com> wrote:
Am 18.06.21 um 17:17 schrieb Daniel Vetter:
[SNIP]
Ignoring _all_ fences is officially ok for pinned dma-buf. This is
what v4l does. Aside from it's definitely not just i915 that does this
even on the drm side, we have a few more drivers nowadays.
No it seriously isn't. If drivers are doing this they are more than broken.
See the comment in dma-resv.h
* Based on bo.c which bears the following copyright notice,
* but is dual licensed:
....
The handling in ttm_bo.c is and always was that the exclusive fence is used for
buffer moves.
As I said multiple times now the *MAIN* purpose of the dma_resv object is
memory management and *NOT* synchronization.
Those restrictions come from the original design of TTM where the dma_resv
object originated from.
The resulting consequences are that:
a) If you access the buffer without waiting for the exclusive fence you run
into a potential information leak.
We kind of let that slip for V4L since they only access the buffers for
writes, so you can't do any harm there.
b) If you overwrite the exclusive fence with a new one without waiting for the
old one to signal you open up the possibility for userspace to access freed up
memory.
This is a complete show stopper since it means that taking over the system
is just a typing exercise.
What you have done by allowing this in is ripping open a major security hole
for any DMA-buf import in i915 from all TTM based driver.
This needs to be fixed ASAP, either by waiting in i915 and all other drivers
doing this for the exclusive fence while importing a DMA-buf or by marking i915
and all other drivers as broken.
Sorry, but if you allowed that in you seriously have no idea what you are
talking about here and where all of this originated from.
Regards,
Christian.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
