[AMD Official Use Only - AMD Internal Distribution Only]
> -----Original Message-----
> From: amd-gfx <[email protected]> On Behalf Of Alex
> Deucher
> Sent: Wednesday, January 28, 2026 6:35 AM
> To: Zhang, Jesse(Jie) <[email protected]>
> Cc: [email protected]; Deucher, Alexander
> <[email protected]>; Koenig, Christian <[email protected]>
> Subject: Re: [PATCH v3 6/6] drm/amdgpu: add MODIFY operation for compute
> queues
>
> On Tue, Jan 27, 2026 at 2:57 AM Jesse.Zhang <[email protected]> wrote:
> >
> > Implement the AMDGPU_USERQ_OP_MODIFY ioctl operation to enable runtime
> > updates of compute queues.
> >
> > Suggested-by: Alex Deucher <[email protected]>
> > Signed-off-by: Jesse Zhang <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 78
> > +++++++++++++++++++++++ drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h |
> > 3 +
> > 2 files changed, 81 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > index 256ceca6d429..eb1e27c27bde 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > @@ -837,6 +837,7 @@ static int amdgpu_userq_input_args_validate(struct
> > drm_device *dev,
> >
> > switch (args->in.op) {
> > case AMDGPU_USERQ_OP_CREATE:
> > + case AMDGPU_USERQ_OP_MODIFY:
> > if (args->in.flags &
> ~(AMDGPU_USERQ_CREATE_FLAGS_QUEUE_PRIORITY_MASK |
> >
> AMDGPU_USERQ_CREATE_FLAGS_QUEUE_SECURE))
> > return -EINVAL; @@ -863,10 +864,29 @@ static
> > int amdgpu_userq_input_args_validate(struct drm_device *dev,
> > drm_file_err(filp, "invalidate userq queue va or
> > size\n");
> > return -EINVAL;
> > }
> > +
> > + if (!is_power_of_2(args->in.queue_size) &&
> > (args->in.queue_size !=
> 0)) {
> > + drm_file_err(filp, "Ring size must be a power of 2
> > or 0\n");
> > + return -EINVAL;
> > + }
>
> Why 0? Also this should probably be a separate patch as it's more of a fix
> than part
> of the modify change.
>
> > +
> > + if (args->in.queue_size > 0 && args->in.queue_size <
> AMDGPU_GPU_PAGE_SIZE) {
> > + args->in.queue_size = AMDGPU_GPU_PAGE_SIZE;
> > + drm_file_err(filp, "Size clamped to
> AMDGPU_GPU_PAGE_SIZE\n");
> > + }
>
> Why not return an error here? Also this check should probably also be part of
> another fix patch like above.
>
> > +
> > + if ((args->in.queue_va) &&
> > + (!access_ok((const void __user *) args->in.queue_va,
> > + sizeof(uint64_t)))) {
>
> This isn't a CPU virtual address, it's a GPU virtual address. It may not be
> a valid
> CPU virtual address.
>
Thanks Alex, will remove the check and separate the patch.
Thanks
Jesse
> > + drm_file_err(filp, "Can't access ring base
> > address\n");
> > + return -EFAULT;
> > + }
> > +
> > if (!args->in.wptr_va || !args->in.rptr_va) {
> > drm_file_err(filp, "invalidate userq queue rptr or
> > wptr\n");
> > return -EINVAL;
> > }
> > +
> > break;
> > case AMDGPU_USERQ_OP_FREE:
> > if (args->in.ip_type || @@ -901,6 +921,58 @@ bool
> > amdgpu_userq_enabled(struct drm_device *dev)
> > return false;
> > }
> >
> > +static int amdgpu_modify_queue(struct drm_file *filp, union
> > +drm_amdgpu_userq *args) {
> > + struct amdgpu_fpriv *fpriv = filp->driver_priv;
> > + struct amdgpu_userq_mgr *uq_mgr = &fpriv->userq_mgr;
> > + struct amdgpu_device *adev = uq_mgr->adev;
> > + struct amdgpu_usermode_queue *queue;
> > + const struct amdgpu_userq_funcs *userq_funcs;
> > + int r;
> > +
> > + mutex_lock(&uq_mgr->userq_mutex);
> > + queue = amdgpu_userq_find(uq_mgr, args->in.queue_id);
> > + if (!queue) {
> > + mutex_unlock(&uq_mgr->userq_mutex);
> > + return -EINVAL;
> > + }
> > +
> > + if (queue->queue_type != AMDGPU_HW_IP_COMPUTE)
> > + return -EOPNOTSUPP;
>
> We'll probably want to support this for non-compute queues as well for
> consistency.
>
> Alex
>
> > +
> > + userq_funcs = adev->userq_funcs[queue->queue_type];
> > +
> > + /*
> > + * Unmap the queue if it's mapped or preempted to ensure a clean
> > update.
> > + * If the queue is already unmapped or hung, we skip this step.
> > + */
> > + if (queue->state == AMDGPU_USERQ_STATE_MAPPED ||
> > + queue->state == AMDGPU_USERQ_STATE_PREEMPTED) {
> > + r = amdgpu_userq_unmap_helper(queue);
> > + if (r)
> > + return r;
> > + }
> > +
> > + r = userq_funcs->mqd_update(queue, &args->in);
> > + if (r)
> > + return r;
> > + /*
> > + * If the queue is considered active (has valid size, address, and
> percentage),
> > + * we attempt to map it. This effectively starts the queue or
> > restarts it
> > + * if it was previously running.
> > + */
> > + if (AMDGPU_USERQ_IS_ACTIVE(queue)) {
> > + r = amdgpu_userq_map_helper(queue);
> > + if (r)
> > + drm_file_err(uq_mgr->file, "Failed to remap queue
> > %llu after
> update\n",
> > + queue->doorbell_index);
> > + }
> > +
> > + mutex_unlock(&uq_mgr->userq_mutex);
> > +
> > + return r;
> > +}
> > +
> > int amdgpu_userq_ioctl(struct drm_device *dev, void *data,
> > struct drm_file *filp) { @@ -920,6 +992,12 @@
> > int amdgpu_userq_ioctl(struct drm_device *dev, void *data,
> > drm_file_err(filp, "Failed to create usermode
> > queue\n");
> > break;
> >
> > +
> > + case AMDGPU_USERQ_OP_MODIFY:
> > + r = amdgpu_modify_queue(filp, args);
> > + if (r)
> > + drm_file_err(filp, "Failed to modify usermode
> > queue\n");
> > + break;
> > case AMDGPU_USERQ_OP_FREE:
> > r = amdgpu_userq_destroy(filp, args->in.queue_id);
> > if (r)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> > index 833468b58603..7cd1ea94e368 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> > @@ -31,6 +31,9 @@
> > #define to_ev_fence(f) container_of(f, struct amdgpu_eviction_fence,
> > base) #define uq_mgr_to_fpriv(u) container_of(u, struct amdgpu_fpriv,
> > userq_mgr) #define work_to_uq_mgr(w, name) container_of(w, struct
> > amdgpu_userq_mgr, name)
> > +#define AMDGPU_USERQ_IS_ACTIVE(q) ((q)->userq_prop->queue_size > 0
> && \
> > + (q)->userq_prop->hqd_base_gpu_addr != 0 && \
> > + (q)->userq_prop->queue_percentage > 0)
> >
> > enum amdgpu_userq_state {
> > AMDGPU_USERQ_STATE_UNMAPPED = 0,
> > --
> > 2.49.0
> >
