linux-kernel - Re: [RFC PATCH v2 7/7] arm64: dts: qcom: Add the Inline Crypto Engine nodes

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: <ZAuMYsH4l2XTsEZ9@sol.localdomain>
Date:   Fri, 10 Mar 2023 12:00:34 -0800
From:   Eric Biggers <ebiggers@...nel.org>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Abel Vesa <abel.vesa@...aro.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Manivannan Sadhasivam <mani@...nel.org>,
        Alim Akhtar <alim.akhtar@...sung.com>,
        Avri Altman <avri.altman@....com>,
        Bart Van Assche <bvanassche@....org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        "James E . J . Bottomley" <jejb@...ux.ibm.com>,
        "Martin K . Petersen" <martin.petersen@...cle.com>,
        linux-mmc@...r.kernel.org, devicetree@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-arm-msm@...r.kernel.org, linux-scsi@...r.kernel.org
Subject: Re: [RFC PATCH v2 7/7] arm64: dts: qcom: Add the Inline Crypto
 Engine nodes

On Fri, Mar 10, 2023 at 09:13:29AM +0100, Krzysztof Kozlowski wrote:
> > FWIW, I tested this patchset on SDA845, and ICE continues to work fine.
> 
> Really?

Yes.

> I clearly see of_find_device_by_node -> "return NULL" and all old code gone,
> so ABI is broken. Are you sure you applied patch 1-6 and ICE was working?

Yes.

But I applied the whole series.  It sounds like you are talking about the case
where *only* patches 1-6 are applied?  I did not test that, sorry.

> > (Though if I understand the patchset correctly, the ICE clock is no longer
> > turned off when the UFS host controller is suspended.  That isn't ideal as it
> > wastes power.  I would like that to be fixed.)
> > 
> > Anyway, when you say "break the ICE", do you really mean "make an incompatible
> > change to the device-tree bindings"?
> 
> It breaks existing users of DTS and kernel.
> 
> > 
> > I'd think there would be no problem with that as long as everything is updated
> > at once, which this patchset does.
> 
> Which is obviously not possible. DTS always goes separate branch,
> always. It cannot be combined with code into the same branch! So how do
> you even imagine this?

It is not obvious.  DTS and drivers are both in the kernel source tree.
It's natural to think they can be updated together as per
Documentation/process/stable-api-nonsense.rst.

I'm *not* saying they can be -- you're the expert here, and it sounds like DTS
are considered a stable UAPI.  I'm merely saying that it's not obvious...
especially when my personal experience is that reviewers explicitly preferred a
patch breaking DTS compatibility to a patch not breaking DTS compatibility
(https://lore.kernel.org/linux-scsi/20200710072013.177481-1-ebiggers@kernel.org/T/#u)

Anyway, based on the follow-up replies, it sounds like Abel has a plan to keep
compatibility with old DTS, so hopefully that works and isn't too much trouble.

- Eric