linux-kernel - Re: [PATCH 1/7] drm/msm: Add missing "location"s to devcoredump

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: <CACSVV02qk59riW4_UAZjd=NTsSLF7qsQW6hkYEz7JcttBJDWTw@mail.gmail.com>
Date: Thu, 31 Jul 2025 13:31:49 -0700
From: Rob Clark <rob.clark@....qualcomm.com>
To: Connor Abbott <cwabbott0@...il.com>
Cc: dri-devel@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
        freedreno@...ts.freedesktop.org,
        Akhil P Oommen <akhilpo@....qualcomm.com>, Sean Paul <sean@...rly.run>,
        Konrad Dybcio <konradybcio@...nel.org>,
        Dmitry Baryshkov <lumag@...nel.org>,
        Abhinav Kumar <abhinav.kumar@...ux.dev>,
        Jessica Zhang <jessica.zhang@....qualcomm.com>,
        Marijn Suijten <marijn.suijten@...ainline.org>,
        David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/7] drm/msm: Add missing "location"s to devcoredump

On Thu, Jul 31, 2025 at 12:16 PM Connor Abbott <cwabbott0@...il.com> wrote:
>
> On Tue, Jul 29, 2025 at 9:40 AM Rob Clark <rob.clark@....qualcomm.com> wrote:
> >
> > On Mon, Jul 28, 2025 at 3:15 PM Rob Clark <rob.clark@....qualcomm.com> wrote:
> > >
> > > On Mon, Jul 28, 2025 at 2:04 PM Connor Abbott <cwabbott0@...il.com> wrote:
> > > >
> > > > On Mon, Jul 28, 2025 at 4:43 PM Rob Clark <robin.clark@....qualcomm.com> wrote:
> > > > >
> > > > > This is needed to properly interpret some of the sections.
> > > > >
> > > > > Signed-off-by: Rob Clark <robin.clark@....qualcomm.com>
> > > > > ---
> > > > >  drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> > > > > index faca2a0243ab..e586577e90de 100644
> > > > > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> > > > > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> > > > > @@ -1796,6 +1796,7 @@ static void a7xx_show_shader(struct a6xx_gpu_state_obj *obj,
> > > > >
> > > > >         print_name(p, "  - type: ", a7xx_statetype_names[block->statetype]);
> > > > >         print_name(p, "    - pipe: ", a7xx_pipe_names[block->pipeid]);
> > > > > +       drm_printf(p, "    - location: %d", block->location);
> > > >
> > > > We should probably at least try to keep it proper YAML by indenting
> > > > everything after another level...
> > >
> > > this made me realize I missed a \n... but otherwise I think the indent
> > > is correct?  Or should location not have a leading '-'?
> >
> > beyond that, even without the added location field, some random online
> > yaml checker is telling me that we were already not proper yaml.. so I
> > guess, :shrug:?
> >
> > BR,
> > -R
>
> Before this change, it looked like this:
>
>   - pipe: A7XX_PIPE_BR
>     - cluster-name: A7XX_CLUSTER_SP_PS
>       - context: 3
>         - { offset: 0x02a718, value: 0x00000003 }
>         ...
>
> Notice that each nested thing (pipe -> cluster -> context) has an
> additional level of indentation. Now, it looks like this:
>
>   - pipe: A7XX_PIPE_BR
>     - cluster-name: A7XX_CLUSTER_SP_PS
>       - context: 3
>       - location: 4
>         - { offset: 0x02a718, value: 0x00000003 }
>         ...
>
> So it looks a bit weird with the context and location not being
> nested. Also, I think the correct nesting HW-wise is cluster ->
> location -> context, rather than context-> location, so the location
> should be first. But ultimately it's up to you.

In terms of nesting, type, pipe, and location are all at the same
level, and then for that tuple there is SPs nested under that, and
then USPTPs nested under the SPs.  Although I guess we already had
pipe nested under type..

BR,
-R