[Public]
Regards,
Prike
> -----Original Message-----
> From: Koenig, Christian <[email protected]>
> Sent: Tuesday, March 17, 2026 3:11 PM
> To: Junrui Luo <[email protected]>; Deucher, Alexander
> <[email protected]>; David Airlie <[email protected]>; Simona Vetter
> <[email protected]>; Liang, Prike <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; Markus Elfring <[email protected]>
> Subject: Re: [PATCH] drm/amdgpu/userq: refactor MQD init into per-IP helpers
>
> On 3/17/26 04:14, Junrui Luo wrote:
> > The three IP-type branches in mes_userq_mqd_create() share a repeated
> > pattern. Extract each branch into a dedicated helper function and
> > introduce mes_userq_mqd_read() to deduplicate the common memdup_user +
> > size check logic.
> >
> > Each helper uses __free(kfree) for cleanup of the memdup'd MQD struct.
>
> Please don't that just looks absolutely messy.
>
> __free(kfree) should only be used with kmalloc() and not like that.
>
> Regards,
> Christian.
>
> >
> > Link:
> >
> https://lore.kernel.org/all/SYBPR01MB7881A279A361F81B670CDEEAAF42A@SY
> B
> > PR01MB7881.ausprd01.prod.outlook.com/
> > Suggested-by: Markus Elfring <[email protected]>
> > Suggested-by: Prike Liang <[email protected]>
> > Signed-off-by: Junrui Luo <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 205
> > +++++++++++++++--------------
> > 1 file changed, 108 insertions(+), 97 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > index faac21ee5739..0d7ccecf7c1e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > @@ -272,6 +272,105 @@ static int mes_userq_detect_and_reset(struct
> amdgpu_device *adev,
> > return r;
> > }
> >
> > +static void *mes_userq_mqd_read(struct drm_amdgpu_userq_in *mqd_user,
> > + size_t size, const char *ip_name) {
> > + void *mqd;
> > +
> > + if (mqd_user->mqd_size != size || !mqd_user->mqd) {
> > + DRM_ERROR("Invalid %s MQD\n", ip_name);
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + mqd = memdup_user(u64_to_user_ptr(mqd_user->mqd), size);
> > + if (IS_ERR(mqd)) {
> > + DRM_ERROR("Failed to read %s user MQD\n", ip_name);
> > + return ERR_PTR(-ENOMEM);
> > + }
> > +
> > + return mqd;
> > +}
> > +
> > +static int mes_userq_mqd_init_compute(struct amdgpu_device *adev,
> > + struct amdgpu_usermode_queue *queue,
> > + struct drm_amdgpu_userq_in *mqd_user,
> > + struct amdgpu_mqd_prop *userq_props) {
> > + struct drm_amdgpu_userq_mqd_compute_gfx11 *mqd __free(kfree) =
> > + mes_userq_mqd_read(mqd_user, sizeof(*mqd), "compute");
> > + int r;
> > +
> > + if (IS_ERR(mqd))
> > + return PTR_ERR(mqd);
We may need to clear mqd before returning on error to guarantee kfree() won’t
see an error pointer at scope exit.
> > + r = amdgpu_userq_input_va_validate(adev, queue, mqd->eop_va, 2048);
> > + if (r)
> > + return r;
> > +
> > + userq_props->eop_gpu_addr = mqd->eop_va;
> > + userq_props->hqd_pipe_priority = AMDGPU_GFX_PIPE_PRIO_NORMAL;
> > + userq_props->hqd_queue_priority =
> AMDGPU_GFX_QUEUE_PRIORITY_MINIMUM;
> > + userq_props->hqd_active = false;
> > + userq_props->tmz_queue =
> > + mqd_user->flags &
> AMDGPU_USERQ_CREATE_FLAGS_QUEUE_SECURE;
> > + return 0;
> > +}
> > +
> > +static int mes_userq_mqd_init_gfx(struct amdgpu_device *adev,
> > + struct amdgpu_usermode_queue *queue,
> > + struct drm_amdgpu_userq_in *mqd_user,
> > + struct amdgpu_mqd_prop *userq_props) {
> > + struct drm_amdgpu_userq_mqd_gfx11 *mqd __free(kfree) =
> > + mes_userq_mqd_read(mqd_user, sizeof(*mqd), "GFX");
> > + struct amdgpu_gfx_shadow_info shadow_info;
> > + int r;
> > +
> > + if (IS_ERR(mqd))
> > + return PTR_ERR(mqd);
> > +
> > + if (adev->gfx.funcs->get_gfx_shadow_info)
> > + adev->gfx.funcs->get_gfx_shadow_info(adev, &shadow_info, true);
> > + else
> > + return -EINVAL;
> > +
> > + userq_props->shadow_addr = mqd->shadow_va;
> > + userq_props->csa_addr = mqd->csa_va;
> > + userq_props->tmz_queue =
> > + mqd_user->flags &
> AMDGPU_USERQ_CREATE_FLAGS_QUEUE_SECURE;
> > +
> > + r = amdgpu_userq_input_va_validate(adev, queue, mqd->shadow_va,
> > + shadow_info.shadow_size);
> > + if (r)
> > + return r;
> > +
> > + r = amdgpu_userq_input_va_validate(adev, queue, mqd->csa_va,
> > + shadow_info.csa_size);
It's better to validate the MQD before assigning the mqd to userq_props.
> > + if (r)
> > + return r;
> > + return 0;
> > +}
> > +
> > +static int mes_userq_mqd_init_sdma(struct amdgpu_device *adev,
> > + struct amdgpu_usermode_queue *queue,
> > + struct drm_amdgpu_userq_in *mqd_user,
> > + struct amdgpu_mqd_prop *userq_props) {
> > + struct drm_amdgpu_userq_mqd_sdma_gfx11 *mqd __free(kfree) =
> > + mes_userq_mqd_read(mqd_user, sizeof(*mqd), "SDMA");
> > + int r;
> > +
> > + if (IS_ERR(mqd))
> > + return PTR_ERR(mqd);
> > +
> > + r = amdgpu_userq_input_va_validate(adev, queue, mqd->csa_va, 32);
> > + if (r)
> > + return r;
> > +
> > + userq_props->csa_addr = mqd->csa_va;
> > + return 0;
> > +}
> > +
> > static int mes_userq_mqd_create(struct amdgpu_usermode_queue *queue,
> > struct drm_amdgpu_userq_in *args_in) { @@ -
> 306,104 +405,16 @@
> > static int mes_userq_mqd_create(struct amdgpu_usermode_queue *queue,
> > userq_props->doorbell_index = queue->doorbell_index;
> > userq_props->fence_address = queue->fence_drv->gpu_addr;
> >
> > - if (queue->queue_type == AMDGPU_HW_IP_COMPUTE) {
> > - struct drm_amdgpu_userq_mqd_compute_gfx11 *compute_mqd;
> > -
> > - if (mqd_user->mqd_size != sizeof(*compute_mqd)) {
> > - DRM_ERROR("Invalid compute IP MQD size\n");
> > - r = -EINVAL;
> > - goto free_mqd;
> > - }
> > -
> > - compute_mqd = memdup_user(u64_to_user_ptr(mqd_user->mqd),
> mqd_user->mqd_size);
> > - if (IS_ERR(compute_mqd)) {
> > - DRM_ERROR("Failed to read user MQD\n");
> > - r = -ENOMEM;
> > - goto free_mqd;
> > - }
> > -
> > - r = amdgpu_userq_input_va_validate(adev, queue, compute_mqd-
> >eop_va,
> > - 2048);
> > - if (r) {
> > - kfree(compute_mqd);
> > - goto free_mqd;
> > - }
> > -
> > - userq_props->eop_gpu_addr = compute_mqd->eop_va;
> > - userq_props->hqd_pipe_priority =
> AMDGPU_GFX_PIPE_PRIO_NORMAL;
> > - userq_props->hqd_queue_priority =
> AMDGPU_GFX_QUEUE_PRIORITY_MINIMUM;
> > - userq_props->hqd_active = false;
> > - userq_props->tmz_queue =
> > - mqd_user->flags &
> AMDGPU_USERQ_CREATE_FLAGS_QUEUE_SECURE;
> > - kfree(compute_mqd);
> > - } else if (queue->queue_type == AMDGPU_HW_IP_GFX) {
> > - struct drm_amdgpu_userq_mqd_gfx11 *mqd_gfx_v11;
> > - struct amdgpu_gfx_shadow_info shadow_info;
> > -
> > - if (adev->gfx.funcs->get_gfx_shadow_info) {
> > - adev->gfx.funcs->get_gfx_shadow_info(adev, &shadow_info,
> true);
> > - } else {
> > - r = -EINVAL;
> > - goto free_mqd;
> > - }
> > -
> > - if (mqd_user->mqd_size != sizeof(*mqd_gfx_v11) ||
> > !mqd_user->mqd)
> {
> > - DRM_ERROR("Invalid GFX MQD\n");
> > - r = -EINVAL;
> > - goto free_mqd;
> > - }
> > -
> > - mqd_gfx_v11 = memdup_user(u64_to_user_ptr(mqd_user->mqd),
> mqd_user->mqd_size);
> > - if (IS_ERR(mqd_gfx_v11)) {
> > - DRM_ERROR("Failed to read user MQD\n");
> > - r = -ENOMEM;
> > - goto free_mqd;
> > - }
> > -
> > - userq_props->shadow_addr = mqd_gfx_v11->shadow_va;
> > - userq_props->csa_addr = mqd_gfx_v11->csa_va;
> > - userq_props->tmz_queue =
> > - mqd_user->flags &
> AMDGPU_USERQ_CREATE_FLAGS_QUEUE_SECURE;
> > -
> > - r = amdgpu_userq_input_va_validate(adev, queue, mqd_gfx_v11-
> >shadow_va,
> > - shadow_info.shadow_size);
> > - if (r) {
> > - kfree(mqd_gfx_v11);
> > - goto free_mqd;
> > - }
> > - r = amdgpu_userq_input_va_validate(adev, queue, mqd_gfx_v11-
> >csa_va,
> > - shadow_info.csa_size);
> > - if (r) {
> > - kfree(mqd_gfx_v11);
> > - goto free_mqd;
> > - }
> > -
> > - kfree(mqd_gfx_v11);
> > - } else if (queue->queue_type == AMDGPU_HW_IP_DMA) {
> > - struct drm_amdgpu_userq_mqd_sdma_gfx11 *mqd_sdma_v11;
> > + if (queue->queue_type == AMDGPU_HW_IP_COMPUTE)
> > + r = mes_userq_mqd_init_compute(adev, queue, mqd_user,
> > + userq_props);
> > + else if (queue->queue_type == AMDGPU_HW_IP_GFX)
> > + r = mes_userq_mqd_init_gfx(adev, queue, mqd_user, userq_props);
> > + else if (queue->queue_type == AMDGPU_HW_IP_DMA)
> > + r = mes_userq_mqd_init_sdma(adev, queue, mqd_user,
> userq_props);
> >
> > - if (mqd_user->mqd_size != sizeof(*mqd_sdma_v11) || !mqd_user-
> >mqd) {
> > - DRM_ERROR("Invalid SDMA MQD\n");
> > - r = -EINVAL;
> > - goto free_mqd;
> > - }
> > -
> > - mqd_sdma_v11 = memdup_user(u64_to_user_ptr(mqd_user->mqd),
> mqd_user->mqd_size);
> > - if (IS_ERR(mqd_sdma_v11)) {
> > - DRM_ERROR("Failed to read sdma user MQD\n");
> > - r = -ENOMEM;
> > - goto free_mqd;
> > - }
> > - r = amdgpu_userq_input_va_validate(adev, queue, mqd_sdma_v11-
> >csa_va,
> > - 32);
> > - if (r) {
> > - kfree(mqd_sdma_v11);
> > - goto free_mqd;
> > - }
> > -
> > - userq_props->csa_addr = mqd_sdma_v11->csa_va;
> > - kfree(mqd_sdma_v11);
> > - }
> > + if (r)
> > + goto free_mqd;
> >
> > queue->userq_prop = userq_props;
> >
> >
> > ---
> > base-commit: 0079dcb07e98346c0722f376ae3436cd28a71fdd
> > change-id: 20260317-fixes-c80fd658c7d5
> >
> > Best regards,
