Re: [PATCH v4] drm/amdkfd: preserve VRAM MQD across hibernation via unpin/repin

On 2026-08-03 06:06, Shikang Fan wrote:
> On gfx9 ASICs with mqd_on_vram(), a compute queue MQD lives in a pinned
> VRAM buffer object. Pinned BOs are skipped by the VRAM eviction done at S4
> suspend, so the MQD contents are lost across hibernation and the first
> submission after resume page-faults on a stale MQD.
>
> Unpin the MQD BO at suspend so the eviction migrates it into the
> hibernation image, and pin it back to VRAM on resume. The BO may return at
> a different VRAM address, so refresh the kernel mapping and cached GPU
> addresses and patch the MQD self-address via a new update_mqd_gpu_addr()
> mqd_manager op; skip eviction with a warning if that op is not implemented.
>
> v3: use unpin/repin instead of shadowing the MQD into a separate buffer.
>
> v4: drop the explicit VRAM->GTT placement at evict (a bare unpin is enough
> for the eviction pass to move the BO out of VRAM), and also repin at queue
> destroy. KFD queue restore runs late - user processes thaw before it, and
> under SR-IOV it is deferred until the VF regains full access - so once the
> VM has resumed an application can destroy a queue before its MQD BO is
> repinned, which would otherwise unpin an already-unpinned BO and touch a
> stale q->mqd.
>
> Signed-off-by: Shikang Fan <[email protected]>
> ---
>   .../drm/amd/amdkfd/kfd_device_queue_manager.c | 123 ++++++++++++++++++
>   drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h  |   8 ++
>   .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c   |  40 ++++++
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |   6 +
>   4 files changed, 177 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 51ee9c39104b..5ce4d4cb423c 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -77,6 +77,7 @@ static struct queue *find_queue_by_doorbell_offset(struct device_queue_manager *
>   static void set_queue_as_reset(struct device_queue_manager *dqm, struct queue *q,
>   			       struct qcm_process_device *qpd);
>   static int reset_queues_mes(struct device_queue_manager *dqm, struct queue *q);
> +static int dqm_repin_mqd_bo(struct device_queue_manager *dqm, struct queue *q);
>   
>   static inline
>   enum KFD_MQD_TYPE get_mqd_type_from_queue_type(enum kfd_queue_type type)
> @@ -1048,6 +1049,11 @@ static int destroy_queue_nocpsch(struct device_queue_manager *dqm,
>   				q->properties.queue_id);
>   	}
>   
> +	/* Repin the MQD BO if it is still evicted for hibernation, before
> +	 * destroy_queue_nocpsch_locked() dereferences q->mqd or it is freed.
> +	 */
> +	dqm_repin_mqd_bo(dqm, q);
> +
>   	dqm_lock(dqm);
>   	retval = destroy_queue_nocpsch_locked(dqm, qpd, q);
>   	if (!retval)
> @@ -1254,6 +1260,98 @@ static int resume_single_queue(struct device_queue_manager *dqm,
>   	return 0;
>   }
>   
> +/* Unpin the MQD BO at S4 suspend so it is evicted into the hibernation image;
> + * dqm_repin_mqd_bo() pins it back on resume. Gated on adev->in_s4 so runtime
> + * eviction is untouched.
> + */
> +static void dqm_evict_mqd_bo(struct device_queue_manager *dqm, struct queue *q)
> +{
> +	struct mqd_manager *mqd_mgr;
> +	struct amdgpu_bo *bo;
> +
> +	if (!dqm->dev->adev->in_s4)
> +		return;
> +	if (!mqd_on_vram(dqm->dev->adev))
> +		return;
> +	if (q->properties.type != KFD_QUEUE_TYPE_COMPUTE)
> +		return;
> +	if (!q->mqd_mem_obj || !q->mqd_mem_obj->mem)
> +		return;
> +
> +	/* Without update_mqd_gpu_addr() the MQD self-address cannot be fixed up
> +	 * after a repin, so skip eviction (with a warning) instead of faulting.
> +	 */
> +	mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(q->properties.type)];
> +	if (!mqd_mgr->update_mqd_gpu_addr) {
> +		dev_warn_once(dqm->dev->adev->dev,
> +			      "MQD is in VRAM but update_mqd_gpu_addr is not implemented; skipping hibernation eviction\n");
> +		return;
> +	}
> +
> +	bo = q->mqd_mem_obj->mem;
> +	if (amdgpu_bo_reserve(bo, false))
> +		return;
> +
> +	amdgpu_bo_unpin(bo);
> +	amdgpu_bo_unreserve(bo);
> +	q->needs_mqd_repin = true;
> +}
> +
> +/* Repin the MQD BO to VRAM and refresh the cached mapping and GPU addresses.
> + * Used both on resume and when a queue is destroyed before resume has repinned
> + * it. A no-op unless a repin is owed (needs_mqd_repin set).
> + */
> +static int dqm_repin_mqd_bo(struct device_queue_manager *dqm, struct queue *q)
> +{
> +	struct mqd_manager *mqd_mgr;
> +	struct amdgpu_bo *bo;
> +	void *cpu_ptr;
> +	int r;
> +
> +	if (!q->needs_mqd_repin)
> +		return 0;
> +	if (!q->mqd_mem_obj || !q->mqd_mem_obj->mem)
> +		return 0;
> +
> +	bo = q->mqd_mem_obj->mem;
> +	r = amdgpu_bo_reserve(bo, false);
> +	if (r)
> +		return r;
> +	r = amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_VRAM);
> +	if (r) {
> +		amdgpu_bo_unreserve(bo);
> +		dev_err(dqm->dev->adev->dev,
> +			"Failed to repin MQD of queue %d to VRAM: %d\n",
> +			q->properties.queue_id, r);
> +		return r;
> +	}
> +	/* The BO may have moved; refresh the kernel mapping and gpu address. */
> +	amdgpu_bo_kunmap(bo);
> +	r = amdgpu_bo_kmap(bo, &cpu_ptr);
> +	amdgpu_bo_unreserve(bo);
> +	if (r) {
> +		dev_err(dqm->dev->adev->dev,
> +			"Failed to remap MQD of queue %d: %d\n",
> +			q->properties.queue_id, r);
> +		return r;
> +	}
> +
> +	q->mqd_mem_obj->cpu_ptr = cpu_ptr;
> +	q->mqd_mem_obj->gpu_addr = amdgpu_bo_gpu_offset(bo);
> +	q->gart_mqd_addr = q->mqd_mem_obj->gpu_addr;
> +	q->mqd = cpu_ptr;
> +
> +	mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
> +			q->properties.type)];
> +	if (mqd_mgr->update_mqd_gpu_addr)
> +		mqd_mgr->update_mqd_gpu_addr(mqd_mgr, q->mqd,
> +					     q->mqd_mem_obj,
> +					     &q->properties);
> +
> +	q->needs_mqd_repin = false;
> +	return 0;
> +}
> +
>   static int evict_process_queues_nocpsch(struct device_queue_manager *dqm,
>   					struct qcm_process_device *qpd)
>   {
> @@ -1297,6 +1395,8 @@ static int evict_process_queues_nocpsch(struct device_queue_manager *dqm,
>   			 * maintain a consistent eviction state
>   			 */
>   			ret = retval;
> +
> +		dqm_evict_mqd_bo(dqm, q);
>   	}
>   
>   out:
> @@ -1350,6 +1450,8 @@ static int evict_process_queues_cpsch(struct device_queue_manager *dqm,
>   				goto out;
>   			}
>   		}
> +
> +		dqm_evict_mqd_bo(dqm, q);
>   	}
>   
>   	if (!dqm->dev->kfd->shared_resources.enable_mes) {
> @@ -1429,6 +1531,10 @@ static int restore_process_queues_nocpsch(struct device_queue_manager *dqm,
>   		if (WARN_ONCE(!dqm->sched_running, "Restore when stopped\n"))
>   			continue;
>   
> +		retval = dqm_repin_mqd_bo(dqm, q);
> +		if (retval && !ret)
> +			ret = retval;
> +
>   		retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, q->pipe,
>   				       q->queue, &q->properties, mm);
>   		if (retval && !ret)
> @@ -1489,6 +1595,13 @@ static int restore_process_queues_cpsch(struct device_queue_manager *dqm,
>   		q->properties.is_active = true;
>   		increment_queue_count(dqm, &pdd->qpd, q);
>   
> +		retval = dqm_repin_mqd_bo(dqm, q);
> +		if (retval) {
> +			dev_err(dev, "Failed to repin MQD for queue %d\n",
> +				q->properties.queue_id);
> +			goto out;
> +		}
> +
>   		if (dqm->dev->kfd->shared_resources.enable_mes) {
>   			retval = add_queue_mes(dqm, q, qpd);
>   			if (retval) {
> @@ -2760,6 +2873,8 @@ static int destroy_queue_cpsch(struct device_queue_manager *dqm,
>   				qpd->pqm->process, q->device,
>   				-1, false, NULL, 0);
>   
> +	/* Repin the MQD BO if still evicted for hibernation, before it is freed. */
> +	dqm_repin_mqd_bo(dqm, q);
>   	mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
>   
>   	return retval;
> @@ -2827,6 +2942,12 @@ static int process_termination_nocpsch(struct device_queue_manager *dqm,
>   		q = list_first_entry(&qpd->queues_list, struct queue, list);
>   		mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
>   				q->properties.type)];
> +		/* Repin the MQD BO before destroy_queue_nocpsch_locked()
> +		 * dereferences q->mqd; drop the DQM lock as reserve may sleep.
> +		 */
> +		dqm_unlock(dqm);
> +		dqm_repin_mqd_bo(dqm, q);
> +		dqm_lock(dqm);

You don't really need to repin before calling destroy_queue. You need it 
before freeing the MQD. That is done a few lines below in another 
section that already drops the DQM lock. You can just move repin into 
that section and avoid some unnecessary churn dropping and re-taking the 
lock repeatedly. I think it's OK to do that after 
destroy_queue_nocpsch_locked, because that function doesn't actually 
free the queue struct.

With that fixed, the patch is

Reviewed-by: Felix Kuehling <[email protected]>


>   		ret = destroy_queue_nocpsch_locked(dqm, qpd, q);
>   		if (ret)
>   			retval = ret;
> @@ -3017,6 +3138,8 @@ static int process_termination_cpsch(struct device_queue_manager *dqm,
>   		list_del(&q->list);
>   		qpd->queue_count--;
>   		dqm_unlock(dqm);
> +		/* Repin the MQD BO if still evicted for hibernation, before free. */
> +		dqm_repin_mqd_bo(dqm, q);
>   		mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
>   		dqm_lock(dqm);
>   	}
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
> index 59eff3389d39..38b46b696243 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
> @@ -117,6 +117,14 @@ struct mqd_manager {
>   				const void *ctl_stack_src,
>   				const u32 ctl_stack_size);
>   
> +	/* Patch the MQD's cached self GPU address after the MQD BO has moved
> +	 * (e.g. repinned to a new VRAM location on hibernation resume). The MQD
> +	 * contents are otherwise preserved.
> +	 */
> +	void	(*update_mqd_gpu_addr)(struct mqd_manager *mm, void *mqd,
> +				       struct kfd_mem_obj *mqd_mem_obj,
> +				       struct queue_properties *p);
> +
>   #if defined(CONFIG_DEBUG_FS)
>   	int	(*debugfs_show_mqd)(struct seq_file *m, void *data);
>   #endif
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
> index 75e5a9f67d50..b95720198e28 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
> @@ -476,6 +476,20 @@ static void restore_mqd(struct mqd_manager *mm, void **mqd,
>   	qp->is_active = 0;
>   }
>   
> +static void update_mqd_gpu_addr(struct mqd_manager *mm, void *mqd,
> +				struct kfd_mem_obj *mqd_mem_obj,
> +				struct queue_properties *qp)
> +{
> +	struct v9_mqd *m = get_mqd(mqd);
> +	uint64_t addr = mqd_mem_obj->gpu_addr;
> +
> +	m->cp_mqd_base_addr_lo = lower_32_bits(addr);
> +	m->cp_mqd_base_addr_hi = upper_32_bits(addr);
> +
> +	if (mqd_on_vram(mm->dev->adev))
> +		amdgpu_device_flush_hdp(mm->dev->adev, NULL);
> +}
> +
>   static void init_mqd_hiq(struct mqd_manager *mm, void **mqd,
>   			struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>   			struct queue_properties *q)
> @@ -860,6 +874,30 @@ static void restore_mqd_v9_4_3(struct mqd_manager *mm, void **mqd,
>   	if (mqd_on_vram(mm->dev->adev))
>   		amdgpu_device_flush_hdp(mm->dev->adev, NULL);
>   }
> +
> +static void update_mqd_gpu_addr_v9_4_3(struct mqd_manager *mm, void *mqd,
> +				       struct kfd_mem_obj *mqd_mem_obj,
> +				       struct queue_properties *qp)
> +{
> +	struct kfd_mem_obj xcc_mqd_mem_obj;
> +	uint64_t offset = mm->mqd_stride(mm, qp);
> +	u32 num_xcc = NUM_XCC(mm->dev->xcc_mask);
> +	struct v9_mqd *m;
> +	int xcc;
> +
> +	memset(&xcc_mqd_mem_obj, 0x0, sizeof(struct kfd_mem_obj));
> +
> +	for (xcc = 0; xcc < num_xcc; xcc++) {
> +		get_xcc_mqd(mqd_mem_obj, &xcc_mqd_mem_obj, offset * xcc);
> +		m = get_mqd(mqd + offset * xcc);
> +		m->cp_mqd_base_addr_lo = lower_32_bits(xcc_mqd_mem_obj.gpu_addr);
> +		m->cp_mqd_base_addr_hi = upper_32_bits(xcc_mqd_mem_obj.gpu_addr);
> +	}
> +
> +	if (mqd_on_vram(mm->dev->adev))
> +		amdgpu_device_flush_hdp(mm->dev->adev, NULL);
> +}
> +
>   static int destroy_mqd_v9_4_3(struct mqd_manager *mm, void *mqd,
>   		   enum kfd_preempt_type type, unsigned int timeout,
>   		   uint32_t pipe_id, uint32_t queue_id)
> @@ -1017,6 +1055,7 @@ struct mqd_manager *mqd_manager_init_v9(enum KFD_MQD_TYPE type,
>   			mqd->get_wave_state = get_wave_state_v9_4_3;
>   			mqd->checkpoint_mqd = checkpoint_mqd_v9_4_3;
>   			mqd->restore_mqd = restore_mqd_v9_4_3;
> +			mqd->update_mqd_gpu_addr = update_mqd_gpu_addr_v9_4_3;
>   		} else {
>   			mqd->init_mqd = init_mqd;
>   			mqd->load_mqd = load_mqd;
> @@ -1025,6 +1064,7 @@ struct mqd_manager *mqd_manager_init_v9(enum KFD_MQD_TYPE type,
>   			mqd->get_wave_state = get_wave_state;
>   			mqd->checkpoint_mqd = checkpoint_mqd;
>   			mqd->restore_mqd = restore_mqd;
> +			mqd->update_mqd_gpu_addr = update_mqd_gpu_addr;
>   		}
>   		break;
>   	case KFD_MQD_TYPE_HIQ:
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index bcb929002839..0dc4f76a36d4 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -637,6 +637,12 @@ struct queue {
>   	void *gang_ctx_cpu_ptr;
>   
>   	struct amdgpu_bo *wptr_bo_gart;
> +
> +	/* The VRAM-resident MQD BO (mqd_on_vram()) is unpinned at S4 suspend so
> +	 * TTM evicts it into the hibernation image, and repinned on resume. Set
> +	 * while the BO is unpinned so the resume path knows to repin it.
> +	 */
> +	bool needs_mqd_repin;
>   };
>   
>   enum KFD_MQD_TYPE {