[PATCH 0/2] powerpc-ieee1275: support larger core.elf images

[PATCH 0/2] powerpc-ieee1275: support larger core.elf images

[Daniel Axtens]

Red Hat recently experienced an issue which I narrowed down to using a
grub core.elf image that was too big: the current limit is just under
2MB.

These are two patches that fix the issue and attempt to future proof
the area.

With these patches, the maximum size will be:

 - SLOF, or PFW with the PowerPC IEEE1275 note: just under 8MB
 - PFW without the PowerPC IEEE1275 note: a bit under 4MB

As such, I recommend people building large grub images for Power
systems (such as secure bootable images) build with the PowerPC
IEEE1275 note from now on.

Daniel Axtens (2):
  powerpc-ieee1275: load grub at 8MB, not 2MB
  ieee1275: set real-base in the PowerPC IEEE1275 Note to 32MB

 grub-core/Makefile.core.def | 2 +-
 include/grub/offsets.h      | 2 +-
 util/grub-mkimagexx.c       | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.32.0

[PATCH 1/2] powerpc-ieee1275: load grub at 8MB, not 2MB

[Daniel Axtens]

RHEL9.0 builds were failing to boot with bizzare errors about module
licensing.

This was first reported under PFW but reproduces under SLOF.

What is going wrong?
--------------------

Much debugging later, it turned out that this was because their image was
greater than 2MB - 16KB in size:

 - The core.elf was 2126152 = 0x207148 bytes in size with the following
   program headers (per readelf):

Entry point 0x200000
There are 4 program headers, starting at offset 52

Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD           0x000160 0x00200000 0x00200000 0x21f98 0x2971c RWE 0x8
  GNU_STACK      0x0220f8 0x00000000 0x00000000 0x00000 0x00000 RWE 0x4
  LOAD           0x0220f8 0x00232000 0x00232000 0x1e4e50 0x1e4e50 RWE 0x4
  NOTE           0x206f48 0x00000000 0x00000000 0x00200 0x00000 R   0x4

 - SLOF places the ELF file at 0x4000 (after the reserved space for
   interrupt handlers etc.) upwards. The image was 2126152 = 0x207148
   bytes in size, so it runs from 0x4000 - 0x20b148. We'll call 0x4000 the
   load address.

0x0        0x4000         0x20b148
 |----------|--------------|
 | reserved | ELF contents |

 - SLOF then copies the first LOAD program header (for .text). That runs
   for 0x21f98 bytes. It runs from
      (load addr + 0x160) to (load addr + 0x160 + 0x21f98)
    = 0x4160 to 0x260f8
   and we copy it to 0x200000 to 0x221f98. This overwrites the end of the
   image:

0x0       0x4000     0x200000        0x221f98
 |----------|------------|---------------|
 | reserved | ELF cont.. | .text section |

 - SLOF zeros the bss up to PhysAddr + MemSize = 0x22971c

0x0       0x4000      0x200000       0x221f98 0x22971c
 |----------|------------|---------------|--------|
 | reserved | ELF cont.. | .text section | bss 0s |

 - SLOF then goes to fulfil the next LOAD header (for mods), which is
   for 0x1e4e50 bytes. We copy from
      (load addr + 0x220f8) to (load addr + 0x220f8 + 0x1e4e50)
    = 0x260f8 to 0x20af48
   and we copy it to 0x232000 to 0x416e50:

0x0       0x4000      0x200000       0x221f98 0x22971c
 |----------|------------|---------------|--------|
 | reserved | ELF cont.. | .text section | bss 0s |
               |-------------|
               | copied area |
            0x260f8      0x20af48

   This goes poorly:

0x0       0x4000      0x200000       0x221f98 0x22971c 0x232000 0x40bf08      0x416e50
 |----------|------------|---------------|--------|-----|-----------|-------------|
 | reserved | ELF cont.. | .text section | bss 0s | pad | some mods | .text start |

This matches the observations on the running system - 0x40bf08 was where
the contents of memory no longer matched the contents of the ELF file.

This was reported as a license verification failure on SLOF as the
last module's .module_license section fell past where the corruption
began.

load-base
---------

From LoPAR:

B.10.1 Load Address

  The client’s load address is specified by the value of the load-base
  Configuration Variable. The value of load-base defines the default
  load address for client programs when using the load
  method. Load-base shall be a real address in real mode or a virtual
  address in virtual mode. Note that this address represents the area,
  within the first LMB, into which the client program file will be
  read by load; it does not correspond to the addresses at which the
  program will be executed. All of physical memory from load-base to
  either the start of OF physical memory or the end of physical
  memory, whichever comes first, shall be available for loading the
  client program.

    Note: The load-base address represents the area into which the
          client program will be read by load and does not correspond
          to the address at which the program will be executed.

We can specify load-base via setting a configuration variable, via
CAS, or via an 1275 PowerPC Note.

We can indeed override load-base and successfully load such a binary
in SLOF. Drop to the OF shell and run:

0 > a000000 to load-base-override
0 > boot

Sadly, asking people to drop to OF is not a super practical
solution. SLOF ignores the 1275 PowerPC Note, which is the only thing
we can (sanely) set in grub rather than in OF.

