Re: [Mesa-dev] [PATCH 1/3] egl/x11_dri3: disable WL_bind_wayland_display for devices without render nodes

2016-08-18 Thread Frank Binns 

On 04/08/16 15:02, Frank Binns wrote:

On 20/06/16 12:05, Axel Davy wrote:

Hi,

The three patches make sense to me.

Reviewed-by: Axel Davy 


Can someone push these patches for me?

Thanks
Frank


Ping



On 17/06/2016 19:41, Frank Binns wrote :
Up until now, DRI3 was only used for devices that have render nodes, 
unless
overridden via an environment variable, with it falling back to DRI2 
otherwise.
This limitation was there in order to support 
WL_bind_wayland_display as it
requires client opened device node fds to be authenticated, which 
isn't possible
when using DRI3. This is an unfortunate compromise as DRI3 provides 
security

benefits over DRI2.

Instead, allow DRI3 to be used for devices without render nodes but 
don't
advertise WL_bind_wayland_display in this case. Applications that 
need this
extension can still be run by disabling DRI3 support via the 
LIBGL_DRI3_DISABLE

environment variable.

Signed-off-by: Frank Binns 
---
  src/egl/drivers/dri2/platform_x11.c  |  3 ++-
  src/egl/drivers/dri2/platform_x11_dri3.c | 33 
+---

  2 files changed, 7 insertions(+), 29 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_x11.c 
b/src/egl/drivers/dri2/platform_x11.c

index c0a4005..1b50889 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -1335,7 +1335,8 @@ dri2_initialize_x11_dri3(_EGLDriver *drv, 
_EGLDisplay *disp)

 disp->Extensions.EXT_buffer_age = EGL_TRUE;
