Re: [PATCH 4/4] dt-bindings: firmware: Add Qualcomm UEFI Secure Application client

[Maximilian Luz <luzmaximilian@gmail.com>]

On 7/28/22 14:35, Ilias Apalodimas wrote:
> Hi Maximilian
> 
> On Thu, 28 Jul 2022 at 13:48, Maximilian Luz <luzmaximilian@gmail.com> wrote:
>>
>> On 7/28/22 08:03, Ilias Apalodimas wrote:
>>> Hi all,
>>>
>>> On Wed, 27 Jul 2022 at 16:24, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>>
>>>> On Wed, Jul 27, 2022 at 03:03:49PM +0200, Maximilian Luz wrote:
>>>>>
>>>>> Is there really a good way around it?
>>>>
>>>> Yes rely on the firmware preferably auto discover, if that is not an option,
>>>> how about query. It seem to be working in your case.
>>>
>>> That's a good point.  We have a similar situation with some Arm
>>> devices and U-Boot.  Let me try to explain a bit.
>>>
>>> There's code plugged in in OP-TEE and U-Boot atm which allows you to
>>> store EFI variables on an RPMB.  This is a nice alternative if your
>>> device doesn't have any other secure storage,  however it presents
>>> some challenges after ExitBootServices, similar to the ones you have
>>> here.
>>>
>>> The eMMC controller usually lives in the non-secure world.  OP-TEE
>>> can't access that, so it relies on a userspace supplicant to perform
>>> the RPMB accesses.  That supplicant is present in U-Boot and
>>> Get/SetVariable works fine before ExitBootServices.  Once Linux boots,
>>>    the 'U-Boot supplicant' goes away and we launch the linux equivalent
>>> one from userspace.  Since variable accessing is a runtime service and
>>> it still has to go through the firmware we can't use those anymore
>>> since U-Boot doesn't preserve the supplicant, the eMMC driver and the
>>> OP-TEE portions needed in the runtime section(and even if it did we
>>> would now have 2 drivers racing to access the same hardware).  Instead
>>> U-Boot copies the variables in runtime memory and
>>> GetVariable/GetNextVariable still works, but SetVariable returns
>>> EFI_UNSUPPORTED.
>>>
>>> I've spent enough time looking at available solutions and although
>>> this indeed breaks the EFI spec, something along the lines of
>>> replacing the runtime services with ones that give you direct access
>>> to the secure world, completely bypassing the firmware is imho our
>>> least bad option.
>>
>> This sounds very similar to what Qualcomm may be doing on some devices.
>> The TrEE interface allows for callbacks and there are indications that
>> one such callback-service is for RPMB. I believe that at least on some
>> platforms, Qualcomm also stores UEFI variables in RPMB and uses the same
>> uefisecapp interface in combination with RPMB listeners installed by the
>> kernel to access them.
>>
>>> I have an ancient branch somewhere that I can polish up and send an
>>> RFC [1],  but the way I enabled that was to install an empty config
>>> table from the firmware.  That empty table is basically an indication
>>> to the kernel saying "Hey I can't store variables, can you do that for
>>> me".
>>>
>>> Is there any chance we can do something similar on that device (or
>>> find a reasonable way of inferring that we need to replace some
>>> services).  That way we could at least have a common entry point to
>>> the kernel and leave out the DT changes.
>>>
>>> [1] https://git.linaro.org/people/ilias.apalodimas/net-next.git/log/?h=setvar_rt_optee_3
>>
>> I would very much like to avoid the need for special bootloaders. The
>> devices we're talking about are WoA devices, meaning they _should_
>> ideally boot just fine with EFI and ACPI.
> 
> I've already responded to following email, but I'll repeat it here for
> completeness. It's not a special bootloader.  It's the opposite, it's
> a generic UEFI compliant bootloader which takes advantage of the fact
> EFI is extensible. We are doing something very similar in how we load
> our initrd via the EFI_LOAD_FILE2 protocol.  Whether Qualcomm can add
> that to their bootloaders is a different topic though.  But at some
> point we need to draw a line than keep overloading the DT because a
> vendor decided to go down it's own path.

But still, you're asking users to install an extra thing in the boot
chain. That's what I mean by "special". So the situation would then be
this: User needs a) GRUB (or something similar) for booting the kernel
(or dual-booting, ...), b) DTBLoader for loading the device-tree because
we don't support the ACPI Qualcomm provided, and c) your thing for EFI
variables and potentially other firmware fix-ups. b) and c) are both
things that "normal" users don't expect. IMHO we should try to get rid
of those "non-standard" things, not add more.

>>   From an end-user perspective, it's annoying enough that we'll have to
>> stick with DTs for the time being due to the use of PEPs in ACPI. I
>> really don't want to add some special bootloader for fixups to that.
>> Also, this would just move the problem from kernel to bootloader.
> 
> But it *is* a bootloader problem.  The bootloader is aware of the fact
> that it can't provide runtime services for X reasons and that's
> exactly why we are trying to set EFI_RT_PROPERTIES_TABLE correctly
> from the firmware.  All we are doing is install a config table to tell
> the OS "I can't do that, can you find a way around it?".

Sure, but is making the Linux installation process more device
dependent and complicated really the best way to solve this?

Regards,
Max