PFW's load-base also defaults to 0x4000 but will honour the note. (The
PFW team has been asking me to get grub to include the note and set
appropriate values. See the next patch.)

Fixing this
-----------

Unless I'm mistaken, if we want backwards compat with SLOF, we will
need to change grub so that we ask the linker instruct the loader to
load the program data somewhere else, giving us more headroom for the
ELF image.

This is fairly straight-forward to do. I've picked 8MB as that's a
fairly common size for the PReP partition. We could potentially pick
4MB, I think I've seen some partitions that size too.

The tests with powerpc32 using the OpenBIOS OF implementation seem to
still work. I have no 32-bit Apple hardware to test on.

Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 grub-core/Makefile.core.def | 2 +-
 include/grub/offsets.h      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 8022e1c0a794..ce338def0f84 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -89,7 +89,7 @@ kernel = {
   i386_xen_pvh_ldflags     = '$(TARGET_IMG_BASE_LDOPT),0x100000';
 
   mips_loongson_ldflags    = '-Wl,-Ttext,0x80200000';
-  powerpc_ieee1275_ldflags = '-Wl,-Ttext,0x200000';
+  powerpc_ieee1275_ldflags = '-Wl,-Ttext,0x800000';
   sparc64_ieee1275_ldflags = '-Wl,-Ttext,0x4400';
   mips_arc_ldflags    = '-Wl,-Ttext,$(TARGET_LINK_ADDR)';
   mips_qemu_mips_ldflags    = '-Wl,-Ttext,0x80200000';
diff --git a/include/grub/offsets.h b/include/grub/offsets.h
index 871e1cd4c38c..8b2d6ab971be 100644
--- a/include/grub/offsets.h
+++ b/include/grub/offsets.h
@@ -63,7 +63,7 @@
 #define GRUB_KERNEL_SPARC64_IEEE1275_LINK_ADDR 0x4400
 
 #define GRUB_KERNEL_POWERPC_IEEE1275_LINK_ALIGN 4
-#define GRUB_KERNEL_POWERPC_IEEE1275_LINK_ADDR 0x200000
+#define GRUB_KERNEL_POWERPC_IEEE1275_LINK_ADDR 0x800000
 
 #define GRUB_KERNEL_MIPS_LOONGSON_LINK_ADDR         0x80200000
 
-- 
2.32.0

Re: [PATCH 1/2] powerpc-ieee1275: load grub at 8MB, not 2MB

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 5555 bytes --]

Daniel Axtens <dja@axtens.net> writes:

