* [PATCH] drm/amdgpu/userq: refactor MQD init into per-IP helpers
@ 2026-03-17 3:14 Junrui Luo
2026-03-17 7:11 ` Christian König
0 siblings, 1 reply; 4+ messages in thread
From: Junrui Luo @ 2026-03-17 3:14 UTC (permalink / raw)
To: Alex Deucher, Christian König, David Airlie, Simona Vetter,
Liang, Prike
Cc: amd-gfx, dri-devel, linux-kernel, Markus Elfring, Junrui Luo
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.
Link: https://lore.kernel.org/all/SYBPR01MB7881A279A361F81B670CDEEAAF42A@SYBPR01MB7881.ausprd01.prod.outlook.com/
Suggested-by: Markus Elfring <Markus.Elfring@web.de>
Suggested-by: Prike Liang <Prike.Liang@amd.com>
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
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);
+
+ 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);
+ 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,
--
Junrui Luo <moonafterrain@outlook.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/amdgpu/userq: refactor MQD init into per-IP helpers
2026-03-17 3:14 [PATCH] drm/amdgpu/userq: refactor MQD init into per-IP helpers Junrui Luo
@ 2026-03-17 7:11 ` Christian König
2026-03-17 13:42 ` Liang, Prike
0 siblings, 1 reply; 4+ messages in thread
From: Christian König @ 2026-03-17 7:11 UTC (permalink / raw)
To: Junrui Luo, Alex Deucher, David Airlie, Simona Vetter, Liang, Prike
Cc: amd-gfx, dri-devel, linux-kernel, Markus Elfring
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@SYBPR01MB7881.ausprd01.prod.outlook.com/
> Suggested-by: Markus Elfring <Markus.Elfring@web.de>
> Suggested-by: Prike Liang <Prike.Liang@amd.com>
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
> ---
> 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);
> +
> + 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);
> + 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,
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] drm/amdgpu/userq: refactor MQD init into per-IP helpers
2026-03-17 7:11 ` Christian König
@ 2026-03-17 13:42 ` Liang, Prike
2026-03-17 13:57 ` Markus Elfring
0 siblings, 1 reply; 4+ messages in thread
From: Liang, Prike @ 2026-03-17 13:42 UTC (permalink / raw)
To: Koenig, Christian, Junrui Luo, Deucher, Alexander, David Airlie,
Simona Vetter
Cc: amd-gfx, dri-devel, linux-kernel, Markus Elfring
[Public]
Regards,
Prike
> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@amd.com>
> Sent: Tuesday, March 17, 2026 3:11 PM
> To: Junrui Luo <moonafterrain@outlook.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; David Airlie <airlied@gmail.com>; Simona Vetter
> <simona@ffwll.ch>; Liang, Prike <Prike.Liang@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-
> kernel@vger.kernel.org; Markus Elfring <Markus.Elfring@web.de>
> 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 <Markus.Elfring@web.de>
> > Suggested-by: Prike Liang <Prike.Liang@amd.com>
> > Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
> > ---
> > 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,
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/amdgpu/userq: refactor MQD init into per-IP helpers
2026-03-17 13:42 ` Liang, Prike
@ 2026-03-17 13:57 ` Markus Elfring
0 siblings, 0 replies; 4+ messages in thread
From: Markus Elfring @ 2026-03-17 13:57 UTC (permalink / raw)
To: Prike Liang, Junrui Luo, amd-gfx, dri-devel
Cc: LKML, Alex Deucher, Christian König, David Airlie,
Simona Vetter, Yuhao Jiang
> We may need to clear mqd before returning on error to guarantee kfree() won’t see an error pointer at scope exit.
Would you like to take another look at a corresponding implementation detail?
https://elixir.bootlin.com/linux/v7.0-rc4/source/include/linux/slab.h#L517
Regards,
Markus
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-03-17 13:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-17 3:14 [PATCH] drm/amdgpu/userq: refactor MQD init into per-IP helpers Junrui Luo
2026-03-17 7:11 ` Christian König
2026-03-17 13:42 ` Liang, Prike
2026-03-17 13:57 ` Markus Elfring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®