linux-kernel - Re: [PATCH 3/5] update virtio gpu driver: add 3d/virgl support

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] [day] [month] [year] [list]

Date:	Thu, 10 Sep 2015 16:45:31 +0200
From:	Gerd Hoffmann <kraxel@...hat.com>
To:	Emil Velikov <emil.l.velikov@...il.com>
Cc:	ML dri-devel <dri-devel@...ts.freedesktop.org>,
	"Michael S. Tsirkin" <mst@...hat.com>,
	"open list:ABI/API" <linux-api@...r.kernel.org>,
	open list <linux-kernel@...r.kernel.org>,
	"open list:VIRTIO GPU DRIVER" 
	<virtualization@...ts.linux-foundation.org>,
	Dave Airlie <airlied@...hat.com>
Subject: Re: [PATCH 3/5] update virtio gpu driver: add 3d/virgl support

  Hi,

> Just a FYI - Daniel Vetter has a series in flight which deprecates
> DRM_UNLOCKED for KMS drivers.

Thanks for the heads up.

> 
> > --- /dev/null
> > +++ b/include/uapi/drm/virtgpu_drm.h
> > @@ -0,0 +1,163 @@
> 
> > +
> > +struct drm_virtgpu_3d_box {
> > +       uint32_t x, y, z;
> > +       uint32_t w, h, d;
> > +};
> > +
> There was a similar case (multiple variables declared on a single
> line) in drm core that caused confusion and we broke the 32bit compat.
> I thought I mention it - not advocating for/against the above declaration.

I highly doubt we'll ever change that.
But we can give each struct field its own row, sure.

> > +struct drm_virtgpu_3d_transfer_to_host {
> > +       uint32_t bo_handle;
> > +       struct drm_virtgpu_3d_box box;
> > +       uint32_t level;
> > +       uint32_t offset;
> > +};
> > +
> > +struct drm_virtgpu_3d_transfer_from_host {
> > +       uint32_t bo_handle;
> > +       struct drm_virtgpu_3d_box box;
> > +       uint32_t level;
> > +       uint32_t offset;
> > +};
> > +
> Afaics these seems to be used by the ioctls. If so the current
> declarations are not 32bit compat safe.

Why?  As long as we have only 32bit fields in the struct the struct
itself gets a 32bit alignment too.  So no layout differences between
32bit and 64bit.

> Things will also go badly if you consider expanding 
> struct drm_virtgpu_3d_box in the distant future.

See above, very unlikely.  And should that really happen we have bigger
problems anyway because it is quite likely that we have to touch the
virtio wire protocol too.

> A u32 pad after bo_handle and a 'pointer' to struct
> drm_virtgpu_3d_box might be the more flexible solution.

IMO this makes things more complicated for no good reason.

cheers,
  Gerd


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/