> RHEL9.0 builds were failing to boot with bizzare errors about module
> licensing.
>
> This was first reported under PFW but reproduces under SLOF.
>
> What is going wrong?
> --------------------
>
> Much debugging later, it turned out that this was because their image was
> greater than 2MB - 16KB in size:
>
>  - The core.elf was 2126152 = 0x207148 bytes in size with the following
>    program headers (per readelf):
>
> Entry point 0x200000
> There are 4 program headers, starting at offset 52
>
> Program Headers:
>   Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>   LOAD           0x000160 0x00200000 0x00200000 0x21f98 0x2971c RWE 0x8
>   GNU_STACK      0x0220f8 0x00000000 0x00000000 0x00000 0x00000 RWE 0x4
>   LOAD           0x0220f8 0x00232000 0x00232000 0x1e4e50 0x1e4e50 RWE 0x4
>   NOTE           0x206f48 0x00000000 0x00000000 0x00200 0x00000 R   0x4
>
>  - SLOF places the ELF file at 0x4000 (after the reserved space for
>    interrupt handlers etc.) upwards. The image was 2126152 = 0x207148
>    bytes in size, so it runs from 0x4000 - 0x20b148. We'll call 0x4000 the
>    load address.
>
> 0x0        0x4000         0x20b148
>  |----------|--------------|
>  | reserved | ELF contents |
>
>  - SLOF then copies the first LOAD program header (for .text). That runs
>    for 0x21f98 bytes. It runs from
>       (load addr + 0x160) to (load addr + 0x160 + 0x21f98)
>     = 0x4160 to 0x260f8
>    and we copy it to 0x200000 to 0x221f98. This overwrites the end of the
>    image:
>
> 0x0       0x4000     0x200000        0x221f98
>  |----------|------------|---------------|
>  | reserved | ELF cont.. | .text section |
>
>  - SLOF zeros the bss up to PhysAddr + MemSize = 0x22971c
>
> 0x0       0x4000      0x200000       0x221f98 0x22971c
>  |----------|------------|---------------|--------|
>  | reserved | ELF cont.. | .text section | bss 0s |
>
>  - SLOF then goes to fulfil the next LOAD header (for mods), which is
>    for 0x1e4e50 bytes. We copy from
>       (load addr + 0x220f8) to (load addr + 0x220f8 + 0x1e4e50)
>     = 0x260f8 to 0x20af48
>    and we copy it to 0x232000 to 0x416e50:
>
> 0x0       0x4000      0x200000       0x221f98 0x22971c
>  |----------|------------|---------------|--------|
>  | reserved | ELF cont.. | .text section | bss 0s |
>                |-------------|
>                | copied area |
>             0x260f8      0x20af48
>
>    This goes poorly:
>
> 0x0       0x4000      0x200000       0x221f98 0x22971c 0x232000 0x40bf08      0x416e50
>  |----------|------------|---------------|--------|-----|-----------|-------------|
>  | reserved | ELF cont.. | .text section | bss 0s | pad | some mods | .text start |
>
> This matches the observations on the running system - 0x40bf08 was where
> the contents of memory no longer matched the contents of the ELF file.
>
> This was reported as a license verification failure on SLOF as the
> last module's .module_license section fell past where the corruption
> began.
>
> load-base
> ---------
>
> From LoPAR:
>
> B.10.1 Load Address
>
>   The client’s load address is specified by the value of the load-base
>   Configuration Variable. The value of load-base defines the default
>   load address for client programs when using the load
>   method. Load-base shall be a real address in real mode or a virtual
>   address in virtual mode. Note that this address represents the area,
>   within the first LMB, into which the client program file will be
>   read by load; it does not correspond to the addresses at which the
>   program will be executed. All of physical memory from load-base to
>   either the start of OF physical memory or the end of physical
>   memory, whichever comes first, shall be available for loading the
>   client program.
>
>     Note: The load-base address represents the area into which the
>           client program will be read by load and does not correspond
>           to the address at which the program will be executed.
>
> We can specify load-base via setting a configuration variable, via
> CAS, or via an 1275 PowerPC Note.
>
> We can indeed override load-base and successfully load such a binary
> in SLOF. Drop to the OF shell and run:
>
> 0 > a000000 to load-base-override
> 0 > boot
>
> Sadly, asking people to drop to OF is not a super practical
> solution. SLOF ignores the 1275 PowerPC Note, which is the only thing
> we can (sanely) set in grub rather than in OF.
>
> PFW's load-base also defaults to 0x4000 but will honour the note. (The
> PFW team has been asking me to get grub to include the note and set
> appropriate values. See the next patch.)
>
> Fixing this
> -----------
>
> Unless I'm mistaken, if we want backwards compat with SLOF, we will
> need to change grub so that we ask the linker instruct the loader to
> load the program data somewhere else, giving us more headroom for the
> ELF image.
>
> This is fairly straight-forward to do. I've picked 8MB as that's a
> fairly common size for the PReP partition. We could potentially pick
> 4MB, I think I've seen some partitions that size too.
>
> The tests with powerpc32 using the OpenBIOS OF implementation seem to
> still work. I have no 32-bit Apple hardware to test on.
>
> Signed-off-by: Daniel Axtens <dja@axtens.net>

Shipping this one already.

Reviewed-by: Robbie Harwood <rharwood@redhat.com>

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH 1/2] powerpc-ieee1275: load grub at 8MB, not 2MB

[Vladimir 'phcoder' Serbinenko]

Was it tested on powerpc-based Macs? They're still useful for
debugging and developpement despite their age

