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

From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
To: Maximilian Luz <luzmaximilian@gmail.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Ard Biesheuvel <ardb@kernel.org>,
	Konrad Dybcio <konrad.dybcio@somainline.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Steev Klimaszewski <steev@kali.org>,
	Shawn Guo <shawn.guo@linaro.org>,
	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
Subject: Re: [PATCH 4/4] dt-bindings: firmware: Add Qualcomm UEFI Secure Application client
Date: Sun, 31 Jul 2022 12:54:11 +0300	[thread overview]
Message-ID: <CAC_iWj+mEEAVzZ-_Cn9KKw6H9sUB9sz8f16neXi-wXFXWSLycg@mail.gmail.com> (raw)
In-Reply-To: <d5a19e17-08eb-8bd6-ea18-5da638d13622@gmail.com>

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.

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

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

Thanks
/Ilias
>
> Regards,
> Max