On Thu, Mar 20, 2025 at 04:06:31PM -0700, Patrick Colp via Grub-devel wrote:
> Update linux_kernel_params to match the latest upstream (v6.13.7)
> version of boot_params. Refactor most things out into structs, as the
> Linux kernel does.
>
> `edid_info` should be a struct with `unsigned char dummy[128]` and
Nit, s/`/"/ and below please...
> `efi_info` should be a struct as well, starting at 0x1c0. However, for
> backwards compatibility, GRUB can have `efi_system_table` at 0x1b8 and
> padding at 0x1bc (or padding at both spots). This cuts into the end of
> edid_info. Make `edid_info` inline and only make it go up to 0x1b8.
>
> Signed-off-by: Patrick Colp <patrick.c...@oracle.com>
> ---
> grub-core/loader/i386/linux.c | 212 +++++++-------
> include/grub/i386/linux.h | 503 ++++++++++++++++++++++------------
> 2 files changed, 436 insertions(+), 279 deletions(-)
[...]
> diff --git a/include/grub/i386/linux.h b/include/grub/i386/linux.h
> index d4b550869534..4b16b0de7b69 100644
> --- a/include/grub/i386/linux.h
> +++ b/include/grub/i386/linux.h
[...]
> +/* Boot parameters for Linux based on 6.13.7 stable. This is used by the
> setup
> + sectors of Linux, and must be simulated by GRUB on EFI, because
> + the setup sectors depend on BIOS. */
Wrong coding style for multiline comments. Please take a look at [1].
> +struct linux_kernel_params
> +{
> + struct grub_screen_info screen_info; /* 0 */
> + struct grub_apm_bios_info apm_bios_info; /* 40 */
> + grub_uint8_t _pad2[4]; /* 54 */
> + grub_uint64_t tboot_addr; /* 58 */
> + struct grub_ist_info ist_info; /* 60 */
> + grub_uint64_t acpi_rsdp_addr; /* 70 */
> + grub_uint8_t _pad3[8]; /* 78 */
> + grub_uint8_t hd0_info[16]; /* 80 */
> + grub_uint8_t hd1_info[16]; /* 90 */
> + struct grub_sys_desc_table sys_desc_table; /* a0 */
> + struct grub_olpc_ofw_header olpc_ofw_header; /* b0 */
> + grub_uint32_t ext_ramdisk_image; /* c0 */
> + grub_uint32_t ext_ramdisk_size; /* c4 */
> + grub_uint32_t ext_cmd_line_ptr; /* c8 */
> + grub_uint8_t _pad4[112]; /* cc */
> + grub_uint32_t cc_blob_address; /* 13c */
> +
> + /* `edid_info` should be a struct with `unsigned char dummy[128]` and
> + `efi_info` should be a struct as well, starting at 0x1c0. However, for
> + backwards compatibility, GRUB can have `efi_system_table` at 0x1b8 and
> + padding at 0x1bc (or padding at both spots). This cuts into the end of
> + edid_info. Make `edid_info` inline and only make it go up to 0x1b8. */
Ditto. Please fix it everywhere in your patch...
> + grub_uint8_t edid_info[0x1b8 - 0x140]; /* 140 */
> + union
> + {
> + struct
> + {
> + grub_uint32_t efi_system_table; /* 1b8 */
s/efi_system_table/efi_systab/?
You are aligning all member names except these ones. Could you fix that?
> + grub_uint32_t padding7_1; /* 1bc */
s/padding7_1/padding7_2/ to be in line with structs below...
> + grub_uint32_t efi_signature; /* 1c0 */
s/efi_signature/efi_loader_signature/
> + grub_uint32_t efi_mem_desc_size; /* 1c4 */
s/efi_mem_desc_size/efi_memdesc_size/ and so on...
> + grub_uint32_t efi_mem_desc_version; /* 1c8 */
> + grub_uint32_t efi_mmap_size; /* 1cc */
> + grub_uint32_t efi_mmap; /* 1d0 */
> + } v0204;
> + struct
> + {
> + grub_uint32_t padding7_1; /* 1b8 */
> + grub_uint32_t padding7_2; /* 1bc */
> + grub_uint32_t efi_signature; /* 1c0 */
> + grub_uint32_t efi_system_table; /* 1c4 */
> + grub_uint32_t efi_mem_desc_size; /* 1c8 */
> + grub_uint32_t efi_mem_desc_version; /* 1cc */
> + grub_uint32_t efi_mmap; /* 1d0 */
> + grub_uint32_t efi_mmap_size; /* 1d4 */
> + } v0206;
> + struct
> + {
> + grub_uint32_t padding7_1; /* 1b8 */
> + grub_uint32_t padding7_2; /* 1bc */
> + grub_uint32_t efi_signature; /* 1c0 */
> + grub_uint32_t efi_system_table; /* 1c4 */
> + grub_uint32_t efi_mem_desc_size; /* 1c8 */
> + grub_uint32_t efi_mem_desc_version; /* 1cc */
> + grub_uint32_t efi_mmap; /* 1d0 */
> + grub_uint32_t efi_mmap_size; /* 1d4 */
> + grub_uint32_t efi_system_table_hi; /* 1d8 */
> + grub_uint32_t efi_mmap_hi; /* 1dc */
> + } v0208;
> + } efi_info;
> +
> + grub_uint32_t alt_mem_k; /* 1e0 */
> + grub_uint32_t scratch; /* 1e4 */
> + grub_uint8_t e820_entries; /* 1e8 */
> + grub_uint8_t eddbuf_entries; /* 1e9 */
> + grub_uint8_t edd_mbr_sig_buf_entries; /* 1ea */
> + grub_uint8_t kbd_status; /* 1eb */
> + grub_uint8_t secure_boot; /* 1ec */
> + grub_uint8_t _pad5[2]; /* 1ed */
> + grub_uint8_t sentinel; /* 1ef */
> + grub_uint8_t _pad6[1]; /* 1f0 */
> + struct grub_setup_header hdr; /* 1f1 */
> + grub_uint8_t _pad7[0x290 - 0x1f1 - sizeof(struct grub_setup_header)];
> grub_uint32_t edd_mbr_sig_buffer[EDD_MBR_SIG_MAX]; /* 290 */
> - struct grub_e820_mmap e820_map[(0x400 - 0x2d0) / 20]; /* 2d0 */
> + struct grub_boot_e820_entry e820_table[GRUB_E820_MAX_ENTRIES_ZEROPAGE];
> /* 2d0 */
> + grub_uint8_t _pad8[48]; /* cd0 */
> + struct grub_edd_info eddbuf[EDDMAXNR]; /* d00 */
> + grub_uint8_t _pad9[276]; /* eec */
> +
> } GRUB_PACKED;
> #endif /* ! ASM_FILE */
Daniel
[1]
https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Multi_002dLine-Comments
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
