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

[Maximilian Luz <luzmaximilian@gmail.com>]

Hi,

On 7/31/22 11:54, Ilias Apalodimas wrote:
> Hi Maximilian,
> 
> On Thu, 28 Jul 2022 at 20:27, Maximilian Luz <luzmaximilian@gmail.com> wrote:
>>
> 
> [...]
> 
>>>>>>>
>>>>>>> [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.
>>>
>>> Not users.  EFI firmware implementations that want to support this in
>>> a generic way.
>>
>> The whole point here is that we don't have control over that. I'd like
>> to fix the firmware, but we're talking about WoA devices where, let's
>> face it, both device and SoC vendor don't really care about Linux. Even
>> if you'd convince them to implement that for future generations, you'd
>> still need them to push firmware updates for older generations.
>> Generations that are end-of-life. IMHO, we should still try support
>> those. Or we just say "sorry, Linux doesn't support that on your WoA
>> device".
> 
> Yea we agree on that.  I've mentioned on a previous mail that whether
> Qualcomm wants to support this in a generic way is questionable, since
> we have no control over the firmware.  My only 'objection' is that the
> kernel has a generic way of discovering which runtime services are
> supported, which we will completely ignore based on some DT entries.

Right, sorry. That makes sense. If we have a realistic possibility, then
I agree that making it discoverable in firmware is the best option. My
point was just that we can't rely on Windows-focused vendors to
implement it.

> In any case let's find something that fits OP-TEE as well, since I can
> send those patches afterwards.

I think it's a great idea to try and find some sort of standardized
solution for OP-TEE and other interested projects similar to it, but we
still have to use a workaround for the Qualcomm WoA devices we have :(

Nevertheless, I'm happy to provide some input for a generic solution,
although I'm not sure I'm the best person to talk to about this.

>>>> 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.
>>>
>>> But that's exactly why EFI is extensible .  You can have non standard
>>> functionality on your firmware for cases like this which doesn't need
>>> to land in the spec.
>>>
>>>>
>>>>>>     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?
>>>
>>> Isn't it device dependent already?  That boat has sailed already since
>>> we need to change the very definition of runtime services and replace
>>> them with OS specific ones.  If we add it on the DT, you'll end up
>>> with different DTs per OS and potentially per use case.  In my head
>>> the DTs should be part of the firmware (and authenticated by the
>>> firmware as well) instead of loading whatever we want each time.  By
>>> using a config table we can add a u64 (random thought),  that tells
>>> the kernel which TEE implementation will handle variable storage.  So
>>> we can have a common extension to boot loaders, which at least uses
>>> EFI interfaces to communicate the functionality.
>>
>> The only thing that is making the installation-process for end-users
>> device dependent is installing the DTB. We can handle the device
>> specific stuff in the kernel, just as we already handle buggy devices.
>>
>> Further, you seem to assume that these devices provide a DT in the first
>> place. WoA devices use ACPI, so they don't. But for the time being (as
>> discussed elsewhere) we unfortunately need to stick with DTs and can't
>> really use ACPI. I agree that we should avoid OS and use-case specific
>> DTs, but I don't see how this would make a DT use-case or OS specific.
>> Things are firmware specific, the interface doesn't change with a
>> different OS, and we're only indicating the presence of that interface.
>>
>> My current suggestion (already sent to Sudeep earlier) is (roughly)
>> this: Add one compatible for the TrEE / TrustZone interface. Then decide
>> to load or instantiate what needs to be loaded in the driver for that.
>> That (depending on maybe SoC / platform / vendor) includes installing
>> the efivar operations. This way we don't have to fill the DT with the
>> specific things running in firmware.
> 
> As far as OP-TEE is concerned, I think we can make the 'feature'
> discoverable.  I'll go propose that to op-tee but if that gets
> accepted, we don't need any extra nodes (other than the existing one),
> to wire up efivars_register().

Right. I think you (either in your patches or mails) already mentioned
having an integer ID for the implementation (or maybe implementation +
vendor?). Apart from that, I think it might also make sense to have a
bit-field similar to efi.runtime_supported_mask that tells the kernel
which functions are taken over.

So with that you could call efivars_register() based on the firmware
table in the driver for linaro,optee-tz (I assume) whether for qcom,tee
(or whatever we'd call that) we'd have to hard-code it based on some
platform/model identifier.

Regards,
Max