mbox series

Pull request for efi-2023-04-rc3-2

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

Pull-request

https://source.denx.de/u-boot/custodians/u-boot-efi.git tags/efi-2023-04-rc3-2

Message

Heinrich Schuchardt Feb. 24, 2023, 10:02 a.m. UTC
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

UEFI:
	Allocate device-tree copy from high memory in bootefi command
	Correct attributes checks in SetVariable()

Other:
	Allow SPL to load main U-Boot via partition type GUID
	Test case for crc8 function

----------------------------------------------------------------
Heinrich Schuchardt (5):
       disk: accessors for conditional partition fields
       kconfig: new macro IF_ENABLED()
       spl: allow loading via partition type GUID
       test: unit test for crc8
       cmd: bootefi: allocate device-tree copy from high memory

Masahisa Kojima (1):
       efi_loader: update SetVariable attribute check

Vincent Stehlé (1):
       doc: uefi: fix links

  cmd/bootefi.c                    | 19 +++----------------
  common/spl/Kconfig               | 27 ++++++++++++++++++++++-----
  common/spl/spl_mmc.c             | 18 ++++++++++++++++++
  doc/develop/uefi/fwu_updates.rst |  3 ++-
  doc/develop/uefi/uefi.rst        |  4 ++--
  include/linux/kconfig.h          |  7 +++++++
  include/part.h                   | 36 ++++++++++++++++++++++++++++++++++++
  lib/efi_loader/efi_variable.c    | 31 +++++++++++++++++++++++++------
  test/lib/Makefile                |  1 +
  test/lib/test_crc8.c             | 29 +++++++++++++++++++++++++++++
  10 files changed, 145 insertions(+), 30 deletions(-)
  create mode 100644 test/lib/test_crc8.c

Comments

Tom Rini Feb. 24, 2023, 2:42 p.m. UTC | #1
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.
Heinrich Schuchardt Feb. 24, 2023, 2:44 p.m. UTC | #2
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.
>
Ilias Apalodimas Feb. 24, 2023, 2:52 p.m. UTC | #3
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.
> >
>
Tom Rini Feb. 24, 2023, 3:15 p.m. UTC | #4
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.
Tom Rini Feb. 24, 2023, 3:16 p.m. UTC | #5
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?