Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 17 Jan 2023 09:24:23 +0100
From:   Johan Hovold <johan@kernel.org>
To:     Maximilian Luz <luzmaximilian@gmail.com>,
        Ard Biesheuvel <ardb@kernel.org>
Cc:     Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Konrad Dybcio <konrad.dybcio@somainline.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Steev Klimaszewski <steev@kali.org>,
        Shawn Guo <shawn.guo@linaro.org>,
        Sudeep Holla <sudeep.holla@arm.com>,
        Cristian Marussi <cristian.marussi@arm.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-arm-msm@vger.kernel.org, linux-efi@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/4] firmware: Add support for Qualcomm UEFI Secure
 Application
Message-ID: <Y8ZbN5LNn2fk0/xi@hovoldconsulting.com>
References: <20220723224949.1089973-1-luzmaximilian@gmail.com>
 <20220723224949.1089973-4-luzmaximilian@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220723224949.1089973-4-luzmaximilian@gmail.com>
Precedence: bulk

On Sun, Jul 24, 2022 at 12:49:48AM +0200, Maximilian Luz wrote:
> On platforms using the Qualcomm UEFI Secure Application (uefisecapp),
> EFI variables cannot be accessed via the standard interface in EFI
> runtime mode. The respective functions return EFI_UNSUPPORTED. On these
> platforms, we instead need to talk to uefisecapp. This commit provides
> support for this and registers the respective efivars operations to
> access EFI variables from the kernel.
> 
> Communication with uefisecapp follows the standard Qualcomm Trusted
> Environment (TEE or TrEE) / Secure OS conventions via the respective SCM
> call interface. This is also the reason why variable access works
> normally while boot services are active. During this time, said SCM
> interface is managed by the boot services. When calling
> ExitBootServices(), the ownership is transferred to the kernel.
> Therefore, UEFI must not use that interface itself (as multiple parties
> accessing this interface at the same time may lead to complications) and
> cannot access variables for us.
> 
> Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
> ---

> +static struct platform_driver qcom_uefisecapp_driver = {
> +	.probe = qcom_uefisecapp_probe,
> +	.remove = qcom_uefisecapp_remove,
> +	.driver = {
> +		.name = "qcom_tee_uefisecapp",
> +		.of_match_table = qcom_uefisecapp_dt_match,
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
> +	},
> +};
> +module_platform_driver(qcom_uefisecapp_driver);

I noticed that for efivarfs to work, you're currently relying on having
the firmware still claim that the variable services are supported in the
RT_PROP table so that efi core registers the default ops at subsys init
time (which are later overridden by this driver).

Otherwise efivarfs may fail to initialise when built in:

	static __init int efivarfs_init(void)
	{
		if (!efivars_kobject())
			return -ENODEV;

		return register_filesystem(&efivarfs_type);
	}

	module_init(efivarfs_init);

With recent X13s firmware the corresponding bit in the RT_PROP table has
been cleared so that efivarfs would fail to initialise. Similar problem
when booting with 'efi=noruntime'.

One way to handle this is to register also the qcom_uefisecapp_driver at
subsys init time and prevent it from being built as a module (e.g. as is
done for the SCM driver). I'm using the below patch for this currently.

I guess the Google GSMI implementation suffers from a similar problem.

Johan


From 8fecce12d215bd8cab1b8c8f9f0d1e1fe20fe6e7 Mon Sep 17 00:00:00 2001
From: Johan Hovold <johan+linaro@kernel.org>
Date: Sun, 15 Jan 2023 15:32:34 +0100
Subject: [PATCH] firmware: qcom_tee_uefisecapp: register at subsys init

Register efivars at subsys init time so that it is available when
efivarfs probes. For the same reason, also prevent building the driver
as a module.

This is specifically needed on platforms such as the Lenovo Thinkpad
X13s where the firmware has cleared the variable services in the RT_PROP
table so that efi core does not register any efivar callbacks at subsys
init time (which are later overridden).

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/firmware/Kconfig               | 2 +-
 drivers/firmware/qcom_tee_uefisecapp.c | 7 ++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 4e9e2c227899..48e712e363da 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -231,7 +231,7 @@ config QCOM_TEE
 	select QCOM_SCM
 
 config QCOM_TEE_UEFISECAPP
-	tristate "Qualcomm TrEE UEFI Secure App client driver"
+	bool "Qualcomm TrEE UEFI Secure App client driver"
 	select QCOM_TEE
 	depends on EFI
 	help
diff --git a/drivers/firmware/qcom_tee_uefisecapp.c b/drivers/firmware/qcom_tee_uefisecapp.c
index 65573e4b815a..e83bce4da70a 100644
--- a/drivers/firmware/qcom_tee_uefisecapp.c
+++ b/drivers/firmware/qcom_tee_uefisecapp.c
@@ -754,7 +754,12 @@ static struct platform_driver qcom_uefisecapp_driver = {
 		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
 	},
 };
-module_platform_driver(qcom_uefisecapp_driver);
+
+static int __init qcom_uefisecapp_init(void)
+{
+	return platform_driver_register(&qcom_uefisecapp_driver);
+}
+subsys_initcall(qcom_uefisecapp_init);
 
 MODULE_AUTHOR("Maximilian Luz <luzmaximilian@gmail.com>");
 MODULE_DESCRIPTION("Client driver for Qualcomm TrEE/TZ UEFI Secure App");
-- 
2.38.2