From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1ojKQ0-0002ay-1F for mharc-grub-devel@gnu.org; Fri, 14 Oct 2022 09:08:04 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:44606) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ojKPw-0002Xw-Oq for grub-devel@gnu.org; Fri, 14 Oct 2022 09:08:02 -0400 Received: from dibed.net-space.pl ([84.10.22.86]:34198) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_3DES_EDE_CBC_SHA1:192) (Exim 4.90_1) (envelope-from ) id 1ojKPu-0003Ed-VW for grub-devel@gnu.org; Fri, 14 Oct 2022 09:08:00 -0400 Received: from router-fw.i.net-space.pl ([192.168.52.1]:52330 "EHLO tomti.i.net-space.pl") by router-fw-old.i.net-space.pl with ESMTP id S2152252AbiJNNHU (ORCPT ); Fri, 14 Oct 2022 15:07:20 +0200 X-Comment: RFC 2476 MSA function at dibed.net-space.pl logged sender identity as: dkiper Date: Fri, 14 Oct 2022 15:07:14 +0200 From: Daniel Kiper To: Ard Biesheuvel Cc: grub-devel@gnu.org, Daniel Kiper , Leif Lindholm , Nikita Ermakov , Atish Patra , Huacai Chen , Heinrich Schuchardt , dann frazier , Julian Andres Klode , Ilias Apalodimas Subject: Re: [PATCH v4 2/6] linux/arm: unify ARM/arm64 vs Xen PE/COFF header handling Message-ID: <20221014130714.tplpa3fp35xnwe3w@tomti.i.net-space.pl> References: <20220908133017.1464494-1-ardb@kernel.org> <20220908133017.1464494-3-ardb@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220908133017.1464494-3-ardb@kernel.org> User-Agent: NeoMutt/20170113 (1.7.2) Received-SPF: pass client-ip=84.10.22.86; envelope-from=dkiper@net-space.pl; helo=dibed.net-space.pl X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Oct 2022 13:08:02 -0000 On Thu, Sep 08, 2022 at 03:30:13PM +0200, Ard Biesheuvel wrote: > Xen has its own version of the image header, to account for the > additional PE/COFF header fields. Since we are adding references to > those in the shared EFI loader code, update the common definitions > and drop the Xen specific one which no longer has a purpose. > > Signed-off-by: Ard Biesheuvel > --- > grub-core/loader/arm64/linux.c | 12 +++++----- > grub-core/loader/arm64/xen_boot.c | 23 ++++---------------- > include/grub/arm/linux.h | 6 +++++ > include/grub/arm64/linux.h | 4 ++++ > include/grub/efi/efi.h | 4 +++- > 5 files changed, 24 insertions(+), 25 deletions(-) > > diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c > index b5b559c236e0..7c0f17cf933d 100644 > --- a/grub-core/loader/arm64/linux.c > +++ b/grub-core/loader/arm64/linux.c > @@ -49,8 +49,13 @@ static grub_addr_t initrd_start; > static grub_addr_t initrd_end; > > grub_err_t > -grub_arch_efi_linux_check_image (struct linux_arch_kernel_header * lh) > +grub_arch_efi_linux_load_image_header (grub_file_t file, > + struct linux_arch_kernel_header * lh) > { > + grub_file_seek (file, 0); > + if (grub_file_read (file, lh, sizeof (*lh)) < (long) sizeof (*lh)) s/(long)/(grub_ssize_t)/ Please mention this change in the commit message. > + return grub_error(GRUB_ERR_FILE_READ_ERROR, "failed to read Linux image header"); > + > if ((lh->code0 & 0xffff) != GRUB_PE32_MAGIC) > return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, > N_("plain image kernel not supported - rebuild with CONFIG_(U)EFI_STUB enabled")); > @@ -301,10 +306,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)), > > kernel_size = grub_file_size (file); > > - if (grub_file_read (file, &lh, sizeof (lh)) < (long) sizeof (lh)) > - return grub_errno; > - > - if (grub_arch_efi_linux_check_image (&lh) != GRUB_ERR_NONE) > + if (grub_arch_efi_linux_load_image_header (file, &lh) != GRUB_ERR_NONE) > goto fail; > > grub_loader_unset(); > diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-core/loader/arm64/xen_boot.c > index 22cc25eccd9d..e5895ee78218 100644 > --- a/grub-core/loader/arm64/xen_boot.c > +++ b/grub-core/loader/arm64/xen_boot.c > @@ -31,7 +31,6 @@ > #include > #include > #include > -#include /* required by struct xen_hypervisor_header */ > #include > #include > > @@ -65,18 +64,6 @@ enum module_type > }; > typedef enum module_type module_type_t; > > -struct xen_hypervisor_header > -{ > - struct linux_arm64_kernel_header efi_head; > - > - /* This is always PE\0\0. */ > - grub_uint8_t signature[GRUB_PE32_SIGNATURE_SIZE]; > - /* The COFF file header. */ > - struct grub_pe32_coff_header coff_header; > - /* The Optional header. */ > - struct grub_pe64_optional_header optional_header; > -}; > - > struct xen_boot_binary > { > struct xen_boot_binary *next; > @@ -452,7 +439,7 @@ static grub_err_t > grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ ((unused)), > int argc, char *argv[]) > { > - struct xen_hypervisor_header sh; > + struct linux_arm64_kernel_header lh; > grub_file_t file = NULL; > > grub_dl_ref (my_mod); > @@ -467,10 +454,7 @@ grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ ((unused)), > if (!file) > goto fail; > > - if (grub_file_read (file, &sh, sizeof (sh)) != (long) sizeof (sh)) > - goto fail; > - if (grub_arch_efi_linux_check_image > - ((struct linux_arch_kernel_header *) &sh) != GRUB_ERR_NONE) > + if (grub_arch_efi_linux_load_image_header (file, &lh) != GRUB_ERR_NONE) > goto fail; > grub_file_seek (file, 0); > > @@ -484,7 +468,8 @@ grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ ((unused)), > return grub_errno; > > xen_hypervisor->is_hypervisor = 1; > - xen_hypervisor->align = (grub_size_t) sh.optional_header.section_alignment; > + xen_hypervisor->align > + = (grub_size_t) lh.coff_image_header.optional_header.section_alignment; > > xen_boot_binary_load (xen_hypervisor, file, argc, argv); > if (grub_errno == GRUB_ERR_NONE) > diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h > index bcd5a7eb186e..ea815db13417 100644 > --- a/include/grub/arm/linux.h > +++ b/include/grub/arm/linux.h > @@ -22,6 +22,8 @@ > > #include "system.h" > > +#include > + > #define GRUB_LINUX_ARM_MAGIC_SIGNATURE 0x016f2818 > > struct linux_arm_kernel_header { > @@ -32,6 +34,10 @@ struct linux_arm_kernel_header { > grub_uint32_t end; /* _edata */ > grub_uint32_t reserved2[3]; > grub_uint32_t hdr_offset; > + Please drop this empty line. > +#if defined GRUB_MACHINE_EFI > + struct grub_coff_image_header coff_image_header; > +#endif > }; > > #if defined(__arm__) > diff --git a/include/grub/arm64/linux.h b/include/grub/arm64/linux.h > index 7e22b4ab6990..c5ea9e8f503a 100644 > --- a/include/grub/arm64/linux.h > +++ b/include/grub/arm64/linux.h > @@ -21,6 +21,8 @@ > > #include > Please drop this empty line. > +#include > + > #define GRUB_LINUX_ARM64_MAGIC_SIGNATURE 0x644d5241 /* 'ARM\x64' */ > > /* From linux/Documentation/arm64/booting.txt */ > @@ -36,6 +38,8 @@ struct linux_arm64_kernel_header > grub_uint64_t res4; /* reserved */ > grub_uint32_t magic; /* Magic number, little endian, "ARM\x64" */ > grub_uint32_t hdr_offset; /* Offset of PE/COFF header */ > + Please drop this empty line. > + struct grub_coff_image_header coff_image_header; Daniel