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

"Fan, Shikang" <[email protected]> Wed, 5 Aug 2026 03:52:22 +0000
Newsgroups org.freedesktop.lists.amd-gfx
Message-ID <SA1PR12MB73430C264CAE719BA49B61EDEBD32@SA1PR12MB7343.namprd12.prod.outlook.com>
AMD General

destroy_queue_nocpsch_locked() calls mqd_mgr->destroy_mqd(mqd_mgr, q->mqd, ...), and on gfx9.4.3 destroy_mqd_v9_4_3() reads the MQD through that pointer  (m->cp_mqd_stride_size) — whose kernel mapping is stale until the repin re-kmaps it. That's why I decided to call repin before destroy_queue.

Regards,
Shikang

________________________________
From: Kuehling, Felix <[email protected]>
Sent: Wednesday, August 5, 2026 6:11 AM
To: Fan, Shikang <[email protected]>; [email protected] <[email protected]>
Cc: Deucher, Alexander <[email protected]>; Koenig, Christian <[email protected]>; Yang, Philip <[email protected]>; Limonciello, Mario <[email protected]>; SHANMUGAM, SRINIVASAN <[email protected]>; Zhang, Tiantian (Celine) <[email protected]>; Zhao, Victor <[email protected]>; Zhang, GuoQing (Sam) <[email protected]>
Subject: 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 {