linux-kernel - Re: [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <3a8a03d6-a544-b9f3-15a2-f6a2411bdb28@collabora.com>
Date:   Thu, 1 Nov 2018 13:56:35 +0100
From:   Robert Foss <robert.foss@...labora.com>
To:     Emil Velikov <emil.l.velikov@...il.com>
Cc:     David Airlie <airlied@...ux.ie>, Gerd Hoffmann <kraxel@...hat.com>,
        ML dri-devel <dri-devel@...ts.freedesktop.org>,
        "open list:VIRTIO GPU DRIVER" 
        <virtualization@...ts.linux-foundation.org>,
        "Linux-Kernel@...r. Kernel. Org" <linux-kernel@...r.kernel.org>,
        Rob Herring <robh@...nel.org>,
        Gustavo Padovan <gustavo.padovan@...labora.com>,
        Emil Velikov <emil.velikov@...labora.com>
Subject: Re: [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences



On 2018-10-31 10:38, Emil Velikov wrote:
> Hi Rob,
> 
> On Thu, 25 Oct 2018 at 19:38, Robert Foss <robert.foss@...labora.com> wrote:
>>
>> Add a new field called fence_fd that will be used by userspace to send
>> in-fences to the kernel and receive out-fences created by the kernel.
>>
>> This uapi enables virtio to take advantage of explicit synchronization of
>> dma-bufs.
>>
>> There are two new flags:
>>
>> * VIRTGPU_EXECBUF_FENCE_FD_IN to be used when passing an in-fence fd.
>> * VIRTGPU_EXECBUF_FENCE_FD_OUT to be used when requesting an out-fence fd
>>
>> The execbuffer IOCTL is now read-write to allow the userspace to read the
>> out-fence.
>>
>> On error -1 should be returned in the fence_fd field.
>>
>> Signed-off-by: Gustavo Padovan <gustavo.padovan@...labora.com>
>> Signed-off-by: Robert Foss <robert.foss@...labora.com>
>> ---
>> Changes since v2:
>>   - Since exbuf-flags is a new flag, check that unsupported
>>     flags aren't set.
>>
>>   drivers/gpu/drm/virtio/virtgpu_ioctl.c |  5 +++++
>>   include/uapi/drm/virtgpu_drm.h         | 13 ++++++++++---
>>   2 files changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>> index d01a9ed100d1..1af289b28fc4 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>> @@ -116,9 +116,14 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
>>          struct ww_acquire_ctx ticket;
>>          void *buf;
>>
>> +       exbuf->fence_fd = -1;
>> +
> Move this after the sanity checking.

Agreed. Fixed in v4

> 
>>          if (vgdev->has_virgl_3d == false)
>>                  return -ENOSYS;
>>
>> +       if ((exbuf->flags & ~VIRTGPU_EXECBUF_FLAGS))
>> +               return -EINVAL;
>> +
> I assume this did this trigger when using old userspace?

No, not as far as I'm aware. This check is there to prevent userspace from
polluting the bitspace of flag, so that all free bits can be used for new flags.

As far as I understand this is pointed out by a drm driver development document
written by danvet, which I unfortunately can't seem to find the link for at the
moment.

> 
> With those the patch is
> Reviewed-by: Emil Velikov <emil.velikov@...labora.com>
> 
> Thanks
> Emil
>