Pull request for efi-2021-10-rc4

Thu Sep 9 22:08:26 CEST 2021

Hi François,

On Thu, 9 Sept 2021 at 06:17, François Ozog <francois.ozog at linaro.org> wrote:
>
> Hi
>
> I think it may be good to step back and talk about the why's rather the how's. (some elements were discussed in DeviceTree Evolution technical report)
>
> We are witnessing changes in the embedded products value chain when it is not controlled in an integrated vertical market (such as as Android for phones or Chromebooks). End-product builders (car makers, robot makers...) are willing to avoid paying over and over for the same things. One identified solution for that is to implement clear boundaries of responsibility between different supply chain providers.
>
> This led to the creation of Arm SystemReady:  a contract between firmware and OS. SystemReady defines UEFI *interface* (not to be confused with EDK2) as the functional aspects and either ACPI or DT for the hardware description. At present three non-secure open source firmware projects are targeted for SysteReady implementation: EDK2, U-Boot and LinuxBoot. SystemReady states that the hardware description is a "given" by the platform: the OS does *not* come with the one it wants to use. I read adoption by RISC V community of the EBBR recipe of SystemReady-IR to signal we are addressing an important and concrete market need.
>
> To identify the consequences on the firmware world, let's drill down into what it means for 4 identical boards that differ from the amount of soldiered memory.
>
> The current practice is often to maintain TFA, OP-TEE, U-Boot and even Linux with that information. This parallel maintenance can be as simple as a single line of code (a #define in TFA a single) or a whole structure of the memory map and assumptions crippled throughout the U-Boot code. The good things is that it avoids need for communication between firmware elements. But if we assume that the tier1 integrator is dealing with U-Boot and the board vendor provides TF-A and OP-TEE, it makes Tier 1 integrator life complicated to also deal with the memory size aspect. Another example of the problems of parallel maintenance: TrustZone is often used by the SoC/board vendor to stick platform code. But car vendors want to stick in DRM, digital wallets, insurance company, rental company trusted applications. In that case, the product maker should be able to easily control TrustZone parameters. That would be greatly facilitated by having to change only one parameter in one project. Looking at a particular case, it has been awfully difficult to update just the secure memory size: single line of codes in TFA,  U-Boot code and structures had to be changed to accommodate a secure memory size change.
>
> So if you stop thinking one single entity controls all aspects of firmware (secure, non-secure, and beyond) as well as operating systems, there are a number of things to do. And the majority of Linaro patches are delivering on making this happen.
>
> I don't say that all projects should follow that. I say that there must be a scheme to properly address SystemReady defined DT lifecycle because it is addressing a major market demand (yes not universal but major).
>
> Other considerations:
> - There may be co-existing alternate schemes such as VBE or Android Verified Boot in U-Boot. SystemReady is not the one that has to be used all the time. It serves a big market but not all markets.
> - DeviceTree abstract language can be used anywhere as a similar technology as protobufs: to represent complex information. For example I see great benefits in using this in blob lists But the lifecycle and content of the one DTB that will reach the OS must be handled as part of the firmware to OS ABI.
> - The DTB lifecycle will be the same in U-Boot and LinuxBoot so that ultimately product vendors may simply choose which non-secure firmware they want. It means that information passing between firmware components (call it HOBs or blob lists) should be standardized across non-secure firmwares.

To this point I think I agree, so far as I understand it.

(please don't use protobuf in firmware...it requires code generation)

> - U-Boot private configuration have no room in such an OS DTB (the one handover by firmware to OS). If U-Boot wants to use DTB format to store some variables that are stored in the U-Boot environment file today: perfect, but that's a separate object from OS DTB. Should U-Boot want to inject some elements through fixups in the OS DTB, fine providing that there is a standardized binding for that. - There may be questions on some elements. For instance inventory information such as part numbers, board name... Some are defined in DT, some are also defined in SMBIOS, some are defined only in SMBIOS. Based on SystemReady compliance and distro tests field feedback, it sounds like we will need to revisit this and be clearer and more exhaustive in how we deal with those parameters. Regardless of that, one need to think of who is providing the information (say the serial number) so that signing firmware is not making the process stiff and complicated (simple for an integrated vertical market, complex in multiparty value chain).

Here you are, with respect and in my opinion, going off into the weeds.

U-Boot *absolutely requires* some properties and nodes in order to
function correctly. In some special cases, this can be avoided, but in
general, it is required. Examples are the u-boot,dm- tags and the
binman image definition, The current 'config' node is there also.

Without them you cannot make use of the available U-Boot features. I
now see that this point is the root of much of the confusion and
misunderstanding of the past month or so. We must resolve it and put
it to bed.

(Honestly I cannot fathom why we would want to use SMBIOS in this day
and age :-) but that does not relate to the point at issue here)

Regards,
Simon

>
>
>
> On Thu, 9 Sept 2021 at 10:57, Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi Heinrich,
>>
>> On Sat, 4 Sept 2021 at 12:08, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>> >
>> >
>> >
>> > Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini <trini at konsulko.com>:
>> > >On Sat, Sep 04, 2021 at 07:03:48PM +0200, Heinrich Schuchardt wrote:
>> > >>
>> > >>
>> > >> Am 4. September 2021 16:37:22 MESZ schrieb Tom Rini <trini at konsulko.com>:
>> > >> >On Sat, Sep 04, 2021 at 03:08:38PM +0200, Heinrich Schuchardt wrote:
>> > >> >>
>> > >> >>
>> > >> >> Am 4. September 2021 15:01:11 MESZ schrieb Tom Rini <trini at konsulko.com>:
>> > >> >> >On Sat, Sep 04, 2021 at 11:56:47AM +0200, Heinrich Schuchardt wrote:
>> > >> >> >
>> > >> >> >> Dear Tom,
>> > >> >> >>
>> > >> >> >> The following changes since commit 94509b79b13e69c209199af0757afbde8d2ebd6d:
>> > >> >> >>
>> > >> >> >>   btrfs: Use default subvolume as filesystem root (2021-09-01 10:11:24
>> > >> >> >> -0400)
>> > >> >> >>
>> > >> >> >> are available in the Git repository at:
>> > >> >> >>
>> > >> >> >>   https://source.denx.de/u-boot/custodians/u-boot-efi.git
>> > >> >> >> tags/efi-2021-10-rc4
>> > >> >> >>
>> > >> >> >> for you to fetch changes up to 1dfa494610c5469cc28cf1f8538abf4be6c00324:
>> > >> >> >>
>> > >> >> >>   efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check
>> > >> >> >> (2021-09-04 09:15:09 +0200)
>> > >> >> >>
>> > >> >> >> ----------------------------------------------------------------
>> > >> >> >> Pull request for efi-2021-10-rc4
>> > >> >> >>
>> > >> >> >> Documentation:
>> > >> >> >>
>> > >> >> >>     Remove invalid reference to configuration variable in UEFI doc
>> > >> >> >>
>> > >> >> >> UEFI:
>> > >> >> >>
>> > >> >> >>     Parameter checks for the EFI_TCG2_PROTOCOL
>> > >> >> >>     Improve support of preseeding UEFI variables.
>> > >> >> >>     Correct the calculation of the size of loaded images.
>> > >> >> >>     Allow for UEFI images with zero VirtualSize
>> > >> >> >>
>> > >> >> >> ----------------------------------------------------------------
>> > >> >> >> Heinrich Schuchardt (5):
>> > >> >> >>       efi_loader: sections with zero VirtualSize
>> > >> >> >>       efi_loader: rounding of image size
>> > >> >> >>       efi_loader: don't load signature database from file
>> > >> >> >>       efi_loader: efi_auth_var_type for AuditMode, DeployedMode
>> > >> >> >>       efi_loader: correct determination of secure boot state
>> > >> >> >>
>> > >> >> >> Masahisa Kojima (3):
>> > >> >> >>       efi_loader: add missing parameter check for EFI_TCG2_PROTOCOL api
>> > >> >> >>       efi_loader: fix boot_service_capability_min calculation
>> > >> >> >>       efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check
>> > >> >> >
>> > >> >> >And I don't see Simon's revert in here either.  And he asked you about
>> > >> >> >that yesterday:
>> > >> >> >https://lore.kernel.org/r/CAPnjgZ3eRdjF0jb9S-cJK6y+feuyRyWf0hNkf2triB4DR4UFBQ@mail.gmail.com/
>> > >> >> >
>> > >> >> >So at this point, are you asserting there is nothing to revert?
>> > >> >>
>> > >> >> Never. Simons "revert" is breaking functionality. The concept for suporting blobs in devicetrees supplied by a prior bootstage has not been defined yet.
>> > >> >
>> > >> >And to be clearer, reverting something that was introduced in one rc in
>> > >> >a later rc isn't breaking functionality.  U-Boot releases (well, the
>> > >> >non-rc ones for sure) are on a very regular schedule.  External projects
>> > >> >may not depend on some feature introduced at -rcN unless they're willing
>> > >> >to accept that some changes could happen before release.
>> > >> >
>> > >>
>> > >> There is no value delivered by Simon's series. Neither does the image get smaller nor does it fix anything. If he wants to enforce a design, it must work for all use cases. But this requires some conceptual work.
>> > >
>> > >Yes, and what's the rush to not do the conceptual work?  If I recall
>> > >part of the thread correctly, yes, Simon didn't get his objections in
>> > >before the patches were merged, but it was early enough in the release
>> > >cycle that taking a step back and reverting was a reasonable request.
>> > >What he had said wouldn't have changed if he had gotten the email out a
>> > >few days earlier.
>> > >
>> > >So yes, please merge Simon's revert, or post and merge new more minimal
>> > >revert that brings things to the same functional end.  There are
>> > >objections to this implementation, and thus far Simon has been
>> > >responding all of the requests to better clarify all of the related code
>> > >and concepts that have been asked of him, so that in the end an
>> > >implementation that fulfills all of the technical requirements can be
>> > >created, that hopefully leaves all parties satisfied.
>> > >
>> >
>> >
>> > There is nothing wrong with the current code.
>>
>> The current code is misconceived and I did go to great effort to
>> explain that in the 'devicetree' series.
>>
>> >
>> > It is Simon's concept of blobs in devicetrees that is borked in that it ignores QEMU and any board that gets the DT from a prior boot stage.
>>
>> That's because I was completely unaware of this strange back-door
>> approach. It would have helped a lot if someone had bothered to create
>> some documentation for the design. Then I would have seen the problem
>> immediately.
>>
>> Anyway, it is now covered by the above series. The use of devicetree
>> in U-Boot is very clear, I think.
>>
>> >
>> > Simon's patches have no functional end. So what do you mean by "same functional end"?
>>
>> So, please, again, will someone apply the revert before the release
>> and people start relying on it.
>>
>> Regards,
>> Simon
>
>
>
> --
> François-Frédéric Ozog | Director Business Development
> T: +33.67221.6485
> francois.ozog at linaro.org | Skype: ffozog
>