* Pull request for efi-2021-10-rc4
@ 2021-09-04 9:56 Heinrich Schuchardt
2021-09-04 10:10 ` Heinrich Schuchardt
2021-09-04 13:01 ` Tom Rini
0 siblings, 2 replies; 26+ messages in thread
From: Heinrich Schuchardt @ 2021-09-04 9:56 UTC (permalink / raw)
To: Tom Rini
Cc: U-Boot Mailing List, Alexander Graf, Masahisa Kojima,
Ilias Apalodimas, AKASHI Takahiro
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
include/efi_tcg2.h | 4 +++-
include/efi_variable.h | 6 +++++-
lib/efi_loader/efi_image_loader.c | 35 +++++++++++++++++++++++++------
lib/efi_loader/efi_tcg2.c | 21 ++++++++++++++++++-
lib/efi_loader/efi_var_common.c | 43
++++++++++++++++++++++++++++++---------
lib/efi_loader/efi_var_file.c | 41
++++++++++++++++++++++---------------
lib/efi_loader/efi_variable.c | 6 +++---
7 files changed, 118 insertions(+), 38 deletions(-)
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: Pull request for efi-2021-10-rc4 2021-09-04 9:56 Pull request for efi-2021-10-rc4 Heinrich Schuchardt @ 2021-09-04 10:10 ` Heinrich Schuchardt 2021-09-06 14:31 ` Tom Rini 2021-09-04 13:01 ` Tom Rini 1 sibling, 1 reply; 26+ messages in thread From: Heinrich Schuchardt @ 2021-09-04 10:10 UTC (permalink / raw) To: Tom Rini Cc: U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro, Michal Simek Hello Tom, the doc patch was missing I have updated tag: 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 538c0f2d3798261161a28a05e445d0c85af56276: efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check (2021-09-04 12:03:57 +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 Michal Simek (1): doc: Remove information about CAPSULE_FMP_HEADER doc/develop/uefi/uefi.rst | 1 - include/efi_tcg2.h | 4 +++- include/efi_variable.h | 6 +++++- lib/efi_loader/efi_image_loader.c | 35 +++++++++++++++++++++++++------ lib/efi_loader/efi_tcg2.c | 21 ++++++++++++++++++- lib/efi_loader/efi_var_common.c | 43 ++++++++++++++++++++++++++++++--------- lib/efi_loader/efi_var_file.c | 41 ++++++++++++++++++++++--------------- lib/efi_loader/efi_variable.c | 6 +++--- 8 files changed, 118 insertions(+), 39 deletions(-) ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-04 10:10 ` Heinrich Schuchardt @ 2021-09-06 14:31 ` Tom Rini 0 siblings, 0 replies; 26+ messages in thread From: Tom Rini @ 2021-09-06 14:31 UTC (permalink / raw) To: Heinrich Schuchardt Cc: U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro, Michal Simek [-- Attachment #1: Type: text/plain, Size: 685 bytes --] On Sat, Sep 04, 2021 at 12:10:07PM +0200, Heinrich Schuchardt wrote: > Hello Tom, > > the doc patch was missing I have updated tag: > > 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 538c0f2d3798261161a28a05e445d0c85af56276: > > efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check > (2021-09-04 12:03:57 +0200) > Applied to u-boot/master, thanks! -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-04 9:56 Pull request for efi-2021-10-rc4 Heinrich Schuchardt 2021-09-04 10:10 ` Heinrich Schuchardt @ 2021-09-04 13:01 ` Tom Rini 2021-09-04 13:08 ` Heinrich Schuchardt 1 sibling, 1 reply; 26+ messages in thread From: Tom Rini @ 2021-09-04 13:01 UTC (permalink / raw) To: Heinrich Schuchardt Cc: U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro, Simon Glass [-- Attachment #1: Type: text/plain, Size: 1909 bytes --] 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? -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-04 13:01 ` Tom Rini @ 2021-09-04 13:08 ` Heinrich Schuchardt 2021-09-04 13:19 ` Tom Rini 2021-09-04 14:37 ` Tom Rini 0 siblings, 2 replies; 26+ messages in thread From: Heinrich Schuchardt @ 2021-09-04 13:08 UTC (permalink / raw) To: Tom Rini Cc: U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro, Simon Glass Am 4. September 2021 15:01:11 MESZ schrieb Tom Rini <trini@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? > >-- >Tom Never. Simons "revert" is breaking functionality. The concept for suporting blobs in devicetrees supplied by a prior bootstage has not been defined yet. See the discussion on the ML. Best regards Heinrich ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-04 13:08 ` Heinrich Schuchardt @ 2021-09-04 13:19 ` Tom Rini 2021-09-04 14:37 ` Tom Rini 1 sibling, 0 replies; 26+ messages in thread From: Tom Rini @ 2021-09-04 13:19 UTC (permalink / raw) To: Heinrich Schuchardt Cc: U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro, Simon Glass [-- Attachment #1: Type: text/plain, Size: 2697 bytes --] 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@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. > > See the discussion on the ML. Yes, I have been following the discussion, which is why I'm wondering why there's still not been a revert in your tree, to bring things back to the state of the previous release, until everyone is in something closer to agreement about how it should be handled moving forward. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-04 13:08 ` Heinrich Schuchardt 2021-09-04 13:19 ` Tom Rini @ 2021-09-04 14:37 ` Tom Rini 2021-09-04 17:03 ` Heinrich Schuchardt 1 sibling, 1 reply; 26+ messages in thread From: Tom Rini @ 2021-09-04 14:37 UTC (permalink / raw) To: Heinrich Schuchardt Cc: U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro, Simon Glass [-- Attachment #1: Type: text/plain, Size: 2728 bytes --] 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@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. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-04 14:37 ` Tom Rini @ 2021-09-04 17:03 ` Heinrich Schuchardt 2021-09-04 17:39 ` Tom Rini 0 siblings, 1 reply; 26+ messages in thread From: Heinrich Schuchardt @ 2021-09-04 17:03 UTC (permalink / raw) To: Tom Rini Cc: U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro, Simon Glass Am 4. September 2021 16:37:22 MESZ schrieb Tom Rini <trini@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@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. We already have other blobs that are not in decicetrees and Simon never complained. So why picking on this blob? Best regards Heinrich ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-04 17:03 ` Heinrich Schuchardt @ 2021-09-04 17:39 ` Tom Rini 2021-09-04 18:02 ` Heinrich Schuchardt 0 siblings, 1 reply; 26+ messages in thread From: Tom Rini @ 2021-09-04 17:39 UTC (permalink / raw) To: Heinrich Schuchardt Cc: U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro, Simon Glass [-- Attachment #1: Type: text/plain, Size: 4145 bytes --] 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@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@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. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-04 17:39 ` Tom Rini @ 2021-09-04 18:02 ` Heinrich Schuchardt 2021-09-04 18:08 ` Tom Rini 2021-09-09 8:56 ` Simon Glass 0 siblings, 2 replies; 26+ messages in thread From: Heinrich Schuchardt @ 2021-09-04 18:02 UTC (permalink / raw) To: Tom Rini Cc: U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro, Simon Glass Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini <trini@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@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@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. 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. Simon's patches have no functional end. So what do you mean by "same functional end"? Best regards Heinrich ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-04 18:02 ` Heinrich Schuchardt @ 2021-09-04 18:08 ` Tom Rini 2021-09-04 20:08 ` Ilias Apalodimas 2021-09-09 8:56 ` Simon Glass 1 sibling, 1 reply; 26+ messages in thread From: Tom Rini @ 2021-09-04 18:08 UTC (permalink / raw) To: Heinrich Schuchardt Cc: U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro, Simon Glass [-- Attachment #1: Type: text/plain, Size: 5183 bytes --] On Sat, Sep 04, 2021 at 08:02:49PM +0200, Heinrich Schuchardt wrote: > > > Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini <trini@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@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@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. > > 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. Then it should be pretty easy to get Simon to withdraw his objections, if there's such a fundamental "this is the only possible way, no changes" path forward. > Simon's patches have no functional end. So what do you mean by "same functional end"? I mean the state of the EFI subsystem, prior to the code in question being merged, without breaking the other assorted EFI changes that have come in since then. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-04 18:08 ` Tom Rini @ 2021-09-04 20:08 ` Ilias Apalodimas 2021-09-06 0:47 ` AKASHI Takahiro 2021-09-09 8:56 ` Simon Glass 0 siblings, 2 replies; 26+ messages in thread From: Ilias Apalodimas @ 2021-09-04 20:08 UTC (permalink / raw) To: Tom Rini Cc: Heinrich Schuchardt, U-Boot Mailing List, Alexander Graf, Masahisa Kojima, AKASHI Takahiro, Simon Glass Hi Tom, On Sat, 4 Sept 2021 at 21:08, Tom Rini <trini@konsulko.com> wrote: > > On Sat, Sep 04, 2021 at 08:02:49PM +0200, Heinrich Schuchardt wrote: > > > > > > Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini <trini@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@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@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. > > > > 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. > > Then it should be pretty easy to get Simon to withdraw his objections, > if there's such a fundamental "this is the only possible way, no > changes" path forward. > > > Simon's patches have no functional end. So what do you mean by "same functional end"? > > I mean the state of the EFI subsystem, prior to the code in question > being merged, without breaking the other assorted EFI changes that have > come in since then. > > -- > Tom I'll sum this up since there's many emails on the topic. The current changes move the public needed for capsule updates in U-Boot's .rodata section. When I sent this, I assumed u-boot was mapping .rodata as read only. Since it doesn't the protection I was hoping for isn't there. So security wise the two different proposals are on par. Arguably it's easier to fix .rodata instead of copying the key from the dtb and switching the pages to RO, but that's really minor. However keeping the key on the DTB has some of limitations, with the most notable being that you *must* only use CONFIG_OF_SEPARATE for your DTB, while there's four different ways available in the Kconfig (and 3 are usable on production). I've repeated enough times, that I don't mind changing the code and keeping the key on the DTB, as long as the limitations are lifted. If that means reverting the patch now and fixing it in the future, I am fine with that as well. To be honest I don't understand why this has to be set in stone. Even if we keep the current patchset and change it to the dtb in the future, that will have zero effect on the users. Once they upgrade to the newer, shinier version, their key will just be read from a different location, but that's all hidden from the user. The only they will have to change, is how they include that key on the final binary. Regards /Ilias ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-04 20:08 ` Ilias Apalodimas @ 2021-09-06 0:47 ` AKASHI Takahiro 2021-09-09 8:56 ` Simon Glass 1 sibling, 0 replies; 26+ messages in thread From: AKASHI Takahiro @ 2021-09-06 0:47 UTC (permalink / raw) To: Ilias Apalodimas Cc: Tom Rini, Heinrich Schuchardt, U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Simon Glass On Sat, Sep 04, 2021 at 11:08:28PM +0300, Ilias Apalodimas wrote: > Hi Tom, > > On Sat, 4 Sept 2021 at 21:08, Tom Rini <trini@konsulko.com> wrote: > > > > On Sat, Sep 04, 2021 at 08:02:49PM +0200, Heinrich Schuchardt wrote: > > > > > > > > > Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini <trini@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@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@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. > > > > > > 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. But it won't prevent the other systems from using their device trees. > > Then it should be pretty easy to get Simon to withdraw his objections, > > if there's such a fundamental "this is the only possible way, no > > changes" path forward. > > > > > Simon's patches have no functional end. So what do you mean by "same functional end"? > > > > I mean the state of the EFI subsystem, prior to the code in question > > being merged, without breaking the other assorted EFI changes that have > > come in since then. > > > > -- > > Tom > I'll sum this up since there's many emails on the topic. > > The current changes move the public needed for capsule updates in > U-Boot's .rodata section. > When I sent this, I assumed u-boot was mapping .rodata as read only. > Since it doesn't the protection I was hoping for isn't there. So > security wise the two different proposals are on par. Arguably it's > easier to fix .rodata instead of copying the key from the dtb and > switching the pages to RO, but that's really minor. > > However keeping the key on the DTB has some of limitations, with the > most notable being that you *must* only use CONFIG_OF_SEPARATE for > your DTB, while there's four different ways available in the Kconfig > (and 3 are usable on production). I've repeated enough times, that I > don't mind changing the code and keeping the key on the DTB, as long > as the limitations are lifted. If that means reverting the patch now > and fixing it in the future, I am fine with that as well. Different people have different requirements/assumptions on different systems. It's also true for firmware update. Some might like Ilias' approach, others are satisfied with device tree. I think that what we need to do is, instead of having a single solution, to allow users to decide which method, or even their own one, they want to use on their systems. Right? > To be honest I don't understand why this has to be set in stone. Even > if we keep the current patchset and change it to the dtb in the > future, that will have zero effect on the users. Once they upgrade to Surely it is system admin/integrators, not users, who care here. -Takahiro Akashi > the newer, shinier version, their key will just be read from a > different location, but that's all hidden from the user. The only they > will have to change, is how they include that key on the final binary. > > Regards > /Ilias ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-04 20:08 ` Ilias Apalodimas 2021-09-06 0:47 ` AKASHI Takahiro @ 2021-09-09 8:56 ` Simon Glass 2021-09-09 11:21 ` Heinrich Schuchardt 1 sibling, 1 reply; 26+ messages in thread From: Simon Glass @ 2021-09-09 8:56 UTC (permalink / raw) To: Ilias Apalodimas Cc: Tom Rini, Heinrich Schuchardt, U-Boot Mailing List, Alexander Graf, Masahisa Kojima, AKASHI Takahiro Hi Ilias, On Sat, 4 Sept 2021 at 14:09, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Tom, > > On Sat, 4 Sept 2021 at 21:08, Tom Rini <trini@konsulko.com> wrote: > > > > On Sat, Sep 04, 2021 at 08:02:49PM +0200, Heinrich Schuchardt wrote: > > > > > > > > > Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini <trini@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@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@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. > > > > > > 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. > > > > Then it should be pretty easy to get Simon to withdraw his objections, > > if there's such a fundamental "this is the only possible way, no > > changes" path forward. > > > > > Simon's patches have no functional end. So what do you mean by "same functional end"? > > > > I mean the state of the EFI subsystem, prior to the code in question > > being merged, without breaking the other assorted EFI changes that have > > come in since then. > > > > -- > > Tom > I'll sum this up since there's many emails on the topic. > > The current changes move the public needed for capsule updates in > U-Boot's .rodata section. > When I sent this, I assumed u-boot was mapping .rodata as read only. > Since it doesn't the protection I was hoping for isn't there. So > security wise the two different proposals are on par. Arguably it's > easier to fix .rodata instead of copying the key from the dtb and > switching the pages to RO, but that's really minor. > > However keeping the key on the DTB has some of limitations, with the > most notable being that you *must* only use CONFIG_OF_SEPARATE for > your DTB, while there's four different ways available in the Kconfig > (and 3 are usable on production). I've repeated enough times, that I > don't mind changing the code and keeping the key on the DTB, as long > as the limitations are lifted. If that means reverting the patch now > and fixing it in the future, I am fine with that as well. > To be honest I don't understand why this has to be set in stone. Even > if we keep the current patchset and change it to the dtb in the > future, that will have zero effect on the users. Once they upgrade to > the newer, shinier version, their key will just be read from a > different location, but that's all hidden from the user. The only they > will have to change, is how they include that key on the final binary. This is a reasonable summary I think. The devicetree series I sent explains how to deal with the limitations here. Heinrich requested that I clarify this which I did. I fully expected that the revert would then be applied. But instead it seems there is not even agreement about the status quo (of use of devicetree in U-Boot). OF_SEPARATE is used by the vast majority of boards, including most qemu builds. I think the OF_BOARD thing should probably be deleted. The OF_EMBED thing should not be used in production. It is needed with the EFI app though and I recently sent a series to support updating the DT there. For OF_PRIOR_STAGE the prior state is responsible for supplying the DT, and needs to do so and meet U-Boot's requirements. I have clearly set that out in the devicetree patch. https://patchwork.ozlabs.org/project/uboot/list/?series=260032&state=* So what has happened here is a short-cut and not the correct approach. It needs to be reverted before the release, since otherwise people will rely on it and we will be stuck with two ways to do signatures. Regards, Simon ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-09 8:56 ` Simon Glass @ 2021-09-09 11:21 ` Heinrich Schuchardt 2021-09-09 12:01 ` Tom Rini 0 siblings, 1 reply; 26+ messages in thread From: Heinrich Schuchardt @ 2021-09-09 11:21 UTC (permalink / raw) To: Simon Glass Cc: Tom Rini, U-Boot Mailing List, Alexander Graf, Masahisa Kojima, AKASHI Takahiro, Ilias Apalodimas On 9/9/21 10:56 AM, Simon Glass wrote: > Hi Ilias, > > On Sat, 4 Sept 2021 at 14:09, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: >> >> Hi Tom, >> >> On Sat, 4 Sept 2021 at 21:08, Tom Rini <trini@konsulko.com> wrote: >>> >>> On Sat, Sep 04, 2021 at 08:02:49PM +0200, Heinrich Schuchardt wrote: >>>> >>>> >>>> Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini <trini@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@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@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. >>>> >>>> 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. >>> >>> Then it should be pretty easy to get Simon to withdraw his objections, >>> if there's such a fundamental "this is the only possible way, no >>> changes" path forward. >>> >>>> Simon's patches have no functional end. So what do you mean by "same functional end"? >>> >>> I mean the state of the EFI subsystem, prior to the code in question >>> being merged, without breaking the other assorted EFI changes that have >>> come in since then. >>> >>> -- >>> Tom >> I'll sum this up since there's many emails on the topic. >> >> The current changes move the public needed for capsule updates in >> U-Boot's .rodata section. >> When I sent this, I assumed u-boot was mapping .rodata as read only. >> Since it doesn't the protection I was hoping for isn't there. So >> security wise the two different proposals are on par. Arguably it's >> easier to fix .rodata instead of copying the key from the dtb and >> switching the pages to RO, but that's really minor. >> >> However keeping the key on the DTB has some of limitations, with the >> most notable being that you *must* only use CONFIG_OF_SEPARATE for >> your DTB, while there's four different ways available in the Kconfig >> (and 3 are usable on production). I've repeated enough times, that I >> don't mind changing the code and keeping the key on the DTB, as long >> as the limitations are lifted. If that means reverting the patch now >> and fixing it in the future, I am fine with that as well. >> To be honest I don't understand why this has to be set in stone. Even >> if we keep the current patchset and change it to the dtb in the >> future, that will have zero effect on the users. Once they upgrade to >> the newer, shinier version, their key will just be read from a >> different location, but that's all hidden from the user. The only they >> will have to change, is how they include that key on the final binary. > > This is a reasonable summary I think. > > The devicetree series I sent explains how to deal with the limitations > here. Heinrich requested that I clarify this which I did. I fully > expected that the revert would then be applied. But instead it seems > there is not even agreement about the status quo (of use of devicetree > in U-Boot). > > OF_SEPARATE is used by the vast majority of boards, including most > qemu builds. I think the OF_BOARD thing should probably be deleted. > The OF_EMBED thing should not be used in production. It is needed with > the EFI app though and I recently sent a series to support updating > the DT there. > > For OF_PRIOR_STAGE the prior state is responsible for supplying the > DT, and needs to do so and meet U-Boot's requirements. I have clearly > set that out in the devicetree patch. Yes, and this was wrong. You cannot impose U-Boot's requirements on other projects. Instead I suggested to use an overlay tree to add our stuff on the devicetree from the prior stage once U-Boot is invoked. Best regards Heinrich > > https://patchwork.ozlabs.org/project/uboot/list/?series=260032&state=* > > So what has happened here is a short-cut and not the correct approach. > It needs to be reverted before the release, since otherwise people > will rely on it and we will be stuck with two ways to do signatures. > > Regards, > Simon > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-09 11:21 ` Heinrich Schuchardt @ 2021-09-09 12:01 ` Tom Rini 0 siblings, 0 replies; 26+ messages in thread From: Tom Rini @ 2021-09-09 12:01 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Simon Glass, U-Boot Mailing List, Alexander Graf, Masahisa Kojima, AKASHI Takahiro, Ilias Apalodimas [-- Attachment #1: Type: text/plain, Size: 9287 bytes --] On Thu, Sep 09, 2021 at 01:21:17PM +0200, Heinrich Schuchardt wrote: > > > On 9/9/21 10:56 AM, Simon Glass wrote: > > Hi Ilias, > > > > On Sat, 4 Sept 2021 at 14:09, Ilias Apalodimas > > <ilias.apalodimas@linaro.org> wrote: > > > > > > Hi Tom, > > > > > > On Sat, 4 Sept 2021 at 21:08, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Sat, Sep 04, 2021 at 08:02:49PM +0200, Heinrich Schuchardt wrote: > > > > > > > > > > > > > > > Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini <trini@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@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@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. > > > > > > > > > > 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. > > > > > > > > Then it should be pretty easy to get Simon to withdraw his objections, > > > > if there's such a fundamental "this is the only possible way, no > > > > changes" path forward. > > > > > > > > > Simon's patches have no functional end. So what do you mean by "same functional end"? > > > > > > > > I mean the state of the EFI subsystem, prior to the code in question > > > > being merged, without breaking the other assorted EFI changes that have > > > > come in since then. > > > > > > > > -- > > > > Tom > > > I'll sum this up since there's many emails on the topic. > > > > > > The current changes move the public needed for capsule updates in > > > U-Boot's .rodata section. > > > When I sent this, I assumed u-boot was mapping .rodata as read only. > > > Since it doesn't the protection I was hoping for isn't there. So > > > security wise the two different proposals are on par. Arguably it's > > > easier to fix .rodata instead of copying the key from the dtb and > > > switching the pages to RO, but that's really minor. > > > > > > However keeping the key on the DTB has some of limitations, with the > > > most notable being that you *must* only use CONFIG_OF_SEPARATE for > > > your DTB, while there's four different ways available in the Kconfig > > > (and 3 are usable on production). I've repeated enough times, that I > > > don't mind changing the code and keeping the key on the DTB, as long > > > as the limitations are lifted. If that means reverting the patch now > > > and fixing it in the future, I am fine with that as well. > > > To be honest I don't understand why this has to be set in stone. Even > > > if we keep the current patchset and change it to the dtb in the > > > future, that will have zero effect on the users. Once they upgrade to > > > the newer, shinier version, their key will just be read from a > > > different location, but that's all hidden from the user. The only they > > > will have to change, is how they include that key on the final binary. > > > > This is a reasonable summary I think. > > > > The devicetree series I sent explains how to deal with the limitations > > here. Heinrich requested that I clarify this which I did. I fully > > expected that the revert would then be applied. But instead it seems > > there is not even agreement about the status quo (of use of devicetree > > in U-Boot). > > > > OF_SEPARATE is used by the vast majority of boards, including most > > qemu builds. I think the OF_BOARD thing should probably be deleted. > > The OF_EMBED thing should not be used in production. It is needed with > > the EFI app though and I recently sent a series to support updating > > the DT there. > > > > For OF_PRIOR_STAGE the prior state is responsible for supplying the > > DT, and needs to do so and meet U-Boot's requirements. I have clearly > > set that out in the devicetree patch. > > Yes, and this was wrong. You cannot impose U-Boot's requirements on > other projects. This is true IF AND ONLY IF the requirements are not in a documented, reviewed and submitted binding. U-Boot is no more, but also no less, special in this regard. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-04 18:02 ` Heinrich Schuchardt 2021-09-04 18:08 ` Tom Rini @ 2021-09-09 8:56 ` Simon Glass 2021-09-09 10:52 ` Heinrich Schuchardt 2021-09-09 12:17 ` François Ozog 1 sibling, 2 replies; 26+ messages in thread From: Simon Glass @ 2021-09-09 8:56 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Tom Rini, U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro Hi Heinrich, On Sat, 4 Sept 2021 at 12:08, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini <trini@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@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@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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-09 8:56 ` Simon Glass @ 2021-09-09 10:52 ` Heinrich Schuchardt 2021-09-09 12:08 ` Tom Rini 2021-09-09 12:17 ` François Ozog 1 sibling, 1 reply; 26+ messages in thread From: Heinrich Schuchardt @ 2021-09-09 10:52 UTC (permalink / raw) To: Simon Glass Cc: Tom Rini, U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro On 9/9/21 10:56 AM, Simon Glass wrote: > Hi Heinrich, > > On Sat, 4 Sept 2021 at 12:08, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> >> >> >> Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini <trini@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@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@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 CONFIG_OF_PRIOR_STAGE is not a backdoor. And you are the DM maintainer. > 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. I see neither a design by you nor a series that covers CONFIG_OF_PRIOR_STAGE. I have suggested that if you really want to move this blob to a devicetree you could apply an overlay tree including U-Boot specific fields to the devicetree of the prior stage. Did you yet respond to this? > >> >> 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. Sure we want capsule updates. My understanding is that you want to modify the current implementation. That is fine. It has no effect on the user where a blob is stored unless you remove it as your series does. So I cannot understand your urgency. Instead collaborate with Ilias and me on the possible design change. Heinrich > > Regards, > Simon > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-09 10:52 ` Heinrich Schuchardt @ 2021-09-09 12:08 ` Tom Rini 2021-09-09 20:08 ` Simon Glass 0 siblings, 1 reply; 26+ messages in thread From: Tom Rini @ 2021-09-09 12:08 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Simon Glass, U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro [-- Attachment #1: Type: text/plain, Size: 7558 bytes --] On Thu, Sep 09, 2021 at 12:52:52PM +0200, Heinrich Schuchardt wrote: > > > On 9/9/21 10:56 AM, Simon Glass wrote: > > Hi Heinrich, > > > > On Sat, 4 Sept 2021 at 12:08, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > > > > > > > Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini <trini@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@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@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 > > CONFIG_OF_PRIOR_STAGE is not a backdoor. And you are the DM maintainer. > > > 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. > > I see neither a design by you nor a series that covers > CONFIG_OF_PRIOR_STAGE. I have suggested that if you really want to move > this blob to a devicetree you could apply an overlay tree including > U-Boot specific fields to the devicetree of the prior stage. Did you yet > respond to this? Given that I feel like "u-boot,dm-spl" and "u-boot,dm-pre-reloc" are unlikely to survive upstream review, I would like to hear what you think about applying overlays, at least in general, Simon. > > > 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. > > Sure we want capsule updates. My understanding is that you want to > modify the current implementation. That is fine. It has no effect on the > user where a blob is stored unless you remove it as your series does. So > I cannot understand your urgency. Instead collaborate with Ilias and me > on the possible design change. I believe Simon's urgency comes from the idea that once we put this method in a release we'll be stuck with using it or supporting it forever. That's certainly a general concern of mine. We can make changes between -rc1 and release and if something else decides to depend on an ABI we changed in the middle there, that was a bad idea on their part. But then it's on us once it's in a full release. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-09 12:08 ` Tom Rini @ 2021-09-09 20:08 ` Simon Glass 2021-09-09 20:44 ` Tom Rini 2021-09-09 21:22 ` Mark Kettenis 0 siblings, 2 replies; 26+ messages in thread From: Simon Glass @ 2021-09-09 20:08 UTC (permalink / raw) To: Tom Rini Cc: Heinrich Schuchardt, U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro Hi Tom, On Thu, 9 Sept 2021 at 06:08, Tom Rini <trini@konsulko.com> wrote: > > On Thu, Sep 09, 2021 at 12:52:52PM +0200, Heinrich Schuchardt wrote: > > > > > > On 9/9/21 10:56 AM, Simon Glass wrote: > > > Hi Heinrich, > > > > > > On Sat, 4 Sept 2021 at 12:08, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > > > > > > > > > > > Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini <trini@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@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@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 > > > > CONFIG_OF_PRIOR_STAGE is not a backdoor. And you are the DM maintainer. > > > > > 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. > > > > I see neither a design by you nor a series that covers > > CONFIG_OF_PRIOR_STAGE. I have suggested that if you really want to move > > this blob to a devicetree you could apply an overlay tree including > > U-Boot specific fields to the devicetree of the prior stage. Did you yet > > respond to this? > > Given that I feel like "u-boot,dm-spl" and "u-boot,dm-pre-reloc" are > unlikely to survive upstream review, I would like to hear what you think > about applying overlays, at least in general, Simon. I would be pretty disappointed if vendor,propname could not survive upstream review, given that it is in the DT spec explicitly, and that linux, is used in Linux. To answer your question, I think it is a terrible idea and would lead to much pain and misery and eventually the failure of U-Boot to function as a useful and efficient bootloader . It moves something that I think can be easily accomplished (from a technical POV anyway) at built-time into the run-time domain. Leaving aside that devicetree overlays are arguably not supported/implemented so far as the DT spec is concerned, it adds overhead to SPL and U-Boot (particularly pre-reloc) that is likely to make the whole thing infeasible. Also, somewhat off-topic, this is the first I have ever heard of the idea of U-Boot needing to put its properties in a separate place. I tried to cover this in 'Why not have two devicetrees?' here: https://patchwork.ozlabs.org/project/uboot/patch/20210828164630.81050-4-sjg@chromium.org/ How hard do we really want to make this? If a DT is provided to U-Boot, it needs to be suitable for U-Boot. The whole idea is driven from a misconception that U-Boot is just borrowing a devicetree from Linux or qemu or TF-A. So to the extent that this implies that U-Boot cannot have its own properties and nodes in the devicetree; No. No. No. U-Boot uses the devicetree for its configuration and it must be supplied based on U-Boot's requirements. I will try to send another version of the devicetree doc series. > > > > > 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. > > > > Sure we want capsule updates. My understanding is that you want to > > modify the current implementation. That is fine. It has no effect on the > > user where a blob is stored unless you remove it as your series does. So > > I cannot understand your urgency. Instead collaborate with Ilias and me > > on the possible design change. > > I believe Simon's urgency comes from the idea that once we put this > method in a release we'll be stuck with using it or supporting it > forever. That's certainly a general concern of mine. We can make > changes between -rc1 and release and if something else decides to depend > on an ABI we changed in the middle there, that was a bad idea on their > part. But then it's on us once it's in a full release. That's exactly it. I am particularly concerned since there does not seem to even be agreement (from Heinrich at least) in how U-Boot uses device tree *today*, some of which is apparently a bug to be fixed :-) Regards, Simon ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-09 20:08 ` Simon Glass @ 2021-09-09 20:44 ` Tom Rini 2021-09-09 21:22 ` Mark Kettenis 1 sibling, 0 replies; 26+ messages in thread From: Tom Rini @ 2021-09-09 20:44 UTC (permalink / raw) To: Simon Glass Cc: Heinrich Schuchardt, U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro [-- Attachment #1: Type: text/plain, Size: 10079 bytes --] On Thu, Sep 09, 2021 at 02:08:24PM -0600, Simon Glass wrote: > Hi Tom, > > On Thu, 9 Sept 2021 at 06:08, Tom Rini <trini@konsulko.com> wrote: > > > > On Thu, Sep 09, 2021 at 12:52:52PM +0200, Heinrich Schuchardt wrote: > > > > > > > > > On 9/9/21 10:56 AM, Simon Glass wrote: > > > > Hi Heinrich, > > > > > > > > On Sat, 4 Sept 2021 at 12:08, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > > > > > > > > > > > > > > > Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini <trini@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@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@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 > > > > > > CONFIG_OF_PRIOR_STAGE is not a backdoor. And you are the DM maintainer. > > > > > > > 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. > > > > > > I see neither a design by you nor a series that covers > > > CONFIG_OF_PRIOR_STAGE. I have suggested that if you really want to move > > > this blob to a devicetree you could apply an overlay tree including > > > U-Boot specific fields to the devicetree of the prior stage. Did you yet > > > respond to this? > > > > Given that I feel like "u-boot,dm-spl" and "u-boot,dm-pre-reloc" are > > unlikely to survive upstream review, I would like to hear what you think > > about applying overlays, at least in general, Simon. > > I would be pretty disappointed if vendor,propname could not survive > upstream review, given that it is in the DT spec explicitly, and that > linux, is used in Linux. > > To answer your question, I think it is a terrible idea and would lead > to much pain and misery and eventually the failure of U-Boot to > function as a useful and efficient bootloader . It moves something > that I think can be easily accomplished (from a technical POV anyway) > at built-time into the run-time domain. Leaving aside that devicetree > overlays are arguably not supported/implemented so far as the DT spec > is concerned, it adds overhead to SPL and U-Boot (particularly > pre-reloc) that is likely to make the whole thing infeasible. Perhaps for u-boot,dm-pre-reloc and dm-spl we can figure out something slightly clever? I really had hoped that by now we might know enough about what really is or isn't needed that early that we could rule-of-thumb our way out of it or something. > Also, somewhat off-topic, this is the first I have ever heard of the > idea of U-Boot needing to put its properties in a separate place. I > tried to cover this in 'Why not have two devicetrees?' here: > > https://patchwork.ozlabs.org/project/uboot/patch/20210828164630.81050-4-sjg@chromium.org/ > > How hard do we really want to make this? If a DT is provided to > U-Boot, it needs to be suitable for U-Boot. > > The whole idea is driven from a misconception that U-Boot is just > borrowing a devicetree from Linux or qemu or TF-A. > > So to the extent that this implies that U-Boot cannot have its own > properties and nodes in the devicetree; If we need properties to exist in the device tree that we're provided then it needs to be in the upstream device tree and bindings. Is u-boot,dm-spl valid and in a registered namespace? Yes. But if we expect to be able to play nicely with the whole of the device tree ecosystem then we need to document what we're doing, in the upstream binding documentation so that validators can validate it. If we expect to be given and then pass on our device tree there's not another option here. Otherwise then yes, we are just borrowing a device tree from someplace else and making it one-off. That this hasn't come up before is at least partly because it's only getting closer to the point where it seems like just maybe the OS has sufficiently complete bindings that the thought of updating the device tree infrequently is becoming feasible. > No. > > No. > > No. > > U-Boot uses the devicetree for its configuration and it must be > supplied based on U-Boot's requirements. I will try to send another > version of the devicetree doc series. Maybe we'll need to have a higher level re-think then too, at least looking forward, about what can and can't live where. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-09 20:08 ` Simon Glass 2021-09-09 20:44 ` Tom Rini @ 2021-09-09 21:22 ` Mark Kettenis 2021-09-10 10:28 ` François Ozog 1 sibling, 1 reply; 26+ messages in thread From: Mark Kettenis @ 2021-09-09 21:22 UTC (permalink / raw) To: Simon Glass Cc: trini, xypron.glpk, u-boot, agraf, masahisa.kojima, ilias.apalodimas, takahiro.akashi > From: Simon Glass <sjg@chromium.org> > Date: Thu, 9 Sep 2021 14:08:24 -0600 Hi Simon, > Hi Tom, > > On Thu, 9 Sept 2021 at 06:08, Tom Rini <trini@konsulko.com> wrote: > > > > On Thu, Sep 09, 2021 at 12:52:52PM +0200, Heinrich Schuchardt wrote: > > > > > > > > > On 9/9/21 10:56 AM, Simon Glass wrote: > > > > Hi Heinrich, > > > > > > > > On Sat, 4 Sept 2021 at 12:08, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > > > > > 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 > > > > > > CONFIG_OF_PRIOR_STAGE is not a backdoor. And you are the DM maintainer. > > > > > > > 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. > > > > > > I see neither a design by you nor a series that covers > > > CONFIG_OF_PRIOR_STAGE. I have suggested that if you really want to move > > > this blob to a devicetree you could apply an overlay tree including > > > U-Boot specific fields to the devicetree of the prior stage. Did you yet > > > respond to this? > > > > Given that I feel like "u-boot,dm-spl" and "u-boot,dm-pre-reloc" are > > unlikely to survive upstream review, I would like to hear what you think > > about applying overlays, at least in general, Simon. > > I would be pretty disappointed if vendor,propname could not survive > upstream review, given that it is in the DT spec explicitly, and that > linux, is used in Linux. Well, the Linux DT maintainers tend to be pretty thorough in their review and will resist crappy ideas ;). I think there is merit in the way we currently do things by augmenting the standard Linux device tree with these properties. At least on platforms where U-Boot is the first bit of firmware that runs (apart from ROM code). But I suspect there would be some resistence if we proposed to add these properties directly to the upstream Linux DTs instead. On the other hand I don't see why maintainers of firmware that runs before U-Boot that provides a DT to U-Boot through a CONFIG_OF_PRIOR_STAGE mechanism would never add "u-boot," properties to their device trees if they use U-Boot as a 2nd stage loader. I'm sure the device trees providied by m1n1 for the Apple M1 could contain additional nodes and "u-boot," properties if necessary. > To answer your question, I think it is a terrible idea and would lead > to much pain and misery and eventually the failure of U-Boot to > function as a useful and efficient bootloader . It moves something > that I think can be easily accomplished (from a technical POV anyway) > at built-time into the run-time domain. Leaving aside that devicetree > overlays are arguably not supported/implemented so far as the DT spec > is concerned, it adds overhead to SPL and U-Boot (particularly > pre-reloc) that is likely to make the whole thing infeasible. I still think that U-Boot TPL/SPL isn't an issue for platforms that use CONFIG_OF_PRIOR_STAGE. The QEMU example that was given is a bit artificial in that it is a configuration specifically designed for testing U-Boot SPL. Folks using a QEMU-based virtualization solution don't care about that configuration and will only use U-Boot proper. > Also, somewhat off-topic, this is the first I have ever heard of the > idea of U-Boot needing to put its properties in a separate place. I > tried to cover this in 'Why not have two devicetrees?' here: > > https://patchwork.ozlabs.org/project/uboot/patch/20210828164630.81050-4-sjg@chromium.org/ > > How hard do we really want to make this? If a DT is provided to > U-Boot, it needs to be suitable for U-Boot. But U-Boot must not make unreasonable demands. > The whole idea is driven from a misconception that U-Boot is just > borrowing a devicetree from Linux or qemu or TF-A. I diasagree that's a misconception. I've already explained why it isn't practical for U-Boot to have hardwired device trees for things like QEMU, Raspberry Pi or Apple M1. I also don't see what specific U-Boot cofiguration is needed for those platforms. Currently U-Boot functions just fine with on these platforms without having any "u-boot," properties in the DT they provide to U-Boot. Granted, I'm not trying to do any sort of verified/secure boot on those platforms. But any meaningful verified/secure boot implementation needs a high degree of agreement between the integrated firmware components anyway. > So to the extent that this implies that U-Boot cannot have its own > properties and nodes in the devicetree; > > No. > > No. > > No. > > U-Boot uses the devicetree for its configuration and it must be > supplied based on U-Boot's requirements. I will try to send another > version of the devicetree doc series. I really think this needs to be judged on a case-by-case basis. Using this as a leading principle is fine, but ruling out binary data linked into the u-boot binary itself out of principle if embedding that data into the device tree isn't practical seems counterproductive to me. Cheers, Mark ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-09 21:22 ` Mark Kettenis @ 2021-09-10 10:28 ` François Ozog 0 siblings, 0 replies; 26+ messages in thread From: François Ozog @ 2021-09-10 10:28 UTC (permalink / raw) To: Mark Kettenis Cc: Simon Glass, Tom Rini, Heinrich Schuchardt, U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, Takahiro Akashi On Thu, 9 Sept 2021 at 23:22, Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > From: Simon Glass <sjg@chromium.org> > > Date: Thu, 9 Sep 2021 14:08:24 -0600 > > Hi Simon, > > > Hi Tom, > > > > On Thu, 9 Sept 2021 at 06:08, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Thu, Sep 09, 2021 at 12:52:52PM +0200, Heinrich Schuchardt wrote: > > > > > > > > > > > > On 9/9/21 10:56 AM, Simon Glass wrote: > > > > > Hi Heinrich, > > > > > > > > > > On Sat, 4 Sept 2021 at 12:08, Heinrich Schuchardt < > xypron.glpk@gmx.de> wrote: > > > > > > > > > > > > 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 > > > > > > > > CONFIG_OF_PRIOR_STAGE is not a backdoor. And you are the DM > maintainer. > > > > > > > > > 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. > > > > > > > > I see neither a design by you nor a series that covers > > > > CONFIG_OF_PRIOR_STAGE. I have suggested that if you really want to > move > > > > this blob to a devicetree you could apply an overlay tree including > > > > U-Boot specific fields to the devicetree of the prior stage. Did you > yet > > > > respond to this? > > > > > > Given that I feel like "u-boot,dm-spl" and "u-boot,dm-pre-reloc" are > > > unlikely to survive upstream review, I would like to hear what you > think > > > about applying overlays, at least in general, Simon. > > > > I would be pretty disappointed if vendor,propname could not survive > > upstream review, given that it is in the DT spec explicitly, and that > > linux, is used in Linux. > > Well, the Linux DT maintainers tend to be pretty thorough in their > review and will resist crappy ideas ;). I think there is merit in the > way we currently do things by augmenting the standard Linux device > tree with these properties. At least on platforms where U-Boot is the > first bit of firmware that runs (apart from ROM code). But I suspect > there would be some resistence if we proposed to add these properties > directly to the upstream Linux DTs instead. > > On the other hand I don't see why maintainers of firmware that runs > before U-Boot that provides a DT to U-Boot through a > CONFIG_OF_PRIOR_STAGE mechanism would never add "u-boot," properties > to their device trees if they use U-Boot as a 2nd stage loader. I'm > sure the device trees providied by m1n1 for the Apple M1 could contain > additional nodes and "u-boot," properties if necessary. > > > To answer your question, I think it is a terrible idea and would lead > > to much pain and misery and eventually the failure of U-Boot to > > function as a useful and efficient bootloader . It moves something > > that I think can be easily accomplished (from a technical POV anyway) > > at built-time into the run-time domain. Leaving aside that devicetree > > overlays are arguably not supported/implemented so far as the DT spec > > is concerned, it adds overhead to SPL and U-Boot (particularly > > pre-reloc) that is likely to make the whole thing infeasible. > > I still think that U-Boot TPL/SPL isn't an issue for platforms that > use CONFIG_OF_PRIOR_STAGE. The QEMU example that was given is a bit > artificial in that it is a configuration specifically designed for > testing U-Boot SPL. Folks using a QEMU-based virtualization solution > don't care about that configuration and will only use U-Boot proper. > > > Also, somewhat off-topic, this is the first I have ever heard of the > > idea of U-Boot needing to put its properties in a separate place. I > > tried to cover this in 'Why not have two devicetrees?' here: > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20210828164630.81050-4-sjg@chromium.org/ > > > > How hard do we really want to make this? If a DT is provided to > > U-Boot, it needs to be suitable for U-Boot. > > But U-Boot must not make unreasonable demands. > > > The whole idea is driven from a misconception that U-Boot is just > > borrowing a devicetree from Linux or qemu or TF-A. > > I diasagree that's a misconception. I've already explained why it > isn't practical for U-Boot to have hardwired device trees for things > like QEMU, Raspberry Pi or Apple M1. I also don't see what specific > U-Boot cofiguration is needed for those platforms. Currently U-Boot > functions just fine with on these platforms without having any > "u-boot," properties in the DT they provide to U-Boot. Granted, I'm > not trying to do any sort of verified/secure boot on those platforms. > But any meaningful verified/secure boot implementation needs a high > degree of agreement between the integrated firmware components anyway. > +1 Let's also take the example of PSCI interface. This may be offered by SCP firmware sitting on an external micro-controller or by OP-TEE Trusted Application or intercepted by Qemu. Let's assume the piece of code offering the PSCI interface change and offer a new version of API. Current situation in most boards: you have to sync up all projects (TFA, OP-TEE, U-Boot, Linux) with that new information. The forward and backward compatibilities are a problem. Desired situation: a single authoritative entity (TFA? U-Boot?) is using a platform specific way to sense the offered API and conduit and injects it in the DT. > > > So to the extent that this implies that U-Boot cannot have its own > > properties and nodes in the devicetree; > > > > No. > > > > No. > > > > No. > > > > U-Boot uses the devicetree for its configuration and it must be > > supplied based on U-Boot's requirements. I will try to send another > > version of the devicetree doc series. > > I really think this needs to be judged on a case-by-case basis. Using > this as a leading principle is fine, but ruling out binary data linked > into the u-boot binary itself out of principle if embedding that data > into the device tree isn't practical seems counterproductive to me. > > Cheers, > > Mark > -- François-Frédéric Ozog | *Director Business Development* T: +33.67221.6485 francois.ozog@linaro.org | Skype: ffozog ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-09 8:56 ` Simon Glass 2021-09-09 10:52 ` Heinrich Schuchardt @ 2021-09-09 12:17 ` François Ozog 2021-09-09 20:08 ` Simon Glass 1 sibling, 1 reply; 26+ messages in thread From: François Ozog @ 2021-09-09 12:17 UTC (permalink / raw) To: Simon Glass Cc: Heinrich Schuchardt, Tom Rini, U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro 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 <https://docs.google.com/document/d/1CLkhLRaz_zcCq44DLGmPZQFPbYHOC6nzPowaL0XmRk0/edit?usp=sharing> ) 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. - 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). On Thu, 9 Sept 2021 at 10:57, Simon Glass <sjg@chromium.org> wrote: > Hi Heinrich, > > On Sat, 4 Sept 2021 at 12:08, Heinrich Schuchardt <xypron.glpk@gmx.de> > wrote: > > > > > > > > Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini <trini@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@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@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@linaro.org | Skype: ffozog ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-09 12:17 ` François Ozog @ 2021-09-09 20:08 ` Simon Glass 2021-09-10 10:40 ` François Ozog 0 siblings, 1 reply; 26+ messages in thread From: Simon Glass @ 2021-09-09 20:08 UTC (permalink / raw) To: François Ozog Cc: Heinrich Schuchardt, Tom Rini, U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro Hi François, On Thu, 9 Sept 2021 at 06:17, François Ozog <francois.ozog@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@chromium.org> wrote: >> >> Hi Heinrich, >> >> On Sat, 4 Sept 2021 at 12:08, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> > >> > >> > >> > Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini <trini@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@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@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@linaro.org | Skype: ffozog > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Pull request for efi-2021-10-rc4 2021-09-09 20:08 ` Simon Glass @ 2021-09-10 10:40 ` François Ozog 0 siblings, 0 replies; 26+ messages in thread From: François Ozog @ 2021-09-10 10:40 UTC (permalink / raw) To: Simon Glass Cc: Heinrich Schuchardt, Tom Rini, U-Boot Mailing List, Alexander Graf, Masahisa Kojima, Ilias Apalodimas, AKASHI Takahiro Hi Simon, On Thu, 9 Sept 2021 at 22:08, Simon Glass <sjg@chromium.org> wrote: > Hi François, > > On Thu, 9 Sept 2021 at 06:17, François Ozog <francois.ozog@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@chromium.org> wrote: > >> > >> Hi Heinrich, > >> > >> On Sat, 4 Sept 2021 at 12:08, Heinrich Schuchardt <xypron.glpk@gmx.de> > wrote: > >> > > >> > > >> > > >> > Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini < > trini@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@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@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@linaro.org | Skype: ffozog > > > -- François-Frédéric Ozog | *Director Business Development* T: +33.67221.6485 francois.ozog@linaro.org | Skype: ffozog ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2021-09-10 10:40 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-04 9:56 Pull request for efi-2021-10-rc4 Heinrich Schuchardt 2021-09-04 10:10 ` Heinrich Schuchardt 2021-09-06 14:31 ` Tom Rini 2021-09-04 13:01 ` Tom Rini 2021-09-04 13:08 ` Heinrich Schuchardt 2021-09-04 13:19 ` Tom Rini 2021-09-04 14:37 ` Tom Rini 2021-09-04 17:03 ` Heinrich Schuchardt 2021-09-04 17:39 ` Tom Rini 2021-09-04 18:02 ` Heinrich Schuchardt 2021-09-04 18:08 ` Tom Rini 2021-09-04 20:08 ` Ilias Apalodimas 2021-09-06 0:47 ` AKASHI Takahiro 2021-09-09 8:56 ` Simon Glass 2021-09-09 11:21 ` Heinrich Schuchardt 2021-09-09 12:01 ` Tom Rini 2021-09-09 8:56 ` Simon Glass 2021-09-09 10:52 ` Heinrich Schuchardt 2021-09-09 12:08 ` Tom Rini 2021-09-09 20:08 ` Simon Glass 2021-09-09 20:44 ` Tom Rini 2021-09-09 21:22 ` Mark Kettenis 2021-09-10 10:28 ` François Ozog 2021-09-09 12:17 ` François Ozog 2021-09-09 20:08 ` Simon Glass 2021-09-10 10:40 ` François Ozog
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.