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

["Kuehling, Felix" <felix.kuehling@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 20414 bytes --]

On 2026-08-04 23:52, Fan, Shikang wrote:
> 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.

OK, that doesn't just affect GFX 9.4.3. kgd_gfx_v9_hqd_destroy accesses 
the CPU pointer of the MQD for all GFX9 version. You're right, that's 
only valid while the BO is pinned. It would be safer to set q->mqd = 
NULL in dqm_evict_mqd_bo to catch any accidental access while the BO is 
evicted.

And maybe we can simplify this. The nocpsch mode is not supported in 
production. It only exists for debugging or triaging HWS problems. It 
doesn't support CWSR, so it's especially useless for virtualization. We 
can simply avoid the whole unpinning and repinning dance in nocpsch mode 
(dqm->sched_policy == KFD_SCHED_POLICY_NO_HWS).

Regards,
   Felix


>
> Regards,
> Shikang
>
> ------------------------------------------------------------------------
> *From:* Kuehling, Felix <Felix.Kuehling@amd.com>
> *Sent:* Wednesday, August 5, 2026 6:11 AM
> *To:* Fan, Shikang <Shikang.Fan@amd.com>; 
> amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
> *Cc:* Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, 
> Christian <Christian.Koenig@amd.com>; Yang, Philip 
> <Philip.Yang@amd.com>; Limonciello, Mario <Mario.Limonciello@amd.com>; 
> SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>; Zhang, Tiantian 
> (Celine) <Tiantian.Zhang@amd.com>; Zhao, Victor <Victor.Zhao@amd.com>; 
> Zhang, GuoQing (Sam) <GuoQing.Zhang@amd.com>
> *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 <shikang.fan@amd.com>
> > ---
> >   .../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 <felix.kuehling@amd.com>
>
>
> >                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 {

[-- Attachment #2: Type: text/html, Size: 39799 bytes --]