Message-ID: <aGKGHvDzlKrBDrjN@hovoldconsulting.com>
Date: Mon, 30 Jun 2025 14:42:06 +0200
From: Johan Hovold <johan@...nel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Bjorn Andersson <andersson@...nel.org>,
	Maximilian Luz <luzmaximilian@...il.com>,
	Konrad Dybcio <konradybcio@...nel.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Ard Biesheuvel <ardb@...nel.org>,
	Steev Klimaszewski <steev@...i.org>, linux-arm-msm@...r.kernel.org,
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	linux-efi@...r.kernel.org,
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
	Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Subject: Re: [PATCH v4 6/8] firmware: qcom: scm: add modparam to control
 QSEECOM enablement

On Sat, Jun 28, 2025 at 06:03:40PM +0300, Dmitry Baryshkov wrote:
> On Fri, Jun 27, 2025 at 02:46:38PM +0200, Johan Hovold wrote:
> > On Fri, Jun 27, 2025 at 02:33:27AM +0300, Dmitry Baryshkov wrote:
> > > On Thu, Jun 26, 2025 at 02:58:52PM +0200, Johan Hovold wrote:
> > > > On Thu, Jun 26, 2025 at 02:08:23PM +0300, Dmitry Baryshkov wrote:
> > > > > On Thu, Jun 26, 2025 at 12:11:20PM +0200, Johan Hovold wrote:

> > > > You basically know by now which machines supports qseecom and which do
> > > > not, right (e.g. UFS storage means non-persistent EFI vars)?
> > 
> > Do you have a theory about why on some platforms, like the one you're
> > currently adding support for, writing UEFI variables does not work?
> > 
> > Can you please include that information in the series so we can consider
> > alternate routes for replacing the current whitelist with this black and
> > white thing you're going for.
> > 
> > Is it related to UFS at all, for example?
> 
> Strictly speaking I have no confirmation (yet), but there are two
> theories:
> 
> - UFS vs SPI-NOR

Someone with time and the sc8280xp and x1e CRDs should be able to set
them up for booting from either UFS or SPI-NOR and see if that makes a
difference to confirm this.

So far my sc8280xp CRD with UFS fails, while Konrad's work with SPI-NOR
(NVMe).

My x1e CRD works but also boots from SPI-NOR (NVMe).

The Yoga C630 booting from UFS is also known to fail.

> - a edk2 PCD which controls whether SetVariable commits immediately or
>   whether it just buffers data until EBS (or other call).
> 
> > 
> > > > And it's a pretty bad user experience to have people trying to write
> > > > efivariables when setting up a machine and then spend hours trying to
> > > > debug why they don't persist after a reboot.
> > > > 
> > > > I don't think that's fair to users.
> > > 
> > > So, is it a user or a developer, trying to port Linux to a new hardware?
> > > Also, R/O implementation makes it obvious, that the variables do not
> > > persist.
> > 
> > A developer enabling support for a new platform can patch the driver and
> > does not need a command line option.
> 
> Yes. But it's easier to debug things this way. Consider all ACPI-related
> or UEFI-related kernel options that we have.

That's because there is a common kernel implementation used across a
host of fw implementations.

Here it's just Qualcomm doing something funny that affects their own
platforms. We should be able to figure this out without forcing users or
distros to pass command line parameters.

> > If you enable it by default, suddenly a bunch of end-users are going to
> > have to debug why storing efi variables silently fails. That would not
> > be fair to them.
> 
> I'm enabling this only for platforms where all existing devices are
> listed in the current whitelist.

Do we know if there are any sc8280xp or x1e machines that boot off UFS?

If not (even with the exception of the CRDs) then it should be fine to
just whitelist the SoCs without any command line parameters.

> > > > Let whoever brings up a new machine figure this out. It's just one
> > > > entry, no scaling issues, and we get accurate information (unless
> > > > Qualcomm, who sits on the documentation, is willing to provide it
> > > > upfront).
> > > 
> > > And that's not really scallable. All other parts of a particular device
> > > are described by the DT only (that's especially true on the PMIC GLINK
> > > machines). If we are to support new laptop in e.g. distro kernel, we
> > > need to provide a DT... and a patch for qcom-scm driver. I'd very much
> > > prefer to do it other way around: provide a DT and patch qcom-scm if the
> > > laptop is any way different from other laptops. E.g. we know that all
> > > X1Elite laptops support R/W EFI variables.
> > 
> > But this is just kicking the can and putting the burden on someone else.
> > Now a user or distro would need to pass command line parameters after
> > spending time debugging why efi variable updates do not persist after a
> > reboot.
> 
> The original developer for new DTS will have to do that anyway, if
> something fails. And once it is done, we can add a quirk for that pure
> platform. However the majority of the case can go without extra quirks.

Adding to a blacklist is bound to be overlooked, while adding to a
whitelist is not.

> As you can see, all X-Elite / X-Plus and majority of SC8280XP platforms
> already are in the whitelist. Once we sort out SC8280XP-CRD issue, all
> SC8280XP platforms supported upstream will have an entry in the
> allowlist, which means we can convert them to the wildcard + quirks.

I'd rather see you get to the bottom of the UFS boot issue and whether
there is some way to determine this programmatically.

> > If we know with reasonable certainty that all, say X1E, devices works,
> > then that that's one thing.
> 
> Yes, we do. You can hand-compare the lists too (I did).

If everything that's currently upstream boots from NVMe that may not
necessarily mean it works for devices using UFS.

> > But if this series now enables broken EFI variable support on every
> > other device then I don't think that's ok (even if you provide a command
> > line parameter that each user now have to pass).
> > 
> > Then I'd rather see a proposal for how to determine which machines
> > support this or not, information which was not available when this
> > interface was reverse engineered and where a conservative whitelist
> > approach made perfect sense.
> 
> WIP

Good. We can manage with adding new entries for a while still while you
guys at Qualcomm work this out.

Johan

Powered by blists - more mailing lists