On Tue, Nov 16, 2021 at 4:42 AM Daniel Axtens <dja@axtens.net> wrote:
>
> RHEL9.0 builds were failing to boot with bizzare errors about module
> licensing.
>
> This was first reported under PFW but reproduces under SLOF.
>
> What is going wrong?
> --------------------
>
> Much debugging later, it turned out that this was because their image was
> greater than 2MB - 16KB in size:
>
>  - The core.elf was 2126152 = 0x207148 bytes in size with the following
>    program headers (per readelf):
>
> Entry point 0x200000
> There are 4 program headers, starting at offset 52
>
> Program Headers:
>   Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>   LOAD           0x000160 0x00200000 0x00200000 0x21f98 0x2971c RWE 0x8
>   GNU_STACK      0x0220f8 0x00000000 0x00000000 0x00000 0x00000 RWE 0x4
>   LOAD           0x0220f8 0x00232000 0x00232000 0x1e4e50 0x1e4e50 RWE 0x4
>   NOTE           0x206f48 0x00000000 0x00000000 0x00200 0x00000 R   0x4
>
>  - SLOF places the ELF file at 0x4000 (after the reserved space for
>    interrupt handlers etc.) upwards. The image was 2126152 = 0x207148
>    bytes in size, so it runs from 0x4000 - 0x20b148. We'll call 0x4000 the
>    load address.
>
> 0x0        0x4000         0x20b148
>  |----------|--------------|
>  | reserved | ELF contents |
>
>  - SLOF then copies the first LOAD program header (for .text). That runs
>    for 0x21f98 bytes. It runs from
>       (load addr + 0x160) to (load addr + 0x160 + 0x21f98)
>     = 0x4160 to 0x260f8
>    and we copy it to 0x200000 to 0x221f98. This overwrites the end of the
>    image:
>
> 0x0       0x4000     0x200000        0x221f98
>  |----------|------------|---------------|
>  | reserved | ELF cont.. | .text section |
>
>  - SLOF zeros the bss up to PhysAddr + MemSize = 0x22971c
>
> 0x0       0x4000      0x200000       0x221f98 0x22971c
>  |----------|------------|---------------|--------|
>  | reserved | ELF cont.. | .text section | bss 0s |
>
>  - SLOF then goes to fulfil the next LOAD header (for mods), which is
>    for 0x1e4e50 bytes. We copy from
>       (load addr + 0x220f8) to (load addr + 0x220f8 + 0x1e4e50)
>     = 0x260f8 to 0x20af48
>    and we copy it to 0x232000 to 0x416e50:
>
> 0x0       0x4000      0x200000       0x221f98 0x22971c
>  |----------|------------|---------------|--------|
>  | reserved | ELF cont.. | .text section | bss 0s |
>                |-------------|
>                | copied area |
>             0x260f8      0x20af48
>
>    This goes poorly:
>
> 0x0       0x4000      0x200000       0x221f98 0x22971c 0x232000 0x40bf08      0x416e50
>  |----------|------------|---------------|--------|-----|-----------|-------------|
>  | reserved | ELF cont.. | .text section | bss 0s | pad | some mods | .text start |
>
> This matches the observations on the running system - 0x40bf08 was where
> the contents of memory no longer matched the contents of the ELF file.
>
> This was reported as a license verification failure on SLOF as the
> last module's .module_license section fell past where the corruption
> began.
>
> load-base
> ---------
>
> From LoPAR:
>
> B.10.1 Load Address
>
>   The client’s load address is specified by the value of the load-base
>   Configuration Variable. The value of load-base defines the default
>   load address for client programs when using the load
>   method. Load-base shall be a real address in real mode or a virtual
>   address in virtual mode. Note that this address represents the area,
>   within the first LMB, into which the client program file will be
>   read by load; it does not correspond to the addresses at which the
>   program will be executed. All of physical memory from load-base to
>   either the start of OF physical memory or the end of physical
>   memory, whichever comes first, shall be available for loading the
>   client program.
>
>     Note: The load-base address represents the area into which the
>           client program will be read by load and does not correspond
>           to the address at which the program will be executed.
>
> We can specify load-base via setting a configuration variable, via
> CAS, or via an 1275 PowerPC Note.
>
> We can indeed override load-base and successfully load such a binary
> in SLOF. Drop to the OF shell and run:
>
> 0 > a000000 to load-base-override
> 0 > boot
>
> Sadly, asking people to drop to OF is not a super practical
> solution. SLOF ignores the 1275 PowerPC Note, which is the only thing
> we can (sanely) set in grub rather than in OF.
>
> PFW's load-base also defaults to 0x4000 but will honour the note. (The
> PFW team has been asking me to get grub to include the note and set
> appropriate values. See the next patch.)
>
> Fixing this
> -----------
>
> Unless I'm mistaken, if we want backwards compat with SLOF, we will
> need to change grub so that we ask the linker instruct the loader to
> load the program data somewhere else, giving us more headroom for the
> ELF image.
>
> This is fairly straight-forward to do. I've picked 8MB as that's a
> fairly common size for the PReP partition. We could potentially pick
> 4MB, I think I've seen some partitions that size too.
>
> The tests with powerpc32 using the OpenBIOS OF implementation seem to
> still work. I have no 32-bit Apple hardware to test on.
>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> ---
>  grub-core/Makefile.core.def | 2 +-
>  include/grub/offsets.h      | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 8022e1c0a794..ce338def0f84 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -89,7 +89,7 @@ kernel = {
>    i386_xen_pvh_ldflags     = '$(TARGET_IMG_BASE_LDOPT),0x100000';
>
>    mips_loongson_ldflags    = '-Wl,-Ttext,0x80200000';
> -  powerpc_ieee1275_ldflags = '-Wl,-Ttext,0x200000';
> +  powerpc_ieee1275_ldflags = '-Wl,-Ttext,0x800000';
>    sparc64_ieee1275_ldflags = '-Wl,-Ttext,0x4400';
>    mips_arc_ldflags    = '-Wl,-Ttext,$(TARGET_LINK_ADDR)';
>    mips_qemu_mips_ldflags    = '-Wl,-Ttext,0x80200000';
> diff --git a/include/grub/offsets.h b/include/grub/offsets.h
> index 871e1cd4c38c..8b2d6ab971be 100644
> --- a/include/grub/offsets.h
> +++ b/include/grub/offsets.h
> @@ -63,7 +63,7 @@
>  #define GRUB_KERNEL_SPARC64_IEEE1275_LINK_ADDR 0x4400
>
>  #define GRUB_KERNEL_POWERPC_IEEE1275_LINK_ALIGN 4
> -#define GRUB_KERNEL_POWERPC_IEEE1275_LINK_ADDR 0x200000
> +#define GRUB_KERNEL_POWERPC_IEEE1275_LINK_ADDR 0x800000
>
>  #define GRUB_KERNEL_MIPS_LOONGSON_LINK_ADDR         0x80200000
>
> --
> 2.32.0
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

Re: [PATCH 1/2] powerpc-ieee1275: load grub at 8MB, not 2MB

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 1168 bytes --]

"Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com> writes:

> Was it tested on powerpc-based Macs? They're still useful for
> debugging and developpement despite their age

