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

From: Stephan Gerhold
Date: Thu Apr 08 2021 - 03:19:55 EST


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