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: <b703f678-b2c5-cdeb-ac40-9646e043d1c3@gmail.com>
Date:   Thu, 28 Jul 2022 14:49:38 +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>
From:   Maximilian Luz <luzmaximilian@gmail.com>
In-Reply-To: <CAC_iWjLWBJLth26ifFfHvimProHZu_w5SjQNWSH_D2Fs_JXjbA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 7/28/22 14:35, Ilias Apalodimas wrote:
> Hi Maximilian
> 
> On Thu, 28 Jul 2022 at 13:48, Maximilian Luz <luzmaximilian@gmail.com> wrote:
>>
>> On 7/28/22 08:03, Ilias Apalodimas wrote:
>>> Hi all,
>>>
>>> On Wed, 27 Jul 2022 at 16:24, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>>
>>>> On Wed, Jul 27, 2022 at 03:03:49PM +0200, Maximilian Luz wrote:
>>>>>
>>>>> Is there really a good way around it?
>>>>
>>>> Yes rely on the firmware preferably auto discover, if that is not an option,
>>>> how about query. It seem to be working in your case.
>>>
>>> That's a good point.  We have a similar situation with some Arm
>>> devices and U-Boot.  Let me try to explain a bit.
>>>
>>> There's code plugged in in OP-TEE and U-Boot atm which allows you to
>>> store EFI variables on an RPMB.  This is a nice alternative if your
>>> device doesn't have any other secure storage,  however it presents
>>> some challenges after ExitBootServices, similar to the ones you have
>>> here.
>>>
>>> The eMMC controller usually lives in the non-secure world.  OP-TEE
>>> can't access that, so it relies on a userspace supplicant to perform
>>> the RPMB accesses.  That supplicant is present in U-Boot and
>>> Get/SetVariable works fine before ExitBootServices.  Once Linux boots,
>>>    the 'U-Boot supplicant' goes away and we launch the linux equivalent
>>> one from userspace.  Since variable accessing is a runtime service and
>>> it still has to go through the firmware we can't use those anymore
>>> since U-Boot doesn't preserve the supplicant, the eMMC driver and the
>>> OP-TEE portions needed in the runtime section(and even if it did we
>>> would now have 2 drivers racing to access the same hardware).  Instead
>>> U-Boot copies the variables in runtime memory and
>>> GetVariable/GetNextVariable still works, but SetVariable returns
>>> EFI_UNSUPPORTED.
>>>
>>> I've spent enough time looking at available solutions and although
>>> this indeed breaks the EFI spec, something along the lines of
>>> replacing the runtime services with ones that give you direct access
>>> to the secure world, completely bypassing the firmware is imho our
>>> least bad option.
>>
>> This sounds very similar to what Qualcomm may be doing on some devices.
>> The TrEE interface allows for callbacks and there are indications that
>> one such callback-service is for RPMB. I believe that at least on some
>> platforms, Qualcomm also stores UEFI variables in RPMB and uses the same
>> uefisecapp interface in combination with RPMB listeners installed by the
>> kernel to access them.
>>
>>> 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. 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.

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

Regards,
Max