Commit 3945e475 authored by Francisco Jerez's avatar Francisco Jerez Committed by Ben Skeggs
Browse files

drm/nouveau: Refactor context destruction to avoid a lock ordering issue. 



The destroy_context() engine hooks call gpuobj management functions to
release the channel resources, these functions use HARDIRQ-unsafe locks
whereas destroy_context() is called with the HARDIRQ-safe
context_switch_lock held, that's a lock ordering violation.

Push the engine-specific channel destruction logic into destroy_context()
and let the hardware-specific code lock and unlock when it's actually
needed. Change the engine destruction order to avoid a race in the small
gap between pgraph and pfifo context uninitialization.

Reported-by: default avatarMarcin Slusarz <marcin.slusarz@gmail.com>
Signed-off-by: default avatarFrancisco Jerez <currojerez@riseup.net>
Signed-off-by: default avatarBen Skeggs <bskeggs@redhat.com>
parent fcccab2e

drivers/gpu/drm/nouveau/nouveau_channel.c

+6 −18
Original line number Diff line number Diff line
@@ -313,32 +313,20 @@ nouveau_channel_put(struct nouveau_channel **pchan) 
	/* boot it off the hardware */

	pfifo->reassign(dev, false);



	/* We want to give pgraph a chance to idle and get rid of all potential

	 * errors. We need to do this before the lock, otherwise the irq handler

	 * is unable to process them.

	/* We want to give pgraph a chance to idle and get rid of all

	 * potential errors. We need to do this without the context

	 * switch lock held, otherwise the irq handler is unable to

	 * process them.

	 */

	if (pgraph->channel(dev) == chan)

		nouveau_wait_for_idle(dev);



	spin_lock_irqsave(&dev_priv->context_switch_lock, flags);



	pgraph->fifo_access(dev, false);

	if (pgraph->channel(dev) == chan)

		pgraph->unload_context(dev);

	pgraph->destroy_context(chan);

	pgraph->fifo_access(dev, true);



	if (pfifo->channel_id(dev) == chan->id) {

		pfifo->disable(dev);

		pfifo->unload_context(dev);

		pfifo->enable(dev);

	}

	/* destroy the engine specific contexts */

	pfifo->destroy_context(chan);

	pgraph->destroy_context(chan);



	pfifo->reassign(dev, true);



	spin_unlock_irqrestore(&dev_priv->context_switch_lock, flags);



	/* aside from its resources, the channel should now be dead,

	 * remove it from the channel list

	 */

drivers/gpu/drm/nouveau/nouveau_drv.h

+0 −2
Original line number Diff line number Diff line
@@ -998,14 +998,12 @@ extern int nv04_fifo_unload_context(struct drm_device *); 
extern int  nv10_fifo_init(struct drm_device *);

extern int  nv10_fifo_channel_id(struct drm_device *);

extern int  nv10_fifo_create_context(struct nouveau_channel *);

extern void nv10_fifo_destroy_context(struct nouveau_channel *);

extern int  nv10_fifo_load_context(struct nouveau_channel *);

extern int  nv10_fifo_unload_context(struct drm_device *);



/* nv40_fifo.c */

extern int  nv40_fifo_init(struct drm_device *);

extern int  nv40_fifo_create_context(struct nouveau_channel *);

extern void nv40_fifo_destroy_context(struct nouveau_channel *);

extern int  nv40_fifo_load_context(struct nouveau_channel *);

extern int  nv40_fifo_unload_context(struct drm_device *);

drivers/gpu/drm/nouveau/nouveau_state.c

+4 −4
Original line number Diff line number Diff line
@@ -137,7 +137,7 @@ static int nouveau_init_engine_ptrs(struct drm_device *dev) 
		engine->fifo.cache_pull		= nv04_fifo_cache_pull;

		engine->fifo.channel_id		= nv10_fifo_channel_id;

		engine->fifo.create_context	= nv10_fifo_create_context;

		engine->fifo.destroy_context	= nv10_fifo_destroy_context;

		engine->fifo.destroy_context	= nv04_fifo_destroy_context;

		engine->fifo.load_context	= nv10_fifo_load_context;

		engine->fifo.unload_context	= nv10_fifo_unload_context;

		engine->display.early_init	= nv04_display_early_init;