#ifdef HAVE_WAYLAND_PLATFORM
-   disp->Extensions.WL_bind_wayland_display = EGL_TRUE;
+   if (dri2_dpy->device_name)
+  disp->Extensions.WL_bind_wayland_display = EGL_TRUE;
  #endif
   if (dri2_dpy->conn) {
diff --git a/src/egl/drivers/dri2/platform_x11_dri3.c 
b/src/egl/drivers/dri2/platform_x11_dri3.c

index 9363a8a..b781987 100644
--- a/src/egl/drivers/dri2/platform_x11_dri3.c
+++ b/src/egl/drivers/dri2/platform_x11_dri3.c
@@ -437,29 +437,6 @@ struct dri2_egl_display_vtbl 
dri3_x11_display_vtbl = {

 .get_dri_drawable = dri3_get_dri_drawable,
  };
  -static char *
-dri3_get_device_name(int fd)
-{
-   char *ret = NULL;
-
-   ret = drmGetRenderDeviceNameFromFd(fd);
-   if (ret)
-  return ret;
-
-   /* For dri3, render node support is required for 
WL_bind_wayland_display.
-* In order not to regress on older systems without kernel or 
libdrm
-* support, fall back to dri2. User can override it with 
environment

-* variable if they don't need to use that extension.
-*/
-   if (getenv("EGL_FORCE_DRI3") == NULL) {
-  _eglLog(_EGL_WARNING, "Render node support not available, 
falling back to dri2");
-  _eglLog(_EGL_WARNING, "If you want to force dri3, set 
EGL_FORCE_DRI3 environment variable");

-   } else
-  ret = loader_get_device_name_for_fd(fd);
-
-   return ret;
-}
-
  EGLBoolean
  dri3_x11_connect(struct dri2_egl_display *dri2_dpy)
  {
@@ -539,11 +516,11 @@ dri3_x11_connect(struct dri2_egl_display 
*dri2_dpy)

return EGL_FALSE;
 }
  -   dri2_dpy->device_name = dri3_get_device_name(dri2_dpy->fd);
-   if (!dri2_dpy->device_name) {
-  close(dri2_dpy->fd);
-  return EGL_FALSE;
-   }
+   /* Only try to get a render device name since it's only needed for
+* WL_bind_wayland_display and dri3 doesn't provide a mechanism for
+* authenticating client opened device node fds. If this fails then
+* don't advertise the extension. */
+   dri2_dpy->device_name = drmGetRenderDeviceNameFromFd(dri2_dpy->fd);
   return EGL_TRUE;
  }






___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] egl/x11_dri3: disable WL_bind_wayland_display for devices without render nodes

2016-08-04 Thread Frank Binns 

On 20/06/16 12:05, Axel Davy wrote:

Hi,

The three patches make sense to me.

Reviewed-by: Axel Davy 


Can someone push these patches for me?

Thanks
Frank



On 17/06/2016 19:41, Frank Binns wrote :
Up until now, DRI3 was only used for devices that have render nodes, 
unless
overridden via an environment variable, with it falling back to DRI2 
otherwise.
This limitation was there in order to support WL_bind_wayland_display 
as it
requires client opened device node fds to be authenticated, which 
isn't possible
when using DRI3. This is an unfortunate compromise as DRI3 provides 
security

benefits over DRI2.

Instead, allow DRI3 to be used for devices without render nodes but 
don't
advertise WL_bind_wayland_display in this case. Applications that 
need this
extension can still be run by disabling DRI3 support via the 
LIBGL_DRI3_DISABLE

environment variable.

Signed-off-by: Frank Binns 
---
  src/egl/drivers/dri2/platform_x11.c  |  3 ++-
  src/egl/drivers/dri2/platform_x11_dri3.c | 33 
+---

  2 files changed, 7 insertions(+), 29 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_x11.c 
b/src/egl/drivers/dri2/platform_x11.c

index c0a4005..1b50889 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -1335,7 +1335,8 @@ dri2_initialize_x11_dri3(_EGLDriver *drv, 
_EGLDisplay *disp)

 disp->Extensions.EXT_buffer_age = EGL_TRUE;
#ifdef HAVE_WAYLAND_PLATFORM
-   disp->Extensions.WL_bind_wayland_display = EGL_TRUE;
+   if (dri2_dpy->device_name)
+  disp->Extensions.WL_bind_wayland_display = EGL_TRUE;
  #endif
   if (dri2_dpy->conn) {
diff --git a/src/egl/drivers/dri2/platform_x11_dri3.c 
b/src/egl/drivers/dri2/platform_x11_dri3.c

index 9363a8a..b781987 100644
--- a/src/egl/drivers/dri2/platform_x11_dri3.c
+++ b/src/egl/drivers/dri2/platform_x11_dri3.c
@@ -437,29 +437,6 @@ struct dri2_egl_display_vtbl 
dri3_x11_display_vtbl = {

 .get_dri_drawable = dri3_get_dri_drawable,
  };
  -static char *
-dri3_get_device_name(int fd)
-{
-   char *ret = NULL;
-
-   ret = drmGetRenderDeviceNameFromFd(fd);
-   if (ret)
-  return ret;
-
-   /* For dri3, render node support is required for 
WL_bind_wayland_display.

-* In order not to regress on older systems without kernel or libdrm
-* support, fall back to dri2. User can override it with environment
-* variable if they don't need to use that extension.
-*/
-   if (getenv("EGL_FORCE_DRI3") == NULL) {
-  _eglLog(_EGL_WARNING, "Render node support not available, 
falling back to dri2");
-  _eglLog(_EGL_WARNING, "If you want to force dri3, set 
EGL_FORCE_DRI3 environment variable");

-   } else
-  ret = loader_get_device_name_for_fd(fd);
-
-   return ret;
-}
-
  EGLBoolean
  dri3_x11_connect(struct dri2_egl_display *dri2_dpy)
  {
@@ -539,11 +516,11 @@ dri3_x11_connect(struct dri2_egl_display 
*dri2_dpy)

return EGL_FALSE;
 }
  -   dri2_dpy->device_name = dri3_get_device_name(dri2_dpy->fd);
-   if (!dri2_dpy->device_name) {
-  close(dri2_dpy->fd);
-  return EGL_FALSE;
-   }
+   /* Only try to get a render device name since it's only needed for
+* WL_bind_wayland_display and dri3 doesn't provide a mechanism for
+* authenticating client opened device node fds. If this fails then
+* don't advertise the extension. */
+   dri2_dpy->device_name = drmGetRenderDeviceNameFromFd(dri2_dpy->fd);
   return EGL_TRUE;
  }




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] egl/x11_dri3: disable WL_bind_wayland_display for devices without render nodes

2016-06-21 Thread Michel Dänzer 
On 21.06.2016 18:49, Emil Velikov wrote:
> On 21 June 2016 at 07:39, Michel Dänzer  wrote:
>> On 21.06.2016 15:24, Axel Davy wrote:
>>> On 21/06/2016 01:26, Michel Dänzer wrote:
 On 20.06.2016 20:06, Frank Binns wrote:
> On 20/06/16 10:48, Michel Dänzer wrote:
>> On 18.06.2016 02:41, Frank Binns wrote:
>>> Up until now, DRI3 was only used for devices that have render nodes,
>>> unless
>>> overridden via an environment variable, with it falling back to DRI2
>>> otherwise.
>>> This limitation was there in order to support WL_bind_wayland_display
>>> as it
>>> requires client opened device node fds to be authenticated, which
>>> isn't possible
>>> when using DRI3. This is an unfortunate compromise as DRI3 provides
>>> security
>>> benefits over DRI2.
>> What exactly is it that works with render nodes but not with
>> unauthenticated non-render nodes? Isn't that a kernel bug?
> The problem isn't that something doesn't work with unauthenticated
> non-render nodes (wouldn't that be the kernel bug?) but that if a
> client opens the primary/legacy node it needs the resulting fd to
> be authenticated, which isn't supported by the X11 DRI3 protocol.
 Authentication is required for using certain ioctls of non-render nodes.
 It sounds like some ioctls are allowed to be used with render nodes but
 not with unauthenticated non-render nodes, which seems like a kernel bug
 — why would an ioctl be safe to use without authentication via a render
 node but not via a non-render node?

 So, which ioctls required by WL_bind_wayland_display don't work with an
 unauthenticated non-render node?


>>> The ioctl to authenticate is possible only when you have the master node
>>> (owned by the DDX when using X, or by the wayland compositor when using
>>> Wayland). There is only one master node.
>>>
>>>
>>> On of the motives of render-nodes is precisely to fix these limitations.
>>
>> I understand all of that, but it doesn't answer my question. :)
>>
>> Since WL_bind_wayland_display works with a render node, it means that
>> all ioctls it needs work without authentication when using a render
>> node. The question is, why do the same ioctls require authentication
>> when using a non-render node?
>>
>>
> I believe the following thread [1] is relevant here.
> 
> -Emil
> [1] https://lists.freedesktop.org/archives/dri-devel/2016-June/110845.html

Bottom line: It's just not a simple as I thought. :) Sorry for the
noise, and thanks for bearing with me.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] egl/x11_dri3: disable WL_bind_wayland_display for devices without render nodes

2016-06-21 Thread Emil Velikov 
On 21 June 2016 at 07:39, Michel Dänzer  wrote:
> On 21.06.2016 15:24, Axel Davy wrote:
>> On 21/06/2016 01:26, Michel Dänzer wrote:
>>> On 20.06.2016 20:06, Frank Binns wrote:
 On 20/06/16 10:48, Michel Dänzer wrote:
> On 18.06.2016 02:41, Frank Binns wrote:
>> Up until now, DRI3 was only used for devices that have render nodes,
>> unless
>> overridden via an environment variable, with it falling back to DRI2
>> otherwise.
>> This limitation was there in order to support WL_bind_wayland_display
>> as it
>> requires client opened device node fds to be authenticated, which
>> isn't possible
>> when using DRI3. This is an unfortunate compromise as DRI3 provides
>> security
>> benefits over DRI2.
> What exactly is it that works with render nodes but not with
> unauthenticated non-render nodes? Isn't that a kernel bug?
 The problem isn't that something doesn't work with unauthenticated
 non-render nodes (wouldn't that be the kernel bug?) but that if a
 client opens the primary/legacy node it needs the resulting fd to
 be authenticated, which isn't supported by the X11 DRI3 protocol.
>>> Authentication is required for using certain ioctls of non-render nodes.
>>> It sounds like some ioctls are allowed to be used with render nodes but
>>> not with unauthenticated non-render nodes, which seems like a kernel bug
>>> — why would an ioctl be safe to use without authentication via a render
>>> node but not via a non-render node?
>>>
>>> So, which ioctls required by WL_bind_wayland_display don't work with an
>>> unauthenticated non-render node?
>>>
>>>
>> The ioctl to authenticate is possible only when you have the master node
>> (owned by the DDX when using X, or by the wayland compositor when using
>> Wayland). There is only one master node.
>>
>>
>> On of the motives of render-nodes is precisely to fix these limitations.
>
> I understand all of that, but it doesn't answer my question. :)
>
> Since WL_bind_wayland_display works with a render node, it means that
> all ioctls it needs work without authentication when using a render
> node. The question is, why do the same ioctls require authentication
> when using a non-render node?
>
>
I believe the following thread [1] is relevant here.

-Emil
[1] https://lists.freedesktop.org/archives/dri-devel/2016-June/110845.html
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] egl/x11_dri3: disable WL_bind_wayland_display for devices without render nodes

2016-06-21 Thread Frank Binns 

On 21/06/16 07:39, Michel Dänzer wrote:

On 21.06.2016 15:24, Axel Davy wrote:

On 21/06/2016 01:26, Michel Dänzer wrote:

On 20.06.2016 20:06, Frank Binns wrote:

On 20/06/16 10:48, Michel Dänzer wrote:

On 18.06.2016 02:41, Frank Binns wrote:

Up until now, DRI3 was only used for devices that have render nodes,
unless
overridden via an environment variable, with it falling back to DRI2
otherwise.
This limitation was there in order to support WL_bind_wayland_display
as it
requires client opened device node fds to be authenticated, which
isn't possible
when using DRI3. This is an unfortunate compromise as DRI3 provides
security
benefits over DRI2.

What exactly is it that works with render nodes but not with
unauthenticated non-render nodes? Isn't that a kernel bug?

The problem isn't that something doesn't work with unauthenticated
non-render nodes (wouldn't that be the kernel bug?) but that if a
client opens the primary/legacy node it needs the resulting fd to
be authenticated, which isn't supported by the X11 DRI3 protocol.

Authentication is required for using certain ioctls of non-render nodes.
It sounds like some ioctls are allowed to be used with render nodes but
not with unauthenticated non-render nodes, which seems like a kernel bug
— why would an ioctl be safe to use without authentication via a render
node but not via a non-render node?

So, which ioctls required by WL_bind_wayland_display don't work with an
unauthenticated non-render node?



The ioctl to authenticate is possible only when you have the master node
(owned by the DDX when using X, or by the wayland compositor when using
Wayland). There is only one master node.


On of the motives of render-nodes is precisely to fix these limitations.

I understand all of that, but it doesn't answer my question. :)

Since WL_bind_wayland_display works with a render node, it means that
all ioctls it needs work without authentication when using a render
node. The question is, why do the same ioctls require authentication
when using a non-render node?


I didn't write all the kernel drivers so I can't possibly answer that :)

To answer your earlier question, I915_GEM_EXECBUFFER2 and AMDGPU_CS ioctls
both work with render nodes but not with unauthenticated non-render nodes.
(I assume both these ioctls submit commands to the HW so are needed for
WL_bind_wayland_display.)

Even if there is a kernel bug, it doesn't really make a difference as,
authentication will still be needed on older kernels.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] egl/x11_dri3: disable WL_bind_wayland_display for devices without render nodes

2016-06-21 Thread Michel Dänzer 
On 21.06.2016 15:24, Axel Davy wrote:
> On 21/06/2016 01:26, Michel Dänzer wrote:
>> On 20.06.2016 20:06, Frank Binns wrote:
>>> On 20/06/16 10:48, Michel Dänzer wrote:
 On 18.06.2016 02:41, Frank Binns wrote:
> Up until now, DRI3 was only used for devices that have render nodes,
> unless
> overridden via an environment variable, with it falling back to DRI2
> otherwise.
> This limitation was there in order to support WL_bind_wayland_display
> as it
> requires client opened device node fds to be authenticated, which
> isn't possible
> when using DRI3. This is an unfortunate compromise as DRI3 provides
> security
> benefits over DRI2.
 What exactly is it that works with render nodes but not with
 unauthenticated non-render nodes? Isn't that a kernel bug?
>>> The problem isn't that something doesn't work with unauthenticated
>>> non-render nodes (wouldn't that be the kernel bug?) but that if a
>>> client opens the primary/legacy node it needs the resulting fd to
>>> be authenticated, which isn't supported by the X11 DRI3 protocol.
>> Authentication is required for using certain ioctls of non-render nodes.
>> It sounds like some ioctls are allowed to be used with render nodes but
>> not with unauthenticated non-render nodes, which seems like a kernel bug
>> — why would an ioctl be safe to use without authentication via a render
>> node but not via a non-render node?
>>
>> So, which ioctls required by WL_bind_wayland_display don't work with an
>> unauthenticated non-render node?
>>
>>
> The ioctl to authenticate is possible only when you have the master node
> (owned by the DDX when using X, or by the wayland compositor when using
> Wayland). There is only one master node.
> 
> 
> On of the motives of render-nodes is precisely to fix these limitations.

I understand all of that, but it doesn't answer my question. :)

Since WL_bind_wayland_display works with a render node, it means that
all ioctls it needs work without authentication when using a render
node. The question is, why do the same ioctls require authentication
when using a non-render node?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] egl/x11_dri3: disable WL_bind_wayland_display for devices without render nodes

2016-06-21 Thread Axel Davy 

On 21/06/2016 01:26, Michel Dänzer wrote:

On 20.06.2016 20:06, Frank Binns wrote:

On 20/06/16 10:48, Michel Dänzer wrote:

On 18.06.2016 02:41, Frank Binns wrote:

Up until now, DRI3 was only used for devices that have render nodes,
unless
overridden via an environment variable, with it falling back to DRI2
otherwise.
This limitation was there in order to support WL_bind_wayland_display
as it
requires client opened device node fds to be authenticated, which
isn't possible
when using DRI3. This is an unfortunate compromise as DRI3 provides
security
benefits over DRI2.

What exactly is it that works with render nodes but not with
unauthenticated non-render nodes? Isn't that a kernel bug?

The problem isn't that something doesn't work with unauthenticated
non-render nodes (wouldn't that be the kernel bug?) but that if a
client opens the primary/legacy node it needs the resulting fd to
be authenticated, which isn't supported by the X11 DRI3 protocol.

Authentication is required for using certain ioctls of non-render nodes.
It sounds like some ioctls are allowed to be used with render nodes but
not with unauthenticated non-render nodes, which seems like a kernel bug
— why would an ioctl be safe to use without authentication via a render
node but not via a non-render node?

So, which ioctls required by WL_bind_wayland_display don't work with an
unauthenticated non-render node?


The ioctl to authenticate is possible only when you have the master node 
(owned by the DDX when using X, or by the wayland compositor when using 
Wayland). There is only one master node.



On of the motives of render-nodes is precisely to fix these limitations.


Axel

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] egl/x11_dri3: disable WL_bind_wayland_display for devices without render nodes

2016-06-20 Thread Michel Dänzer 
On 20.06.2016 20:06, Frank Binns wrote:
> On 20/06/16 10:48, Michel Dänzer wrote:
>> On 18.06.2016 02:41, Frank Binns wrote:
>>> Up until now, DRI3 was only used for devices that have render nodes,
>>> unless
>>> overridden via an environment variable, with it falling back to DRI2
>>> otherwise.
>>> This limitation was there in order to support WL_bind_wayland_display
>>> as it
>>> requires client opened device node fds to be authenticated, which
>>> isn't possible
>>> when using DRI3. This is an unfortunate compromise as DRI3 provides
>>> security
>>> benefits over DRI2.
>> What exactly is it that works with render nodes but not with
>> unauthenticated non-render nodes? Isn't that a kernel bug?
> 
> The problem isn't that something doesn't work with unauthenticated
> non-render nodes (wouldn't that be the kernel bug?) but that if a
> client opens the primary/legacy node it needs the resulting fd to
> be authenticated, which isn't supported by the X11 DRI3 protocol.

Authentication is required for using certain ioctls of non-render nodes.
It sounds like some ioctls are allowed to be used with render nodes but
not with unauthenticated non-render nodes, which seems like a kernel bug
— why would an ioctl be safe to use without authentication via a render
node but not via a non-render node?

So, which ioctls required by WL_bind_wayland_display don't work with an
unauthenticated non-render node?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] egl/x11_dri3: disable WL_bind_wayland_display for devices without render nodes

2016-06-20 Thread Axel Davy 

Le 20/06/2016 11:48, Michel Dänzer a écrit :

On 18.06.2016 02:41, Frank Binns wrote:

Up until now, DRI3 was only used for devices that have render nodes, unless
overridden via an environment variable, with it falling back to DRI2 otherwise.
This limitation was there in order to support WL_bind_wayland_display as it
requires client opened device node fds to be authenticated, which isn't possible
when using DRI3. This is an unfortunate compromise as DRI3 provides security
benefits over DRI2.

What exactly is it that works with render nodes but not with
unauthenticated non-render nodes? Isn't that a kernel bug?


Wayland egl authentication mechanism is that the client opens the device 
advertised by the server (or it can use another device, but must use 
render nodes then),
and if it is not a render-node, it asks the server to authenticate the 
client opened fd of the device for rendering.


On DRI3, you don't ask the X server for authentication (contrary to DRI2 
and Wayland egl), instead the X server gives you an already 
authenticated fd.


