linux-kernel - Re: [PATCH 5/5] drm/dsc: Prevent negative BPG offsets from shadowing adjacent bitfields

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: <20221004223940.stfsyvubx7ecd3a3@SoMainline.org>
Date:   Wed, 5 Oct 2022 00:39:40 +0200
From:   Marijn Suijten <marijn.suijten@...ainline.org>
To:     Abhinav Kumar <quic_abhinavk@...cinc.com>
Cc:     phone-devel@...r.kernel.org, Rob Clark <robdclark@...il.com>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Vinod Koul <vkoul@...nel.org>,
        ~postmarketos/upstreaming@...ts.sr.ht,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...ainline.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Martin Botka <martin.botka@...ainline.org>,
        Jami Kettunen <jami.kettunen@...ainline.org>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>, Sean Paul <sean@...rly.run>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Javier Martinez Canillas <javierm@...hat.com>,
        Alex Deucher <alexander.deucher@....com>,
        Douglas Anderson <dianders@...omium.org>,
        Vladimir Lypak <vladimir.lypak@...il.com>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, freedreno@...ts.freedesktop.org,
        Lyude Paul <lyude@...hat.com>
Subject: Re: [PATCH 5/5] drm/dsc: Prevent negative BPG offsets from shadowing
 adjacent bitfields

On 2022-10-04 15:31:10, Abhinav Kumar wrote:
> 
> 
> On 10/4/2022 2:57 PM, Marijn Suijten wrote:
> > [..]
> > Alas, as explained in the cover letter I opted to perform the masking in
> > the PPS packing code as the DSC block code also reads these values, and
> > would suddenly write 6-bit intead of 8-bit values to the
> > DSC_RANGE_BPG_OFFSET registers.  Quick testing on the mentioned sdm845
> > platform shows no regressions, but I'm not sure if that's safe to rely
> > on?
> 
> I looked up the MDP_DSC_0_RANGE_BPG_OFFSET_* registers.
> They take only a 6-bit value according to the SW documentation ( bits 5:0 )
> 
> It was always expecting only a 6-bit value and not 8.
> 
> So this change is safe.

Ack, I think that implies I should make this change and move the masks
to the DSI driver?

> >> If you want to move to helper, other drivers need to be changed too to
> >> remove duplicate & 0x3f.
> > 
> > Sure, we only have to confirm whether those drivers also read back the
> > value(s) in rc_range_params, and expect / allow this to be 8 instead of
> > 6 bits.
> > 
> >> FWIW, this too has already been fixed in the latest downstream driver too.
> > 
> > What is this supposed to mean?  Is there a downstream DPU project that
> > has pending patches needing to be upstreamed?  Or is the downstream SDE,
> > techpack/display, or whatever it is called nowadays, slowly using more
> > DRM structs like drm_dsc_config and this drm_dsc_pps_payload_pack()
> > helper function as pointed out in an earlier mail?
> > 
> 
> No, what I meant was, the version of downstream driver based on which 
> the upstream DSC was made seems to be an older version. Downstream 
> drivers keep getting updated and we always keep trying to align with 
> upstream structs.
> 
> This is true not just for DSC but even other blocks.
> 
> So as part of that effort, we started using struct drm_dsc_config . That 
> change was made on newer chipsets. But the downstream SW on sdm845 based 
> on which the DSC was upstreamed seems like didnt have that. Hence all 
> this redundant math happened.
> 
> So this comment was more of a explanation about why this issue happened 
> even though latest downstream didnt have this issue.

Thanks, I understood most of that but wasn't aware these exact "issues"
were also addressed downstream (by i.e. also using the upstream
structs).

> > Offtopic: are SDE and DPU growing closer together, hopefully achieving
> > feature parity allowing the SDE project to be dropped in favour of a
> > fully upstreamed DPU driver for day-one out-of-the-box mainline support
> > for new SoCs (as long as work is published and on its way upstream)?
> > 
> 
> There is still a lot of gap between SDE and DPU drivers at this point. 
> We keep trying to upstream as many features as possible to minimize the 
> gap but there is still a lot of work to do.

Glad to hear, but that sounds like a very hard to close gap unless
downstream "just works on DPU" instead of having parallel development on
two "competing" drivers for the exact same hardware.

- Marijn