Per Daniel's message:

    The tests with powerpc32 using the OpenBIOS OF implementation seem
    to still work. I have no 32-bit Apple hardware to test on.

But generally, I don't think this is a reasonable requirement.

Not only are powerpc macs very old at this point (discontinued 2006 - 15
years ago), but there also aren't really distros that supports them
(Debian dropped them in 8, Fedora around 12, Suse in it looks like 7).
Sure, Gentoo might work - but these are by no means fast machines.

To be constructive: I submit that if users/developers want to test on
these machines that they have and use, we should incorporate their
feedback where reasonable.  But let's not hold back patches over
hypothetical concerns for a dead architecture. [1]

Be well,
--Robbie

1: I love these machines.  I grew up with them.  But they're solidly
   retro now.  And many of them have design failures that cause them to
   self-destruct, which makes them particularly difficult retro to
   manage.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH 1/2] powerpc-ieee1275: load grub at 8MB, not 2MB

[John Paul Adrian Glaubitz]

Hello!

On 11/16/21 17:37, Robbie Harwood wrote:
> Per Daniel's message:
> 
>     The tests with powerpc32 using the OpenBIOS OF implementation seem
>     to still work. I have no 32-bit Apple hardware to test on.
> 
> But generally, I don't think this is a reasonable requirement.

I do.

> Not only are powerpc macs very old at this point (discontinued 2006 - 15
> years ago), but there also aren't really distros that supports them
> (Debian dropped them in 8, Fedora around 12, Suse in it looks like 7).
> Sure, Gentoo might work - but these are by no means fast machines.

Debian supports 32-bit PowerPC in Debian Ports. I'm regularly creating updated
images:

> https://cdimage.debian.org/cdimage/ports/snapshots/

We even support Debian on m68k :-).

> To be constructive: I submit that if users/developers want to test on
> these machines that they have and use, we should incorporate their
> feedback where reasonable.  But let's not hold back patches over
> hypothetical concerns for a dead architecture. [1]

I can test them later this week.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 1/2] powerpc-ieee1275: load grub at 8MB, not 2MB

[John Paul Adrian Glaubitz]

On 11/16/21 17:48, John Paul Adrian Glaubitz wrote:
>> But generally, I don't think this is a reasonable requirement.
> 
> I do.

And, FWIW, we have quite an active user base for Debian on PowerMacs and
I would eventually get all these user reports if something breaks.

I work a commercial Linux vendor myself, but I find it a bit frustrating
when some people think Linux development should care about commercial use
cases only.

The charm of Linux and free software is that it works well on old hardware
and hacking on these old machines is still very valuable for educational
purposes and as a hobby.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 1/2] powerpc-ieee1275: load grub at 8MB, not 2MB

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 594 bytes --]

John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> writes:

> On 11/16/21 17:48, John Paul Adrian Glaubitz wrote:
>
> I work a commercial Linux vendor myself, but I find it a bit frustrating
> when some people think Linux development should care about commercial use
> cases only.

This is a mischaracterization of what I said - I didn't mention anything
about use cases, commercial or otherwise.  My statement was that we
shouldn't be bound by behavior on retro hardware that we can't test.  If
you're willing and able to test them, that's great and I'll go be quiet.

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH 1/2] powerpc-ieee1275: load grub at 8MB, not 2MB

[Vladimir 'phcoder' Serbinenko]

On Tue, Nov 16, 2021 at 7:07 PM Robbie Harwood <rharwood@redhat.com> wrote:
>
> John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> writes:
>
> > On 11/16/21 17:48, John Paul Adrian Glaubitz wrote:
> >
> > I work a commercial Linux vendor myself, but I find it a bit frustrating
> > when some people think Linux development should care about commercial use
> > cases only.
>
> This is a mischaracterization of what I said - I didn't mention anything
> about use cases, commercial or otherwise.  My statement was that we
> shouldn't be bound by behavior on retro hardware that we can't test.  If
> you're willing and able to test them, that's great and I'll go be quiet.

I'll be able to test in O(2 weeks) if nobody gets to it earlier.

Also my suggestion if this breaks powermacs isn't to kill the patch
altogether but to switch powerpc to choose linking address at mkimage
time like we already do on ARM. This would be little effort as most
code is already there and I can do it in an hour or less.
It seems that the lowest amount of RAM on powermacs is 32 MiB. However
at least 64 MiB seems to be more typical. My two models have 384 MiB
(powermac G3) and 512 MiB (powerbook G4). With 128 MiB or more,
loading at 2MiB or 8 MiB shouldn't be much of a difference probably
(still needs to be tested). However I wonder of the impact on 32 MiB
and 64 MiB machines but not able to test outside of emulator
>
> Be well,
> --Robbie



-- 
Regards
Vladimir 'phcoder' Serbinenko

Re: [PATCH 1/2] powerpc-ieee1275: load grub at 8MB, not 2MB

[John Paul Adrian Glaubitz]

Hi!

On 11/16/21 16:31, Vladimir 'phcoder' Serbinenko wrote:
> Was it tested on powerpc-based Macs? They're still useful for
> debugging and developpement despite their age

