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
| ||
|
Message-ID: <CAF6AEGut-75ri+U=B2eBtNeYQu5ECKPmk51b2_pCgu91uKy1ow@mail.gmail.com> Date: Wed, 23 Feb 2022 07:46:21 -0800 From: Rob Clark <robdclark@...il.com> To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org> Cc: dri-devel <dri-devel@...ts.freedesktop.org>, freedreno <freedreno@...ts.freedesktop.org>, linux-arm-msm <linux-arm-msm@...r.kernel.org>, Rob Clark <robdclark@...omium.org>, Linux Kernel Functional Testing <lkft@...aro.org>, Anders Roxell <anders.roxell@...aro.org>, Sean Paul <sean@...rly.run>, Abhinav Kumar <quic_abhinavk@...cinc.com>, David Airlie <airlied@...ux.ie>, Daniel Vetter <daniel@...ll.ch>, open list <linux-kernel@...r.kernel.org> Subject: Re: [PATCH] drm/msm/gpu: Fix crash on devices without devfreq support On Tue, Feb 22, 2022 at 7:11 PM Dmitry Baryshkov <dmitry.baryshkov@...aro.org> wrote: > > On 19/02/2022 21:33, Rob Clark wrote: > > From: Rob Clark <robdclark@...omium.org> > > > > Avoid going down devfreq paths on devices where devfreq is not > > initialized. > > > > Reported-by: Linux Kernel Functional Testing <lkft@...aro.org> > > Reported-by: Anders Roxell <anders.roxell@...aro.org> > > Signed-off-by: Rob Clark <robdclark@...omium.org> > > --- > > drivers/gpu/drm/msm/msm_gpu_devfreq.c | 31 +++++++++++++++++++++------ > > 1 file changed, 25 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c b/drivers/gpu/drm/msm/msm_gpu_devfreq.c > > index 9bf319be11f6..26a3669a97b3 100644 > > --- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c > > +++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c > > @@ -83,12 +83,17 @@ static struct devfreq_dev_profile msm_devfreq_profile = { > > static void msm_devfreq_boost_work(struct kthread_work *work); > > static void msm_devfreq_idle_work(struct kthread_work *work); > > > > +static bool has_devfreq(struct msm_gpu *gpu) > > +{ > > + return !!gpu->funcs->gpu_busy; > > I see that devfreq init will be skipped if gpu_busy is NULL. > Can we use gpu->devfreq instead of this condition? We could, but then we couldn't also use the same has_devfreq() helper in msm_devfreq_init(). I thought it was clearer to use the same helper everywhere. > I noticed that you have replaced some of gpu->devfreq checks with > has_devreq() calls. Is there any difference? It amounts to the same thing because if you don't have gpu_busy, then devfreq is never initialized. I just thought it clearer to use the same check in all places. BR, -R > > +} > > + > > void msm_devfreq_init(struct msm_gpu *gpu) > > { > > struct msm_gpu_devfreq *df = &gpu->devfreq; > > > > /* We need target support to do devfreq */ > > - if (!gpu->funcs->gpu_busy) > > + if (!has_devfreq(gpu)) > > return; > > > > dev_pm_qos_add_request(&gpu->pdev->dev, &df->idle_freq, > > @@ -149,6 +154,9 @@ void msm_devfreq_cleanup(struct msm_gpu *gpu) > > { > > struct msm_gpu_devfreq *df = &gpu->devfreq; > > > > + if (!has_devfreq(gpu)) > > + return; > > + > > devfreq_cooling_unregister(gpu->cooling); > > dev_pm_qos_remove_request(&df->boost_freq); > > dev_pm_qos_remove_request(&df->idle_freq); > > @@ -156,16 +164,24 @@ void msm_devfreq_cleanup(struct msm_gpu *gpu) > > > > void msm_devfreq_resume(struct msm_gpu *gpu) > > { > > - gpu->devfreq.busy_cycles = 0; > > - gpu->devfreq.time = ktime_get(); > > + struct msm_gpu_devfreq *df = &gpu->devfreq; > > > > - devfreq_resume_device(gpu->devfreq.devfreq); > > + if (!has_devfreq(gpu)) > > + return; > > + > > + df->busy_cycles = 0; > > + df->time = ktime_get(); > > + > > + devfreq_resume_device(df->devfreq); > > } > > > > void msm_devfreq_suspend(struct msm_gpu *gpu) > > { > > struct msm_gpu_devfreq *df = &gpu->devfreq; > > > > + if (!has_devfreq(gpu)) > > + return; > > + > > devfreq_suspend_device(df->devfreq); > > > > cancel_idle_work(df); > > @@ -185,6 +201,9 @@ void msm_devfreq_boost(struct msm_gpu *gpu, unsigned factor) > > struct msm_gpu_devfreq *df = &gpu->devfreq; > > uint64_t freq; > > > > + if (!has_devfreq(gpu)) > > + return; > > + > > freq = get_freq(gpu); > > freq *= factor; > > > > @@ -207,7 +226,7 @@ void msm_devfreq_active(struct msm_gpu *gpu) > > struct devfreq_dev_status status; > > unsigned int idle_time; > > > > - if (!df->devfreq) > > + if (!has_devfreq(gpu)) > > return; > > > > /* > > @@ -253,7 +272,7 @@ void msm_devfreq_idle(struct msm_gpu *gpu) > > { > > struct msm_gpu_devfreq *df = &gpu->devfreq; > > > > - if (!df->devfreq) > > + if (!has_devfreq(gpu)) > > return; > > > > msm_hrtimer_queue_work(&df->idle_work, ms_to_ktime(1), > > > -- > With best wishes > Dmitry
Powered by blists - more mailing lists