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

[Ilias Apalodimas]

On Thu, 28 Jul 2022 at 15:49, Maximilian Luz <luzmaximilian@xxxxxxxxx> wrote:
>

[...]

> >>
> >>> 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.

Not users.  EFI firmware implementations that want to support this in
a generic way.

> 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.

I really prefer this to adding it on a DT, but I am not that picky.
Your email raises an important topic of replacing runtime services
with OS specific ones,  which is unfortunately very much needed and we
should fix that.

Thanks
/Ilias
>
> Regards,
> Max