Re: [PATCH v4 6/8] firmware: qcom: scm: add modparam to control QSEECOM enablement

[Dmitry Baryshkov]

On Mon, Jun 30, 2025 at 02:42:06PM +0200, Johan Hovold wrote:
> 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.

This is not intended for the normal working course, but for the initial
bringup / nailing out issues after the bringup (e.g. after firmware
upgrade).

> 
> > > 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.

I'm not aware of such platforms.

> 
> > > > > 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.

You can't overlook it since it is required as a part of almost any
distro setup - point UEFI boot sequence to your new bootloader entry.

> 
> > 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.

I don't see a good way to do that - UFS might be probed very late, it
might be unused for the boot at all, etc.

> 
> > > 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.

And? I don't care that much about theoretical devices here.

> 
> > > 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.

You (we) guys at Linaro could have figured that out too ;-)

-- 
With best wishes
Dmitry