Re: [PATCH 5/7] arm: reuse arm64 linux loader on efi systems

[Leif Lindholm]

On Thu, Jul 27, 2017 at 05:06:25PM +0200, Daniel Kiper wrote:
> On Mon, Jun 12, 2017 at 03:53:39PM +0100, Leif Lindholm wrote:
> > The original 32-bit arm EFI Linux loader reused the 32-bit Linux
> > loader for U-Boot. However, this meant it was acting in an
> > entirely not UEFI-compliant fashion.
> >
> > Since EFI stub loader support for arm went into upstream Linux
> > for 4.5, we can now reuse the same loader as is used on arm64.
> >
> > This results in some now-redundant code being dropped from the U-Boot
> > linux loader, and the arm efi port in general. This also drops the
> > ability to boot kernels without the efi stub support on 32-bit arm efi
> > grub.
> 
> I have a feeling that this patch does more than one thing. Please
> split it if possible.

If you're happy with keeping dead code temporarily, I can split into
at least two.

> > Signed-off-by: Leif Lindholm <address@hidden>
> > ---
> >  grub-core/Makefile.am         |   1 -
> >  grub-core/Makefile.core.def   |   6 +-
> >  grub-core/kern/arm/efi/misc.c | 202 
> > ------------------------------------------
> >  grub-core/loader/arm/linux.c  |  39 ++------
> >  include/grub/arm/efi/loader.h |  26 ------
> >  include/grub/arm/efi/memory.h |   3 +
> >  include/grub/arm/linux.h      |  30 +++----
> >  7 files changed, 22 insertions(+), 285 deletions(-)
> >  delete mode 100644 grub-core/kern/arm/efi/misc.c
> >  delete mode 100644 include/grub/arm/efi/loader.h
> 
> [...]
> 
> > diff --git a/include/grub/arm/efi/memory.h b/include/grub/arm/efi/memory.h
> > index 2c64918e3..986f656d6 100644
> > --- a/include/grub/arm/efi/memory.h
> > +++ b/include/grub/arm/efi/memory.h
> > @@ -3,4 +3,7 @@
> >
> >  #define GRUB_EFI_MAX_USABLE_ADDRESS 0xffffffff
> >
> > +#define GRUB_EFI_PAGE_SHIFT        12
> > +#define GRUB_EFI_BYTES_TO_PAGES(bytes)   (((bytes) + 0xfff) >> 
> > GRUB_EFI_PAGE_SHIFT)
> 
> Hmmm... It looks familiar. Why do you duplicate this?

Sorry, rebase mess-up.

> > +
> >  #endif /* ! GRUB_MEMORY_CPU_HEADER */
> > diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h
> > index f217f8281..98ff4c004 100644
> > --- a/include/grub/arm/linux.h
> > +++ b/include/grub/arm/linux.h
> > @@ -20,32 +20,26 @@
> >  #ifndef GRUB_LINUX_CPU_HEADER
> >  #define GRUB_LINUX_CPU_HEADER 1
> >
> > -#define LINUX_ZIMAGE_OFFSET 0x24
> > -#define LINUX_ZIMAGE_MAGIC  0x016f2818
> > +#define GRUB_LINUX_MAGIC_SIGNATURE 0x016f2818
> >
> > -#include "system.h"
> > +struct grub_linux_kernel_header {
> > +  grub_uint32_t code0;
> > +  grub_uint32_t reserved1[8];
> > +  grub_uint32_t magic;
> > +  grub_uint32_t start; /* _start */
> > +  grub_uint32_t end;   /* _edata */
> > +  grub_uint32_t reserved2[4];
> > +  grub_uint32_t hdr_offset;
> > +};
> 
> ???

A struct describing the start of the 32-bit ARM zImage for the sanity
check in grub_efi_linux_check_image.

/
    Leif