Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d5a19e17-08eb-8bd6-ea18-5da638d13622@gmail.com>
Date:   Thu, 28 Jul 2022 19:27:19 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH 4/4] dt-bindings: firmware: Add Qualcomm UEFI Secure
 Application client
Content-Language: en-US
To:     Ilias Apalodimas <ilias.apalodimas@linaro.org>
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
References: <20220723224949.1089973-1-luzmaximilian@gmail.com>
 <20220723224949.1089973-5-luzmaximilian@gmail.com>
 <20220726143005.wt4be7yo7sbd3xut@bogus>
 <829c8fee-cae5-597d-933d-784b4b57bd73@gmail.com>
 <20220726154138.74avqs6iqlzqpzjk@bogus>
 <d1bc99bb-82ce-aa6e-7fad-e9309fa1c19b@gmail.com>
 <7284953b-52bb-37ac-fbe1-1fa845c44ff9@linaro.org>
 <3d752603-365d-3a33-e13e-ca241cee9a11@gmail.com>
 <20220727132437.pjob3z2nyxsuxgam@bogus>
 <CAC_iWj+Pn+h8k=fuDHzYwqD0g4m6jGRt8sCzcz+5+rYqvz9q4w@mail.gmail.com>
 <fd922f0f-99fd-55a3-a0b5-b62ad2dbfb45@gmail.com>
 <CAC_iWjLWBJLth26ifFfHvimProHZu_w5SjQNWSH_D2Fs_JXjbA@mail.gmail.com>
 <b703f678-b2c5-cdeb-ac40-9646e043d1c3@gmail.com>
 <CAC_iWjLrntWuJUzVuRi0ZOtG6JXNwz7SbS2mrqpuTgU5TV6rQA@mail.gmail.com>
From:   Maximilian Luz <luzmaximilian@gmail.com>
In-Reply-To: <CAC_iWjLrntWuJUzVuRi0ZOtG6JXNwz7SbS2mrqpuTgU5TV6rQA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 7/28/22 18:56, Ilias Apalodimas wrote:
> On Thu, 28 Jul 2022 at 15:49, Maximilian Luz <luzmaximilian@gmail.com> 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.

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

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

Regards,
Max