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;
Message-ID: <468b6e00-5d77-da64-164d-149bb640e902@gmail.com>
Date:   Mon, 25 Jul 2022 22:16:53 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH 0/4] firmware: Add support for Qualcomm UEFI Secure
 Application
Content-Language: en-US
To:     Rob Herring <robh@kernel.org>
Cc:     Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Ard Biesheuvel <ardb@kernel.org>,
        Konrad Dybcio <konrad.dybcio@somainline.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
References: <20220723224949.1089973-1-luzmaximilian@gmail.com>
 <20220725192705.GA2536409-robh@kernel.org>
From:   Maximilian Luz <luzmaximilian@gmail.com>
In-Reply-To: <20220725192705.GA2536409-robh@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 7/25/22 21:27, Rob Herring wrote:
> On Sun, Jul 24, 2022 at 12:49:45AM +0200, Maximilian Luz wrote:
>> On modern Qualcomm platforms, access to EFI variables is restricted to
>> the secure world / TrustZone, i.e. the Trusted Execution Environment
>> (TrEE or TEE) as Qualcomm seems to call it. To access EFI variables, we
>> therefore need to talk to the UEFI Secure Application (uefisecapp),
>> residing in the TrEE.
> 
> The whole point of UEFI is providing a standard interface. Why can't the
> UEFI implementation call the TEE itself?
> 
> I'm not sure custom interfaces is something we want.

Unfortunately, I'm not a Qualcomm engineer and in no way affiliated with
them. So I probably can't convince them otherwise. Believe me, I'd like
to :/

First: The uefisecapp-driver is based on reverse-engineering. So please
take things below with a grain of salt, I may be wrong. I've tried to
lay this out in a bit more detail in patch 3, but I'll try to be more
precise here:

For some reason unknown to me, Qualcomm decided to lock away UEFI
variable access via their TrEE framework for applications running in the
TrustZone (or whatever that is exactly). To call to TrEE applications
(like uefisecapp), they use SCM calls. Those SCM calls unfortunately can
be a bit more complex. As far as I can tell, you essentially call to
some hypervisor in the TrustZone which redistributes them (if necessary)
to the respective application. Their downstream driver for that is at
[1] and supports callbacks and re-entrant calls. As far as I can tell,
the latter means that you can't run multiple SCM calls in parallel (at
least not to that TzOS/TrEE interface) and can only ever have one
"client" performing them.

And as you can only ever have one entity performing those SCM calls, you
cannot have both UEFI and the kernel doing them. To me, it seems to be a
deliberate decision by Qualcomm to return EFI_UNSUPPORTED from the
GetVariable etc. calls after exiting boot services. They do work just
fine before that. Essentially exiting boot services transfers ownership
of that SCM interface from UEFI to the kernel.

Note: uefisecapp is also not the only TrEE application in use by those
kinds of devices. For example, on the SC8180X based Surface Pro X that
I'm using, there are at least an app for the TPM, a bunch of apps for
HDCP, some winsecapp, and as far as I can tell also other cryptographic
interfaces. I've tried to collect my findings about those in [2].

>> This series adds support for accessing EFI variables on those platforms.
>>
>> To do this, we first need to add some SCM call functions used to manage
>> and talk to Secure Applications. A very small subset of this interface
>> is added in the second patch (whereas the first one exports the required
>> functions for that). Interface specifications are extracted from [1].
>> While this does not (yet) support re-entrant SCM calls (including
>> callbacks and listeners), this is enough to talk to the aforementioned
>> uefisecapp on a couple of platforms (I've tested this on a Surface Pro X
>> and heard reports from Lenovo Flex 5G, Lenovo Thinkpad x13s, and Lenovo
>> Yoga C630 devices).
> 
> What does Windows do on these devices? I'm surprised something like this
> would fly with Microsoft.

It looks like Microsoft accepts this. They even seem to have some sort
of interface for EFI variables via trusted execution environments: [3].
This is essentially what the QcTrEE8180.sys driver I've reverse
engineered this from seems to provide.

So unless there's some way to make EFI variables work via the standard
functions that I've missed, I don't see any alternatives. I think it's
fairly unlikely that we can convince Qualcomm to make their UEFI
implementation behave properly (variables are not the only issue, it
seems that other functions are either partially or completely broken in
some way or another...) and then also push updates for a bunch of
devices (e.g. the Lenovo C630 also using this stuff is discontinued, as
far as I can tell).

I am open to suggestions though...

Note that this series also doesn't really introduce a new interface for
EFI variables themselves to the kernel. It relies on the existing
efivars_register() / efivars_unregister() functions and the interface
provided by them to enable access to EFI variables. So we already do
have a "workaround" for broken UEFI variable access in the kernel.

Regards,
Max

[1]: https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/auto-kernel.lnx.4.14.c34/drivers/misc/qseecom.c
[2]: https://github.com/linux-surface/surface-pro-x/issues/37
[3]: https://github.com/tpn/winsdk-10/blob/9b69fd26ac0c7d0b83d378dba01080e93349c2ed/Include/10.0.16299.0/km/treevariableservice.h#L11-L12