linux-kernel - Re: [PATCH 1/7] drm/msm: Fix bv_fence being used as bv

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACu1E7H6g=8thZfoRh8-svjqhdTOPg5diKoj+ENa4F5==d5RxA@mail.gmail.com>
Date: Tue, 20 Aug 2024 11:45:13 +0100
From: Connor Abbott <cwabbott0@...il.com>
To: Konrad Dybcio <konradybcio@...nel.org>
Cc: Antonino Maniscalco <antomani103@...il.com>, Rob Clark <robdclark@...il.com>, 
	Sean Paul <sean@...rly.run>, Abhinav Kumar <quic_abhinavk@...cinc.com>, 
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, 
	Marijn Suijten <marijn.suijten@...ainline.org>, David Airlie <airlied@...il.com>, 
	Daniel Vetter <daniel@...ll.ch>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, linux-arm-msm@...r.kernel.org, 
	dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/7] drm/msm: Fix bv_fence being used as bv_rptr

On Tue, Aug 20, 2024 at 11:15 AM Konrad Dybcio <konradybcio@...nel.org> wrote:
>
> On 15.08.2024 8:26 PM, Antonino Maniscalco wrote:
> > The bv_fence field of rbmemptrs was being used incorrectly as the BV
> > rptr shadow pointer in some places.
> >
> > Add a bv_rptr field and change the code to use that instead.
> >
> > Signed-off-by: Antonino Maniscalco <antomani103@...il.com>
> > ---
> >  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
> >  drivers/gpu/drm/msm/msm_ringbuffer.h  | 1 +
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > index bcaec86ac67a..32a4faa93d7f 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > @@ -1132,7 +1132,7 @@ static int hw_init(struct msm_gpu *gpu)
> >       /* ..which means "always" on A7xx, also for BV shadow */
> >       if (adreno_is_a7xx(adreno_gpu)) {
> >               gpu_write64(gpu, REG_A7XX_CP_BV_RB_RPTR_ADDR,
> > -                         rbmemptr(gpu->rb[0], bv_fence));
> > +                         rbmemptr(gpu->rb[0], bv_rptr));
> >       }
> >
> >       /* Always come up on rb 0 */
> > diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.h b/drivers/gpu/drm/msm/msm_ringbuffer.h
> > index 0d6beb8cd39a..40791b2ade46 100644
> > --- a/drivers/gpu/drm/msm/msm_ringbuffer.h
> > +++ b/drivers/gpu/drm/msm/msm_ringbuffer.h
> > @@ -31,6 +31,7 @@ struct msm_rbmemptrs {
> >       volatile uint32_t rptr;
> >       volatile uint32_t fence;
> >       /* Introduced on A7xx */
> > +     volatile uint32_t bv_rptr;
>
> This is never initialized or assigned any value, no?
>
> Konrad

Neither is the original (retroactively BR) shadow RPTR, except
apparently on suspend (no idea why). It's written by the GPU as it
reads the ringbuffer, because CP_BV_RPTR_ADDR is set to its address.
For the BV shadow RPTR, we aren't really using it for anything (and
neither is kgsl) so we just need to point the register to a valid
"dummy" address that isn't used by anything else.

Connor