@@ -191,7 +191,7 @@ static int nouveau_init_engine_ptrs(struct drm_device *dev) 
		engine->fifo.cache_pull		= nv04_fifo_cache_pull;

		engine->fifo.channel_id		= nv10_fifo_channel_id;

		engine->fifo.create_context	= nv10_fifo_create_context;

		engine->fifo.destroy_context	= nv10_fifo_destroy_context;

		engine->fifo.destroy_context	= nv04_fifo_destroy_context;

		engine->fifo.load_context	= nv10_fifo_load_context;

		engine->fifo.unload_context	= nv10_fifo_unload_context;

		engine->display.early_init	= nv04_display_early_init;
@@ -245,7 +245,7 @@ static int nouveau_init_engine_ptrs(struct drm_device *dev) 
		engine->fifo.cache_pull		= nv04_fifo_cache_pull;

		engine->fifo.channel_id		= nv10_fifo_channel_id;

		engine->fifo.create_context	= nv10_fifo_create_context;

		engine->fifo.destroy_context	= nv10_fifo_destroy_context;

		engine->fifo.destroy_context	= nv04_fifo_destroy_context;

		engine->fifo.load_context	= nv10_fifo_load_context;

		engine->fifo.unload_context	= nv10_fifo_unload_context;

		engine->display.early_init	= nv04_display_early_init;
@@ -302,7 +302,7 @@ static int nouveau_init_engine_ptrs(struct drm_device *dev) 
		engine->fifo.cache_pull		= nv04_fifo_cache_pull;

		engine->fifo.channel_id		= nv10_fifo_channel_id;

		engine->fifo.create_context	= nv40_fifo_create_context;

		engine->fifo.destroy_context	= nv40_fifo_destroy_context;

		engine->fifo.destroy_context	= nv04_fifo_destroy_context;

		engine->fifo.load_context	= nv40_fifo_load_context;

		engine->fifo.unload_context	= nv40_fifo_unload_context;

		engine->display.early_init	= nv04_display_early_init;

drivers/gpu/drm/nouveau/nv04_fifo.c

+19 −2
Original line number Diff line number Diff line
@@ -151,10 +151,27 @@ void 
nv04_fifo_destroy_context(struct nouveau_channel *chan)

{

	struct drm_device *dev = chan->dev;

	struct drm_nouveau_private *dev_priv = dev->dev_private;

	struct nouveau_fifo_engine *pfifo = &dev_priv->engine.fifo;

	unsigned long flags;



	nv_wr32(dev, NV04_PFIFO_MODE,

		nv_rd32(dev, NV04_PFIFO_MODE) & ~(1 << chan->id));

	spin_lock_irqsave(&dev_priv->context_switch_lock, flags);

	pfifo->reassign(dev, false);



	/* Unload the context if it's the currently active one */

	if (pfifo->channel_id(dev) == chan->id) {

		pfifo->disable(dev);

		pfifo->unload_context(dev);

		pfifo->enable(dev);

	}



	/* Keep it from being rescheduled */

	nv_mask(dev, NV04_PFIFO_MODE, 1 << chan->id, 0);



	pfifo->reassign(dev, true);

	spin_unlock_irqrestore(&dev_priv->context_switch_lock, flags);



	/* Free the channel resources */

	nouveau_gpuobj_ref(NULL, &chan->ramfc);

}

drivers/gpu/drm/nouveau/nv04_graph.c

+15 −0
Original line number Diff line number Diff line
@@ -412,10 +412,25 @@ int nv04_graph_create_context(struct nouveau_channel *chan) 


void nv04_graph_destroy_context(struct nouveau_channel *chan)

{

	struct drm_device *dev = chan->dev;

	struct drm_nouveau_private *dev_priv = dev->dev_private;

	struct nouveau_pgraph_engine *pgraph = &dev_priv->engine.graph;

	struct graph_state *pgraph_ctx = chan->pgraph_ctx;

	unsigned long flags;



	spin_lock_irqsave(&dev_priv->context_switch_lock, flags);

	pgraph->fifo_access(dev, false);



	/* Unload the context if it's the currently active one */

	if (pgraph->channel(dev) == chan)

		pgraph->unload_context(dev);



	/* Free the context resources */

	kfree(pgraph_ctx);

	chan->pgraph_ctx = NULL;



	pgraph->fifo_access(dev, true);

	spin_unlock_irqrestore(&dev_priv->context_switch_lock, flags);

}



int nv04_graph_load_context(struct nouveau_channel *chan)