From: Akhil P Oommen <akhilpo@codeaurora.org>
To: Rob Clark <robdclark@gmail.com>
Cc: "Rob Clark" <robdclark@chromium.org>,
freedreno <freedreno@lists.freedesktop.org>,
"David Airlie" <airlied@linux.ie>,
linux-arm-msm <linux-arm-msm@vger.kernel.org>,
"open list" <linux-kernel@vger.kernel.org>,
dri-devel <dri-devel@lists.freedesktop.org>,
"moderated list:DMA BUFFER SHARING FRAMEWORK"
<linaro-mm-sig@lists.linaro.org>, "Sean Paul" <sean@poorly.run>,
"Christian König" <christian.koenig@amd.com>,
"open list:DMA BUFFER SHARING FRAMEWORK"
<linux-media@vger.kernel.org>
Subject: Re: [PATCH 2/2] drm/msm: Restore error return on invalid fence
Date: Tue, 16 Nov 2021 11:28:57 +0530 [thread overview]
Message-ID: <32e0798f-37b5-e255-7e1d-fab3af4858b0@codeaurora.org> (raw)
In-Reply-To: <CAF6AEGvMf2pQO9LmUanOGzWgU34=gO3ZPPH=6dea=AvfavWTVg@mail.gmail.com>
On 11/15/2021 10:26 PM, Rob Clark wrote:
> On Mon, Nov 15, 2021 at 6:43 AM Akhil P Oommen <akhilpo@codeaurora.org> wrote:
>>
>> On 11/12/2021 12:54 AM, Rob Clark wrote:
>>> From: Rob Clark <robdclark@chromium.org>
>>>
>>> When converting to use an idr to map userspace fence seqno values back
>>> to a dma_fence, we lost the error return when userspace passes seqno
>>> that is larger than the last submitted fence. Restore this check.
>>>
>>> Reported-by: Akhil P Oommen <akhilpo@codeaurora.org>
>>> Fixes: a61acbbe9cf8 ("drm/msm: Track "seqno" fences by idr")
>>> Signed-off-by: Rob Clark <robdclark@chromium.org>
>>> ---
>>> Note: I will rebase "drm/msm: Handle fence rollover" on top of this,
>>> to simplify backporting this patch to stable kernels
>>>
>>> drivers/gpu/drm/msm/msm_drv.c | 6 ++++++
>>> drivers/gpu/drm/msm/msm_gem_submit.c | 1 +
>>> drivers/gpu/drm/msm/msm_gpu.h | 3 +++
>>> 3 files changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
>>> index cb14d997c174..56500eb5219e 100644
>>> --- a/drivers/gpu/drm/msm/msm_drv.c
>>> +++ b/drivers/gpu/drm/msm/msm_drv.c
>>> @@ -967,6 +967,12 @@ static int wait_fence(struct msm_gpu_submitqueue *queue, uint32_t fence_id,
>>> struct dma_fence *fence;
>>> int ret;
>>>
>>> + if (fence_id > queue->last_fence) {
>>
>> But fence_id can wrap around and then this check won't be valid.
>
> that is correct, but see my note about rebasing "drm/msm: Handle fence
> rollover" on top of this patch, so this patch could be more easily
> cherry-picked to stable/lts branches
>
> BR,
> -R
Missed that. Thanks.
Reviewed-by: Akhil P Oommen <akhilpo@codeaurora.org>
-Akhil.
>
>> -Akhil.
>>
>>> + DRM_ERROR_RATELIMITED("waiting on invalid fence: %u (of %u)\n",
>>> + fence_id, queue->last_fence);
>>> + return -EINVAL;
>>> + }
>>> +
>>> /*
>>> * Map submitqueue scoped "seqno" (which is actually an idr key)
>>> * back to underlying dma-fence
>>> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
>>> index 151d19e4453c..a38f23be497d 100644
>>> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
>>> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
>>> @@ -911,6 +911,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
>>> drm_sched_entity_push_job(&submit->base, queue->entity);
>>>
>>> args->fence = submit->fence_id;
>>> + queue->last_fence = submit->fence_id;
>>>
>>> msm_reset_syncobjs(syncobjs_to_reset, args->nr_in_syncobjs);
>>> msm_process_post_deps(post_deps, args->nr_out_syncobjs,
>>> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
>>> index bd4e0024033e..e73a5bb03544 100644
>>> --- a/drivers/gpu/drm/msm/msm_gpu.h
>>> +++ b/drivers/gpu/drm/msm/msm_gpu.h
>>> @@ -376,6 +376,8 @@ static inline int msm_gpu_convert_priority(struct msm_gpu *gpu, int prio,
>>> * @ring_nr: the ringbuffer used by this submitqueue, which is determined
>>> * by the submitqueue's priority
>>> * @faults: the number of GPU hangs associated with this submitqueue
>>> + * @last_fence: the sequence number of the last allocated fence (for error
>>> + * checking)
>>> * @ctx: the per-drm_file context associated with the submitqueue (ie.
>>> * which set of pgtables do submits jobs associated with the
>>> * submitqueue use)
>>> @@ -391,6 +393,7 @@ struct msm_gpu_submitqueue {
>>> u32 flags;
>>> u32 ring_nr;
>>> int faults;
>>> + uint32_t last_fence;
>>> struct msm_file_private *ctx;
>>> struct list_head node;
>>> struct idr fence_idr;
>>>
>>
prev parent reply other threads:[~2021-11-16 6:00 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-11 19:24 [PATCH 0/2] drm/msm: wait_fence fixes Rob Clark
2021-11-11 19:24 ` [PATCH 1/2] drm/msm: Fix wait_fence submitqueue leak Rob Clark
2021-11-11 19:24 ` [PATCH 2/2] drm/msm: Restore error return on invalid fence Rob Clark
2021-11-15 14:43 ` Akhil P Oommen
2021-11-15 16:56 ` Rob Clark
2021-11-16 5:58 ` Akhil P Oommen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=32e0798f-37b5-e255-7e1d-fab3af4858b0@codeaurora.org \
--to=akhilpo@codeaurora.org \
--cc=airlied@linux.ie \
--cc=christian.koenig@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=linaro-mm-sig@lists.linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=robdclark@chromium.org \
--cc=robdclark@gmail.com \
--cc=sean@poorly.run \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).