Re: [PATCH v3 2/2] drm/uapi: add explicit virtgpu context debug name

[Gurchetan Singh <gurchetansingh@chromium.org>]

[-- Attachment #1: Type: text/plain, Size: 1539 bytes --]

On Sat, Nov 11, 2023 at 2:37 PM Dmitry Osipenko <
dmitry.osipenko@collabora.com> wrote:

> On 10/18/23 21:17, Gurchetan Singh wrote:
> > +             case VIRTGPU_CONTEXT_PARAM_DEBUG_NAME:
> > +                     if (vfpriv->explicit_debug_name) {
> > +                             ret = -EINVAL;
> > +                             goto out_unlock;
> > +                     }
> > +
> > +                     ret = strncpy_from_user(vfpriv->debug_name,
> > +                                             u64_to_user_ptr(value),
> > +                                             DEBUG_NAME_MAX_LEN - 1);
> > +
> > +                     if (ret < 0) {
> > +                             ret = -EFAULT;
> > +                             goto out_unlock;
> > +                     }
> > +
> > +                     vfpriv->explicit_debug_name = true;
> > +                     break;
>
> Spotted a problem here. The ret needs to be set to zero on success. I'll
> send the fix shortly. Gurchetan you should've been getting the
> DRM_IOCTL_VIRTGPU_CONTEXT_INIT failure from gfxstream when you tested
> this patch, haven't you?
>

To accommodate older kernels/QEMU, gfxstream doesn't fail if CONTEXT_INIT
fails.  So the guest thought it failed and didn't react, but the value was
propagated to the host.


>
> Also noticed that the patch title says "drm/uapi" instead of
> "drm/virtio". My bad for not noticing it earlier. Please be more careful
> next time too :)
>
> --
> Best regards,
> Dmitry
>
>

[-- Attachment #2: Type: text/html, Size: 2371 bytes --]