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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e472cbe8-44ec-110a-1ad7-bc561cd0be88@linux.microsoft.com>
Date:   Fri, 14 Jan 2022 10:52:50 -0800
From:   Iouri Tarassov <iourit@...ux.microsoft.com>
To:     kys@...rosoft.com, haiyangz@...rosoft.com, sthemmin@...rosoft.com,
        wei.liu@...nel.org, linux-hyperv@...r.kernel.org,
        linux-kernel@...r.kernel.org, spronovo@...rosoft.com,
        gregkh@...uxfoundation.org,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        jenatali@...rosoft.com
Subject: Re: [PATCH v1 9/9] drivers: hv: dxgkrnl: Implement DXGSYNCFILE


On 1/14/2022 10:03 AM, Daniel Vetter wrote:
> Hi all,
>
> On Wed, Jan 12, 2022 at 11:55:14AM -0800, Iouri Tarassov wrote:
> > 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 <iourit@...ux.microsoft.com>
>
> Adding dri-devel, which get_maintainers.pl should have done automatically
> with the dma_fence wildcard match. Not sure why that didn't happen.
>
> > +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.

Iouri