linux-kernel - Re: [PATCH v2] drm/msm: Fix objtool warning in submit_lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CACSVV03b+tAN4o9kFFaNVJrcO6OgaCSmajL-LpvCd_wDzWPSBQ@mail.gmail.com>
Date: Thu, 7 Aug 2025 09:34:09 -0700
From: Rob Clark <rob.clark@....qualcomm.com>
To: Sasha Levin <sashal@...nel.org>
Cc: lumag@...nel.org, abhinav.kumar@...ux.dev, jessica.zhang@....qualcomm.com,
        sean@...rly.run, marijn.suijten@...ainline.org, airlied@...il.com,
        simona@...ll.ch, antomani103@...il.com, linux-arm-msm@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/msm: Fix objtool warning in submit_lock_objects()

On Thu, Aug 7, 2025 at 6:11 AM Sasha Levin <sashal@...nel.org> wrote:
>
> Split the vmbind case into a separate helper function
> submit_lock_objects_vmbind() to fix objtool warning:
>
>   drivers/gpu/drm/msm/msm.o: warning: objtool: submit_lock_objects+0x451:
>   sibling call from callable instruction with modified stack frame
>
> The drm_exec_until_all_locked() macro uses computed gotos internally
> for its retry loop. Having return statements inside this macro, or
> immediately after it in certain code paths, confuses objtool's static
> analysis of stack frames, causing it to incorrectly flag tail call
> optimizations.
>
> Fixes: 92395af63a99 ("drm/msm: Add VM_BIND submitqueue")
> Signed-off-by: Sasha Levin <sashal@...nel.org>

Thanks, I've pushed this to my staging branch for msm-fixes

BR,
-R

> ---
>
> Changes since v1:
>  - Extract helper submit_lock_objects_vmbind() instead of refactoring
>    single loop
>
>  drivers/gpu/drm/msm/msm_gem_submit.c | 49 +++++++++++++++-------------
>  1 file changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
> index 5f8e939a5906..1ce90e351b7a 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -271,32 +271,37 @@ static int submit_lookup_cmds(struct msm_gem_submit *submit,
>         return ret;
>  }
>
> -/* This is where we make sure all the bo's are reserved and pin'd: */
> -static int submit_lock_objects(struct msm_gem_submit *submit)
> +static int submit_lock_objects_vmbind(struct msm_gem_submit *submit)
>  {
> -       unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT;
> +       unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT | DRM_EXEC_IGNORE_DUPLICATES;
>         struct drm_exec *exec = &submit->exec;
> -       int ret;
> +       int ret = 0;
>
> -       if (msm_context_is_vmbind(submit->queue->ctx)) {
> -               flags |= DRM_EXEC_IGNORE_DUPLICATES;
> +       drm_exec_init(&submit->exec, flags, submit->nr_bos);
>
> -               drm_exec_init(&submit->exec, flags, submit->nr_bos);
> +       drm_exec_until_all_locked (&submit->exec) {
> +               ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
> +               drm_exec_retry_on_contention(exec);
> +               if (ret)
> +                       break;
>
> -               drm_exec_until_all_locked (&submit->exec) {
> -                       ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
> -                       drm_exec_retry_on_contention(exec);
> -                       if (ret)
> -                               return ret;
> +               ret = drm_gpuvm_prepare_objects(submit->vm, exec, 1);
> +               drm_exec_retry_on_contention(exec);
> +               if (ret)
> +                       break;
> +       }
>
> -                       ret = drm_gpuvm_prepare_objects(submit->vm, exec, 1);
> -                       drm_exec_retry_on_contention(exec);
> -                       if (ret)
> -                               return ret;
> -               }
> +       return ret;
> +}
>
> -               return 0;
> -       }
> +/* This is where we make sure all the bo's are reserved and pin'd: */
> +static int submit_lock_objects(struct msm_gem_submit *submit)
> +{
> +       unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT;
> +       int ret = 0;
> +
> +       if (msm_context_is_vmbind(submit->queue->ctx))
> +               return submit_lock_objects_vmbind(submit);
>
>         drm_exec_init(&submit->exec, flags, submit->nr_bos);
>
> @@ -305,17 +310,17 @@ static int submit_lock_objects(struct msm_gem_submit *submit)
>                                         drm_gpuvm_resv_obj(submit->vm));
>                 drm_exec_retry_on_contention(&submit->exec);
>                 if (ret)
> -                       return ret;
> +                       break;
>                 for (unsigned i = 0; i < submit->nr_bos; i++) {
>                         struct drm_gem_object *obj = submit->bos[i].obj;
>                         ret = drm_exec_prepare_obj(&submit->exec, obj, 1);
>                         drm_exec_retry_on_contention(&submit->exec);
>                         if (ret)
> -                               return ret;
> +                               break;
>                 }
>         }
>
> -       return 0;
> +       return ret;
>  }
>
>  static int submit_fence_sync(struct msm_gem_submit *submit)
> --
> 2.39.5
>