Re: [PATCH] drm/amdgpu/userq: refactor MQD init into per-IP helpers

["Christian König" <christian.koenig@amd.com>]

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,