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/sybpr01mb7881a279a361f81b670cdeeaaf...@sybpr01mb7881.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); > + > + 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,