Fired up my iBook G4 now and updating everything the latest package versions
in Debian unstable. Will test the patch after that.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

[PATCH 2/2] ieee1275: set real-base in the PowerPC IEEE1275 Note to 32MB

[Daniel Axtens]

Background
----------

Figuring out how to lay out memory in powerpc OpenFirmware is a bit
of a complicate dance.

Firstly, firmware needs to reserve a little bit of space for things
like interrupt vectors that live at a fixed address. This is usually
0x4000 bytes. PFW calls this "stage 1" (of the firmware, not the
bootloader!)

As discussed in the previous patch, the ELF image is loaded at
'load-base' before it's unpacked. load-base is, by default, set at the
end of stage 1. From there, the ELF loader unpacks the binary to
wherever the program headers say and the program is executed.

Next in memory, we need space for the rest of firmware. (PFW calls
this "stage 2" of firmware.) The starting address begins at
'real-base'.* We need to not run past real-base when loading our ELF
binary.

load-base and real-base have defaults from firmware, and may be
configurable in various ways.

On PFW:

 - real-base defaults to 12MB (0xc00000)

 - real-base is configurable with the IEEE1275 ELF note.

 - a proprietary OS bootloader configures real-base to
   0x2000000 (32MB).

On SLOF:

 - real-base is set dynamically towards the end of accessible memory.

 - real-base is not configurable.

On 32-bit, I'm not sure quite what the story is. I don't have any
32-bit Apple hardware to test with. However, with this change, grub
still loads and runs, including when I hack up the tests to install
the note by default.

* I'm assuming here that the firmware doesn't do virtual mode. This is
true on Power for both PFW and SLOF; Apple OF implementations may do
things differently.

Moving real-base
----------------

To account for growing grub binary sizes, we want to try to expand the
amount of memory that we have to work with. We have the following
constraints and inputs:

 - A good practical limit for the size of the grub binary is 8MB, as
   that's about the size of the PReP partition on a number of distro
   setups. (See previous patch.)

 - We also want things to work if people forget to include the
   note. Even if distros build it in to signed images, people
   running grub-install will probably forget to set it and we don't
   want to break their systems. (Helpfully, if you're running
   grub-install manually there's a decent chance you're not building
   in a bunch of modules so things are likely to be smaller anyway.)

 - We want to avoid doing anything too novel with enterprise software.

So when the note is included, set real-base to 32MB.

Results
-------

