Pull request for efi-2021-10-rc4

[François Ozog]

Hi Simon,

On Thu, 9 Sept 2021 at 22:08, Simon Glass <sjg at chromium.org> wrote:

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

I think I understand your point and still view things differently. The DT
passed to the OS shall essentially be a downstream information passing
construct  (from TF-A to U-Boot, from U-Boot to TFA). Should U-Boot need
information from Linux (what you describe above) then it is about
downstream to upstream and that is configuration information. The
configuration information storage is irrelevant to the OS. So I would place
that in either U-Boot environment or U-Boot private DT.
In EDK2 or LinuxBoot, the information you refer to is kept separate from
ACPI table or DT.


> (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)
>
(Well, I admit I am not a SMBIOS fan either....if we could forget entirely
that would be great and I would prefer the OS guys to introduce proper
abstraction to get the information rather than just being a passive
passthrough for SMBIOS related information).

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


-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog at linaro.org | Skype: ffozog