On Tue, Nov 8, 2022 at 7:56 AM Daniel Kiper <dki...@net-space.pl> wrote:
> On Fri, Nov 04, 2022 at 04:26:06PM -0700, 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")
>
> The latest commit which updates Documentation/riscv/boot-image-header.rst
> is
> 1d5c17e47028 (RISC-V: Typo fixes in image header and documentation).
>
>
Yes. I was pointing out the commit the image header was actually modified.
I will modify the commit text to reflect the latest commit for the
documentation as you suggested.
> > Acked-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
> > Signed-off-by: Atish Patra <ati...@rivosinc.com>
>
> The order of these lines should be reversed...
>
>
Sure. Will do.
> > ---
> > include/grub/riscv32/linux.h | 19 ++++++++++---------
> > include/grub/riscv64/linux.h | 19 +++++++++++--------
> > 2 files changed, 21 insertions(+), 17 deletions(-)
> >
> > diff --git a/include/grub/riscv32/linux.h b/include/grub/riscv32/linux.h
> > index 512b777c8a41..a884d4f4760c 100644
> > --- a/include/grub/riscv32/linux.h
> > +++ b/include/grub/riscv32/linux.h
> > @@ -19,21 +19,22 @@
> > #ifndef GRUB_RISCV32_LINUX_HEADER
> > #define GRUB_RISCV32_LINUX_HEADER 1
> >
> > -#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x52534356 /* 'RSCV' */
> > -
> > -/* 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 */
>
> I can agree that hdr_offset makes more sense but
> Documentation/riscv/boot-image-header.rst names this member as res3.
> So, I would rename hdr_offset to res3 too. Or fix
> Documentation/riscv/boot-image-header.rst in the kernel. Or, least
> preferred, explain in the commit message why you do not rename this
> member and add to the comment that this member is named res3 in the
> documentation.
>
> > +
> > + struct grub_pe_image_header pe_image_header;
>
> This struct should not be part of linux_riscv_kernel_header struct.
> Please take a look at the commit 6d7bb89ef (efi: Move MS-DOS stub out of
> generic PE header definition). And right now I can see this comes from
> ARM headers. I am not sure why we did not spotted and fixed this issue
> when we worked on LoadFile2 series. Atish, could you fix this too? Of
> course in separate patch before this one. And if you could align ARM
> headers structs with current Linux documentation that would be perfect...
>
> Same comments apply below...
>
> > };
> >
> > #define linux_arch_kernel_header linux_riscv_kernel_header
> > diff --git a/include/grub/riscv64/linux.h b/include/grub/riscv64/linux.h
> > index 3630c30fbf1a..a69046de34ef 100644
> > --- a/include/grub/riscv64/linux.h
> > +++ b/include/grub/riscv64/linux.h
> > @@ -19,23 +19,26 @@
> > #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_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_pe_image_header pe_image_header;
> > };
>
> Daniel
>
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