This means that:

 - The ELF file will be loaded at 0x4000 and have just short of 8MB to
   fit into. (This is entirely the work of the previous patch - the
   8MB limitation is because we now load at 8MB instead of 2MB.)

 - Nothing changes on SLOF, because SLOF ignores the note. We have
   oodles of space in SLOF.

 - On PFW when the note is not installed, we have about 4MB of space
   for ELF loader to put the program into (the gap between 8MB and the
   default real-base of 12MB). This is well more than any current grub
   image uses.

 - On PFW when the note _is_ installed, we will have about 24MB of
   space to put the program bits into. This is well more than we're
   likely to use unless people start putting very large arrays in BSS,
   but it means we're aligned with a proprietary bootloader and
   therefore we can be assured that some testing has been done.

Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 util/grub-mkimagexx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
index d78fa3e53308..2ac40b82a22d 100644
--- a/util/grub-mkimagexx.c
+++ b/util/grub-mkimagexx.c
@@ -467,7 +467,7 @@ SUFFIX (grub_mkimage_generate_elf) (const struct grub_install_image_target_desc
       note_ptr->header.n_type = grub_host_to_target32 (GRUB_IEEE1275_NOTE_TYPE);
       strcpy (note_ptr->name, GRUB_IEEE1275_NOTE_NAME);
       note_ptr->descriptor.real_mode = grub_host_to_target32 (0xffffffff);
-      note_ptr->descriptor.real_base = grub_host_to_target32 (0x00c00000);
+      note_ptr->descriptor.real_base = grub_host_to_target32 (0x02000000);
       note_ptr->descriptor.real_size = grub_host_to_target32 (0xffffffff);
       note_ptr->descriptor.virt_base = grub_host_to_target32 (0xffffffff);
       note_ptr->descriptor.virt_size = grub_host_to_target32 (0xffffffff);
-- 
2.32.0

Re: [PATCH 2/2] ieee1275: set real-base in the PowerPC IEEE1275 Note to 32MB

[Michael Chang]

On Tue, Nov 16, 2021 at 02:42:05PM +1100, Daniel Axtens wrote:
> Background
> ----------
> 
> Figuring out how to lay out memory in powerpc OpenFirmware is a bit
> of a complicate dance.
> 
> Firstly, firmware needs to reserve a little bit of space for things
> like interrupt vectors that live at a fixed address. This is usually
> 0x4000 bytes. PFW calls this "stage 1" (of the firmware, not the
> bootloader!)
> 
> As discussed in the previous patch, the ELF image is loaded at
> 'load-base' before it's unpacked. load-base is, by default, set at the
> end of stage 1. From there, the ELF loader unpacks the binary to
> wherever the program headers say and the program is executed.
> 
> Next in memory, we need space for the rest of firmware. (PFW calls
> this "stage 2" of firmware.) The starting address begins at
> 'real-base'.* We need to not run past real-base when loading our ELF
> binary.
> 
> load-base and real-base have defaults from firmware, and may be
> configurable in various ways.
> 
> On PFW:
> 
>  - real-base defaults to 12MB (0xc00000)
> 
>  - real-base is configurable with the IEEE1275 ELF note.
> 
>  - a proprietary OS bootloader configures real-base to
>    0x2000000 (32MB).
> 
> On SLOF:
> 
>  - real-base is set dynamically towards the end of accessible memory.
> 
>  - real-base is not configurable.
> 
> On 32-bit, I'm not sure quite what the story is. I don't have any
> 32-bit Apple hardware to test with. However, with this change, grub
> still loads and runs, including when I hack up the tests to install
> the note by default.
> 
> * I'm assuming here that the firmware doesn't do virtual mode. This is
> true on Power for both PFW and SLOF; Apple OF implementations may do
> things differently.
> 
> Moving real-base
> ----------------
> 
> To account for growing grub binary sizes, we want to try to expand the
> amount of memory that we have to work with. We have the following
> constraints and inputs:
> 
>  - A good practical limit for the size of the grub binary is 8MB, as
>    that's about the size of the PReP partition on a number of distro
>    setups. (See previous patch.)
> 
>  - We also want things to work if people forget to include the
>    note. Even if distros build it in to signed images, people
>    running grub-install will probably forget to set it and we don't
>    want to break their systems. (Helpfully, if you're running
>    grub-install manually there's a decent chance you're not building
>    in a bunch of modules so things are likely to be smaller anyway.)
> 
>  - We want to avoid doing anything too novel with enterprise software.
> 
> So when the note is included, set real-base to 32MB.
> 
> Results
> -------
> 
> This means that:
> 
>  - The ELF file will be loaded at 0x4000 and have just short of 8MB to
>    fit into. (This is entirely the work of the previous patch - the
>    8MB limitation is because we now load at 8MB instead of 2MB.)
> 
>  - Nothing changes on SLOF, because SLOF ignores the note. We have
>    oodles of space in SLOF.
> 
>  - On PFW when the note is not installed, we have about 4MB of space
>    for ELF loader to put the program into (the gap between 8MB and the
>    default real-base of 12MB). This is well more than any current grub
>    image uses.
> 
>  - On PFW when the note _is_ installed, we will have about 24MB of
>    space to put the program bits into. This is well more than we're
>    likely to use unless people start putting very large arrays in BSS,
>    but it means we're aligned with a proprietary bootloader and
>    therefore we can be assured that some testing has been done.

May I ask why 'proprietary' is used here to describe the usecase ? Does
it possess any meaningful words like supportability to the firmware
vendor that OS vendor have to think over before enabling the note for a
'general' feature like powerpc secure boot ?

Thanks,
Michael

> 
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> ---
>  util/grub-mkimagexx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
> index d78fa3e53308..2ac40b82a22d 100644
> --- a/util/grub-mkimagexx.c
> +++ b/util/grub-mkimagexx.c
> @@ -467,7 +467,7 @@ SUFFIX (grub_mkimage_generate_elf) (const struct grub_install_image_target_desc
>        note_ptr->header.n_type = grub_host_to_target32 (GRUB_IEEE1275_NOTE_TYPE);
>        strcpy (note_ptr->name, GRUB_IEEE1275_NOTE_NAME);
>        note_ptr->descriptor.real_mode = grub_host_to_target32 (0xffffffff);
> -      note_ptr->descriptor.real_base = grub_host_to_target32 (0x00c00000);
> +      note_ptr->descriptor.real_base = grub_host_to_target32 (0x02000000);
>        note_ptr->descriptor.real_size = grub_host_to_target32 (0xffffffff);
>        note_ptr->descriptor.virt_base = grub_host_to_target32 (0xffffffff);
>        note_ptr->descriptor.virt_size = grub_host_to_target32 (0xffffffff);
> -- 
> 2.32.0
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 0/2] powerpc-ieee1275: support larger core.elf images

[John Paul Adrian Glaubitz]

Hello Daniel!

On 11/16/21 04:42, Daniel Axtens wrote:
> Daniel Axtens (2):
>   powerpc-ieee1275: load grub at 8MB, not 2MB
>   ieee1275: set real-base in the PowerPC IEEE1275 Note to 32MB

I have applied these now and tested them on my iBook G4 but that introduced the
following regression:

root@ibook-g4-14:/home/glaubitz/grub# grub-install --macppc-directory=/boot/grub --no-nvram
Installing for powerpc-ieee1275 platform.
grub-install: warning: cannot open directory `/usr/local/share/locale': No such file or directory.
grub-install: error: `/usr/local/lib/grub/powerpc-ieee1275/kernel.img' is miscompiled: its start address is 0x200000 instead of 0x800000: ld.gold bug?.
root@ibook-g4-14:/home/glaubitz/grub#

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 0/2] powerpc-ieee1275: support larger core.elf images

