Messages in this thread | | | From | Ard Biesheuvel <> | Date | Fri, 7 Oct 2022 09:20:46 +0200 | Subject | Re: [v4 PATCH 2/3] RISC-V: Update image header |
| |
On Fri, 7 Oct 2022 at 01:51, Heinrich Schuchardt <heinrich.schuchardt@canonical.com> wrote: > > > > On 10/7/22 01:00, Atish Patra wrote: > > Update the RISC-V Linux kernel image headers as per the current header. > > > > Reference: > > <Linux kernel source>/Documentation/riscv/boot-image-header.rst > > > > 474efecb65dc: ("riscv: modify the Image header to improve compatibility with the ARM64 header") > > > > Signed-off-by: Atish Patra <atishp@rivosinc.com> > > --- > > include/grub/riscv32/linux.h | 15 ++++++++------- > > include/grub/riscv64/linux.h | 21 +++++++++++++-------- > > 2 files changed, 21 insertions(+), 15 deletions(-) > > > > diff --git a/include/grub/riscv32/linux.h b/include/grub/riscv32/linux.h > > index 512b777c8a41..de0dbdcd1be5 100644 > > --- a/include/grub/riscv32/linux.h > > +++ b/include/grub/riscv32/linux.h > > @@ -19,20 +19,21 @@ > > #ifndef GRUB_RISCV32_LINUX_HEADER > > #define GRUB_RISCV32_LINUX_HEADER 1 > > > > -#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x52534356 /* 'RSCV' */ > > +#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x05435352 /* 'RSC\0x5' */ > > Thanks for following up on this series. > > Considering 69edb312056 ("loader/arm64/linux: Remove magic number header > field check") why should this constant exist in GRUB at all? > > In a follow up patch we could remove it together with > GRUB_LINUX_ARM_MAGIC_SIGNATURE, GRUB_LINUX_ARMXX_MAGIC_SIGNATURE, and > GRUB_LINUX_ARMXX_MAGIC_SIGNATURE. >
Indeed.
But by the same reasoning, why do we need per-arch kernel header typedefs at all? The only fields we access are generic PE/COFF header fields.
> > > > -/* From linux/Documentation/riscv/booting.txt */ > > +/* From linux/Documentation/riscv/boot-image-header.rst */ > > struct linux_riscv_kernel_header > > { > > grub_uint32_t code0; /* Executable code */ > > grub_uint32_t code1; /* Executable code */ > > - grub_uint64_t text_offset; /* Image load offset */ > > - grub_uint64_t res0; /* reserved */ > > - grub_uint64_t res1; /* reserved */ > > + grub_uint64_t text_offset; /* Image load offset, little endian */ > > + grub_uint64_t image_size; /* Effective Image size, little endian */ > > + grub_uint64_t flags; /* kernel flags, little endian */ > > + grub_uint32_t version; /* Version of this header */ > > + grub_uint32_t res1; /* reserved */ > > grub_uint64_t res2; /* reserved */ > > grub_uint64_t res3; /* reserved */ > > According to tag next-20221006 of > Documentation/riscv/boot-image-header.rst and of > arch/riscv/include/asm/image.h this field is called 'magic' and filled > it with the string 'RISCV\0\0\0'. > > > - grub_uint64_t res4; /* reserved */ > > - grub_uint32_t magic; /* Magic number, little endian, "RSCV" */ > > + grub_uint32_t magic; /* Magic number, little endian, "RSC\x05" */ > > The Linux kernel documentation calls this field magic2. > > Of course this is functionally irrelevant as we don't care about the > content of both fields. > > > grub_uint32_t hdr_offset; /* Offset of PE/COFF header */ > > }; > > > > diff --git a/include/grub/riscv64/linux.h b/include/grub/riscv64/linux.h > > index 3630c30fbf1a..ea77f8718222 100644 > > --- a/include/grub/riscv64/linux.h > > +++ b/include/grub/riscv64/linux.h > > @@ -19,23 +19,28 @@ > > #ifndef GRUB_RISCV64_LINUX_HEADER > > #define GRUB_RISCV64_LINUX_HEADER 1 > > > > -#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x52534356 /* 'RSCV' */ > > +#include <grub/efi/pe32.h> > > + > > +#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x05435352 /* 'RSC\0x5' */ > > to be removed in future > > Best regards > > Heinrich > > > > > #define GRUB_EFI_PE_MAGIC 0x5A4D > > > > -/* From linux/Documentation/riscv/booting.txt */ > > +/* From linux/Documentation/riscv/boot-image-header.rst */ > > struct linux_riscv_kernel_header > > { > > grub_uint32_t code0; /* Executable code */ > > grub_uint32_t code1; /* Executable code */ > > - grub_uint64_t text_offset; /* Image load offset */ > > - grub_uint64_t res0; /* reserved */ > > - grub_uint64_t res1; /* reserved */ > > + grub_uint64_t text_offset; /* Image load offset, little endian */ > > + grub_uint64_t image_size; /* Effective Image size, little endian */ > > + grub_uint64_t flags; /* kernel flags, little endian */ > > + grub_uint32_t version; /* Version of this header */ > > + grub_uint32_t res1; /* reserved */ > > grub_uint64_t res2; /* reserved */ > > - grub_uint64_t res3; /* reserved */ > > - grub_uint64_t res4; /* reserved */ > > - grub_uint32_t magic; /* Magic number, little endian, "RSCV" */ > > + grub_uint64_t magic; /* magic (RISC-V specifc, deprecated)*/ > > + grub_uint32_t magic2; /* Magic number 2 (to match the ARM64 'magic' field pos) */ > > grub_uint32_t hdr_offset; /* Offset of PE/COFF header */ > > + > > + struct grub_coff_image_header coff_image_header; > > }; > > > > #define linux_arch_kernel_header linux_riscv_kernel_header
| |