Re: [PATCH v2 1/3] x86/boot: Introduce the kernel_info

From: hpa
Date: Fri Jul 12 2019 - 12:04:45 EST

Next message: Tom Zanussi: "Re: [PATCH v3] trace:Add "gfp_t" support in synthetic_events"
Previous message: Thomas Gleixner: "Re: [PATCH 1/3] x86/Kconfig: Move ARCH_HAS_MEM_ENCRYPT to arch/Kconfig"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On July 4, 2019 9:36:10 AM PDT, Daniel Kiper <daniel.kiper@xxxxxxxxxx> wrote:
>The relationships between the headers are analogous to the various data
>sections:
>
> setup_header = .data
> boot_params/setup_data = .bss
>
>What is missing from the above list? That's right:
>
> kernel_info = .rodata
>
>We have been (ab)using .data for things that could go into .rodata or
>.bss for
>a long time, for lack of alternatives and -- especially early on --
>inertia.
>Also, the BIOS stub is responsible for creating boot_params, so it
>isn't
>available to a BIOS-based loader (setup_data is, though).
>
>setup_header is permanently limited to 144 bytes due to the reach of
>the
>2-byte jump field, which doubles as a length field for the structure,
>combined
>with the size of the "hole" in struct boot_params that a protected-mode
>loader
>or the BIOS stub has to copy it into. It is currently 119 bytes long,
>which
>leaves us with 25 very precious bytes. This isn't something that can be
>fixed
>without revising the boot protocol entirely, breaking backwards
>compatibility.
>
>boot_params proper is limited to 4096 bytes, but can be arbitrarily
>extended
>by adding setup_data entries. It cannot be used to communicate
>properties of
>the kernel image, because it is .bss and has no image-provided content.
>
>kernel_info solves this by providing an extensible place for
>information about
>the kernel image. It is readonly, because the kernel cannot rely on a
>bootloader copying its contents anywhere, but that is OK; if it becomes
>necessary it can still contain data items that an enabled bootloader
>would be
>expected to copy into a setup_data chunk.
>
>This patch does not bump setup_header version in arch/x86/boot/header.S
>because it will be followed by additional changes coming into the
>Linux/x86 boot protocol.
>
>Suggested-by: H. Peter Anvin <hpa@xxxxxxxxx>
>Signed-off-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>
>Reviewed-by: Eric Snowberg <eric.snowberg@xxxxxxxxxx>
>Reviewed-by: Ross Philipson <ross.philipson@xxxxxxxxxx>
>---
>v2 - suggestions/fixes:
> - rename setup_header2 to kernel_info,
> (suggested by H. Peter Anvin),
> - change kernel_info.header value to "InfO" (0x4f666e49),
> - new kernel_info description in Documentation/x86/boot.rst,
> (suggested by H. Peter Anvin),
> - drop kernel_info_offset_update() as an overkill and
> update kernel_info offset directly from main(),
> (suggested by Eric Snowberg),
> - new commit message
> (suggested by H. Peter Anvin),
> - fix some commit message misspellings
> (suggested by Eric Snowberg).
>---
>Documentation/x86/boot.rst | 89
>++++++++++++++++++++++++++++++++++
> arch/x86/boot/Makefile | 2 +-
> arch/x86/boot/compressed/Makefile | 4 +-
> arch/x86/boot/compressed/kernel_info.S | 12 +++++
> arch/x86/boot/header.S | 1 +
> arch/x86/boot/tools/build.c | 5 ++
> arch/x86/include/uapi/asm/bootparam.h | 1 +
> 7 files changed, 111 insertions(+), 3 deletions(-)
> create mode 100644 arch/x86/boot/compressed/kernel_info.S
>
>diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
>index 08a2f100c0e6..a934a56f0516 100644
>--- a/Documentation/x86/boot.rst
>+++ b/Documentation/x86/boot.rst
>@@ -68,8 +68,25 @@ Protocol 2.12 (Kernel 3.8) Added the xloadflags
>field and extension fields
> Protocol 2.13 (Kernel 3.14) Support 32- and 64-bit flags being set in
> xloadflags to support booting a 64-bit kernel from 32-bit
> EFI
>+
>+Protocol 2.14: BURNT BY INCORRECT COMMIT
>ae7e1238e68f2a472a125673ab506d49158c1889
>+ (x86/boot: Add ACPI RSDP address to setup_header)
>+ DO NOT USE!!! ASSUME SAME AS 2.13.
>+
>+Protocol 2.15: (Kernel 5.3) Added the kernel_info.
>============= ============================================================
>
>+.. note::
>+ The protocol version number should be changed only if the setup
>header
>+ is changed. There is no need to update the version number if
>boot_params
>+ or kernel_info are changed. Additionally, it is recommended to
>use
>+ xloadflags (in this case the protocol version number should not
>be
>+ updated either) or kernel_info to communicate supported Linux
>kernel
>+ features to the boot loader. Due to very limited space available
>in
>+ the original setup header every update to it should be considered
>+ with great care. Starting from the protocol 2.15 the primary way
>to
>+ communicate things to the boot loader is the kernel_info.
>+
>
> Memory Layout
> =============
>@@ -207,6 +224,7 @@ Offset/Size Proto Name Meaning
> 0258/8 2.10+ pref_address Preferred loading address
> 0260/4 2.10+ init_size Linear memory required during initialization
> 0264/4 2.11+ handover_offset Offset of handover entry point
>+0268/4 2.15+ kernel_info_offset Offset of the kernel_info
>=========== ======== ===================== ============================================
>
> .. note::
>@@ -855,6 +873,77 @@ Offset/size: 0x264/4
>
> See EFI HANDOVER PROTOCOL below for more details.
>
>+============ ==================
>+Field name: kernel_info_offset
>+Type: read
>+Offset/size: 0x268/4
>+Protocol: 2.15+
>+============ ==================
>+
>+ This field is the offset from the beginning of the kernel image to
>the
>+ kernel_info. It is embedded in the Linux image in the uncompressed
>+ protected mode region.
>+
>+
>+The kernel_info
>+===============
>+
>+The relationships between the headers are analogous to the various
>data
>+sections:
>+
>+ setup_header = .data
>+ boot_params/setup_data = .bss
>+
>+What is missing from the above list? That's right:
>+
>+ kernel_info = .rodata
>+
>+We have been (ab)using .data for things that could go into .rodata or
>.bss for
>+a long time, for lack of alternatives and -- especially early on --
>inertia.
>+Also, the BIOS stub is responsible for creating boot_params, so it
>isn't
>+available to a BIOS-based loader (setup_data is, though).
>+
>+setup_header is permanently limited to 144 bytes due to the reach of
>the
>+2-byte jump field, which doubles as a length field for the structure,
>combined
>+with the size of the "hole" in struct boot_params that a
>protected-mode loader
>+or the BIOS stub has to copy it into. It is currently 119 bytes long,
>which
>+leaves us with 25 very precious bytes. This isn't something that can
>be fixed
>+without revising the boot protocol entirely, breaking backwards
>compatibility.
>+
>+boot_params proper is limited to 4096 bytes, but can be arbitrarily
>extended
>+by adding setup_data entries. It cannot be used to communicate
>properties of
>+the kernel image, because it is .bss and has no image-provided
>content.
>+
>+kernel_info solves this by providing an extensible place for
>information about
>+the kernel image. It is readonly, because the kernel cannot rely on a
>+bootloader copying its contents anywhere, but that is OK; if it
>becomes
>+necessary it can still contain data items that an enabled bootloader
>would be
>+expected to copy into a setup_data chunk.
>+
>+It is recommended to not store large data chunks, e.g. strings,
>directly in the
>+kernel_info struct. Such data should be placed outside of it and
>pointed from
>+the kernel_info structure using offsets from the beginning of the
>structure,
>+the kernel_info.header field.
>+
>+
>+Details of the kernel_info Fields
>+=================================
>+
>+============ ========
>+Field name: header
>+Offset/size: 0x0000/4
>+============ ========
>+
>+ Contains the magic number "InfO" (0x4f666e49).
>+
>+============ ========
>+Field name: size
>+Offset/size: 0x0004/4
>+============ ========
>+
>+ This field contains the size of the kernel_info including
>kernel_info.header.
>+ It should be used by the boot loader to detect supported fields in
>the kernel_info.
>+
>
> The Image Checksum
> ==================
>diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
>index e2839b5c246c..c30a9b642a86 100644
>--- a/arch/x86/boot/Makefile
>+++ b/arch/x86/boot/Makefile
>@@ -87,7 +87,7 @@ $(obj)/vmlinux.bin: $(obj)/compressed/vmlinux FORCE
>
> SETUP_OBJS = $(addprefix $(obj)/,$(setup-y))
>
>-sed-zoffset := -e 's/^$[0-9a-fA-F]*$ [ABCDGRSTVW]
>$startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|input_data\|_end\|_ehead\|_text\|z_.*$$$/\#define
>ZO_\2 0x\1/p'
>+sed-zoffset := -e 's/^$[0-9a-fA-F]*$ [ABCDGRSTVW]
>$startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|input_data\|kernel_info\|_end\|_ehead\|_text\|z_.*$$$/\#define
>ZO_\2 0x\1/p'
>
> quiet_cmd_zoffset = ZOFFSET $@
> cmd_zoffset = $(NM) $< | sed -n $(sed-zoffset) > $@
>diff --git a/arch/x86/boot/compressed/Makefile
>b/arch/x86/boot/compressed/Makefile
>index 6b84afdd7538..fad3b18e2cc3 100644
>--- a/arch/x86/boot/compressed/Makefile
>+++ b/arch/x86/boot/compressed/Makefile
>@@ -72,8 +72,8 @@ $(obj)/../voffset.h: vmlinux FORCE
>
> $(obj)/misc.o: $(obj)/../voffset.h
>
>-vmlinux-objs-y := $(obj)/vmlinux.lds $(obj)/head_$(BITS).o
>$(obj)/misc.o \
>- $(obj)/string.o $(obj)/cmdline.o $(obj)/error.o \
>+vmlinux-objs-y := $(obj)/vmlinux.lds $(obj)/kernel_info.o
>$(obj)/head_$(BITS).o \
>+ $(obj)/misc.o $(obj)/string.o $(obj)/cmdline.o $(obj)/error.o \
> $(obj)/piggy.o $(obj)/cpuflags.o
>
> vmlinux-objs-$(CONFIG_EARLY_PRINTK) += $(obj)/early_serial_console.o
>diff --git a/arch/x86/boot/compressed/kernel_info.S
>b/arch/x86/boot/compressed/kernel_info.S
>new file mode 100644
>index 000000000000..3f1cb301b9ff
>--- /dev/null
>+++ b/arch/x86/boot/compressed/kernel_info.S
>@@ -0,0 +1,12 @@
>+/* SPDX-License-Identifier: GPL-2.0 */
>+
>+ .section ".rodata.kernel_info", "a"
>+
>+ .global kernel_info
>+
>+kernel_info:
>+ /* Header. */
>+ .ascii "InfO"
>+ /* Size. */
>+ .long kernel_info_end - kernel_info
>+kernel_info_end:
>diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
>index 850b8762e889..ec6a25a43148 100644
>--- a/arch/x86/boot/header.S
>+++ b/arch/x86/boot/header.S
>@@ -557,6 +557,7 @@ pref_address: .quad LOAD_PHYSICAL_ADDR # preferred
>load addr
>
> init_size: .long INIT_SIZE # kernel initialization size
> handover_offset: .long 0 # Filled in by build.c
>+kernel_info_offset: .long 0 # Filled in by build.c
>
># End of setup header
>#####################################################
>
>diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
>index a93d44e58f9c..55e669d29e54 100644
>--- a/arch/x86/boot/tools/build.c
>+++ b/arch/x86/boot/tools/build.c
>@@ -56,6 +56,7 @@ u8 buf[SETUP_SECT_MAX*512];
> unsigned long efi32_stub_entry;
> unsigned long efi64_stub_entry;
> unsigned long efi_pe_entry;
>+unsigned long kernel_info;
> unsigned long startup_64;
>
>/*----------------------------------------------------------------------*/
>@@ -321,6 +322,7 @@ static void parse_zoffset(char *fname)
> PARSE_ZOFS(p, efi32_stub_entry);
> PARSE_ZOFS(p, efi64_stub_entry);
> PARSE_ZOFS(p, efi_pe_entry);
>+ PARSE_ZOFS(p, kernel_info);
> PARSE_ZOFS(p, startup_64);
>
> p = strchr(p, '\n');
>@@ -410,6 +412,9 @@ int main(int argc, char ** argv)
>
> efi_stub_entry_update();
>
>+ /* Update kernel_info offset. */
>+ put_unaligned_le32(kernel_info, &buf[0x268]);
>+
> crc = partial_crc32(buf, i, crc);
> if (fwrite(buf, 1, i, dest) != i)
> die("Writing setup failed");
>diff --git a/arch/x86/include/uapi/asm/bootparam.h
>b/arch/x86/include/uapi/asm/bootparam.h
>index 60733f137e9a..b05318112452 100644
>--- a/arch/x86/include/uapi/asm/bootparam.h
>+++ b/arch/x86/include/uapi/asm/bootparam.h
>@@ -86,6 +86,7 @@ struct setup_header {
> __u64 pref_address;
> __u32 init_size;
> __u32 handover_offset;
>+ __u32 kernel_info_offset;
> } __attribute__((packed));
>
> struct sys_desc_table {

I should like to make make things a bit more stringent: additional data should be made offsets from the kernel_info structure *and they should live in the .rodata.kernel_info section*. We should add a size field for the entire .kernel_info section, thus ensuring it is a single self-contained blob.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Next message: Tom Zanussi: "Re: [PATCH v3] trace:Add "gfp_t" support in synthetic_events"
Previous message: Thomas Gleixner: "Re: [PATCH 1/3] x86/Kconfig: Move ARCH_HAS_MEM_ENCRYPT to arch/Kconfig"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]