AMD General
> -----Original Message-----
> From: Francis, David <[email protected]>
> Sent: Monday, September 21, 2026 3:23 PM
> To: [email protected]
> Cc: Kuehling, Felix <[email protected]>; Yat Sin, David
> <[email protected]>; Francis, David <[email protected]>
> Subject: [PATCH 1/2] drm/amdkfd: Acquire buffers in CRIU queue restore (again)
>
> CRIU queue restore doesn't acquire the bo buffers in the same way normal queue
> creation does, which results in it failing a check in init_user_queue when
> its wptr_bo
> is null.
>
> Acquire them, but don't get the svm bo. SVM isn't yet fully restored by this
> point in
> CRIU and so there's no svm range to get.
> By the time the queue is actually in use the svm range will be there.
>
> Signed-off-by: David Francis <[email protected]>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +-
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 2 +-
> .../amd/amdkfd/kfd_process_queue_manager.c | 34 ++++++++++++++++---
> drivers/gpu/drm/amd/amdkfd/kfd_queue.c | 8 ++++-
> 4 files changed, 38 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index 7fcfc150a7fc..ed6e0f817e7c 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -392,7 +392,7 @@ static int kfd_ioctl_create_queue(struct file *filep,
> struct
> kfd_process *p,
> }
> }
>
> - err = kfd_queue_acquire_buffers(pdd, &q_properties);
> + err = kfd_queue_acquire_buffers(pdd, &q_properties, false);
> if (err) {
> pr_debug("failed to acquire user queue buffers\n");
> goto err_acquire_queue_buf;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index d8631847f0eb..59267f8c5840 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -1377,7 +1377,7 @@ void print_queue(struct queue *q); int
> kfd_queue_buffer_get(struct amdgpu_vm *vm, void __user *addr, struct amdgpu_bo
> **pbo,
> u64 expected_size);
> void kfd_queue_buffer_put(struct amdgpu_bo **bo); -int
> kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct
> queue_properties
> *properties);
> +int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct
> +queue_properties *properties, bool is_criu);
> int kfd_queue_release_buffers(struct kfd_process_device *pdd, struct
> queue_properties *properties); void kfd_queue_unref_bo_va(struct amdgpu_vm
> *vm, struct amdgpu_bo **bo); int kfd_queue_unref_bo_vas(struct
> kfd_process_device *pdd, diff --git
> a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> index a15a826519ad..55569d555758 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> @@ -983,8 +983,14 @@ int kfd_criu_checkpoint_queues(struct kfd_process *p, }
>
> static void set_queue_properties_from_criu(struct queue_properties *qp,
> - struct kfd_criu_queue_priv_data
> *q_data,
> uint32_t num_xcc)
> + struct kfd_criu_queue_priv_data
> *q_data,
> + struct kfd_process_device *pdd)
> {
> + struct kfd_topology_device *topo_dev;
> + uint32_t num_xcc = NUM_XCC(pdd->dev->xcc_mask);
> +
> + topo_dev = kfd_topology_device_by_id(pdd->dev->id);
> +
> qp->is_interop = false;
> qp->queue_percent = q_data->q_percent;
> qp->priority = q_data->priority;
> @@ -996,10 +1002,14 @@ static void set_queue_properties_from_criu(struct
> queue_properties *qp,
> qp->eop_ring_buffer_size = q_data->eop_ring_buffer_size;
> qp->ctx_save_restore_area_address = q_data-
> >ctx_save_restore_area_address;
> qp->ctx_save_restore_area_size = q_data->ctx_save_restore_area_size;
> - if (q_data->type == KFD_QUEUE_TYPE_COMPUTE)
> - qp->ctl_stack_size = q_data->ctl_stack_size / num_xcc;
> - else
> + if (q_data->type == KFD_QUEUE_TYPE_COMPUTE) {
> + if (q_data->ctl_stack_size)
> + qp->ctl_stack_size = q_data->ctl_stack_size / num_xcc;
> + else if (topo_dev)
> + qp->ctl_stack_size =
> topo_dev->node_props.ctl_stack_size;
> + } else {
> qp->ctl_stack_size = q_data->ctl_stack_size;
> + }
>
> qp->type = q_data->type;
> qp->format = q_data->format;
> @@ -1078,13 +1088,27 @@ int kfd_criu_restore_queue(struct kfd_process *p,
> ctl_stack = mqd + q_data->mqd_size;
>
> memset(&qp, 0, sizeof(qp));
> - set_queue_properties_from_criu(&qp, q_data, NUM_XCC(pdd->dev-
> >xcc_mask));
> + set_queue_properties_from_criu(&qp, q_data, pdd);
> +
> + if (q_data->type == KFD_QUEUE_TYPE_COMPUTE &&
> !qp.ctl_stack_size) {
> + pr_err("Failed to determine ctl_stack_size for compute
> queue\n");
> + ret = -EINVAL;
> + goto exit;
> + }
I think that if q_data->ctl_stack_size is zero when kfd_criu_restore_queue is
called, this should be an error because that means this value was not stored
correctly during checkpoint.
So the qp.ctl_stack_size check should be at the beginning of
kfd_criu_restore_queue function and do an early exit instead.
Remaining changes look ok.
>
> print_queue_properties(&qp);
>
> + ret = kfd_queue_acquire_buffers(pdd, &qp, true);
> + if (ret) {
> + pr_err("Failed to acquire user queue buffers err:%d\n", ret);
> + goto exit;
> + }
> +
> ret = pqm_create_queue(&p->pqm, pdd->dev, &qp, &queue_id, q_data, mqd,
> ctl_stack, NULL);
> if (ret) {
> pr_err("Failed to create new queue err:%d\n", ret);
> + kfd_queue_unref_bo_vas(pdd, &qp);
> + kfd_queue_release_buffers(pdd, &qp);
> goto exit;
> }
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
> index ae3e7c54014a..0e8833716381 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
> @@ -230,7 +230,7 @@ void kfd_queue_buffer_put(struct amdgpu_bo **bo)
> amdgpu_bo_unref(bo);
> }
>
> -int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct
> queue_properties *properties)
> +int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct
> +queue_properties *properties, bool is_criu)
> {
> struct kfd_topology_device *topo_dev;
> u64 expected_queue_size;
> @@ -325,6 +325,12 @@ int kfd_queue_acquire_buffers(struct kfd_process_device
> *pdd, struct queue_prope
>
> amdgpu_bo_unreserve(vm->root.bo);
>
> + /* CRIU svm buffers are restored at a later stage than queues. They'll
> be in
> + * before the queue is actually used, so no risk here
> + */
> + if (is_criu)
> + return 0;
> +
> err = kfd_queue_buffer_svm_get(pdd, properties-
> >ctx_save_restore_area_address,
> total_cwsr_size);
> if (err)
> --
> 2.34.1
