linux-kernel - Re: [PATCH 4/4] drm/msm: bump UAPI version

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: <CAK8P3a2FZfVbGKEud+T54EUR0ZMKPeW9VzLmU2s97+TPQhroCg@mail.gmail.com>
Date:   Fri, 30 Nov 2018 16:36:07 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Rob Clark <robdclark@...il.com>
Cc:     dri-devel <dri-devel@...ts.freedesktop.org>,
        Jordan Crouse <jcrouse@...eaurora.org>,
        David Airlie <airlied@...ux.ie>, linux-arm-msm@...r.kernel.org,
        freedreno@...ts.freedesktop.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/4] drm/msm: bump UAPI version

On Fri, Nov 30, 2018 at 4:31 PM Rob Clark <robdclark@...il.com> wrote:
>
> On Fri, Nov 30, 2018 at 10:12 AM Arnd Bergmann <arnd@...db.de> wrote:
> >
> > On Fri, Nov 30, 2018 at 4:02 PM Rob Clark <robdclark@...il.com> wrote:
> > >
> > > Signed-off-by: Rob Clark <robdclark@...il.com>
> > > ---
> > >  drivers/gpu/drm/msm/msm_drv.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> > > index 6ebbd5010722..782cc33916d6 100644
> > > --- a/drivers/gpu/drm/msm/msm_drv.c
> > > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > > @@ -36,9 +36,11 @@
> > >   * - 1.3.0 - adds GMEM_BASE + NR_RINGS params, SUBMITQUEUE_NEW +
> > >   *           SUBMITQUEUE_CLOSE ioctls, and MSM_INFO_IOVA flag for
> > >   *           MSM_GEM_INFO ioctl.
> > > + * - 1.4.0 - softpin, MSM_RELOC_BO_DUMP, and GEM_INFO support to set/get
> > > + *           GEM object's debug name
> > >   */
> > >  #define MSM_VERSION_MAJOR      1
> > > -#define MSM_VERSION_MINOR      3
> > > +#define MSM_VERSION_MINOR      4
> > >  #define MSM_VERSION_PATCHLEVEL 0
> > >
> >
> > I don't know the background here, but generally speaking we don't have
> > version numbers for ioctls in kernel drivers. Instead, the old ioctls
> > need to remain functional, but you can add new ioctl commands
> > in addition.
> >
> > Is there something that makes this driver special?
> >
>
> The version # indicates to userspace that some new features are
> supported, so that new userspace on kernel can work.  For example, the
> userspace side of setting a GEM obj debug name is:

Ok, got it.

> static void msm_bo_set_name(struct fd_bo *bo, const char *fmt, va_list ap)
> {
>     struct drm_msm_gem_info req = {
>             .handle = bo->handle,
>             .info = MSM_INFO_SET_NAME,
>     };
>     char buf[32];
>     int sz;
>
>     /* bail if kernel doesn't support this: */
>     if (bo->dev->version < FD_VERSION_SOFTPIN)
>         return;
>
>     sz = vsnprintf(buf, sizeof(buf), fmt, ap);
>
>     req.value = VOID2U64(buf);
>     req.len = MIN2(sz, sizeof(buf));
>
>     drmCommandWrite(bo->dev->fd, DRM_MSM_GEM_INFO, &req, sizeof(req));
> }

So that version check seems harmless, but also not necessary,
at least in this case, right? I would assume that calling into
drmCommandWrite() with an invalid command will only return
an error, which then gets ignored, where with the check, we
would skip the call, knowing that it wont't work.

      Arnd