Hi,
Following to my shared talk with krh, danvet and Timothée Ravier @ XDC2012, I have actually taken the time to start fixing some security holes found in the graphics stack.
Today, I would like to request your comments on the render node patchset. Keep in mind that I am not asking for inclusion. However, I know this patchset works on my nvidia card and I would like to know if anyone has anything against this architecture.
## DRM If I'm not mistaken, the idea originated from airlied which got simplified later by krh. Both only provided drm patches.
Here is what I did: - I took krh's patchset - rebased it on top on drm 3.7-rc8 - added support for Nouveau - fixed a few bugs along the way (as stated in the commit logs)
The kernel can be found here: https://gitorious.org/linux-nouveau-pm/linux-nouveau-pm/commits/render_nodes The patches will also be sent in reply, to let you comment on specific parts of the patches.
## Libdrm
Here are the two main goals of this patchset: - Add a new symbol called drmOpenType that allows to open a specific type of device (usual node, render node) - Add a new symbol called drmGetSameDeviceButType to get the path to the same drm_device but with a different type
The patches are available here: http://cgit.freedesktop.org/~mperes/drm/
## DRI2Proto
What we want here is to let the ddx send the render node instead of the usual one. However, authentication is not necessary and not shouldn't be done on a render node.
This modification to DRI2Proto adds a boolean in the Connection response to tell the dri2 client that no authentication is required.
The patches are available here: http://cgit.freedesktop.org/~mperes/dri2proto/
## XServer
The X-Server is responsible for collecting the DRI2 informations from the ddx. In this patch, we provide the way for the ddx to specify whether the DRI2 client should authenticate or not.
The patches are available here: http://cgit.freedesktop.org/~mperes/xserver/
## xf86-video-nouveau
In this patch, we simply tell the DRI2 extension to use the render node if available (using drmGetSameDeviceButType), and if it is the case, we set the "require_authentication" attribute to 0.
The patches are available here: http://cgit.freedesktop.org/~mperes/xf86-video-nouveau/
## Mesa
In this patch, I simply check whether I should authenticate or not using the information from the DRI2 protocol.
The patches are available here: http://cgit.freedesktop.org/~mperes/mesa/
Cheers,
Martin
From: Kristian Høgsberg krh@bitplanet.net
We got the minor number base right, but limit is too big and causes the minor numer ranges for the control and render nodes to overlap.
v2: fix a off-by-one overlap as suggested by ihadzic@research.bell-labs.com
Signed-off-by: Martin Peres martin.peres@labri.fr --- drivers/gpu/drm/drm_stub.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index c236fd2..b56e977 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -107,10 +107,10 @@ static int drm_minor_get_id(struct drm_device *dev, int type)
if (type == DRM_MINOR_CONTROL) { base += 64; - limit = base + 127; + limit = base + 63; } else if (type == DRM_MINOR_RENDER) { base += 128; - limit = base + 255; + limit = base + 63; }
again:
From: Kristian Høgsberg krh@bitplanet.net
This patch introduces a new kind of drm device node: the render node. A render node exposes a safe subset of the legacy drm device ioctls and can only be used for rendering. The legacy node supports modesetting, DRI1 and global buffer sharing, while the render node only supports rendering and limited buffer sharing. A render node can either export a buffer using the gem flink mechanism, or it can import and export buffers using the prime fd passing mechanism.
A render node is not associated with any drm master and does not require the legacy authentication step. Render nodes can be used without a master, potentially in headless setups such as video-transcoding or GPGPU work.
v2: Martin Peres - Allow render nodes to access DRM_UNLOCKED IOCTLs)
Signed-off-by: Kristian Høgsberg krh@bitplanet.net Signed-off-by: Martin Peres martin.peres@labri.fr --- drivers/gpu/drm/drm_drv.c | 13 +++++++------ drivers/gpu/drm/drm_fops.c | 9 ++++++--- drivers/gpu/drm/drm_pci.c | 9 +++++++++ include/drm/drmP.h | 3 +++ 4 files changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index be174ca..7c74959 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -131,14 +131,14 @@ static struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_UPDATE_DRAW, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
- DRM_IOCTL_DEF(DRM_IOCTL_GEM_CLOSE, drm_gem_close_ioctl, DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_GEM_FLINK, drm_gem_flink_ioctl, DRM_AUTH|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_GEM_CLOSE, drm_gem_close_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_GEM_FLINK, drm_gem_flink_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_GEM_OPEN, drm_gem_open_ioctl, DRM_AUTH|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
- DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_CONTROL_ALLOW|DRM_UNLOCKED), @@ -427,9 +427,10 @@ long drm_ioctl(struct file *filp, DRM_DEBUG("no function\n"); retcode = -EINVAL; } else if (((ioctl->flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN)) || - ((ioctl->flags & DRM_AUTH) && !file_priv->authenticated) || + ((ioctl->flags & DRM_AUTH) && (file_priv->minor->type != DRM_MINOR_RENDER) && !file_priv->authenticated) || ((ioctl->flags & DRM_MASTER) && !file_priv->is_master) || - (!(ioctl->flags & DRM_CONTROL_ALLOW) && (file_priv->minor->type == DRM_MINOR_CONTROL))) { + (!(ioctl->flags & DRM_CONTROL_ALLOW) && (file_priv->minor->type == DRM_MINOR_CONTROL)) || + (!(ioctl->flags & (DRM_RENDER_ALLOW | DRM_UNLOCKED)) && (file_priv->minor->type == DRM_MINOR_RENDER))) { retcode = -EACCES; } else { if (cmd & (IOC_IN | IOC_OUT)) { diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 133b413..a27e8a4 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -295,7 +295,7 @@ static int drm_open_helper(struct inode *inode, struct file *filp,
/* if there is no current master make this fd it */ mutex_lock(&dev->struct_mutex); - if (!priv->minor->master) { + if (!priv->minor->master && priv->minor->type != DRM_MINOR_RENDER) { /* create a new master */ priv->minor->master = drm_master_create(priv->minor); if (!priv->minor->master) { @@ -334,10 +334,12 @@ static int drm_open_helper(struct inode *inode, struct file *filp, } } mutex_unlock(&dev->struct_mutex); - } else { + } else if (priv->minor->type != DRM_MINOR_RENDER) { /* get a reference to the master */ priv->master = drm_master_get(priv->minor->master); mutex_unlock(&dev->struct_mutex); + } else { + mutex_unlock(&dev->struct_mutex); }
mutex_lock(&dev->struct_mutex); @@ -529,7 +531,8 @@ int drm_release(struct inode *inode, struct file *filp) iput(container_of(dev->dev_mapping, struct inode, i_data));
/* drop the reference held my the file priv */ - drm_master_put(&file_priv->master); + if (file_priv->master) + drm_master_put(&file_priv->master); file_priv->is_master = 0; list_del(&file_priv->lhead); mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index ba33144..b0d8fda 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -348,6 +348,12 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, goto err_g2; }
+ if (drm_core_check_feature(dev, DRIVER_RENDER)) { + ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER); + if (ret) + goto err_g21; + } + if ((ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY))) goto err_g3;
@@ -377,6 +383,9 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, err_g4: drm_put_minor(&dev->primary); err_g3: + if (drm_core_check_feature(dev, DRIVER_RENDER)) + drm_put_minor(&dev->render); +err_g21: if (drm_core_check_feature(dev, DRIVER_MODESET)) drm_put_minor(&dev->control); err_g2: diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 3fd8280..741b9cd 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -153,6 +153,7 @@ int drm_err(const char *func, const char *format, ...); #define DRIVER_GEM 0x1000 #define DRIVER_MODESET 0x2000 #define DRIVER_PRIME 0x4000 +#define DRIVER_RENDER 0x8000
#define DRIVER_BUS_PCI 0x1 #define DRIVER_BUS_PLATFORM 0x2 @@ -307,6 +308,7 @@ typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd, #define DRM_ROOT_ONLY 0x4 #define DRM_CONTROL_ALLOW 0x8 #define DRM_UNLOCKED 0x10 +#define DRM_RENDER_ALLOW 0x20
struct drm_ioctl_desc { unsigned int cmd; @@ -1186,6 +1188,7 @@ struct drm_device { struct drm_local_map *agp_buffer_map; unsigned int agp_buffer_token; struct drm_minor *control; /**< Control node for card */ + struct drm_minor *render; /**< Render node for card */ struct drm_minor *primary; /**< render type primary screen head */
struct drm_mode_config mode_config; /**< Current mode config */
From: Kristian Høgsberg krh@bitplanet.net
Enable support for drm render nodes for i915 by flagging the ioctls that are safe and just needed for rendering.
Signed-off-by: Kristian Høgsberg krh@bitplanet.net --- drivers/gpu/drm/i915/i915_dma.c | 36 ++++++++++++++++++------------------ drivers/gpu/drm/i915/i915_drv.c | 3 ++- 2 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 61ae104..d0554b0 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1851,7 +1851,7 @@ struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_BATCHBUFFER, i915_batchbuffer, DRM_AUTH), DRM_IOCTL_DEF_DRV(I915_IRQ_EMIT, i915_irq_emit, DRM_AUTH), DRM_IOCTL_DEF_DRV(I915_IRQ_WAIT, i915_irq_wait, DRM_AUTH), - DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam, DRM_AUTH), + DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_SETPARAM, i915_setparam, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), DRM_IOCTL_DEF_DRV(I915_ALLOC, drm_noop, DRM_AUTH), DRM_IOCTL_DEF_DRV(I915_FREE, drm_noop, DRM_AUTH), @@ -1864,34 +1864,34 @@ struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_HWS_ADDR, i915_set_status_page, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), DRM_IOCTL_DEF_DRV(I915_GEM_INIT, i915_gem_init_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED), DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER, i915_gem_execbuffer, DRM_AUTH|DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2, i915_gem_execbuffer2, DRM_AUTH|DRM_UNLOCKED), + DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2, i915_gem_execbuffer2, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_PIN, i915_gem_pin_ioctl, DRM_AUTH|DRM_ROOT_ONLY|DRM_UNLOCKED), DRM_IOCTL_DEF_DRV(I915_GEM_UNPIN, i915_gem_unpin_ioctl, DRM_AUTH|DRM_ROOT_ONLY|DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, DRM_AUTH|DRM_UNLOCKED), + DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_SET_CACHING, i915_gem_set_caching_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF_DRV(I915_GEM_GET_CACHING, i915_gem_get_caching_ioctl, DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, DRM_AUTH|DRM_UNLOCKED), + DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_ENTERVT, i915_gem_entervt_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED), DRM_IOCTL_DEF_DRV(I915_GEM_LEAVEVT, i915_gem_leavevt_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl, DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling, DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_GET_TILING, i915_gem_get_tiling, DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_GET_TILING, i915_gem_get_tiling, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GET_PIPE_FROM_CRTC_ID, intel_get_pipe_from_crtc_id, DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_OVERLAY_PUT_IMAGE, intel_overlay_put_image, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF_DRV(I915_SET_SPRITE_COLORKEY, intel_sprite_set_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF_DRV(I915_GET_SPRITE_COLORKEY, intel_sprite_get_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_AUTH|DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED), - DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED), };
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 6770ee6..344061e 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -967,7 +967,8 @@ static struct drm_driver driver = { */ .driver_features = DRIVER_USE_AGP | DRIVER_REQUIRE_AGP | /* DRIVER_USE_MTRR |*/ - DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME, + DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME | + DRIVER_RENDER, .load = i915_driver_load, .unload = i915_driver_unload, .open = i915_driver_open,
From: Martin Peres martin.peres@labri.fr
Enable support for drm render nodes for nouveau by flagging the ioctls that are safe and just needed for rendering.
Signed-off-by: Martin Peres martin.peres@labri.fr --- drivers/gpu/drm/nouveau/nouveau_drm.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index fdd8813..30def9b 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -586,18 +586,18 @@ nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv)
static struct drm_ioctl_desc nouveau_ioctls[] = { - DRM_IOCTL_DEF_DRV(NOUVEAU_GETPARAM, nouveau_abi16_ioctl_getparam, DRM_UNLOCKED|DRM_AUTH), + DRM_IOCTL_DEF_DRV(NOUVEAU_GETPARAM, nouveau_abi16_ioctl_getparam, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(NOUVEAU_SETPARAM, nouveau_abi16_ioctl_setparam, DRM_UNLOCKED|DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), - DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_ALLOC, nouveau_abi16_ioctl_channel_alloc, DRM_UNLOCKED|DRM_AUTH), - DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_FREE, nouveau_abi16_ioctl_channel_free, DRM_UNLOCKED|DRM_AUTH), - DRM_IOCTL_DEF_DRV(NOUVEAU_GROBJ_ALLOC, nouveau_abi16_ioctl_grobj_alloc, DRM_UNLOCKED|DRM_AUTH), - DRM_IOCTL_DEF_DRV(NOUVEAU_NOTIFIEROBJ_ALLOC, nouveau_abi16_ioctl_notifierobj_alloc, DRM_UNLOCKED|DRM_AUTH), - DRM_IOCTL_DEF_DRV(NOUVEAU_GPUOBJ_FREE, nouveau_abi16_ioctl_gpuobj_free, DRM_UNLOCKED|DRM_AUTH), - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_NEW, nouveau_gem_ioctl_new, DRM_UNLOCKED|DRM_AUTH), - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_PUSHBUF, nouveau_gem_ioctl_pushbuf, DRM_UNLOCKED|DRM_AUTH), - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_UNLOCKED|DRM_AUTH), - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_UNLOCKED|DRM_AUTH), - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_UNLOCKED|DRM_AUTH), + DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_ALLOC, nouveau_abi16_ioctl_channel_alloc, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_FREE, nouveau_abi16_ioctl_channel_free, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_GROBJ_ALLOC, nouveau_abi16_ioctl_grobj_alloc, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_NOTIFIEROBJ_ALLOC, nouveau_abi16_ioctl_notifierobj_alloc, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_GPUOBJ_FREE, nouveau_abi16_ioctl_gpuobj_free, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_NEW, nouveau_gem_ioctl_new, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_PUSHBUF, nouveau_gem_ioctl_pushbuf, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW), };
static const struct file_operations @@ -621,7 +621,7 @@ driver = { .driver_features = DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG | DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | - DRIVER_MODESET | DRIVER_PRIME, + DRIVER_MODESET | DRIVER_PRIME | DRIVER_RENDER,
.load = nouveau_drm_load, .unload = nouveau_drm_unload,
From: Martin Peres martin.peres@labri.fr
Signed-off-by: Martin Peres martin.peres@labri.fr --- tests/name_from_fd.c | 19 ++++++---- xf86drm.c | 105 ++++++++++++++++++++++++++++++++++++++++----------- xf86drm.h | 7 ++++ 3 files changed, 100 insertions(+), 31 deletions(-)
diff --git a/tests/name_from_fd.c b/tests/name_from_fd.c index 330c8ff..8e1a197 100644 --- a/tests/name_from_fd.c +++ b/tests/name_from_fd.c @@ -41,17 +41,20 @@ int main(int argc, char **argv) { int fd, ret; drm_set_version_t sv, version; - const char *name = "/dev/dri/card0"; + const char *name[2] = { "/dev/dri/card0", "/dev/dri/renderD128" }; char *v; + int i;
- fd = open("/dev/dri/card0", O_RDWR); - if (fd == -1) - return 0; + for (i = 0; i < 2; i++) { + fd = open(name[i], O_RDWR); + if (fd > 0) { + v = drmGetDeviceNameFromFd(fd); + close(fd); + assert(v != NULL); + assert(strcmp(name[i], v) == 0); + } + }
- v = drmGetDeviceNameFromFd(fd); - close(fd); - - assert(strcmp(name, v) == 0); drmFree(v);
return 0; diff --git a/xf86drm.c b/xf86drm.c index 2a74c80..eb0549d 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -85,9 +85,6 @@
#define DRM_MSG_VERBOSITY 3
-#define DRM_NODE_CONTROL 0 -#define DRM_NODE_RENDER 1 - static drmServerInfoPtr drm_server_info;
void drmSetServerInfo(drmServerInfoPtr info) @@ -286,10 +283,51 @@ static int chown_check_return(const char *path, uid_t owner, gid_t group) }
/** + * Generate the device name according to its type. + * + * \param buf buffer that will hold the device path after the call. + * \param len length of the buffer. + * \param base_dir base directory used to generate the path. If NULL, it + * defaults to DRM_DIR_NAME ("/dev/dri"). + * \param type the type of device (control, render or render_only). + * \param minor number of the device. + */ +static void drmDevicePath(char *buf, size_t len, const char *base_dir, int type, + int minor) +{ + const char *dev_name = NULL; + + switch (type) + { + case DRM_NODE_CONTROL: + dev_name = DRM_CONTROL_DEV_NAME; + /* we do not increase minor by 64 not to change the current + * behaviour even though I couldn't find who uses this type! + */ + break; + case DRM_NODE_RENDER_ONLY: + dev_name = DRM_RENDER_ONLY_DEV_NAME; + minor += 128; + break; + default: + drmMsg("drmDevicePath: unknown type %d, default to render.\n", type); + case DRM_NODE_RENDER: + dev_name = DRM_DEV_NAME; + break; + } + + if (!base_dir) + base_dir = DRM_DIR_NAME; + + snprintf(buf, len, dev_name, base_dir, minor); +} + +/** * Open the DRM device, creating it if necessary. * * \param dev major and minor numbers of the device. * \param minor minor number of the device. + * \param type the type of device we want to open (control, render or render_only). * * \return a file descriptor on success, or a negative value on error. * @@ -308,7 +346,7 @@ static int drmOpenDevice(long dev, int minor, int type) uid_t user = DRM_DEV_UID; gid_t group = DRM_DEV_GID, serv_group;
- sprintf(buf, type ? DRM_DEV_NAME : DRM_CONTROL_DEV_NAME, DRM_DIR_NAME, minor); + drmDevicePath(buf, sizeof(buf), NULL, type, minor); drmMsg("drmOpenDevice: node name is %s\n", buf);
if (drm_server_info) { @@ -417,7 +455,7 @@ static int drmOpenMinor(int minor, int create, int type) if (create) return drmOpenDevice(makedev(DRM_MAJOR, minor), minor, type);
- sprintf(buf, type ? DRM_DEV_NAME : DRM_CONTROL_DEV_NAME, DRM_DIR_NAME, minor); + drmDevicePath(buf, sizeof(buf), NULL, type, minor); if ((fd = open(buf, O_RDWR, 0)) >= 0) return fd; return -errno; @@ -472,7 +510,7 @@ int drmAvailable(void) * * \sa drmOpenMinor() and drmGetBusid(). */ -static int drmOpenByBusid(const char *busid) +static int drmOpenByBusid(const char *busid, int type) { int i, pci_domain_ok = 1; int fd; @@ -481,7 +519,7 @@ static int drmOpenByBusid(const char *busid)
drmMsg("drmOpenByBusid: Searching for BusID %s\n", busid); for (i = 0; i < DRM_MAX_MINOR; i++) { - fd = drmOpenMinor(i, 1, DRM_NODE_RENDER); + fd = drmOpenMinor(i, 1, type); drmMsg("drmOpenByBusid: drmOpenMinor returns %d\n", fd); if (fd >= 0) { /* We need to try for 1.4 first for proper PCI domain support @@ -531,7 +569,7 @@ static int drmOpenByBusid(const char *busid) * * \sa drmOpenMinor(), drmGetVersion() and drmGetBusid(). */ -static int drmOpenByName(const char *name) +static int drmOpenByName(const char *name, int type) { int i; int fd; @@ -556,7 +594,7 @@ static int drmOpenByName(const char *name) * already in use. If it's in use it will have a busid assigned already. */ for (i = 0; i < DRM_MAX_MINOR; i++) { - if ((fd = drmOpenMinor(i, 1, DRM_NODE_RENDER)) >= 0) { + if ((fd = drmOpenMinor(i, 1, type)) >= 0) { if ((version = drmGetVersion(fd))) { if (!strcmp(version->name, name)) { drmFreeVersion(version); @@ -598,9 +636,9 @@ static int drmOpenByName(const char *name) for (devstring = ++pt; *pt && *pt != ' '; ++pt) ; if (*pt) { /* Found busid */ - return drmOpenByBusid(++pt); + return drmOpenByBusid(++pt, type); } else { /* No busid */ - return drmOpenDevice(strtol(devstring, NULL, 0),i, DRM_NODE_RENDER); + return drmOpenDevice(strtol(devstring, NULL, 0),i, type); } } } @@ -621,6 +659,7 @@ static int drmOpenByName(const char *name) * * \param name driver name. Not referenced if bus ID is supplied. * \param busid bus ID. Zero if not known. + * \param type type of node wanted (control, render or render_only). * * \return a file descriptor on success, or a negative value on error. * @@ -628,7 +667,7 @@ static int drmOpenByName(const char *name) * It calls drmOpenByBusid() if \p busid is specified or drmOpenByName() * otherwise. */ -int drmOpen(const char *name, const char *busid) +int drmOpenType(const char *name, const char *busid, int type) { if (!drmAvailable() && name != NULL && drm_server_info) { /* try to load the kernel */ @@ -639,17 +678,37 @@ int drmOpen(const char *name, const char *busid) }
if (busid) { - int fd = drmOpenByBusid(busid); + int fd = drmOpenByBusid(busid, type); if (fd >= 0) return fd; }
if (name) - return drmOpenByName(name); + return drmOpenByName(name, type);
return -1; }
+/** + * Open the DRM device. + * + * Looks up the specified name and bus ID, and opens the device found. The + * entry in /dev/dri is created if necessary and if called by root. + * + * \param name driver name. Not referenced if bus ID is supplied. + * \param busid bus ID. Zero if not known. + * + * \return a file descriptor on success, or a negative value on error. + * + * \internal + * It calls drmOpenByBusid() if \p busid is specified or drmOpenByName() + * otherwise. + */ +int drmOpen(const char *name, const char *busid) +{ + return drmOpenType(name, busid, DRM_NODE_RENDER); +} + int drmOpenControl(int minor) { return drmOpenMinor(minor, 0, DRM_NODE_CONTROL); @@ -2522,7 +2581,7 @@ char *drmGetDeviceNameFromFd(int fd) char name[128]; struct stat sbuf; dev_t d; - int i; + int type, i;
/* The whole drmOpen thing is a fiasco and we need to find a way * back to just using open(2). For now, however, lets just make @@ -2532,15 +2591,15 @@ char *drmGetDeviceNameFromFd(int fd) fstat(fd, &sbuf); d = sbuf.st_rdev;
- for (i = 0; i < DRM_MAX_MINOR; i++) { - snprintf(name, sizeof name, DRM_DEV_NAME, DRM_DIR_NAME, i); - if (stat(name, &sbuf) == 0 && sbuf.st_rdev == d) - break; + for (type = DRM_NODE_RENDER; type <= DRM_NODE_RENDER_ONLY; type++) { + for (i = 0; i < DRM_MAX_MINOR; i++) { + drmDevicePath(name, sizeof(name), NULL, type, i); + if (stat(name, &sbuf) == 0 && sbuf.st_rdev == d) + return strdup(name); + } } - if (i == DRM_MAX_MINOR) - return NULL; - - return strdup(name); + + return NULL; }
int drmPrimeHandleToFD(int fd, uint32_t handle, uint32_t flags, int *prime_fd) diff --git a/xf86drm.h b/xf86drm.h index 5ecb284..d727ce1 100644 --- a/xf86drm.h +++ b/xf86drm.h @@ -47,6 +47,10 @@ extern "C" { #define DRM_MAX_MINOR 16 #endif
+#define DRM_NODE_CONTROL 0 +#define DRM_NODE_RENDER 1 +#define DRM_NODE_RENDER_ONLY 2 + #if defined(__linux__)
#define DRM_IOCTL_NR(n) _IOC_NR(n) @@ -78,6 +82,7 @@ extern "C" {
#define DRM_DIR_NAME "/dev/dri" #define DRM_DEV_NAME "%s/card%d" +#define DRM_RENDER_ONLY_DEV_NAME "%s/renderD%d" #define DRM_CONTROL_DEV_NAME "%s/controlD%d" #define DRM_PROC_NAME "/proc/dri/" /* For backward Linux compatibility */
@@ -544,6 +549,7 @@ do { register unsigned int __old __asm("o0"); \
/* General user-level programmer's API: unprivileged */ extern int drmAvailable(void); +extern int drmOpenType(const char *name, const char *busid, int type); extern int drmOpen(const char *name, const char *busid); extern int drmOpenControl(int minor); extern int drmClose(int fd); @@ -694,6 +700,7 @@ extern int drmSLLookupNeighbors(void *l, unsigned long key, unsigned long *prev_key, void **prev_value, unsigned long *next_key, void **next_value);
+extern int drmOpenTypeOnce(void *unused, const char *BusID, int *newlyopened, int type); extern int drmOpenOnce(void *unused, const char *BusID, int *newlyopened); extern void drmCloseOnce(int fd); extern void drmMsg(const char *format, ...);
From: Martin Peres martin.peres@labri.fr
Signed-off-by: Martin Peres martin.peres@labri.fr --- .gitignore | 1 + tests/Makefile.am | 1 + tests/same_device_but_type.c | 52 ++++++++++++++++++++++++++++++++++++++++++++ xf86drm.c | 44 +++++++++++++++++++++++++++++++++++++ xf86drm.h | 1 + 5 files changed, 99 insertions(+) create mode 100644 tests/same_device_but_type.c
diff --git a/.gitignore b/.gitignore index 28c77c5..318e129 100644 --- a/.gitignore +++ b/.gitignore @@ -74,6 +74,7 @@ tests/gem_readwrite tests/openclose tests/setversion tests/updatedraw +tests/same_device_but_type tests/modeprint/modeprint tests/modetest/modetest tests/kmstest/kmstest diff --git a/tests/Makefile.am b/tests/Makefile.am index a3a59bd..ddc73ca 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -46,6 +46,7 @@ TESTS = \ setversion \ updatedraw \ name_from_fd \ + same_device_but_type \ $(NULL)
SUBDIRS += vbltest $(NULL) diff --git a/tests/same_device_but_type.c b/tests/same_device_but_type.c new file mode 100644 index 0000000..f35951f --- /dev/null +++ b/tests/same_device_but_type.c @@ -0,0 +1,52 @@ +/* + * Copyright © 2012 + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: + * Martin Peres martin.peres@labri.fr + * + */ + +#include <unistd.h> +#include <fcntl.h> +#include <limits.h> +#include "drmtest.h" + +/** + * Checks drmGetSameDeviceButType + */ +int main(int argc, char **argv) +{ + int fd, ret; + drm_set_version_t sv, version; + const char *name = "/dev/dri/card0"; + char *render; + int i; + + render = drmGetSameDeviceButType(name, DRM_NODE_RENDER); + assert(strcmp(render, name) == 0); + drmFree(render); + + render = drmGetSameDeviceButType("/no_device", DRM_NODE_RENDER); + assert(render == NULL); + + return 0; +} diff --git a/xf86drm.c b/xf86drm.c index eb0549d..cca8d8a 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -2602,6 +2602,50 @@ char *drmGetDeviceNameFromFd(int fd) return NULL; }
+/** + * Generate the path to a different type (render node, control node, etc...) + * but for the same actual drm device. + * + * \param device a path to an existing drm device type. + * \param type the new type that should be returned. + * + * \return the path to the same drm device but to a different type if success. + * NULL otherwise. + */ +char *drmGetSameDeviceButType(const char *device, int type) +{ + char base_path[64], name[64], *path, *device_name; + struct stat sbuf; + int i; + + /* get the device name (ie /card0) */ + device_name=strrchr(device, '/'); + if (!device_name) + return NULL; + + /* get the device's drm folder (ie /sys/class/drm/card0/device/drm */ + snprintf(base_path, sizeof(base_path), "/sys/class/drm/%s/device/drm", + device_name + 1); + + /* search for the type in the base path */ + device_name = NULL; + for (i = 0; i < DRM_MAX_MINOR; i++) { + drmDevicePath(name, sizeof(name), base_path, type, i); + if (stat(name, &sbuf) == 0) { + device_name = strrchr(name, '/'); + break; + } + } + if (!device_name) + return NULL; + + /* create a more appropriate path for the device (/dev/dri/card0) */ + path = malloc(64); + snprintf(path, 64, "%s/%s", DRM_DIR_NAME, device_name + 1); + + return path; +} + int drmPrimeHandleToFD(int fd, uint32_t handle, uint32_t flags, int *prime_fd) { struct drm_prime_handle args; diff --git a/xf86drm.h b/xf86drm.h index d727ce1..658dfd7 100644 --- a/xf86drm.h +++ b/xf86drm.h @@ -733,6 +733,7 @@ typedef struct _drmEventContext { extern int drmHandleEvent(int fd, drmEventContextPtr evctx);
extern char *drmGetDeviceNameFromFd(int fd); +extern char *drmGetSameDeviceButType(const char *device, int type);
extern int drmPrimeHandleToFD(int fd, uint32_t handle, uint32_t flags, int *prime_fd); extern int drmPrimeFDToHandle(int fd, int prime_fd, uint32_t *handle);
From: Martin Peres martin.peres@labri.fr
Signed-off-by: Martin Peres martin.peres@labri.fr --- nouveau/nouveau.c | 11 +++++++++-- nouveau/nouveau.h | 2 ++ 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/nouveau/nouveau.c b/nouveau/nouveau.c index 940d933..1402852 100644 --- a/nouveau/nouveau.c +++ b/nouveau/nouveau.c @@ -128,9 +128,10 @@ nouveau_device_wrap(int fd, int close, struct nouveau_device **pdev) }
int -nouveau_device_open(const char *busid, struct nouveau_device **pdev) +nouveau_device_open_type(const char *busid, struct nouveau_device **pdev, + int type) { - int ret = -ENODEV, fd = drmOpen("nouveau", busid); + int ret = -ENODEV, fd = drmOpenType("nouveau", busid, type); if (fd >= 0) { ret = nouveau_device_wrap(fd, 1, pdev); if (ret) @@ -139,6 +140,12 @@ nouveau_device_open(const char *busid, struct nouveau_device **pdev) return ret; }
+int +nouveau_device_open(const char *busid, struct nouveau_device **pdev) +{ + return nouveau_device_open_type(busid, pdev, DRM_NODE_RENDER); +} + void nouveau_device_del(struct nouveau_device **pdev) { diff --git a/nouveau/nouveau.h b/nouveau/nouveau.h index c42eea7..cbc0dc9 100644 --- a/nouveau/nouveau.h +++ b/nouveau/nouveau.h @@ -66,6 +66,8 @@ struct nouveau_device { };
int nouveau_device_wrap(int fd, int close, struct nouveau_device **); +int nouveau_device_open_type(const char *busid, struct nouveau_device **pdev, + int type); int nouveau_device_open(const char *busid, struct nouveau_device **); void nouveau_device_del(struct nouveau_device **); int nouveau_getparam(struct nouveau_device *, uint64_t param, uint64_t *value);
From: Martin Peres martin.peres@labri.fr
Signed-off-by: Martin Peres martin.peres@labri.fr --- nouveau/libdrm_nouveau.pc.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/nouveau/libdrm_nouveau.pc.in b/nouveau/libdrm_nouveau.pc.in index 6170613..f3b99cf 100644 --- a/nouveau/libdrm_nouveau.pc.in +++ b/nouveau/libdrm_nouveau.pc.in @@ -5,7 +5,7 @@ includedir=@includedir@
Name: libdrm_nouveau Description: Userspace interface to nouveau kernel DRM services -Version: 2.4.33 +Version: 2.4.34 Libs: -L${libdir} -ldrm_nouveau Cflags: -I${includedir} -I${includedir}/libdrm -I${includedir}/nouveau Requires.private: libdrm
From: Martin Peres martin.peres@labri.fr
Signed-off-by: Martin Peres martin.peres@labri.fr --- configure.ac | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/configure.ac b/configure.ac index 0c19929..df56066 100644 --- a/configure.ac +++ b/configure.ac @@ -20,7 +20,7 @@
AC_PREREQ([2.63]) AC_INIT([libdrm], - [2.4.40], + [2.4.41], [https://bugs.freedesktop.org/enter_bug.cgi?product=DRI], [libdrm])
@@ -68,10 +68,6 @@ LT_PREREQ([2.2]) LT_INIT([disable-static])
-PKG_CHECK_MODULES(PTHREADSTUBS, pthread-stubs) -AC_SUBST(PTHREADSTUBS_CFLAGS) -AC_SUBST(PTHREADSTUBS_LIBS) - pkgconfigdir=${libdir}/pkgconfig AC_SUBST(pkgconfigdir) AC_ARG_ENABLE([udev],
From: Martin Peres martin.peres@labri.fr
Signed-off-by: Martin Peres martin.peres@labri.fr --- dri2proto.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/dri2proto.h b/dri2proto.h index 128b807..4d11ba7 100644 --- a/dri2proto.h +++ b/dri2proto.h @@ -35,7 +35,7 @@
#define DRI2_NAME "DRI2" #define DRI2_MAJOR 1 -#define DRI2_MINOR 4 +#define DRI2_MINOR 5
#define DRI2NumberErrors 0 #define DRI2NumberEvents 2 @@ -109,10 +109,12 @@ typedef struct { CARD32 length B32; CARD32 driverNameLength B32; CARD32 deviceNameLength B32; - CARD32 pad2 B32; - CARD32 pad3 B32; + BYTE deviceRequiresAuth; + BYTE pad2; + CARD16 pad3 B16; CARD32 pad4 B32; CARD32 pad5 B32; + CARD32 pad6 B32; } xDRI2ConnectReply; #define sz_xDRI2ConnectReply 32
From: Martin Peres martin.peres@labri.fr
Signed-off-by: Martin Peres martin.peres@labri.fr --- glx/glxdri2.c | 3 ++- hw/xfree86/dri2/dri2.c | 11 ++++++++++- hw/xfree86/dri2/dri2.h | 8 ++++++-- hw/xfree86/dri2/dri2ext.c | 8 ++++++-- 4 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/glx/glxdri2.c b/glx/glxdri2.c index bce1bfa..ead24bc 100644 --- a/glx/glxdri2.c +++ b/glx/glxdri2.c @@ -929,6 +929,7 @@ __glXDRIscreenProbe(ScreenPtr pScreen) __GLXDRIscreen *screen; size_t buffer_size; ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen); + unsigned char devReqAuth;
screen = calloc(1, sizeof *screen); if (screen == NULL) @@ -936,7 +937,7 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
if (!xf86LoaderCheckSymbol("DRI2Connect") || !DRI2Connect(serverClient, pScreen, DRI2DriverDRI, - &screen->fd, &driverName, &deviceName)) { + &screen->fd, &driverName, &deviceName, &devReqAuth)) { LogMessage(X_INFO, "AIGLX: Screen %d is not DRI2 capable\n", pScreen->myNum); return NULL; diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 40963c3..6238caa 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -115,6 +115,7 @@ typedef struct _DRI2Screen { int fd; unsigned int lastSequence; int prime_id; + unsigned int device_requires_auth;
DRI2CreateBufferProcPtr CreateBuffer; DRI2DestroyBufferProcPtr DestroyBuffer; @@ -1317,7 +1318,8 @@ DRI2HasSwapControl(ScreenPtr pScreen) Bool DRI2Connect(ClientPtr client, ScreenPtr pScreen, unsigned int driverType, int *fd, - const char **driverName, const char **deviceName) + const char **driverName, const char **deviceName, + unsigned char *deviceRequiresAuth) { DRI2ScreenPtr ds; uint32_t prime_id = DRI2DriverPrimeId(driverType); @@ -1503,6 +1505,13 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) ds->DestroyBuffer2 = info->DestroyBuffer2; ds->CopyRegion2 = info->CopyRegion2; } + + if (info->version >= 10) { + ds->device_requires_auth = info->device_requires_auth; + xf86DrvMsg(pScreen->myNum, X_INFO, + "[DRI2] The device %s authentication\n", + info->device_requires_auth?"requires":"doesn't require"); + }
/* * if the driver doesn't provide an AuthMagic function or the info struct diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h index 1e7afdd..f855565 100644 --- a/hw/xfree86/dri2/dri2.h +++ b/hw/xfree86/dri2/dri2.h @@ -205,7 +205,7 @@ typedef int (*DRI2GetParamProcPtr) (ClientPtr client, /** * Version of the DRI2InfoRec structure defined in this header */ -#define DRI2INFOREC_VERSION 9 +#define DRI2INFOREC_VERSION 10
typedef struct { unsigned int version; /**< Version of this struct */ @@ -252,6 +252,9 @@ typedef struct { DRI2CreateBuffer2ProcPtr CreateBuffer2; DRI2DestroyBuffer2ProcPtr DestroyBuffer2; DRI2CopyRegion2ProcPtr CopyRegion2; + + /* added in version 10 */ + unsigned char device_requires_auth; } DRI2InfoRec, *DRI2InfoPtr;
extern _X_EXPORT Bool DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info); @@ -264,7 +267,8 @@ extern _X_EXPORT Bool DRI2Connect(ClientPtr client, ScreenPtr pScreen, unsigned int driverType, int *fd, const char **driverName, - const char **deviceName); + const char **deviceName, + unsigned char *deviceRequiresAuth);
extern _X_EXPORT Bool DRI2Authenticate(ClientPtr client, ScreenPtr pScreen, uint32_t magic);
diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c index e1decec..1a78916 100644 --- a/hw/xfree86/dri2/dri2ext.c +++ b/hw/xfree86/dri2/dri2ext.c @@ -107,12 +107,14 @@ ProcDRI2Connect(ClientPtr client) .sequenceNumber = client->sequence, .length = 0, .driverNameLength = 0, - .deviceNameLength = 0 + .deviceNameLength = 0, + .deviceRequiresAuth = 1 }; DrawablePtr pDraw; int fd, status; const char *driverName; const char *deviceName; + unsigned char deviceRequiresAuth;
REQUEST_SIZE_MATCH(xDRI2ConnectReq); if (!validDrawable(client, stuff->window, DixGetAttrAccess, @@ -120,11 +122,13 @@ ProcDRI2Connect(ClientPtr client) return status;
if (!DRI2Connect(client, pDraw->pScreen, - stuff->driverType, &fd, &driverName, &deviceName)) + stuff->driverType, &fd, &driverName, &deviceName, + &deviceRequiresAuth)) goto fail;
rep.driverNameLength = strlen(driverName); rep.deviceNameLength = strlen(deviceName); + rep.deviceRequiresAuth = deviceRequiresAuth; rep.length = (rep.driverNameLength + 3) / 4 + (rep.deviceNameLength + 3) / 4;
From: Martin Peres martin.peres@labri.fr
Signed-off-by: Martin Peres martin.peres@labri.fr --- configure.ac | 2 +- src/nouveau_dri2.c | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac index ff9c337..61dfb01 100644 --- a/configure.ac +++ b/configure.ac @@ -67,7 +67,7 @@ XORG_DRIVER_CHECK_EXT(XV, videoproto) XORG_DRIVER_CHECK_EXT(DPMSExtension, xextproto)
# Checks for pkg-config packages -PKG_CHECK_MODULES(LIBDRM_NOUVEAU, [libdrm_nouveau >= 2.4.25]) +PKG_CHECK_MODULES(LIBDRM_NOUVEAU, [libdrm_nouveau >= 2.4.34]) AC_SUBST(LIBDRM_NOUVEAU_CFLAGS) AC_SUBST(LIBDRM_NOUVEAU_LIBS)
diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c index 036bcff..f852d8e 100644 --- a/src/nouveau_dri2.c +++ b/src/nouveau_dri2.c @@ -754,6 +754,7 @@ nouveau_dri2_init(ScreenPtr pScreen) { "nouveau", "nouveau" }, { "nouveau_vieux", "nouveau_vieux" } }; + char *device_name = NULL;
if (pNv->Architecture >= NV_ARCH_30) dri2.driverNames = drivernames[0]; @@ -788,6 +789,16 @@ nouveau_dri2_init(ScreenPtr pScreen) dri2.DestroyBuffer2 = nouveau_dri2_destroy_buffer2; dri2.CopyRegion2 = nouveau_dri2_copy_region2; #endif + +#if DRI2INFOREC_VERSION >= 10 + dri2.version = 10; + + /* try to use the render node if available */ + device_name = drmGetSameDeviceButType(pNv->drm_device_name, DRM_NODE_RENDER_ONLY); + if (device_name) + dri2.deviceName = device_name; + dri2.device_requires_auth = (device_name == NULL); +#endif return DRI2ScreenInit(pScreen, &dri2); }
From: Martin Peres martin.peres@labri.fr
Signed-off-by: Martin Peres martin.peres@labri.fr --- src/gallium/state_trackers/egl/x11/x11_screen.c | 2 +- src/glx/dri2.c | 6 +++++- src/glx/dri2.h | 3 ++- src/glx/dri2_glx.c | 20 +++++++++++--------- src/glx/dri_glx.c | 3 ++- 5 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/src/gallium/state_trackers/egl/x11/x11_screen.c b/src/gallium/state_trackers/egl/x11/x11_screen.c index effac0e..f648e91 100644 --- a/src/gallium/state_trackers/egl/x11/x11_screen.c +++ b/src/gallium/state_trackers/egl/x11/x11_screen.c @@ -234,7 +234,7 @@ x11_screen_probe_dri2(struct x11_screen *xscr, int *major, int *minor) /* get the driver name and the device name */ if (!xscr->dri_driver) { if (!DRI2Connect(xscr->dpy, RootWindow(xscr->dpy, xscr->number), - &xscr->dri_driver, &xscr->dri_device)) + &xscr->dri_driver, &xscr->dri_device, NULL)) xscr->dri_driver = xscr->dri_device = NULL; } if (major) diff --git a/src/glx/dri2.c b/src/glx/dri2.c index bcd1f9c..1c2c91f 100644 --- a/src/glx/dri2.c +++ b/src/glx/dri2.c @@ -264,7 +264,8 @@ DRI2QueryVersion(Display * dpy, int *major, int *minor) }
Bool -DRI2Connect(Display * dpy, XID window, char **driverName, char **deviceName) +DRI2Connect(Display * dpy, XID window, char **driverName, char **deviceName, + unsigned char *deviceRequiresAuth) { XExtDisplayInfo *info = DRI2FindDisplay(dpy); xDRI2ConnectReply rep; @@ -327,6 +328,9 @@ DRI2Connect(Display * dpy, XID window, char **driverName, char **deviceName) } _XReadPad(dpy, *deviceName, rep.deviceNameLength); (*deviceName)[rep.deviceNameLength] = '\0'; + + if (deviceRequiresAuth) + *deviceRequiresAuth = rep.deviceRequiresAuth;
UnlockDisplay(dpy); SyncHandle(); diff --git a/src/glx/dri2.h b/src/glx/dri2.h index a6fe66e..798cba1 100644 --- a/src/glx/dri2.h +++ b/src/glx/dri2.h @@ -53,7 +53,8 @@ DRI2QueryVersion(Display * display, int *major, int *minor);
extern Bool DRI2Connect(Display * display, XID window, - char **driverName, char **deviceName); + char **driverName, char **deviceName, + unsigned char *deviceRequiresAuth);
extern Bool DRI2Authenticate(Display * display, XID window, drm_magic_t magic); diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c index 1b3cf2b..e6354ef 100644 --- a/src/glx/dri2_glx.c +++ b/src/glx/dri2_glx.c @@ -1119,6 +1119,7 @@ dri2CreateScreen(int screen, struct glx_display * priv) __GLXDRIscreen *psp; struct glx_config *configs = NULL, *visuals = NULL; char *driverName, *deviceName, *tmp; + unsigned char deviceRequiresAuth; drm_magic_t magic; int i;
@@ -1134,7 +1135,7 @@ dri2CreateScreen(int screen, struct glx_display * priv) }
if (!DRI2Connect(priv->dpy, RootWindow(priv->dpy, screen), - &driverName, &deviceName)) { + &driverName, &deviceName, &deviceRequiresAuth)) { glx_screen_cleanup(&psc->base); free(psc); InfoMessageF("screen %d does not appear to be DRI2 capable\n", screen); @@ -1179,16 +1180,17 @@ dri2CreateScreen(int screen, struct glx_display * priv) goto handle_error; }
- if (drmGetMagic(psc->fd, &magic)) { - ErrorMessageF("failed to get magic\n"); - goto handle_error; - } + if (deviceRequiresAuth) { + if (drmGetMagic(psc->fd, &magic)) { + ErrorMessageF("failed to get magic\n"); + goto handle_error; + }
- if (!DRI2Authenticate(priv->dpy, RootWindow(priv->dpy, screen), magic)) { - ErrorMessageF("failed to authenticate magic %d\n", magic); - goto handle_error; + if (!DRI2Authenticate(priv->dpy, RootWindow(priv->dpy, screen), magic)) { + ErrorMessageF("failed to authenticate magic %d\n", magic); + goto handle_error; + } } -
/* If the server does not support the protocol for * DRI2GetBuffersWithFormat, don't supply that interface to the driver. diff --git a/src/glx/dri_glx.c b/src/glx/dri_glx.c index ba8fda2..2a03c57 100644 --- a/src/glx/dri_glx.c +++ b/src/glx/dri_glx.c @@ -133,7 +133,8 @@ driGetDriverName(Display * dpy, int scrNum, char **driverName) } else if (DRI2QueryExtension(dpy, &event, &error)) { /* DRI2 */ char *dev; - Bool ret = DRI2Connect(dpy, RootWindow(dpy, scrNum), driverName, &dev); + Bool ret = DRI2Connect(dpy, RootWindow(dpy, scrNum), driverName, &dev, + NULL);
if (ret) free(dev);
On Mon, Dec 17, 2012 at 02:42:00AM +0100, Martin Peres wrote:
Hi,
Following to my shared talk with krh, danvet and Timothée Ravier @ XDC2012, I have actually taken the time to start fixing some security holes found in the graphics stack.
Today, I would like to request your comments on the render node patchset. Keep in mind that I am not asking for inclusion. However, I know this patchset works on my nvidia card and I would like to know if anyone has anything against this architecture.
## DRM If I'm not mistaken, the idea originated from airlied which got simplified later by krh. Both only provided drm patches.
Here is what I did:
- I took krh's patchset
- rebased it on top on drm 3.7-rc8
- added support for Nouveau
- fixed a few bugs along the way (as stated in the commit logs)
The kernel can be found here: https://gitorious.org/linux-nouveau-pm/linux-nouveau-pm/commits/render_nodes The patches will also be sent in reply, to let you comment on specific parts of the patches.
One thing which stalled this on the kernel side, and I think we really need this before it's useful, is per-open-file mmap spaces. Otherwise everyone can still read back your textures easily ...
I've looked around a bit, and besides the plumbing to set up per-open-file mmap offset infrastructure it shouldn't be hard, the linux mm already passes in the file pointer to the ->mmap driver callback. One thing though is to clean up things a bit maybe - iirc ttm based drivers use their own lookup cache for mmap offset, and there's still the legacy drm map support which really shouldn't be exposed to rendernodes. I have this on my todo somewhere, but at the current rate&backlog it'll take a while for me to get around to this, so maybe I can volunteer you?
Also I think we need this implemented just in case some aweful userspace breaks due to the per-file mmap space (there's been some decently aweful code in intel-land ...).
I can't really comment on the other pieces of this due to lack of knowledge.
Cheers, Daniel
## Libdrm
Here are the two main goals of this patchset:
- Add a new symbol called drmOpenType that allows to open a specific
type of device (usual node, render node)
- Add a new symbol called drmGetSameDeviceButType to get the path to
the same drm_device but with a different type
The patches are available here: http://cgit.freedesktop.org/~mperes/drm/
## DRI2Proto
What we want here is to let the ddx send the render node instead of the usual one. However, authentication is not necessary and not shouldn't be done on a render node.
This modification to DRI2Proto adds a boolean in the Connection response to tell the dri2 client that no authentication is required.
The patches are available here: http://cgit.freedesktop.org/~mperes/dri2proto/
## XServer
The X-Server is responsible for collecting the DRI2 informations from the ddx. In this patch, we provide the way for the ddx to specify whether the DRI2 client should authenticate or not.
The patches are available here: http://cgit.freedesktop.org/~mperes/xserver/
## xf86-video-nouveau
In this patch, we simply tell the DRI2 extension to use the render node if available (using drmGetSameDeviceButType), and if it is the case, we set the "require_authentication" attribute to 0.
The patches are available here: http://cgit.freedesktop.org/~mperes/xf86-video-nouveau/
## Mesa
In this patch, I simply check whether I should authenticate or not using the information from the DRI2 protocol.
The patches are available here: http://cgit.freedesktop.org/~mperes/mesa/
Cheers,
Martin _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Le 18/12/2012 13:03, Daniel Vetter a écrit :
On Mon, Dec 17, 2012 at 02:42:00AM +0100, Martin Peres wrote:
Hi,
Following to my shared talk with krh, danvet and Timothée Ravier @ XDC2012, I have actually taken the time to start fixing some security holes found in the graphics stack.
Today, I would like to request your comments on the render node patchset. Keep in mind that I am not asking for inclusion. However, I know this patchset works on my nvidia card and I would like to know if anyone has anything against this architecture.
## DRM If I'm not mistaken, the idea originated from airlied which got simplified later by krh. Both only provided drm patches.
Here is what I did:
- I took krh's patchset
- rebased it on top on drm 3.7-rc8
- added support for Nouveau
- fixed a few bugs along the way (as stated in the commit logs)
The kernel can be found here: https://gitorious.org/linux-nouveau-pm/linux-nouveau-pm/commits/render_nodes The patches will also be sent in reply, to let you comment on specific parts of the patches.
One thing which stalled this on the kernel side, and I think we really need this before it's useful, is per-open-file mmap spaces. Otherwise everyone can still read back your textures easily ...
Yeah, krh told me about it. It was on this week's todo list but it may be pushed back to this week end.
I've looked around a bit, and besides the plumbing to set up per-open-file mmap offset infrastructure it shouldn't be hard, the linux mm already passes in the file pointer to the ->mmap driver callback. One thing though is to clean up things a bit maybe - iirc ttm based drivers use their own lookup cache for mmap offset, and there's still the legacy drm map support which really shouldn't be exposed to rendernodes. I have this on my todo somewhere, but at the current rate&backlog it'll take a while for me to get around to this, so maybe I can volunteer you?
Sure! When I sent the mail, I realized I forgot to talk about my DRM todo list. This was the number one issue I had.
Thanks for the suggestions, I will look into it promptly.
Also I think we need this implemented just in case some aweful userspace breaks due to the per-file mmap space (there's been some decently aweful code in intel-land ...).
So, you mean some intel-oriented sw communicate the mapping offset between each others? That's ugly as hell!
I can't really comment on the other pieces of this due to lack of knowledge.
Thanks, I do not expect for every one to have some feedback on the whole patchset. I'm new to these user-space projects too...
Cheers, Daniel
Thanks for taking the time to answer me!
Cheers, Martin
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Martin Peres -
martin.peres＠free.fr