From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
In preparation to enabling QSEECOM for the platforms rather than
individual machines provide a mechanism for the user to override default
selection. Allow users to use qcom_scm.qseecom modparam.
Setting it to 'force' will enable QSEECOM even if it disabled or not
handled by the allowlist.
Setting it to 'off' will forcibly disable the QSEECOM interface,
allowing incompatible machines to function.
Setting it to 'roefivars' will enable the QSEECOM interface, making UEFI
variables read-only.
All other values mean 'auto', trusting the allowlist in the module.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/firmware/qcom/qcom_scm.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 27ef2497089e11b5a902d949de2e16b7443a2ca4..5bf59eba2a863ba16e59df7fa2de1c50b0a218d0 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1983,9 +1983,14 @@ EXPORT_SYMBOL_GPL(qcom_scm_qseecom_app_send);
static unsigned long qcom_qseecom_ro_uefi = QCOM_QSEECOM_QUIRK_RO_UEFIVARS;
+static char *qseecom = "auto";
+MODULE_PARM_DESC(qseecom, "Enable QSEECOM interface (force | roefivars | off | auto)");
+module_param(qseecom, charp, 0);
+
/*
* We do not yet support re-entrant calls via the qseecom interface. To prevent
- + any potential issues with this, only allow validated machines for now.
+ * any potential issues with this, only allow validated machines for now. Users
+ * still can manually enable or disable it via the qcom_scm.qseecom modparam.
*/
static const struct of_device_id qcom_scm_qseecom_allowlist[] __maybe_unused = {
{ .compatible = "asus,vivobook-s15" },
@@ -2013,11 +2018,27 @@ static const struct of_device_id qcom_scm_qseecom_allowlist[] __maybe_unused = {
{ }
};
-static bool qcom_scm_qseecom_machine_is_allowed(unsigned long *quirks)
+static bool qcom_scm_qseecom_machine_is_allowed(struct device *scm_dev,
+ unsigned long *quirks)
{
const struct of_device_id *match;
struct device_node *np;
+ if (!strcmp(qseecom, "off")) {
+ dev_info(scm_dev, "qseecom: disabled by modparam\n");
+ return false;
+ } else if (!strcmp(qseecom, "force")) {
+ dev_info(scm_dev, "qseecom: forcibly enabled\n");
+ *quirks = 0;
+ return true;
+ } else if (!strcmp(qseecom, "roefivars")) {
+ dev_info(scm_dev, "qseecom: enabling with R/O UEFI variables\n");
+ *quirks = QCOM_QSEECOM_QUIRK_RO_UEFIVARS;
+ return true;
+ } else if (strcmp(qseecom, "auto")) {
+ dev_warn(scm_dev, "qseecom: invalid value for the modparam, ignoring\n");
+ }
+
np = of_find_node_by_path("/");
if (!np)
return false;
@@ -2065,7 +2086,7 @@ static int qcom_scm_qseecom_init(struct qcom_scm *scm)
dev_info(scm->dev, "qseecom: found qseecom with version 0x%x\n", version);
- if (!qcom_scm_qseecom_machine_is_allowed(&quirks)) {
+ if (!qcom_scm_qseecom_machine_is_allowed(scm->dev, &quirks)) {
dev_info(scm->dev, "qseecom: untested machine, skipping\n");
return 0;
}
--
2.39.5On Wed, Jun 25, 2025 at 01:53:25AM +0300, Dmitry Baryshkov wrote: > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > In preparation to enabling QSEECOM for the platforms rather than > individual machines provide a mechanism for the user to override default > selection. Allow users to use qcom_scm.qseecom modparam. > > Setting it to 'force' will enable QSEECOM even if it disabled or not > handled by the allowlist. > > Setting it to 'off' will forcibly disable the QSEECOM interface, > allowing incompatible machines to function. > > Setting it to 'roefivars' will enable the QSEECOM interface, making UEFI > variables read-only. > > All other values mean 'auto', trusting the allowlist in the module. I don't see the need for this. The kernel should just provide sensible defaults. Johan
On Thu, Jun 26, 2025 at 12:11:20PM +0200, Johan Hovold wrote: > On Wed, Jun 25, 2025 at 01:53:25AM +0300, Dmitry Baryshkov wrote: > > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > In preparation to enabling QSEECOM for the platforms rather than > > individual machines provide a mechanism for the user to override default > > selection. Allow users to use qcom_scm.qseecom modparam. > > > > Setting it to 'force' will enable QSEECOM even if it disabled or not > > handled by the allowlist. > > > > Setting it to 'off' will forcibly disable the QSEECOM interface, > > allowing incompatible machines to function. > > > > Setting it to 'roefivars' will enable the QSEECOM interface, making UEFI > > variables read-only. > > > > All other values mean 'auto', trusting the allowlist in the module. > > I don't see the need for this. The kernel should just provide sensible > defaults. It does provide _defaults_. However with the next commit we mass-enable QSEECOM for SoC families, which includes untested WoA devices. If the user observes a misbehaviour of the UEFI vars or any other QSEECOM-related driver on those platforms, it is much easier to let users test and workaround UEFI misbehaviour. I can probably add an explicit message that if the modparam is used, it must be reported to linux-arm-msm@. > > Johan -- With best wishes Dmitry
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: > > On Wed, Jun 25, 2025 at 01:53:25AM +0300, Dmitry Baryshkov wrote: > > > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > In preparation to enabling QSEECOM for the platforms rather than > > > individual machines provide a mechanism for the user to override default > > > selection. Allow users to use qcom_scm.qseecom modparam. > > > > > > Setting it to 'force' will enable QSEECOM even if it disabled or not > > > handled by the allowlist. > > > > > > Setting it to 'off' will forcibly disable the QSEECOM interface, > > > allowing incompatible machines to function. > > > > > > Setting it to 'roefivars' will enable the QSEECOM interface, making UEFI > > > variables read-only. > > > > > > All other values mean 'auto', trusting the allowlist in the module. > > > > I don't see the need for this. The kernel should just provide sensible > > defaults. > > It does provide _defaults_. However with the next commit we mass-enable > QSEECOM for SoC families, which includes untested WoA devices. If the > user observes a misbehaviour of the UEFI vars or any other > QSEECOM-related driver on those platforms, it is much easier to let > users test and workaround UEFI misbehaviour. You basically know by now which machines supports qseecom and which do not, right (e.g. UFS storage means non-persistent EFI vars)? 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. 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). Johan
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: > > > On Wed, Jun 25, 2025 at 01:53:25AM +0300, Dmitry Baryshkov wrote: > > > > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > > > In preparation to enabling QSEECOM for the platforms rather than > > > > individual machines provide a mechanism for the user to override default > > > > selection. Allow users to use qcom_scm.qseecom modparam. > > > > > > > > Setting it to 'force' will enable QSEECOM even if it disabled or not > > > > handled by the allowlist. > > > > > > > > Setting it to 'off' will forcibly disable the QSEECOM interface, > > > > allowing incompatible machines to function. > > > > > > > > Setting it to 'roefivars' will enable the QSEECOM interface, making UEFI > > > > variables read-only. > > > > > > > > All other values mean 'auto', trusting the allowlist in the module. > > > > > > I don't see the need for this. The kernel should just provide sensible > > > defaults. > > > > It does provide _defaults_. However with the next commit we mass-enable > > QSEECOM for SoC families, which includes untested WoA devices. If the > > user observes a misbehaviour of the UEFI vars or any other > > QSEECOM-related driver on those platforms, it is much easier to let > > users test and workaround UEFI misbehaviour. > > You basically know by now which machines supports qseecom and which do > not, right (e.g. UFS storage means non-persistent EFI vars)? > > 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. > > 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. Except for X1-CRD, which deserves an entry in the driver. -- With best wishes Dmitry
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: > > > > On Wed, Jun 25, 2025 at 01:53:25AM +0300, Dmitry Baryshkov wrote: > > > > > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > > > > > In preparation to enabling QSEECOM for the platforms rather than > > > > > individual machines provide a mechanism for the user to override default > > > > > selection. Allow users to use qcom_scm.qseecom modparam. > > > > > > > > > > Setting it to 'force' will enable QSEECOM even if it disabled or not > > > > > handled by the allowlist. > > > > > > > > > > Setting it to 'off' will forcibly disable the QSEECOM interface, > > > > > allowing incompatible machines to function. > > > > > > > > > > Setting it to 'roefivars' will enable the QSEECOM interface, making UEFI > > > > > variables read-only. > > > > > > > > > > All other values mean 'auto', trusting the allowlist in the module. > > > > > > > > I don't see the need for this. The kernel should just provide sensible > > > > defaults. > > > > > > It does provide _defaults_. However with the next commit we mass-enable > > > QSEECOM for SoC families, which includes untested WoA devices. If the > > > user observes a misbehaviour of the UEFI vars or any other > > > QSEECOM-related driver on those platforms, it is much easier to let > > > users test and workaround UEFI misbehaviour. > > > > 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? > > 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. 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. > > 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. If we know with reasonable certainty that all, say X1E, devices works, then that that's one thing. 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. > Except for X1-CRD, which deserves an entry in the driver. I think you meant my sc8280xp CRD here. Johan
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: > > > > > On Wed, Jun 25, 2025 at 01:53:25AM +0300, Dmitry Baryshkov wrote: > > > > > > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > > > > > > > In preparation to enabling QSEECOM for the platforms rather than > > > > > > individual machines provide a mechanism for the user to override default > > > > > > selection. Allow users to use qcom_scm.qseecom modparam. > > > > > > > > > > > > Setting it to 'force' will enable QSEECOM even if it disabled or not > > > > > > handled by the allowlist. > > > > > > > > > > > > Setting it to 'off' will forcibly disable the QSEECOM interface, > > > > > > allowing incompatible machines to function. > > > > > > > > > > > > Setting it to 'roefivars' will enable the QSEECOM interface, making UEFI > > > > > > variables read-only. > > > > > > > > > > > > All other values mean 'auto', trusting the allowlist in the module. > > > > > > > > > > I don't see the need for this. The kernel should just provide sensible > > > > > defaults. > > > > > > > > It does provide _defaults_. However with the next commit we mass-enable > > > > QSEECOM for SoC families, which includes untested WoA devices. If the > > > > user observes a misbehaviour of the UEFI vars or any other > > > > QSEECOM-related driver on those platforms, it is much easier to let > > > > users test and workaround UEFI misbehaviour. > > > > > > 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 - 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. > > 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. > > > > 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. 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. > 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). > 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 > > > Except for X1-CRD, which deserves an entry in the driver. > > I think you meant my sc8280xp CRD here. Yes. > > Johan -- With best wishes Dmitry
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
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
On Tue, Jul 01, 2025 at 02:10:49PM +0300, Dmitry Baryshkov wrote: > On Mon, Jun 30, 2025 at 02:42:06PM +0200, Johan Hovold wrote: > > 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). And for that you do not need a module parameter either. > > 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. Then go for it. > > 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. The distros don't do bring ups of these machines themselves. > > 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. How about asking the Qualcomm firmware team? Again, there's no rush here. Whitelisting is perfectly fine until then. > > 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. It's not theoretical. We know that the UEFI vars on the CRDs are not persistent when booting off UFS. Not to mention your Yoga. > > > > 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 ;-) Linaro relies on Qualcomm to provide details on things like this. As you know. Johan
© 2016 - 2025 Red Hat, Inc.