Thus X egl/dri3 cannot authenticate clients opened fd.

That's why render-nodes are required for the extension with egl/dri3

Axel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] egl/x11_dri3: disable WL_bind_wayland_display for devices without render nodes

2016-06-20 Thread Frank Binns 

On 20/06/16 10:48, Michel Dänzer wrote:

On 18.06.2016 02:41, Frank Binns wrote:

Up until now, DRI3 was only used for devices that have render nodes, unless
overridden via an environment variable, with it falling back to DRI2 otherwise.
This limitation was there in order to support WL_bind_wayland_display as it
requires client opened device node fds to be authenticated, which isn't possible
when using DRI3. This is an unfortunate compromise as DRI3 provides security
benefits over DRI2.

What exactly is it that works with render nodes but not with
unauthenticated non-render nodes? Isn't that a kernel bug?



The problem isn't that something doesn't work with unauthenticated
non-render nodes (wouldn't that be the kernel bug?) but that if a
client opens the primary/legacy node it needs the resulting fd to
be authenticated, which isn't supported by the X11 DRI3 protocol.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] egl/x11_dri3: disable WL_bind_wayland_display for devices without render nodes

2016-06-20 Thread Axel Davy 

Hi,

The three patches make sense to me.

Reviewed-by: Axel Davy 

On 17/06/2016 19:41, Frank Binns wrote :

Up until now, DRI3 was only used for devices that have render nodes, unless
overridden via an environment variable, with it falling back to DRI2 otherwise.
This limitation was there in order to support WL_bind_wayland_display as it
requires client opened device node fds to be authenticated, which isn't possible
when using DRI3. This is an unfortunate compromise as DRI3 provides security
benefits over DRI2.

Instead, allow DRI3 to be used for devices without render nodes but don't
advertise WL_bind_wayland_display in this case. Applications that need this
extension can still be run by disabling DRI3 support via the LIBGL_DRI3_DISABLE
environment variable.

Signed-off-by: Frank Binns 
---
  src/egl/drivers/dri2/platform_x11.c  |  3 ++-
  src/egl/drivers/dri2/platform_x11_dri3.c | 33 +---
  2 files changed, 7 insertions(+), 29 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_x11.c 
b/src/egl/drivers/dri2/platform_x11.c
index c0a4005..1b50889 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -1335,7 +1335,8 @@ dri2_initialize_x11_dri3(_EGLDriver *drv, _EGLDisplay 
*disp)
 disp->Extensions.EXT_buffer_age = EGL_TRUE;
  
  #ifdef HAVE_WAYLAND_PLATFORM

-   disp->Extensions.WL_bind_wayland_display = EGL_TRUE;
+   if (dri2_dpy->device_name)
+  disp->Extensions.WL_bind_wayland_display = EGL_TRUE;
  #endif
  
 if (dri2_dpy->conn) {

diff --git a/src/egl/drivers/dri2/platform_x11_dri3.c 
b/src/egl/drivers/dri2/platform_x11_dri3.c
index 9363a8a..b781987 100644
--- a/src/egl/drivers/dri2/platform_x11_dri3.c
+++ b/src/egl/drivers/dri2/platform_x11_dri3.c
@@ -437,29 +437,6 @@ struct dri2_egl_display_vtbl dri3_x11_display_vtbl = {
 .get_dri_drawable = dri3_get_dri_drawable,
  };
  
-static char *

-dri3_get_device_name(int fd)
-{
-   char *ret = NULL;
-
-   ret = drmGetRenderDeviceNameFromFd(fd);
-   if (ret)
-  return ret;
-
-   /* For dri3, render node support is required for WL_bind_wayland_display.
-* In order not to regress on older systems without kernel or libdrm
-* support, fall back to dri2. User can override it with environment
-* variable if they don't need to use that extension.
-*/
-   if (getenv("EGL_FORCE_DRI3") == NULL) {
-  _eglLog(_EGL_WARNING, "Render node support not available, falling back to 
dri2");
-  _eglLog(_EGL_WARNING, "If you want to force dri3, set EGL_FORCE_DRI3 
environment variable");
-   } else
-  ret = loader_get_device_name_for_fd(fd);
-
-   return ret;
-}
-
  EGLBoolean
  dri3_x11_connect(struct dri2_egl_display *dri2_dpy)
  {
@@ -539,11 +516,11 @@ dri3_x11_connect(struct dri2_egl_display *dri2_dpy)
return EGL_FALSE;
 }
  
-   dri2_dpy->device_name = dri3_get_device_name(dri2_dpy->fd);

-   if (!dri2_dpy->device_name) {
-  close(dri2_dpy->fd);
-  return EGL_FALSE;
-   }
+   /* Only try to get a render device name since it's only needed for
+* WL_bind_wayland_display and dri3 doesn't provide a mechanism for
+* authenticating client opened device node fds. If this fails then
+* don't advertise the extension. */
+   dri2_dpy->device_name = drmGetRenderDeviceNameFromFd(dri2_dpy->fd);
  
 return EGL_TRUE;

  }


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] egl/x11_dri3: disable WL_bind_wayland_display for devices without render nodes

2016-06-20 Thread Michel Dänzer 
On 18.06.2016 02:41, Frank Binns wrote:
> Up until now, DRI3 was only used for devices that have render nodes, unless
> overridden via an environment variable, with it falling back to DRI2 
> otherwise.
> This limitation was there in order to support WL_bind_wayland_display as it
> requires client opened device node fds to be authenticated, which isn't 
> possible
> when using DRI3. This is an unfortunate compromise as DRI3 provides security
> benefits over DRI2.

What exactly is it that works with render nodes but not with
unauthenticated non-render nodes? Isn't that a kernel bug?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/3] egl/x11_dri3: disable WL_bind_wayland_display for devices without render nodes

2016-06-17 Thread Frank Binns 
Up until now, DRI3 was only used for devices that have render nodes, unless
overridden via an environment variable, with it falling back to DRI2 otherwise.
This limitation was there in order to support WL_bind_wayland_display as it
requires client opened device node fds to be authenticated, which isn't possible
when using DRI3. This is an unfortunate compromise as DRI3 provides security
benefits over DRI2.

Instead, allow DRI3 to be used for devices without render nodes but don't
advertise WL_bind_wayland_display in this case. Applications that need this
extension can still be run by disabling DRI3 support via the LIBGL_DRI3_DISABLE
environment variable.

Signed-off-by: Frank Binns 
---
 src/egl/drivers/dri2/platform_x11.c  |  3 ++-
 src/egl/drivers/dri2/platform_x11_dri3.c | 33 +---
 2 files changed, 7 insertions(+), 29 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_x11.c 
b/src/egl/drivers/dri2/platform_x11.c
index c0a4005..1b50889 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -1335,7 +1335,8 @@ dri2_initialize_x11_dri3(_EGLDriver *drv, _EGLDisplay 
*disp)
disp->Extensions.EXT_buffer_age = EGL_TRUE;
 
 #ifdef HAVE_WAYLAND_PLATFORM
-   disp->Extensions.WL_bind_wayland_display = EGL_TRUE;
+   if (dri2_dpy->device_name)
+  disp->Extensions.WL_bind_wayland_display = EGL_TRUE;
 #endif
 
if (dri2_dpy->conn) {
diff --git a/src/egl/drivers/dri2/platform_x11_dri3.c 
b/src/egl/drivers/dri2/platform_x11_dri3.c
index 9363a8a..b781987 100644
--- a/src/egl/drivers/dri2/platform_x11_dri3.c
+++ b/src/egl/drivers/dri2/platform_x11_dri3.c
@@ -437,29 +437,6 @@ struct dri2_egl_display_vtbl dri3_x11_display_vtbl = {
.get_dri_drawable = dri3_get_dri_drawable,
 };
 
-static char *
-dri3_get_device_name(int fd)
-{
-   char *ret = NULL;
-
-   ret = drmGetRenderDeviceNameFromFd(fd);
-   if (ret)
-  return ret;
-
-   /* For dri3, render node support is required for WL_bind_wayland_display.
-* In order not to regress on older systems without kernel or libdrm
-* support, fall back to dri2. User can override it with environment
-* variable if they don't need to use that extension.
-*/
-   if (getenv("EGL_FORCE_DRI3") == NULL) {
-  _eglLog(_EGL_WARNING, "Render node support not available, falling back 
to dri2");
-  _eglLog(_EGL_WARNING, "If you want to force dri3, set EGL_FORCE_DRI3 
environment variable");
-   } else
-  ret = loader_get_device_name_for_fd(fd);
-
-   return ret;
-}
-
 EGLBoolean
 dri3_x11_connect(struct dri2_egl_display *dri2_dpy)
 {
@@ -539,11 +516,11 @@ dri3_x11_connect(struct dri2_egl_display *dri2_dpy)
   return EGL_FALSE;
}
 
-   dri2_dpy->device_name = dri3_get_device_name(dri2_dpy->fd);
-   if (!dri2_dpy->device_name) {
-  close(dri2_dpy->fd);
-  return EGL_FALSE;
-   }
+   /* Only try to get a render device name since it's only needed for
+* WL_bind_wayland_display and dri3 doesn't provide a mechanism for
+* authenticating client opened device node fds. If this fails then
+* don't advertise the extension. */
+   dri2_dpy->device_name = drmGetRenderDeviceNameFromFd(dri2_dpy->fd);
 
return EGL_TRUE;
 }
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev