Message ID | 2dd2cacb-607f-2dc3-cbdf-acd7e586d28a@canonical.com |
---|---|
State | Changes Requested |
Delegated to: | Tom Rini |
Headers | show |
Series | Pull request for efi-2023-04-rc3-2 | expand |
On Fri, Feb 24, 2023 at 11:02:57AM +0100, Heinrich Schuchardt wrote: > The following changes since commit 8c39999acb726ef083d3d5de12f20318ee0e5070: > > Merge branch 'master' of > https://source.denx.de/u-boot/custodians/u-boot-usb (2023-02-22 13:36:29 > -0500) > > are available in the Git repository at: > > https://source.denx.de/u-boot/custodians/u-boot-efi.git > tags/efi-2023-04-rc3-2 > > for you to fetch changes up to 0bb5d6b1e4105ec4215239958830a396658bfed8: > > cmd: bootefi: allocate device-tree copy from high memory (2023-02-24 > 07:44:35 +0100) > > Gitlab CI showed no issues: > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/15340 > > ---------------------------------------------------------------- > Pull request for efi-2023-04-rc3-2 > > Documentation: > Fix links to fwu_updates.rst This is a fix. > > UEFI: > Allocate device-tree copy from high memory in bootefi command This I think counts as a fix, but I'm a little confused about allocate in this context. Can you point me at the patch please? > Correct attributes checks in SetVariable() This is a fix.. > Other: > Allow SPL to load main U-Boot via partition type GUID This is new functionality, and I didn't quite see that everyone agreed this was a good idea? Not for master at this point in the cycle. > Test case for crc8 function This I suppose can count as a fix.
On 2/24/23 15:42, Tom Rini wrote: > On Fri, Feb 24, 2023 at 11:02:57AM +0100, Heinrich Schuchardt wrote: >> The following changes since commit 8c39999acb726ef083d3d5de12f20318ee0e5070: >> >> Merge branch 'master' of >> https://source.denx.de/u-boot/custodians/u-boot-usb (2023-02-22 13:36:29 >> -0500) >> >> are available in the Git repository at: >> >> https://source.denx.de/u-boot/custodians/u-boot-efi.git >> tags/efi-2023-04-rc3-2 >> >> for you to fetch changes up to 0bb5d6b1e4105ec4215239958830a396658bfed8: >> >> cmd: bootefi: allocate device-tree copy from high memory (2023-02-24 >> 07:44:35 +0100) >> >> Gitlab CI showed no issues: >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/15340 >> >> ---------------------------------------------------------------- >> Pull request for efi-2023-04-rc3-2 >> >> Documentation: >> Fix links to fwu_updates.rst > > This is a fix. > >> >> UEFI: >> Allocate device-tree copy from high memory in bootefi command > > This I think counts as a fix, but I'm a little confused about allocate > in this context. Can you point me at the patch please? > >> Correct attributes checks in SetVariable() > > This is a fix.. > >> Other: >> Allow SPL to load main U-Boot via partition type GUID > > This is new functionality, and I didn't quite see that everyone agreed > this was a good idea? Not for master at this point in the cycle. Would you take it for next? Best regards Heinrich > >> Test case for crc8 function > > This I suppose can count as a fix. >
Hi all, Just to put some context On Fri, 24 Feb 2023 at 16:44, Heinrich Schuchardt <heinrich.schuchardt@canonical.com> wrote: > > On 2/24/23 15:42, Tom Rini wrote: > > On Fri, Feb 24, 2023 at 11:02:57AM +0100, Heinrich Schuchardt wrote: > >> The following changes since commit 8c39999acb726ef083d3d5de12f20318ee0e5070: > >> > >> Merge branch 'master' of > >> https://source.denx.de/u-boot/custodians/u-boot-usb (2023-02-22 13:36:29 > >> -0500) > >> > >> are available in the Git repository at: > >> > >> https://source.denx.de/u-boot/custodians/u-boot-efi.git > >> tags/efi-2023-04-rc3-2 > >> > >> for you to fetch changes up to 0bb5d6b1e4105ec4215239958830a396658bfed8: > >> > >> cmd: bootefi: allocate device-tree copy from high memory (2023-02-24 > >> 07:44:35 +0100) > >> > >> Gitlab CI showed no issues: > >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/15340 > >> > >> ---------------------------------------------------------------- > >> Pull request for efi-2023-04-rc3-2 > >> > >> Documentation: > >> Fix links to fwu_updates.rst > > > > This is a fix. > > > >> > >> UEFI: > >> Allocate device-tree copy from high memory in bootefi command > > > > This I think counts as a fix, but I'm a little confused about allocate > > in this context. Can you point me at the patch please? https://source.denx.de/u-boot/custodians/u-boot-efi/-/commit/0bb5d6b1e4105ec4215239958830a396658bfed8 The TL;DR is that we were placing the DT on 127mb. This might overwrite previously loaded binaries. The efi-stub of the linux kernel doesn't use the DTB in place. Instead, it copies it and adds some nodes for the efi-stub -> kernel proper handover (e.g a kaslr-seed). So we as far as linux is concerned we are free to place the DT wherever we want. Unless people from risc-v want this in *now*, we are better of landing this in -next. > > > >> Correct attributes checks in SetVariable() This can wait. This fixes one minor issue on the SystemReady Security extensions test suite. This is a test suite (mostly running as EFI apps) from Arm to check conformance for UEFI Secure Boot and Measured boot. > > > > This is a fix.. > > > >> Other: > >> Allow SPL to load main U-Boot via partition type GUID > > > > This is new functionality, and I didn't quite see that everyone agreed > > this was a good idea? Not for master at this point in the cycle. > > Would you take it for next? Regards /Ilias > > Best regards > > Heinrich > > > > >> Test case for crc8 function > > > > This I suppose can count as a fix. > > >
On Fri, Feb 24, 2023 at 04:52:06PM +0200, Ilias Apalodimas wrote: > Hi all, > > Just to put some context > > On Fri, 24 Feb 2023 at 16:44, Heinrich Schuchardt > <heinrich.schuchardt@canonical.com> wrote: > > > > On 2/24/23 15:42, Tom Rini wrote: > > > On Fri, Feb 24, 2023 at 11:02:57AM +0100, Heinrich Schuchardt wrote: > > >> The following changes since commit 8c39999acb726ef083d3d5de12f20318ee0e5070: > > >> > > >> Merge branch 'master' of > > >> https://source.denx.de/u-boot/custodians/u-boot-usb (2023-02-22 13:36:29 > > >> -0500) > > >> > > >> are available in the Git repository at: > > >> > > >> https://source.denx.de/u-boot/custodians/u-boot-efi.git > > >> tags/efi-2023-04-rc3-2 > > >> > > >> for you to fetch changes up to 0bb5d6b1e4105ec4215239958830a396658bfed8: > > >> > > >> cmd: bootefi: allocate device-tree copy from high memory (2023-02-24 > > >> 07:44:35 +0100) > > >> > > >> Gitlab CI showed no issues: > > >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/15340 > > >> > > >> ---------------------------------------------------------------- > > >> Pull request for efi-2023-04-rc3-2 > > >> > > >> Documentation: > > >> Fix links to fwu_updates.rst > > > > > > This is a fix. > > > > > >> > > >> UEFI: > > >> Allocate device-tree copy from high memory in bootefi command > > > > > > This I think counts as a fix, but I'm a little confused about allocate > > > in this context. Can you point me at the patch please? > > https://source.denx.de/u-boot/custodians/u-boot-efi/-/commit/0bb5d6b1e4105ec4215239958830a396658bfed8 > > The TL;DR is that we were placing the DT on 127mb. This might > overwrite previously loaded binaries. The efi-stub of the linux kernel > doesn't use the DTB in place. Instead, it copies it and adds some > nodes for the efi-stub -> kernel proper handover (e.g a kaslr-seed). > So we as far as linux is concerned we are free to place the DT > wherever we want. > > Unless people from risc-v want this in *now*, we are better of landing > this in -next. Ah, thanks. So the sinking feeling that I had that we had duplicated the complex logic of "where _IS_ a safe place in memory to put things" was done again, but without being configurable, was right. Sigh. I don't have an idea on here's how we solve it, but I do worry we're going to have, once again, some of the overlap and silent corruption problems of old. But maybe not this time, since we're finally getting payloads that we can determine the BSS of, etc. > > >> Correct attributes checks in SetVariable() > > This can wait. This fixes one minor issue on the SystemReady Security > extensions test suite. This is a test suite (mostly running as EFI > apps) from Arm to check conformance for UEFI Secure Boot and Measured > boot. Ah, OK.
On Fri, Feb 24, 2023 at 03:44:19PM +0100, Heinrich Schuchardt wrote: > On 2/24/23 15:42, Tom Rini wrote: > > On Fri, Feb 24, 2023 at 11:02:57AM +0100, Heinrich Schuchardt wrote: > > > The following changes since commit 8c39999acb726ef083d3d5de12f20318ee0e5070: > > > > > > Merge branch 'master' of > > > https://source.denx.de/u-boot/custodians/u-boot-usb (2023-02-22 13:36:29 > > > -0500) > > > > > > are available in the Git repository at: > > > > > > https://source.denx.de/u-boot/custodians/u-boot-efi.git > > > tags/efi-2023-04-rc3-2 > > > > > > for you to fetch changes up to 0bb5d6b1e4105ec4215239958830a396658bfed8: > > > > > > cmd: bootefi: allocate device-tree copy from high memory (2023-02-24 > > > 07:44:35 +0100) > > > > > > Gitlab CI showed no issues: > > > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/15340 > > > > > > ---------------------------------------------------------------- > > > Pull request for efi-2023-04-rc3-2 > > > > > > Documentation: > > > Fix links to fwu_updates.rst > > > > This is a fix. > > > > > > > > UEFI: > > > Allocate device-tree copy from high memory in bootefi command > > > > This I think counts as a fix, but I'm a little confused about allocate > > in this context. Can you point me at the patch please? > > > > > Correct attributes checks in SetVariable() > > > > This is a fix.. > > > > > Other: > > > Allow SPL to load main U-Boot via partition type GUID > > > > This is new functionality, and I didn't quite see that everyone agreed > > this was a good idea? Not for master at this point in the cycle. > > Would you take it for next? The last feedback I saw was that the idea isn't exactly workable and presents its own challenges for distributions / OSes, or did I miss some further messages?