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

[Maximilian Luz]

Hi,

On 7/31/22 11:54, Ilias Apalodimas wrote:
> Hi Maximilian,
>
> On Thu, 28 Jul 2022 at 20:27, Maximilian Luz <luzmaximilian@xxxxxxxxx> 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