Refine the locking of the DRM. Most significant is covering the driver

ioctls with dev_lock, which is a major step toward being able to remove
    Giant. Covers some new pieces (dev->unique*) in the core, and avoids
    one call down into system internals with the drm lock held, which is
    usually bad (FreeBSD LOR #23, #27).
main
Eric Anholt 2004-11-07 04:11:15 +00:00
parent c5bededa51
commit a1d9e5abaf
8 changed files with 130 additions and 93 deletions

View File

@ -197,6 +197,7 @@ MALLOC_DECLARE(M_DRM);
#define DRM_SPINUNINIT(l)
#define DRM_SPINLOCK(l)
#define DRM_SPINUNLOCK(u)
#define DRM_SPINLOCK_ASSERT(l)
#define DRM_CURRENTPID curproc->p_pid
#define DRM_LOCK()
#define DRM_UNLOCK()
@ -353,9 +354,7 @@ do { \
DRM_ERROR("filp doesn't match curproc\n"); \
return EINVAL; \
} \
DRM_LOCK(); \
_priv = drm_find_file_by_proc(dev, DRM_CURPROC); \
DRM_UNLOCK(); \
if (_priv == NULL) { \
DRM_ERROR("can't find authenticator\n"); \
return EINVAL; \
@ -375,6 +374,7 @@ do { \
#define DRM_GETSAREA() \
do { \
drm_map_list_entry_t *listentry; \
DRM_SPINLOCK_ASSERT(&dev->dev_lock); \
TAILQ_FOREACH(listentry, dev->maplist, link) { \
drm_local_map_t *map = listentry->map; \
if (map->type == _DRM_SHM && \
@ -388,11 +388,13 @@ do { \
#if defined(__FreeBSD__) && __FreeBSD_version > 500000
#define DRM_WAIT_ON( ret, queue, timeout, condition ) \
for ( ret = 0 ; !ret && !(condition) ; ) { \
DRM_UNLOCK(); \
mtx_lock(&dev->irq_lock); \
if (!(condition)) \
ret = msleep(&(queue), &dev->irq_lock, \
ret = msleep(&(queue), &dev->irq_lock, \
PZERO | PCATCH, "drmwtq", (timeout)); \
mtx_unlock(&dev->irq_lock); \
mtx_unlock(&dev->irq_lock); \
DRM_LOCK(); \
}
#else
#define DRM_WAIT_ON( ret, queue, timeout, condition ) \
@ -527,10 +529,6 @@ typedef struct drm_device_dma {
_DRM_DMA_USE_AGP = 0x01,
_DRM_DMA_USE_SG = 0x02
} flags;
/* DMA support */
drm_buf_t *this_buffer; /* Buffer being sent */
drm_buf_t *next_buffer; /* Selected buffer to send */
} drm_device_dma_t;
typedef struct drm_agp_mem {

View File

@ -116,12 +116,18 @@ static int drm_remove_magic(drm_device_t *dev, drm_magic_t magic)
int drm_getmagic(DRM_IOCTL_ARGS)
{
DRM_DEVICE;
static drm_magic_t sequence = 0;
drm_auth_t auth;
drm_file_t *priv;
DRM_DEVICE;
DRM_GET_PRIV_WITH_RETURN(priv, filp);
DRM_LOCK();
priv = drm_find_file_by_proc(dev, p);
DRM_UNLOCK();
if (priv == NULL) {
DRM_ERROR("can't find authenticator\n");
return EINVAL;
}
/* Find unique magic */
if (priv->magic) {

View File

@ -229,15 +229,12 @@ int drm_context_switch_complete(drm_device_t *dev, int new)
int drm_resctx(DRM_IOCTL_ARGS)
{
drm_ctx_res_t res;
drm_ctx_t ctx;
int i;
DRM_COPY_FROM_USER_IOCTL( res, (drm_ctx_res_t *)data, sizeof(res) );
if ( res.count >= DRM_RESERVED_CONTEXTS ) {
bzero(&ctx, sizeof(ctx));
for ( i = 0 ; i < DRM_RESERVED_CONTEXTS ; i++ ) {
ctx.handle = i;
if ( DRM_COPY_TO_USER( &res.contexts[i],
&i, sizeof(i) ) )
return DRM_ERR(EFAULT);
@ -269,8 +266,11 @@ int drm_addctx(DRM_IOCTL_ARGS)
return DRM_ERR(ENOMEM);
}
if (dev->context_ctor && ctx.handle != DRM_KERNEL_CONTEXT)
if (dev->context_ctor && ctx.handle != DRM_KERNEL_CONTEXT) {
DRM_LOCK();
dev->context_ctor(dev, ctx.handle);
DRM_UNLOCK();
}
DRM_COPY_TO_USER_IOCTL( (drm_ctx_t *)data, ctx, sizeof(ctx) );
@ -330,8 +330,11 @@ int drm_rmctx(DRM_IOCTL_ARGS)
DRM_DEBUG( "%d\n", ctx.handle );
if ( ctx.handle != DRM_KERNEL_CONTEXT ) {
if (dev->context_dtor)
if (dev->context_dtor) {
DRM_LOCK();
dev->context_dtor(dev, ctx.handle);
DRM_UNLOCK();
}
drm_ctxbitmap_free(dev, ctx.handle);
}

View File

@ -809,9 +809,17 @@ int drm_ioctl(struct cdev *kdev, u_long cmd, caddr_t data, int flags,
drm_ioctl_desc_t *ioctl;
int (*func)(DRM_IOCTL_ARGS);
int nr = DRM_IOCTL_NR(cmd);
int is_driver_ioctl = 0;
drm_file_t *priv;
DRMFILE filp = (DRMFILE)(uintptr_t)DRM_CURRENTPID;
DRM_GET_PRIV_WITH_RETURN(priv, (DRMFILE)(uintptr_t)DRM_CURRENTPID);
DRM_LOCK();
priv = drm_find_file_by_proc(dev, p);
DRM_UNLOCK();
if (priv == NULL) {
DRM_ERROR("can't find authenticator\n");
return EINVAL;
}
atomic_inc( &dev->counts[_DRM_STAT_IOCTLS] );
++priv->ioctl_count;
@ -868,6 +876,7 @@ int drm_ioctl(struct cdev *kdev, u_long cmd, caddr_t data, int flags,
return EINVAL;
}
ioctl = &dev->driver_ioctls[nr];
is_driver_ioctl = 1;
}
func = ioctl->func;
@ -879,7 +888,11 @@ int drm_ioctl(struct cdev *kdev, u_long cmd, caddr_t data, int flags,
!priv->authenticated))
return EACCES;
retcode = func(kdev, cmd, data, flags, p, (void *)(uintptr_t)DRM_CURRENTPID);
if (is_driver_ioctl)
DRM_LOCK();
retcode = func(kdev, cmd, data, flags, p, filp);
if (is_driver_ioctl)
DRM_UNLOCK();
if (retcode != 0)
DRM_DEBUG(" returning %d\n", retcode);

View File

@ -64,40 +64,53 @@ int drm_setunique(DRM_IOCTL_ARGS)
DRM_DEVICE;
drm_unique_t u;
int domain, bus, slot, func, ret;
if (dev->unique_len || dev->unique)
return DRM_ERR(EBUSY);
char *busid;
DRM_COPY_FROM_USER_IOCTL( u, (drm_unique_t *)data, sizeof(u) );
/* Check and copy in the submitted Bus ID */
if (!u.unique_len || u.unique_len > 1024)
return DRM_ERR(EINVAL);
dev->unique_len = u.unique_len;
dev->unique = malloc(u.unique_len + 1, M_DRM, M_NOWAIT);
if (dev->unique == NULL)
busid = malloc(u.unique_len + 1, M_DRM, M_WAITOK);
if (busid == NULL)
return DRM_ERR(ENOMEM);
if (DRM_COPY_FROM_USER(dev->unique, u.unique, dev->unique_len))
if (DRM_COPY_FROM_USER(busid, u.unique, u.unique_len)) {
free(busid, M_DRM);
return DRM_ERR(EFAULT);
dev->unique[dev->unique_len] = '\0';
}
busid[u.unique_len] = '\0';
/* Return error if the busid submitted doesn't match the device's actual
* busid.
*/
ret = sscanf(dev->unique, "PCI:%d:%d:%d", &bus, &slot, &func);
if (ret != 3)
ret = sscanf(busid, "PCI:%d:%d:%d", &bus, &slot, &func);
if (ret != 3) {
free(busid, M_DRM);
return DRM_ERR(EINVAL);
}
domain = bus >> 8;
bus &= 0xff;
if ((domain != dev->pci_domain) ||
(bus != dev->pci_bus) ||
(slot != dev->pci_slot) ||
(func != dev->pci_func))
(func != dev->pci_func)) {
free(busid, M_DRM);
return DRM_ERR(EINVAL);
}
/* Actually set the device's busid now. */
DRM_LOCK();
if (dev->unique_len || dev->unique) {
DRM_UNLOCK();
return DRM_ERR(EBUSY);
}
dev->unique_len = u.unique_len;
dev->unique = busid;
DRM_UNLOCK();
return 0;
}
@ -107,17 +120,25 @@ static int
drm_set_busid(drm_device_t *dev)
{
if (dev->unique != NULL)
DRM_LOCK();
if (dev->unique != NULL) {
DRM_UNLOCK();
return EBUSY;
}
dev->unique_len = 20;
dev->unique = malloc(dev->unique_len + 1, M_DRM, M_NOWAIT);
if (dev->unique == NULL)
if (dev->unique == NULL) {
DRM_UNLOCK();
return ENOMEM;
}
snprintf(dev->unique, dev->unique_len, "pci:%04x:%02x:%02x.%1x",
dev->pci_domain, dev->pci_bus, dev->pci_slot, dev->pci_func);
DRM_UNLOCK();
return 0;
}
@ -264,9 +285,6 @@ int drm_setversion(DRM_IOCTL_ARGS)
if (sv.drm_dd_major != dev->driver_major ||
sv.drm_dd_minor < 0 || sv.drm_dd_minor > dev->driver_minor)
return EINVAL;
#ifdef DRIVER_SETVERSION
DRIVER_SETVERSION(dev, &sv);
#endif
}
return 0;
}

View File

@ -73,25 +73,22 @@ int drm_irq_install(drm_device_t *dev)
if (dev->irq == 0 || dev->dev_private == NULL)
return DRM_ERR(EINVAL);
DRM_DEBUG( "%s: irq=%d\n", __FUNCTION__, dev->irq );
DRM_LOCK();
if (dev->irq_enabled) {
DRM_UNLOCK();
return DRM_ERR(EBUSY);
}
dev->irq_enabled = 1;
DRM_UNLOCK();
DRM_DEBUG( "%s: irq=%d\n", __FUNCTION__, dev->irq );
dev->context_flag = 0;
dev->dma->next_buffer = NULL;
dev->dma->this_buffer = NULL;
DRM_SPININIT(dev->irq_lock, "DRM IRQ lock");
/* Before installing handler */
dev->irq_preinstall(dev);
DRM_UNLOCK();
/* Install handler */
#ifdef __FreeBSD__
@ -125,7 +122,9 @@ int drm_irq_install(drm_device_t *dev)
#endif
/* After installing handler */
DRM_LOCK();
dev->irq_postinstall(dev);
DRM_UNLOCK();
return 0;
err:
@ -143,9 +142,6 @@ err:
return retcode;
}
/* XXX: This function needs to be called with the device lock held. In some
* cases it isn't, so far.
*/
int drm_irq_uninstall(drm_device_t *dev)
{
int irqrid;
@ -162,8 +158,10 @@ int drm_irq_uninstall(drm_device_t *dev)
dev->irq_uninstall(dev);
#ifdef __FreeBSD__
DRM_UNLOCK();
bus_teardown_intr(dev->device, dev->irqr, dev->irqh);
bus_release_resource(dev->device, SYS_RES_IRQ, irqrid, dev->irqr);
DRM_LOCK();
#elif defined(__NetBSD__) || defined(__OpenBSD__)
pci_intr_disestablish(&dev->pa.pa_pc, dev->irqh);
#endif
@ -242,7 +240,9 @@ int drm_wait_vblank(DRM_IOCTL_ARGS)
#endif
ret = EINVAL;
} else {
DRM_LOCK();
ret = dev->vblank_wait(dev, &vblwait.request.sequence);
DRM_UNLOCK();
microtime(&now);
vblwait.reply.tval_sec = now.tv_sec;

View File

@ -36,7 +36,6 @@
void drm_sg_cleanup(drm_sg_mem_t *entry)
{
free(entry->virtual, M_DRM);
free(entry->busaddr, M_DRM);
free(entry, M_DRM);
}
@ -56,7 +55,7 @@ int drm_sg_alloc(DRM_IOCTL_ARGS)
DRM_COPY_FROM_USER_IOCTL(request, (drm_scatter_gather_t *)data,
sizeof(request) );
entry = malloc(sizeof(*entry), M_DRM, M_NOWAIT | M_ZERO);
entry = malloc(sizeof(*entry), M_DRM, M_WAITOK | M_ZERO);
if ( !entry )
return ENOMEM;
@ -66,16 +65,15 @@ int drm_sg_alloc(DRM_IOCTL_ARGS)
entry->pages = pages;
entry->busaddr = malloc(pages * sizeof(*entry->busaddr), M_DRM,
M_NOWAIT | M_ZERO);
M_WAITOK | M_ZERO);
if ( !entry->busaddr ) {
free(entry, M_DRM);
drm_sg_cleanup(entry);
return ENOMEM;
}
entry->virtual = malloc(pages << PAGE_SHIFT, M_DRM, M_WAITOK | M_ZERO);
if ( !entry->virtual ) {
free(entry->busaddr, M_DRM);
free(entry, M_DRM);
drm_sg_cleanup(entry);
return ENOMEM;
}
@ -90,12 +88,16 @@ int drm_sg_alloc(DRM_IOCTL_ARGS)
request,
sizeof(request) );
DRM_LOCK();
if (dev->sg) {
DRM_UNLOCK();
drm_sg_cleanup(entry);
return EINVAL;
}
dev->sg = entry;
DRM_UNLOCK();
return 0;
drm_sg_cleanup(entry);
return ENOMEM;
}
int drm_sg_free(DRM_IOCTL_ARGS)
@ -107,8 +109,10 @@ int drm_sg_free(DRM_IOCTL_ARGS)
DRM_COPY_FROM_USER_IOCTL( request, (drm_scatter_gather_t *)data,
sizeof(request) );
DRM_LOCK();
entry = dev->sg;
dev->sg = NULL;
DRM_UNLOCK();
if ( !entry || entry->handle != request.handle )
return EINVAL;

View File

@ -25,35 +25,6 @@
#include "drmP.h"
#include "drm.h"
#if defined(__FreeBSD__) && __FreeBSD_version >= 500102
static int drm_dma_mmap(struct cdev *kdev, vm_offset_t offset,
vm_paddr_t *paddr, int prot)
#elif defined(__FreeBSD__)
static int drm_dma_mmap(dev_t kdev, vm_offset_t offset, int prot)
#elif defined(__NetBSD__) || defined(__OpenBSD__)
static paddr_t drm_dma_mmap(dev_t kdev, vm_offset_t offset, int prot)
#endif
{
DRM_DEVICE;
drm_device_dma_t *dma = dev->dma;
unsigned long physical;
unsigned long page;
if (dma == NULL || dma->pagelist == NULL)
return -1;
page = offset >> PAGE_SHIFT;
physical = dma->pagelist[page];
DRM_DEBUG("0x%08lx (page %lu) => 0x%08lx\n", (long)offset, page, physical);
#if defined(__FreeBSD__) && __FreeBSD_version >= 500102
*paddr = physical;
return 0;
#else
return atop(physical);
#endif
}
#if defined(__FreeBSD__) && __FreeBSD_version >= 500102
int drm_mmap(struct cdev *kdev, vm_offset_t offset, vm_paddr_t *paddr,
int prot)
@ -67,20 +38,40 @@ paddr_t drm_mmap(dev_t kdev, off_t offset, int prot)
drm_local_map_t *map = NULL;
drm_map_list_entry_t *listentry = NULL;
drm_file_t *priv;
drm_map_type_t type;
DRM_GET_PRIV_WITH_RETURN(priv, (DRMFILE)(uintptr_t)DRM_CURRENTPID);
DRM_LOCK();
priv = drm_find_file_by_proc(dev, DRM_CURPROC);
DRM_UNLOCK();
if (priv == NULL) {
DRM_ERROR("can't find authenticator\n");
return EINVAL;
}
if (!priv->authenticated)
return DRM_ERR(EACCES);
if (dev->dma
&& offset >= 0
&& offset < ptoa(dev->dma->page_count))
DRM_SPINLOCK(&dev->dma_lock);
if (dev->dma && offset >= 0 && offset < ptoa(dev->dma->page_count)) {
drm_device_dma_t *dma = dev->dma;
if (dma->pagelist != NULL) {
unsigned long page = offset >> PAGE_SHIFT;
unsigned long phys = dma->pagelist[page];
#if defined(__FreeBSD__) && __FreeBSD_version >= 500102
return drm_dma_mmap(kdev, offset, paddr, prot);
*paddr = phys;
DRM_SPINUNLOCK(&dev->dma_lock);
return 0;
#else
return drm_dma_mmap(kdev, offset, prot);
return atop(phys);
#endif
} else {
DRM_SPINUNLOCK(&dev->dma_lock);
return -1;
}
}
DRM_SPINUNLOCK(&dev->dma_lock);
/* A sequential search of a linked list is
fine here because: 1) there will only be
@ -89,23 +80,27 @@ paddr_t drm_mmap(dev_t kdev, off_t offset, int prot)
once, so it doesn't have to be optimized
for performance, even if the list was a
bit longer. */
DRM_LOCK();
TAILQ_FOREACH(listentry, dev->maplist, link) {
map = listentry->map;
/* DRM_DEBUG("considering 0x%x..0x%x\n", map->offset, map->offset + map->size - 1);*/
if (offset >= map->offset
&& offset < map->offset + map->size) break;
if (offset >= map->offset && offset < map->offset + map->size)
break;
}
if (!listentry) {
DRM_UNLOCK();
DRM_DEBUG("can't find map\n");
return -1;
}
if (((map->flags&_DRM_RESTRICTED) && DRM_SUSER(DRM_CURPROC))) {
DRM_UNLOCK();
DRM_DEBUG("restricted map\n");
return -1;
}
type = map->type;
DRM_UNLOCK();
switch (map->type) {
switch (type) {
case _DRM_FRAME_BUFFER:
case _DRM_REGISTERS:
case _DRM_AGP: