From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Listing individual machines in qcom_scm_qseecom_allowlist doesn't scale.
Allow it to function as allow and disallow list at the same time by the
means of the match->data and list the SoC families instead of devices.
In case a particular device has buggy or incompatible firmware user
still can disable QSEECOM by specifying qcom_scm.qseecom=off kernel
param and (in the longer term) adding machine-specific entry to the
qcom_scm_qseecom_allowlist table.
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 | 49 ++++++++++++++----------------
include/linux/firmware/qcom/qcom_qseecom.h | 1 +
2 files changed, 24 insertions(+), 26 deletions(-)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 5bf59eba2a863ba16e59df7fa2de1c50b0a218d0..025f834e95b537b76d41b59b63871a4ce5bed717 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1981,6 +1981,7 @@ int qcom_scm_qseecom_app_send(u32 app_id, void *req, size_t req_size,
}
EXPORT_SYMBOL_GPL(qcom_scm_qseecom_app_send);
+static unsigned long qcom_qseecom_disable = QCOM_QSEECOM_QUIRK_DISABLE;
static unsigned long qcom_qseecom_ro_uefi = QCOM_QSEECOM_QUIRK_RO_UEFIVARS;
static char *qseecom = "auto";
@@ -1989,32 +1990,20 @@ 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. Users
+ * any potential issues with this, only allow validated platforms for now. Users
* still can manually enable or disable it via the qcom_scm.qseecom modparam.
+ *
+ * To disable QSEECOM for a particular machine, add compatible entry and set
+ * data to &qcom_qseecom_disable.
*/
static const struct of_device_id qcom_scm_qseecom_allowlist[] __maybe_unused = {
- { .compatible = "asus,vivobook-s15" },
- { .compatible = "asus,zenbook-a14-ux3407qa" },
- { .compatible = "asus,zenbook-a14-ux3407ra" },
- { .compatible = "dell,xps13-9345" },
- { .compatible = "hp,elitebook-ultra-g1q" },
- { .compatible = "hp,omnibook-x14" },
- { .compatible = "huawei,gaokun3" },
- { .compatible = "lenovo,flex-5g" },
- { .compatible = "lenovo,thinkpad-t14s" },
- { .compatible = "lenovo,thinkpad-x13s", },
{ .compatible = "lenovo,yoga-c630", .data = &qcom_qseecom_ro_uefi, },
- { .compatible = "lenovo,yoga-slim7x" },
- { .compatible = "microsoft,arcata", },
- { .compatible = "microsoft,blackrock" },
- { .compatible = "microsoft,romulus13", },
- { .compatible = "microsoft,romulus15", },
- { .compatible = "qcom,sc8180x-primus" },
+ { .compatible = "qcom,sc8180x", },
+ { .compatible = "qcom,sc8280xp", },
{ .compatible = "qcom,sc8280xp-crd", .data = &qcom_qseecom_ro_uefi, },
- { .compatible = "qcom,x1e001de-devkit" },
- { .compatible = "qcom,x1e80100-crd" },
- { .compatible = "qcom,x1e80100-qcp" },
- { .compatible = "qcom,x1p42100-crd" },
+ { .compatible = "qcom,sdm845", .data = &qcom_qseecom_disable, },
+ { .compatible = "qcom,x1e80100", },
+ { .compatible = "qcom,x1p42100", },
{ }
};
@@ -2046,12 +2035,22 @@ static bool qcom_scm_qseecom_machine_is_allowed(struct device *scm_dev,
match = of_match_node(qcom_scm_qseecom_allowlist, np);
of_node_put(np);
- if (match && match->data)
+ if (!match) {
+ dev_info(scm_dev, "qseecom: untested machine, skipping\n");
+ return false;
+ }
+
+ if (match->data)
*quirks = *(unsigned long *)(match->data);
else
*quirks = 0;
- return match;
+ if (*quirks & QCOM_QSEECOM_QUIRK_DISABLE) {
+ dev_info(scm_dev, "qseecom: disabled by the quirk\n");
+ return false;
+ }
+
+ return true;
}
static void qcom_scm_qseecom_free(void *data)
@@ -2086,10 +2085,8 @@ 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(scm->dev, &quirks)) {
- dev_info(scm->dev, "qseecom: untested machine, skipping\n");
+ if (!qcom_scm_qseecom_machine_is_allowed(scm->dev, &quirks))
return 0;
- }
/*
* Set up QSEECOM interface device. All application clients will be
diff --git a/include/linux/firmware/qcom/qcom_qseecom.h b/include/linux/firmware/qcom/qcom_qseecom.h
index 8d6d660e854fdb0fabbef10ab5ee6ff23ad79826..d48044ece20cc9ebac3357a642dc671c349d4343 100644
--- a/include/linux/firmware/qcom/qcom_qseecom.h
+++ b/include/linux/firmware/qcom/qcom_qseecom.h
@@ -52,5 +52,6 @@ static inline int qcom_qseecom_app_send(struct qseecom_client *client,
}
#define QCOM_QSEECOM_QUIRK_RO_UEFIVARS BIT(0)
+#define QCOM_QSEECOM_QUIRK_DISABLE BIT(1)
#endif /* __QCOM_QSEECOM_H */
--
2.39.5On Wed, Jun 25, 2025 at 01:53:26AM +0300, Dmitry Baryshkov wrote:
> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
> Listing individual machines in qcom_scm_qseecom_allowlist doesn't scale.
> Allow it to function as allow and disallow list at the same time by the
> means of the match->data and list the SoC families instead of devices.
>
> In case a particular device has buggy or incompatible firmware user
> still can disable QSEECOM by specifying qcom_scm.qseecom=off kernel
> param and (in the longer term) adding machine-specific entry to the
> qcom_scm_qseecom_allowlist table.
>
> 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>
> /*
> * 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. Users
> + * any potential issues with this, only allow validated platforms for now. Users
> * still can manually enable or disable it via the qcom_scm.qseecom modparam.
> + *
> + * To disable QSEECOM for a particular machine, add compatible entry and set
> + * data to &qcom_qseecom_disable.
> */
> static const struct of_device_id qcom_scm_qseecom_allowlist[] __maybe_unused = {
> - { .compatible = "asus,vivobook-s15" },
> - { .compatible = "asus,zenbook-a14-ux3407qa" },
> - { .compatible = "asus,zenbook-a14-ux3407ra" },
> - { .compatible = "dell,xps13-9345" },
> - { .compatible = "hp,elitebook-ultra-g1q" },
> - { .compatible = "hp,omnibook-x14" },
> - { .compatible = "huawei,gaokun3" },
> - { .compatible = "lenovo,flex-5g" },
> - { .compatible = "lenovo,thinkpad-t14s" },
> - { .compatible = "lenovo,thinkpad-x13s", },
> { .compatible = "lenovo,yoga-c630", .data = &qcom_qseecom_ro_uefi, },
> - { .compatible = "lenovo,yoga-slim7x" },
> - { .compatible = "microsoft,arcata", },
> - { .compatible = "microsoft,blackrock" },
> - { .compatible = "microsoft,romulus13", },
> - { .compatible = "microsoft,romulus15", },
> - { .compatible = "qcom,sc8180x-primus" },
> + { .compatible = "qcom,sc8180x", },
> + { .compatible = "qcom,sc8280xp", },
> { .compatible = "qcom,sc8280xp-crd", .data = &qcom_qseecom_ro_uefi, },
You need to have the machine specific entries before the SoC fallbacks
for this to work.
Perhaps this should be made more clear in the table by adding a
separator comment before the SoC entries or similar.
> - { .compatible = "qcom,x1e001de-devkit" },
> - { .compatible = "qcom,x1e80100-crd" },
> - { .compatible = "qcom,x1e80100-qcp" },
> - { .compatible = "qcom,x1p42100-crd" },
> + { .compatible = "qcom,sdm845", .data = &qcom_qseecom_disable, },
> + { .compatible = "qcom,x1e80100", },
> + { .compatible = "qcom,x1p42100", },
> { }
> };
>
> @@ -2046,12 +2035,22 @@ static bool qcom_scm_qseecom_machine_is_allowed(struct device *scm_dev,
> match = of_match_node(qcom_scm_qseecom_allowlist, np);
> of_node_put(np);
>
> - if (match && match->data)
> + if (!match) {
> + dev_info(scm_dev, "qseecom: untested machine, skipping\n");
> + return false;
> + }
> +
> + if (match->data)
> *quirks = *(unsigned long *)(match->data);
> else
> *quirks = 0;
>
> - return match;
> + if (*quirks & QCOM_QSEECOM_QUIRK_DISABLE) {
> + dev_info(scm_dev, "qseecom: disabled by the quirk\n");
Not sure this is needed since it presumably has been disabled because it
has been tested and found not to work. No need to spam the logs with
that on every boot.
In any case I don't think you should be referring to "the quirk" which
makes little sense without looking at the implementation.
> + return false;
> + }
> +
> + return true;
> }
Johan
On Thu, Jun 26, 2025 at 11:56:01AM +0200, Johan Hovold wrote:
> On Wed, Jun 25, 2025 at 01:53:26AM +0300, Dmitry Baryshkov wrote:
> > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >
> > Listing individual machines in qcom_scm_qseecom_allowlist doesn't scale.
> > Allow it to function as allow and disallow list at the same time by the
> > means of the match->data and list the SoC families instead of devices.
> >
> > In case a particular device has buggy or incompatible firmware user
> > still can disable QSEECOM by specifying qcom_scm.qseecom=off kernel
> > param and (in the longer term) adding machine-specific entry to the
> > qcom_scm_qseecom_allowlist table.
> >
> > 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>
>
> > /*
> > * 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. Users
> > + * any potential issues with this, only allow validated platforms for now. Users
> > * still can manually enable or disable it via the qcom_scm.qseecom modparam.
> > + *
> > + * To disable QSEECOM for a particular machine, add compatible entry and set
> > + * data to &qcom_qseecom_disable.
> > */
> > static const struct of_device_id qcom_scm_qseecom_allowlist[] __maybe_unused = {
> > - { .compatible = "asus,vivobook-s15" },
> > - { .compatible = "asus,zenbook-a14-ux3407qa" },
> > - { .compatible = "asus,zenbook-a14-ux3407ra" },
> > - { .compatible = "dell,xps13-9345" },
> > - { .compatible = "hp,elitebook-ultra-g1q" },
> > - { .compatible = "hp,omnibook-x14" },
> > - { .compatible = "huawei,gaokun3" },
> > - { .compatible = "lenovo,flex-5g" },
> > - { .compatible = "lenovo,thinkpad-t14s" },
> > - { .compatible = "lenovo,thinkpad-x13s", },
> > { .compatible = "lenovo,yoga-c630", .data = &qcom_qseecom_ro_uefi, },
> > - { .compatible = "lenovo,yoga-slim7x" },
> > - { .compatible = "microsoft,arcata", },
> > - { .compatible = "microsoft,blackrock" },
> > - { .compatible = "microsoft,romulus13", },
> > - { .compatible = "microsoft,romulus15", },
> > - { .compatible = "qcom,sc8180x-primus" },
> > + { .compatible = "qcom,sc8180x", },
> > + { .compatible = "qcom,sc8280xp", },
> > { .compatible = "qcom,sc8280xp-crd", .data = &qcom_qseecom_ro_uefi, },
>
> You need to have the machine specific entries before the SoC fallbacks
> for this to work.
I don't think so. It's not how OF matching works.
>
> Perhaps this should be made more clear in the table by adding a
> separator comment before the SoC entries or similar.
>
> > - { .compatible = "qcom,x1e001de-devkit" },
> > - { .compatible = "qcom,x1e80100-crd" },
> > - { .compatible = "qcom,x1e80100-qcp" },
> > - { .compatible = "qcom,x1p42100-crd" },
> > + { .compatible = "qcom,sdm845", .data = &qcom_qseecom_disable, },
> > + { .compatible = "qcom,x1e80100", },
> > + { .compatible = "qcom,x1p42100", },
> > { }
> > };
> >
> > @@ -2046,12 +2035,22 @@ static bool qcom_scm_qseecom_machine_is_allowed(struct device *scm_dev,
> > match = of_match_node(qcom_scm_qseecom_allowlist, np);
> > of_node_put(np);
> >
> > - if (match && match->data)
> > + if (!match) {
> > + dev_info(scm_dev, "qseecom: untested machine, skipping\n");
> > + return false;
> > + }
> > +
> > + if (match->data)
> > *quirks = *(unsigned long *)(match->data);
> > else
> > *quirks = 0;
> >
> > - return match;
> > + if (*quirks & QCOM_QSEECOM_QUIRK_DISABLE) {
> > + dev_info(scm_dev, "qseecom: disabled by the quirk\n");
>
> Not sure this is needed since it presumably has been disabled because it
> has been tested and found not to work. No need to spam the logs with
> that on every boot.
>
> In any case I don't think you should be referring to "the quirk" which
> makes little sense without looking at the implementation.
ack
>
> > + return false;
> > + }
> > +
> > + return true;
> > }
>
> Johan
--
With best wishes
Dmitry
On Thu, Jun 26, 2025 at 02:09:08PM +0300, Dmitry Baryshkov wrote:
> On Thu, Jun 26, 2025 at 11:56:01AM +0200, Johan Hovold wrote:
> > On Wed, Jun 25, 2025 at 01:53:26AM +0300, Dmitry Baryshkov wrote:
> > > + { .compatible = "qcom,sc8180x", },
> > > + { .compatible = "qcom,sc8280xp", },
> > > { .compatible = "qcom,sc8280xp-crd", .data = &qcom_qseecom_ro_uefi, },
> >
> > You need to have the machine specific entries before the SoC fallbacks
> > for this to work.
>
> I don't think so. It's not how OF matching works.
Ah, right, too used to USB matching.
Johan
© 2016 - 2025 Red Hat, Inc.