[Daniel Axtens]

Hi Adrian,

Thanks for testing!

> root@ibook-g4-14:/home/glaubitz/grub# grub-install --macppc-directory=/boot/grub --no-nvram
> Installing for powerpc-ieee1275 platform.
> grub-install: warning: cannot open directory `/usr/local/share/locale': No such file or directory.
> grub-install: error: `/usr/local/lib/grub/powerpc-ieee1275/kernel.img' is miscompiled: its start address is 0x200000 instead of 0x800000: ld.gold bug?.
> root@ibook-g4-14:/home/glaubitz/grub#

Fascinating. I saw a similar issue when I only updated the kernel
(Makefile.core.def) and not the utils and fixed it with the change to
offsets.h.

Interestingly you're seeing the issue in reverse: the tools are
expecting the new offset but
/usr/local/lib/grub/powerpc-ieee1275/kernel.img is built with the old
offset.

Could you humour me and:
 - check that your kernel.img in /usr is indeed up to date
 - try grub-install ... -d ./grub-core?

Many thanks!

Kind regards,
Daniel

Re: [PATCH 0/2] powerpc-ieee1275: support larger core.elf images

[John Paul Adrian Glaubitz]

Hi Daniel!

On 11/24/21 01:51, Daniel Axtens wrote:
> Could you humour me and:
>  - check that your kernel.img in /usr is indeed up to date
>  - try grub-install ... -d ./grub-core?
> 
> Many thanks!

Apologies for the late reply. I will give it a try later today and report back.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 0/2] powerpc-ieee1275: support larger core.elf images

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 443 bytes --]

John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> writes:

> On 11/24/21 01:51, Daniel Axtens wrote:
>> Could you humour me and:
>>  - check that your kernel.img in /usr is indeed up to date
>>  - try grub-install ... -d ./grub-core?
>> 
>> Many thanks!
>
> Apologies for the late reply. I will give it a try later today and
> report back.

Hi Adrian, did you get a chance to try Daniel's suggestion?

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH 0/2] powerpc-ieee1275: support larger core.elf images

[John Paul Adrian Glaubitz]

Hi Robbie!

On 1/14/22 17:43, Robbie Harwood wrote:
> John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> writes:
> 
>> On 11/24/21 01:51, Daniel Axtens wrote:
>>> Could you humour me and:
>>>  - check that your kernel.img in /usr is indeed up to date
>>>  - try grub-install ... -d ./grub-core?
>>>
>>> Many thanks!
>>
>> Apologies for the late reply. I will give it a try later today and
>> report back.
> 
> Hi Adrian, did you get a chance to try Daniel's suggestion?

Hmm, I must have forgotten that. Let me give it a try after dinner. The laptop
just happens to be sitting next to me.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 0/2] powerpc-ieee1275: support larger core.elf images

[John Paul Adrian Glaubitz]

Hi!

On 1/14/22 17:54, John Paul Adrian Glaubitz wrote:
> Hmm, I must have forgotten that. Let me give it a try after dinner. The laptop
> just happens to be sitting next to me.

I gave it a try now and made sure to use the proper grub-core directory and that
the kernel.img in /usr/local/lib/grub/powerpc-ieee1275/ is up-to-date, so installation
went without an error message:

root@ibook-g4-14:/home/glaubitz/grub# grub-install -d ./grub-core --macppc-directory=/boot/grub --no-nvram
Installing for powerpc-ieee1275 platform.
grub-install: warning: cannot open directory `/usr/local/share/locale': No such file or directory.
Installation finished. No error reported.
root@ibook-g4-14:/home/glaubitz/grub#

However, after rebooting the machine, the machine is now unbootable. The firmware
is unable to even load GRUB. I'm just getting the question mark sign which indicates
that the firmware cannot find the bootloader.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 0/2] powerpc-ieee1275: support larger core.elf images

[Daniel Axtens]

Hi,

Sorry, I missed this as well - my otherwise lovely threaded email client
doesn't sort on the basis of most recent message, but thread start date.

> root@ibook-g4-14:/home/glaubitz/grub# grub-install -d ./grub-core --macppc-directory=/boot/grub --no-nvram
> Installing for powerpc-ieee1275 platform.
> grub-install: warning: cannot open directory `/usr/local/share/locale': No such file or directory.
> Installation finished. No error reported.
> root@ibook-g4-14:/home/glaubitz/grub#
>
> However, after rebooting the machine, the machine is now unbootable. The firmware
> is unable to even load GRUB. I'm just getting the question mark sign which indicates
> that the firmware cannot find the bootloader.

That's really irritating. (and apologies for rendering your machine
unbootable!)

I don't have a good idea what the cause of that would be. I don't think
I saw anything similar under qemu's mac99 model.

Is there any chance you could dump the `available` property of the
/memory node before grub is loaded? I don't think it's likely to be very
revealing but it's my best idea so far...

I've also since left IBM so I'm not likely to have a heap of time to
follow it up, unfortnately. The patch does solve a real issue on Power
systems, so I guess the next best thing if we can't root-cause it is
to make the link address compile-time configurable :/

Kind regards,
Daniel