Re: [PATCH v4 7/8] firmware: qcom: scm: rework QSEECOM allowlist

[Tobias Heider]

Hi Dmitry,

I think making the list more generic would be great. Currently onboarding new
devices always requires a kernel/driver rebuild on top of the device-tree which
isn't ideal. If we could allow all of sc8280xp, x1e80100 and glymur by default
adding new devices to our Ubuntu images would be a lot easier.

Since this seems to have stalled a while ago I was wondering if you
had any plans to
continue this work. Is there a technical reason this never went anywhere?

- Tobias

On Wed, Jun 25, 2025 at 12:55 AM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxxxxxxxx> wrote:
>
> From: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> ---
>  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.5
>
>