linux-kernel - Re: [PATCH v2] firmware: qcom_scm: Only compile legacy calls on ARM

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]

Date:   Thu, 8 Apr 2021 09:19:44 +0200
From:   Stephan Gerhold <stephan@...hold.net>
To:     Stephen Boyd <swboyd@...omium.org>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Elliot Berman <eberman@...eaurora.org>,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        Brian Masney <masneyb@...tation.org>,
        Jeffrey Hugo <jhugo@...eaurora.org>,
        Douglas Anderson <dianders@...omium.org>
Subject: Re: [PATCH v2] firmware: qcom_scm: Only compile legacy calls on ARM

On Wed, Apr 07, 2021 at 05:12:06PM -0700, Stephen Boyd wrote:
> Quoting Stephan Gerhold (2021-04-05 05:50:26)
> > On Fri, Apr 02, 2021 at 10:21:58AM -0700, Stephen Boyd wrote:
> > > 
> > > Ah right, the whole secure world running in 32-bit mode thing. Is
> > > msm8916 the only SoC that's using that? Or are there more? If only
> > > msm8916 is affected then we could use a combination of CONFIG_ARM64 and
> > > the compatible string to differentiate and then if more SoCs use 32-bit
> > > secure world then we could have a new compatible string like qcom,scm-32
> > > that tells us this fact. Maybe this was all discussed before and I
> > > missed it. Either way, I'm trying to get rid of this boot call so that
> > > we don't have to bounce to the firmware an extra time to figure out
> > > something we can figure out from the kernel arch and scm compatible
> > > string.
> > 
> > At least MSM8994 also uses SMC32 from what I heard. Overall it's
> > probably quite hard to get that right now since all boards were tested
> > with the dynamic detection so far. I suppose you could do the opposite,
> > add an optional qcom,scm-64 to skip the detection step and force SMC64.
> 
> Isn't SMC64 going to be the overall majority going forward? Legacy
> convention is for sure limited to CONFIG_ARM so I'll send another
> follow-up patch to add a warning if we find legacy on CONFIG_ARM64.
> SMC32 is hopefully no longer being produced given that it was introduced
> at the time that the bootloader team wasn't supporting PSCI and didn't
> want to support it. So we're making all new boards/SoCs/firmwares do
> this calling convention probing to figure out something they already
> know?
> 
> Maybe we should probe the calling convention on msm8994/msm8916 and
> otherwise assume SMC64 on CONFIG_ARM64 kernels. I'd expect the exception
> list to be smaller that way.
> 

Personally, I think it would be best to introduce a new, SMC64 only
compatible (e.g. "qcom,scm-64" like I mentioned). Then you can skip the
detection check for the boards that opt-in by adding the compatible.
You can then use it on all newer boards/SoCs/firmwares where you know
exactly that there is SMC64.

I would just like to avoid breaking any existing boards where we don't
know exactly if they have SMC32 or SMC64.

> > 
> > Also note that this could even be firmware-specific, not necessarily
> > SoC-specific. There are some ancient MSM8916 firmwares that have legacy
> > instead of SMC32. I could also imagine that there is also some SoC where
> > there are different firmware versions with SMC32 or SMC64.
> 
> Sure but in theory the firmware would update the DT to indicate what
> sort of firmware is there.
> 

In a perfect world the firmware would do that, but there is certainly
no such mechanism on any of the old SoCs :/

> > 
> > Plus, IMO the overhead for this detection is negligible. At least it
> > ensures that we always use the right calling convention. The PSCI code
> > probably does much more firmware calls to figure out all supported
> > features.
> > 
> 
> Heh, it tried to ensure we use the right calling convention but broke
> things in the process, because the way of detecting the convention isn't
> always there. I wouldn't be surprised if this comes up again for other
> boards that use TF-A.

Ah okay, this sounds like a better reason than just trying to avoid the
"overhead" of the detection step. :) I still think it should work if you
just start marking all newer boards/SoCs/... as "qcom,scm-64" or
something like that, right?

Thanks,
Stephan