Re: [PATCH v2] drm: Copy drm_wait_vblank to user before returning

[Mark Yacoub <markyacoub@chromium.org>]

Thanks for your review Michel!
@MAINTAINER, could you please strip the Change-Id when applying.
Thanks!

On Fri, Aug 13, 2021 at 3:33 AM Michel Dänzer <michel@daenzer.net> wrote:
>
> On 2021-08-12 9:49 p.m., Mark Yacoub wrote:
> > From: Mark Yacoub <markyacoub@google.com>
> >
> > [Why]
> > Userspace should get back a copy of drm_wait_vblank that's been modified
> > even when drm_wait_vblank_ioctl returns a failure.
> >
> > Rationale:
> > drm_wait_vblank_ioctl modifies the request and expects the user to read
> > it back. When the type is RELATIVE, it modifies it to ABSOLUTE and updates
> > the sequence to become current_vblank_count + sequence (which was
> > RELATIVE), but now it became ABSOLUTE.
> > drmWaitVBlank (in libdrm) expects this to be the case as it modifies
> > the request to be Absolute so it expects the sequence to would have been
> > updated.
> >
> > The change is in compat_drm_wait_vblank, which is called by
> > drm_compat_ioctl. This change of copying the data back regardless of the
> > return number makes it en par with drm_ioctl, which always copies the
> > data before returning.
> >
> > [How]
> > Return from the function after everything has been copied to user.
> >
> > Fixes: IGT:kms_flip::modeset-vs-vblank-race-interruptible
> > Tested on ChromeOS Trogdor(msm)
> >
> > Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
> > Change-Id: I98da279a5f1329c66a9d1e06b88d40b247b51313
>
> With the Gerrit Change-Id removed,
>
> Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
>
>
> --
> Earthling Michel Dänzer               |               https://redhat.com
> Libre software enthusiast             |             Mesa and X developer