Thread (27 messages) 27 messages, 5 authors, 2022-02-08

Re: [PATCH v1 9/9] drivers: hv: dxgkrnl: Implement DXGSYNCFILE

From: Daniel Vetter <hidden>
Date: 2022-01-17 09:35:46
Also in: dri-devel, lkml

On Mon, Jan 17, 2022 at 9:34 AM Iouri Tarassov
[off-list ref] wrote:

On 1/14/2022 10:03 AM, Daniel Vetter wrote:
quoted
Hi all,

On Wed, Jan 12, 2022 at 11:55:14AM -0800, Iouri Tarassov wrote:
quoted
Implement the LX_DXCREATESYNCFILE IOCTL (D3DKMTCreateSyncFile).

dxgsyncfile is built on top of the Linux sync_file object and
provides a way for the user mode to synchronize with the execution
of the device DMA packets.

The IOCTL creates a dxgsyncfile object for the given GPU synchronization
object and a fence value. A sync_object file descriptor is returned to
the caller. The caller could wait for the object by using poll().
When the GPU synchronization object is signaled on the host, the host
sends a message to the virtual machine and the sync_file object is
signaled.

Signed-off-by: Iouri Tarassov <redacted>
Adding dri-devel, which get_maintainers.pl should have done automatically
with the dma_fence wildcard match. Not sure why that didn't happen.
quoted
+struct dxgsyncpoint {
+   struct dxghostevent     hdr;
+   struct dma_fence        base;
This doesn't work unfortuntately. For better or worse memory fences like
monitored fences from wddm have completely different semantics from
dma_fence. You could probably hack this to be self-consistent for hyper-v,
but the problem is that then hv would have incompatible locking/nesting
rules compared to everything else, and dma_fence matter for memory
management so this includes whether you're allowed to kmalloc(GFP_KERNEL)
or not, and that's just a bit too much.

I discussed this quickly with Jesse on irc and it sounds like the reason
you want the dma_fence is just to emulate the sync_file interface for
android. I think the correct solution here is to create a hv_dxg_sync_file
fd, which emulates the exact ioctls that Android needs, but with a wddm
monitored fence underneath instead of a dma_fence underneath.

This way we guarantee that no one ever accidentally mixes these
incompatible concepts up in the kernel, and Android should still be able
to happily run under hyperv.

Thoughts?

Also pls cc me on this sync work since even if you drop dma_fence use
completely I'd like to follow this a bit.
Hi Daniel,

Thank you for the review and feedback.
I will get this addressed.
btw another idea I had over the w/e: Another option might be to allow
different backends for sync_file, and then making sure that you cannot
ever mix dma_fence and hv_dxg_fence type sync_file up (in e.g. the
merge ioctl).

The issue is that fundamentally dma_fence and memory fences (or umf
for userspace memory fences as we tend to call them) aren't
compatible, but some of the interop plans we have is to allow stuffing
either of them into fence container objects like sync_file. So going
that route for wddm monitored fence support too could be a really
future-proof approach, plus it'd allow you to still share the
sync_file interface code. Not that it's going to be much code sharing,
since all the implementation code needs to be distinct.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help