On Tue, Jun 17, 2025 at 4:11 AM Prike Liang <prike.li...@amd.com> wrote:
>
> This will help on validating the userq input args, and
> rejecting for the invalidate userq request at the IOCTLs
> first place.
>
> Signed-off-by: Prike Liang <prike.li...@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 80 +++++++++++++++-------
> drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 14 ++--
> 2 files changed, 62 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 295e7186e156..f67969312c39 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -359,27 +359,10 @@ amdgpu_userq_create(struct drm_file *filp, union
> drm_amdgpu_userq *args)
> (args->in.flags &
> AMDGPU_USERQ_CREATE_FLAGS_QUEUE_PRIORITY_MASK) >>
> AMDGPU_USERQ_CREATE_FLAGS_QUEUE_PRIORITY_SHIFT;
>
> - /* Usermode queues are only supported for GFX IP as of now */
> - if (args->in.ip_type != AMDGPU_HW_IP_GFX &&
> - args->in.ip_type != AMDGPU_HW_IP_DMA &&
> - args->in.ip_type != AMDGPU_HW_IP_COMPUTE) {
> - drm_file_err(uq_mgr->file, "Usermode queue doesn't support IP
> type %u\n",
> - args->in.ip_type);
> - return -EINVAL;
> - }
> -
> r = amdgpu_userq_priority_permit(filp, priority);
> if (r)
> return r;
>
> - if ((args->in.flags & AMDGPU_USERQ_CREATE_FLAGS_QUEUE_SECURE) &&
> - (args->in.ip_type != AMDGPU_HW_IP_GFX) &&
> - (args->in.ip_type != AMDGPU_HW_IP_COMPUTE) &&
> - !amdgpu_is_tmz(adev)) {
> - drm_file_err(uq_mgr->file, "Secure only supported on
> GFX/Compute queues\n");
> - return -EINVAL;
> - }
> -
> r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
> if (r < 0) {
> drm_file_err(uq_mgr->file, "pm_runtime_get_sync() failed for
> userqueue create\n");
> @@ -485,22 +468,44 @@ amdgpu_userq_create(struct drm_file *filp, union
> drm_amdgpu_userq *args)
> return r;
> }
>
> -int amdgpu_userq_ioctl(struct drm_device *dev, void *data,
> - struct drm_file *filp)
> +static int amdgpu_userq_input_args_validate(struct drm_device *dev,
> + union drm_amdgpu_userq *args,
> + struct drm_file *filp)
> {
> - union drm_amdgpu_userq *args = data;
> - int r;
> + struct amdgpu_device *adev = drm_to_adev(dev);
>
> switch (args->in.op) {
> case AMDGPU_USERQ_OP_CREATE:
> if (args->in.flags &
> ~(AMDGPU_USERQ_CREATE_FLAGS_QUEUE_PRIORITY_MASK |
>
> AMDGPU_USERQ_CREATE_FLAGS_QUEUE_SECURE))
> return -EINVAL;
> - r = amdgpu_userq_create(filp, args);
> - if (r)
> - drm_file_err(filp, "Failed to create usermode
> queue\n");
> - break;
> + /* Usermode queues are only supported for GFX IP as of now */
> + if (args->in.ip_type != AMDGPU_HW_IP_GFX &&
> + args->in.ip_type != AMDGPU_HW_IP_DMA &&
> + args->in.ip_type != AMDGPU_HW_IP_COMPUTE) {
> + drm_file_err(filp, "Usermode queue doesn't support IP
> type %u\n",
> + args->in.ip_type);
> + return -EINVAL;
> + }
> +
> + if ((args->in.flags & AMDGPU_USERQ_CREATE_FLAGS_QUEUE_SECURE)
> &&
> + (args->in.ip_type != AMDGPU_HW_IP_GFX) &&
> + (args->in.ip_type != AMDGPU_HW_IP_COMPUTE) &&
> + !amdgpu_is_tmz(adev)) {
> + drm_file_err(filp, "Secure only supported on
> GFX/Compute queues\n");
> + return -EINVAL;
> + }
>
> + if (args->in.queue_va == AMDGPU_BO_INVALID_OFFSET ||
args->in.queue_va of 0 is also invalid.
> + args->in.queue_size == 0) {
> + drm_file_err(filp, "invalidate userq queue va or
> size\n");
> + return -EINVAL;
> + }
> + if (!args->in.wptr_va || !args->in.rptr_va) {
AMDGPU_BO_INVALID_OFFSET is also invalid for these pointers.
> + drm_file_err(filp, "invalidate userq queue rptr or
> wptr\n");
> + return -EINVAL;
> + }
> + break;
> case AMDGPU_USERQ_OP_FREE:
> if (args->in.ip_type ||
> args->in.doorbell_handle ||
> @@ -514,6 +519,31 @@ int amdgpu_userq_ioctl(struct drm_device *dev, void
> *data,
> args->in.mqd ||
> args->in.mqd_size)
> return -EINVAL;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +int amdgpu_userq_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *filp)
> +{
> + union drm_amdgpu_userq *args = data;
> + int r;
> +
> + if (amdgpu_userq_input_args_validate(dev, args, filp) < 0)
> + return -EINVAL;
> +
> + switch (args->in.op) {
> + case AMDGPU_USERQ_OP_CREATE:
> + r = amdgpu_userq_create(filp, args);
> + if (r)
> + drm_file_err(filp, "Failed to create usermode
> queue\n");
> + break;
> +
> + case AMDGPU_USERQ_OP_FREE:
> r = amdgpu_userq_destroy(filp, args->in.queue_id);
> if (r)
> drm_file_err(filp, "Failed to destroy usermode
> queue\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> index d6f50b13e2ba..39decc0b00f7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> @@ -208,6 +208,13 @@ static int mes_userq_mqd_create(struct amdgpu_userq_mgr
> *uq_mgr,
> struct amdgpu_mqd_prop *userq_props;
> int r;
>
> + /* validate the user va at early time to avoid the time cost on
> building memory resource*/
> + if (!mqd_user->wptr_va || !mqd_user->rptr_va ||
> + !mqd_user->queue_va || mqd_user->queue_size == 0) {
> + DRM_ERROR("Invalid MQD parameters for userqueue\n");
> + return -EINVAL;
> + }
I think we can drop this as you already validate these in
amdgpu_userq_input_args_validate().
Alex
> +
> /* Structure to initialize MQD for userqueue using generic MQD init
> function */
> userq_props = kzalloc(sizeof(struct amdgpu_mqd_prop), GFP_KERNEL);
> if (!userq_props) {
> @@ -215,13 +222,6 @@ static int mes_userq_mqd_create(struct amdgpu_userq_mgr
> *uq_mgr,
> return -ENOMEM;
> }
>
> - if (!mqd_user->wptr_va || !mqd_user->rptr_va ||
> - !mqd_user->queue_va || mqd_user->queue_size == 0) {
> - DRM_ERROR("Invalid MQD parameters for userqueue\n");
> - r = -EINVAL;
> - goto free_props;
> - }
> -
> r = amdgpu_userq_create_object(uq_mgr, &queue->mqd,
> mqd_hw_default->mqd_size);
> if (r) {
> DRM_ERROR("Failed to create MQD object for userqueue\n");
> --
> 2.34.1
>
