[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <143DFBDE-E604-48E0-8072-6DB68E3E83C1@zytor.com>
Date: Fri, 12 Jul 2019 08:56:44 -0700
From: hpa@...or.com
To: Daniel Kiper <daniel.kiper@...cle.com>, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, x86@...nel.org
CC: bp@...en8.de, corbet@....net, dpsmith@...rtussolutions.com,
eric.snowberg@...cle.com, kanth.ghatraju@...cle.com,
konrad.wilk@...cle.com, mingo@...hat.com,
ross.philipson@...cle.com, tglx@...utronix.de
Subject: Re: [PATCH v2 2/3] x86/boot: Introduce the setup_indirect
On July 4, 2019 9:36:11 AM PDT, Daniel Kiper <daniel.kiper@...cle.com> wrote:
>The setup_data is a bit awkward to use for extremely large data
>objects,
>both because the setup_data header has to be adjacent to the data
>object
>and because it has a 32-bit length field. However, it is important that
>intermediate stages of the boot process have a way to identify which
>chunks of memory are occupied by kernel data.
>
>Thus we introduce a uniform way to specify such indirect data as
>setup_indirect struct and SETUP_INDIRECT type.
>
>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@...or.com>
>Signed-off-by: Daniel Kiper <daniel.kiper@...cle.com>
>Reviewed-by: Eric Snowberg <eric.snowberg@...cle.com>
>Reviewed-by: Ross Philipson <ross.philipson@...cle.com>
>---
>v2 - suggestions/fixes:
> - add setup_indirect usage example
> (suggested by Eric Snowberg and Ross Philipson).
>---
>Documentation/x86/boot.rst | 38
>++++++++++++++++++++++++++++++++++-
> arch/x86/include/uapi/asm/bootparam.h | 11 +++++++++-
> 2 files changed, 47 insertions(+), 2 deletions(-)
>
>diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
>index a934a56f0516..23d3726d54fc 100644
>--- a/Documentation/x86/boot.rst
>+++ b/Documentation/x86/boot.rst
>@@ -73,7 +73,7 @@ Protocol 2.14: BURNT BY INCORRECT COMMIT
>ae7e1238e68f2a472a125673ab506d49158c188
> (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.
>+Protocol 2.15: (Kernel 5.3) Added the kernel_info and setup_indirect.
>============= ============================================================
>
> .. note::
>@@ -827,6 +827,42 @@ Protocol: 2.09+
> sure to consider the case where the linked list already contains
> entries.
>
>+ The setup_data is a bit awkward to use for extremely large data
>objects,
>+ both because the setup_data header has to be adjacent to the data
>object
>+ and because it has a 32-bit length field. However, it is important
>that
>+ intermediate stages of the boot process have a way to identify which
>+ chunks of memory are occupied by kernel data.
>+
>+ Thus setup_indirect struct and SETUP_INDIRECT type were introduced
>in
>+ protocol 2.15.
>+
>+ struct setup_indirect {
>+ __u32 type;
>+ __u32 reserved; /* Reserved, must be set to zero. */
>+ __u64 len;
>+ __u64 addr;
>+ };
>+
>+ The type member is itself simply a SETUP_* type. However, it cannot
>be
>+ SETUP_INDIRECT since making the setup_indirect a tree structure
>could
>+ require a lot of stack space in something that needs to parse it and
>+ stack space can be limited in boot contexts.
>+
>+ Let's give an example how to point to SETUP_E820_EXT data using
>setup_indirect.
>+ In this case setup_data and setup_indirect will look like this:
>+
>+ struct setup_data {
>+ __u64 next = 0 or <addr_of_next_setup_data_struct>;
>+ __u32 type = SETUP_INDIRECT;
>+ __u32 len = sizeof(setup_data);
>+ __u8 data[sizeof(setup_indirect)] = struct setup_indirect {
>+ __u32 type = SETUP_E820_EXT;
>+ __u32 reserved = 0;
>+ __u64 len = <len_of_SETUP_E820_EXT_data>;
>+ __u64 addr = <addr_of_SETUP_E820_EXT_data>;
>+ }
>+ }
>+
> ============ ============
> Field name: pref_address
> Type: read (reloc)
>diff --git a/arch/x86/include/uapi/asm/bootparam.h
>b/arch/x86/include/uapi/asm/bootparam.h
>index b05318112452..aaaa17fa6ad6 100644
>--- a/arch/x86/include/uapi/asm/bootparam.h
>+++ b/arch/x86/include/uapi/asm/bootparam.h
>@@ -2,7 +2,7 @@
> #ifndef _ASM_X86_BOOTPARAM_H
> #define _ASM_X86_BOOTPARAM_H
>
>-/* setup_data types */
>+/* setup_data/setup_indirect types */
> #define SETUP_NONE 0
> #define SETUP_E820_EXT 1
> #define SETUP_DTB 2
>@@ -10,6 +10,7 @@
> #define SETUP_EFI 4
> #define SETUP_APPLE_PROPERTIES 5
> #define SETUP_JAILHOUSE 6
>+#define SETUP_INDIRECT 7
>
> /* ram_size flags */
> #define RAMDISK_IMAGE_START_MASK 0x07FF
>@@ -47,6 +48,14 @@ struct setup_data {
> __u8 data[0];
> };
>
>+/* extensible setup indirect data node */
>+struct setup_indirect {
>+ __u32 type;
>+ __u32 reserved; /* Reserved, must be set to zero. */
>+ __u64 len;
>+ __u64 addr;
>+};
>+
> struct setup_header {
> __u8 setup_sects;
> __u16 root_flags;
This needs actual implementation; we can't advertise it until the kernel knows how to consume the data! It probably should be moved to after the 3/3 patch.
Implementing this has two parts:
1. The kernel needs to be augmented so it can find current objects via indirection.
2. And this is the main reason for this in the first place: the early code needs to walk the list and map out the memory areas which are occupied so it doesn't clobber anything; this allows this code to be generic as opposed to having to know what is a pointer and what size it might point to.
(The decompressor didn't need this until kaslr entered the picture, but now it does, sadly.)
Optional/future enhancements that might be nice:
3. Add some kind of description (e.g. foo=u64 ; bar=str ; baz=blob) to make it possible to write a bootloader that can load these kinds of objects without specific enabling.
4. Add support for mapping initramfs fragments this way.
5. Add support for passingload-on-boot kernel modules.
6. ... ?
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
Powered by blists - more mailing lists