LinuxLists.cc - [PATCH v5 0/4] arch/x86: Remove unnecessary dependencies on bootparam.h

2024-01-12 09:52:36

Subject: [PATCH v5 0/4] arch/x86: Remove unnecessary dependencies on bootparam.h

Reduce build time in some cases by removing unnecessary include statements
for <asm/bootparam.h>. Reorganize some header files accordingly.

While working on the kernel's boot-up graphics, I noticed that touching
include/linux/screen_info.h triggers a complete rebuild of the kernel
on x86. It turns out that the architecture's PCI and EFI headers include
<asm/bootparam.h>, which depends on <linux/screen_info.h>. But none of
the drivers have any business with boot parameters or the screen_info
state.

The patchset moves code from bootparam.h and efi.h into separate header
files and removes obsolete include statements on x86. I did

make allmodconfig
make -j28
touch include/linux/screen_info.h
time make -j28

to measure the time it takes to rebuild. Results without the patchset
are around 20 minutes.

real 20m46,705s
user 354m29,166s
sys 28m27,359s

And with the patchset applied it goes down to less than one minute.

real 0m56,643s
user 4m0,661s
sys 0m32,956s

The test system is an Intel i5-13500.

v5:
* silence clang warnings for real-mode code (Nathan)
* revert boot/compressed/misc.h (kernel test robot)
v4:
* fix fwd declaration in compressed/misc.h (Ard)
v3:
* keep setup_header in bootparam.h (Ard)
* implement arch_ima_efi_boot_mode() in source file (Ard)
v2:
* only keep struct boot_params in bootparam.h (Ard)
* simplify arch_ima_efi_boot_mode define (Ard)
* updated cover letter

Thomas Zimmermann (4):
arch/x86: Move UAPI setup structures into setup_data.h
arch/x86: Move internal setup_data structures into setup_data.h
arch/x86: Implement arch_ima_efi_boot_mode() in source file
arch/x86: Do not include <asm/bootparam.h> in several files

arch/x86/Makefile | 3 +
arch/x86/boot/compressed/acpi.c | 2 +
arch/x86/boot/compressed/cmdline.c | 2 +
arch/x86/boot/compressed/efi.c | 2 +
arch/x86/boot/compressed/efi.h | 9 ---
arch/x86/boot/compressed/pgtable_64.c | 1 +
arch/x86/boot/compressed/sev.c | 1 +
arch/x86/include/asm/efi.h | 14 +----
arch/x86/include/asm/kexec.h | 1 -
arch/x86/include/asm/mem_encrypt.h | 2 +-
arch/x86/include/asm/pci.h | 13 ----
arch/x86/include/asm/setup_data.h | 32 ++++++++++
arch/x86/include/asm/sev.h | 3 +-
arch/x86/include/asm/x86_init.h | 2 -
arch/x86/include/uapi/asm/bootparam.h | 72 +---------------------
arch/x86/include/uapi/asm/setup_data.h | 83 ++++++++++++++++++++++++++
arch/x86/kernel/crash.c | 1 +
arch/x86/kernel/sev-shared.c | 2 +
arch/x86/platform/efi/efi.c | 5 ++
arch/x86/platform/pvh/enlighten.c | 1 +
arch/x86/xen/enlighten_pvh.c | 1 +
arch/x86/xen/vga.c | 1 -
22 files changed, 143 insertions(+), 110 deletions(-)
create mode 100644 arch/x86/include/asm/setup_data.h
create mode 100644 arch/x86/include/uapi/asm/setup_data.h

--
2.43.0

2024-01-12 09:52:46

by Thomas Zimmermann

[permalink] [raw]

Subject: [PATCH v5 1/4] arch/x86: Move UAPI setup structures into setup_data.h

The type definition of struct pci_setup_rom in <asm/pci.h> requires
struct setup_data from <asm/bootparam.h>. Many drivers include
<linux/pci.h>, but do not use boot parameters. Changes to bootparam.h
or its included header files could easily trigger a large, unnecessary
rebuild of the kernel.

Moving struct setup_data and related code into its own header file
avoids including <asm/bootparam.h> in <asm/pci.h>. Instead include the
new header <asm/screen_data.h> and remove the include statement for
x86_init.h, which is unnecessary but pulls in bootparams.h.

Suggested-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Thomas Zimmermann <[email protected]>

---

v3:
* keep setup_header and friends in bootparam.h (Ard)
---
arch/x86/include/asm/pci.h | 2 +-
arch/x86/include/uapi/asm/bootparam.h | 72 +---------------------
arch/x86/include/uapi/asm/setup_data.h | 83 ++++++++++++++++++++++++++
3 files changed, 85 insertions(+), 72 deletions(-)
create mode 100644 arch/x86/include/uapi/asm/setup_data.h

diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index b40c462b4af3..f6100df3652e 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -10,7 +10,7 @@
#include <linux/numa.h>
#include <asm/io.h>
#include <asm/memtype.h>
-#include <asm/x86_init.h>
+#include <asm/setup_data.h>

struct pci_sysdata {
int domain; /* PCI domain */
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index 01d19fc22346..4a38e7917756 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -2,21 +2,7 @@
#ifndef _ASM_X86_BOOTPARAM_H
#define _ASM_X86_BOOTPARAM_H

-/* setup_data/setup_indirect types */
-#define SETUP_NONE 0
-#define SETUP_E820_EXT 1
-#define SETUP_DTB 2
-#define SETUP_PCI 3
-#define SETUP_EFI 4
-#define SETUP_APPLE_PROPERTIES 5
-#define SETUP_JAILHOUSE 6
-#define SETUP_CC_BLOB 7
-#define SETUP_IMA 8
-#define SETUP_RNG_SEED 9
-#define SETUP_ENUM_MAX SETUP_RNG_SEED
-
-#define SETUP_INDIRECT (1<<31)
-#define SETUP_TYPE_MAX (SETUP_ENUM_MAX | SETUP_INDIRECT)
+#include <asm/setup_data.h>

/* ram_size flags */
#define RAMDISK_IMAGE_START_MASK 0x07FF
@@ -48,22 +34,6 @@
#include <asm/ist.h>
#include <video/edid.h>

-/* extensible setup data list node */
-struct setup_data {
- __u64 next;
- __u32 type;
- __u32 len;
- __u8 data[];
-};
-
-/* 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;
@@ -136,51 +106,11 @@ struct efi_info {
*/
#define E820_MAX_ENTRIES_ZEROPAGE 128

-/*
- * The E820 memory region entry of the boot protocol ABI:
- */
-struct boot_e820_entry {
- __u64 addr;
- __u64 size;
- __u32 type;
-} __attribute__((packed));
-
/*
* Smallest compatible version of jailhouse_setup_data required by this kernel.
*/
#define JAILHOUSE_SETUP_REQUIRED_VERSION 1

-/*
- * The boot loader is passing platform information via this Jailhouse-specific
- * setup data structure.
- */
-struct jailhouse_setup_data {
- struct {
- __u16 version;
- __u16 compatible_version;
- } __attribute__((packed)) hdr;
- struct {
- __u16 pm_timer_address;
- __u16 num_cpus;
- __u64 pci_mmconfig_base;
- __u32 tsc_khz;
- __u32 apic_khz;
- __u8 standard_ioapic;
- __u8 cpu_ids[255];
- } __attribute__((packed)) v1;
- struct {
- __u32 flags;
- } __attribute__((packed)) v2;
-} __attribute__((packed));
-
-/*
- * IMA buffer setup data information from the previous kernel during kexec
- */
-struct ima_setup_data {
- __u64 addr;
- __u64 size;
-} __attribute__((packed));
-
/* The so-called "zeropage" */
struct boot_params {
struct screen_info screen_info; /* 0x000 */
diff --git a/arch/x86/include/uapi/asm/setup_data.h b/arch/x86/include/uapi/asm/setup_data.h
new file mode 100644
index 000000000000..b111b0c18544
--- /dev/null
+++ b/arch/x86/include/uapi/asm/setup_data.h
@@ -0,0 +1,83 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_ASM_X86_SETUP_DATA_H
+#define _UAPI_ASM_X86_SETUP_DATA_H
+
+/* setup_data/setup_indirect types */
+#define SETUP_NONE 0
+#define SETUP_E820_EXT 1
+#define SETUP_DTB 2
+#define SETUP_PCI 3
+#define SETUP_EFI 4
+#define SETUP_APPLE_PROPERTIES 5
+#define SETUP_JAILHOUSE 6
+#define SETUP_CC_BLOB 7
+#define SETUP_IMA 8
+#define SETUP_RNG_SEED 9
+#define SETUP_ENUM_MAX SETUP_RNG_SEED
+
+#define SETUP_INDIRECT (1<<31)
+#define SETUP_TYPE_MAX (SETUP_ENUM_MAX | SETUP_INDIRECT)
+
+#ifndef __ASSEMBLY__
+
+#include <linux/types.h>
+
+/* extensible setup data list node */
+struct setup_data {
+ __u64 next;
+ __u32 type;
+ __u32 len;
+ __u8 data[];
+};
+
+/* extensible setup indirect data node */
+struct setup_indirect {
+ __u32 type;
+ __u32 reserved; /* Reserved, must be set to zero. */
+ __u64 len;
+ __u64 addr;
+};
+
+/*
+ * The E820 memory region entry of the boot protocol ABI:
+ */
+struct boot_e820_entry {
+ __u64 addr;
+ __u64 size;
+ __u32 type;
+} __attribute__((packed));
+
+/*
+ * The boot loader is passing platform information via this Jailhouse-specific
+ * setup data structure.
+ */
+struct jailhouse_setup_data {
+ struct {
+ __u16 version;
+ __u16 compatible_version;
+ } __attribute__((packed)) hdr;
+ struct {
+ __u16 pm_timer_address;
+ __u16 num_cpus;
+ __u64 pci_mmconfig_base;
+ __u32 tsc_khz;
+ __u32 apic_khz;
+ __u8 standard_ioapic;
+ __u8 cpu_ids[255];
+ } __attribute__((packed)) v1;
+ struct {
+ __u32 flags;
+ } __attribute__((packed)) v2;
+} __attribute__((packed));
+
+/*
+ * IMA buffer setup data information from the previous kernel during kexec
+ */
+struct ima_setup_data {
+ __u64 addr;
+ __u64 size;
+} __attribute__((packed));
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _UAPI_ASM_X86_SETUP_DATA_H */
--
2.43.0

2024-01-12 09:52:50

by Thomas Zimmermann

[permalink] [raw]

Subject: [PATCH v5 2/4] arch/x86: Move internal setup_data structures into setup_data.h

2024-01-12 09:52:55

by Thomas Zimmermann

[permalink] [raw]

Subject: [PATCH v5 3/4] arch/x86: Implement arch_ima_efi_boot_mode() in source file

The x86 implementation of arch_ima_efi_boot_mode() uses the global
boot_param state. Move it into a source file to clean up the header.
Avoid potential rebuilds of unrelated source files if boot_params
changes.

Suggested-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Thomas Zimmermann <[email protected]>
---
arch/x86/include/asm/efi.h | 5 +++--
arch/x86/platform/efi/efi.c | 5 +++++
2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index a5d7a83e4c2a..1dc600fa3ba5 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -409,8 +409,9 @@ extern int __init efi_memmap_split_count(efi_memory_desc_t *md,
extern void __init efi_memmap_insert(struct efi_memory_map *old_memmap,
void *buf, struct efi_mem_range *mem);

-#define arch_ima_efi_boot_mode \
- ({ extern struct boot_params boot_params; boot_params.secure_boot; })
+extern enum efi_secureboot_mode __x86_ima_efi_boot_mode(void);
+
+#define arch_ima_efi_boot_mode __x86_ima_efi_boot_mode()

#ifdef CONFIG_EFI_RUNTIME_MAP
int efi_get_runtime_map_size(void);
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index e9f99c56f3ce..f090ec972d7b 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -950,3 +950,8 @@ umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n)
}
return attr->mode;
}
+
+enum efi_secureboot_mode __x86_ima_efi_boot_mode(void)
+{
+ return boot_params.secure_boot;
+}
--
2.43.0

2024-01-12 09:56:14

by Thomas Zimmermann

[permalink] [raw]

Subject: [PATCH v5 4/4] arch/x86: Do not include <asm/bootparam.h> in several files

Remove the include statement for <asm/bootparam.h> from several files
that don't require it. Limits the exposure of the boot parameters
within the Linux kernel code.

Signed-off-by: Thomas Zimmermann <[email protected]>
Acked-by: Ard Biesheuvel <[email protected]>

---

v5:
* revert of boot/compressed/misc.h (kernel test robot)
v3:
* revert of e820/types.h required
v2:
* clean up misc.h and e820/types.h
* include bootparam.h in several source files
---
arch/x86/boot/compressed/acpi.c | 2 ++
arch/x86/boot/compressed/cmdline.c | 2 ++
arch/x86/boot/compressed/efi.c | 2 ++
arch/x86/boot/compressed/pgtable_64.c | 1 +
arch/x86/boot/compressed/sev.c | 1 +
arch/x86/include/asm/kexec.h | 1 -
arch/x86/include/asm/mem_encrypt.h | 2 +-
arch/x86/include/asm/sev.h | 3 ++-
arch/x86/include/asm/x86_init.h | 2 --
arch/x86/kernel/crash.c | 1 +
arch/x86/kernel/sev-shared.c | 2 ++
arch/x86/platform/pvh/enlighten.c | 1 +
arch/x86/xen/enlighten_pvh.c | 1 +
arch/x86/xen/vga.c | 1 -
14 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 18d15d1ce87d..f196b1d1ddf8 100644
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -5,6 +5,8 @@
#include "../string.h"
#include "efi.h"

+#include <asm/bootparam.h>
+
#include <linux/numa.h>

/*
diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c
index c1bb180973ea..e162d7f59cc5 100644
--- a/arch/x86/boot/compressed/cmdline.c
+++ b/arch/x86/boot/compressed/cmdline.c
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: GPL-2.0
#include "misc.h"

+#include <asm/bootparam.h>
+
static unsigned long fs;
static inline void set_fs(unsigned long seg)
{
diff --git a/arch/x86/boot/compressed/efi.c b/arch/x86/boot/compressed/efi.c
index 6edd034b0b30..f2e50f9758e6 100644
--- a/arch/x86/boot/compressed/efi.c
+++ b/arch/x86/boot/compressed/efi.c
@@ -7,6 +7,8 @@

#include "misc.h"

+#include <asm/bootparam.h>
+
/**
* efi_get_type - Given a pointer to boot_params, determine the type of EFI environment.
*
diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
index 51f957b24ba7..c882e1f67af0 100644
--- a/arch/x86/boot/compressed/pgtable_64.c
+++ b/arch/x86/boot/compressed/pgtable_64.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
#include "misc.h"
+#include <asm/bootparam.h>
#include <asm/e820/types.h>
#include <asm/processor.h>
#include "pgtable.h"
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index 454acd7a2daf..13beae767e48 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -12,6 +12,7 @@
*/
#include "misc.h"

+#include <asm/bootparam.h>
#include <asm/pgtable_types.h>
#include <asm/sev.h>
#include <asm/trapnr.h>
diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
index c9f6a6c5de3c..91ca9a9ee3a2 100644
--- a/arch/x86/include/asm/kexec.h
+++ b/arch/x86/include/asm/kexec.h
@@ -25,7 +25,6 @@

#include <asm/page.h>
#include <asm/ptrace.h>
-#include <asm/bootparam.h>

struct kimage;

diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
index 359ada486fa9..c1a8a3408c18 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -15,7 +15,7 @@
#include <linux/init.h>
#include <linux/cc_platform.h>

-#include <asm/bootparam.h>
+struct boot_params;

#ifdef CONFIG_X86_MEM_ENCRYPT
void __init mem_encrypt_init(void);
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 5b4a1ce3d368..8dad8b1613bf 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -13,7 +13,6 @@

#include <asm/insn.h>
#include <asm/sev-common.h>
-#include <asm/bootparam.h>
#include <asm/coco.h>

#define GHCB_PROTOCOL_MIN 1ULL
@@ -22,6 +21,8 @@

#define VMGEXIT() { asm volatile("rep; vmmcall\n\r"); }

+struct boot_params;
+
enum es_result {
ES_OK, /* All good */
ES_UNSUPPORTED, /* Requested operation not supported */
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index c878616a18b8..f062715578a0 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -2,8 +2,6 @@
#ifndef _ASM_X86_PLATFORM_H
#define _ASM_X86_PLATFORM_H

-#include <asm/bootparam.h>
-
struct ghcb;
struct mpc_bus;
struct mpc_cpu;
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index c92d88680dbf..564cff7ed33a 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -26,6 +26,7 @@
#include <linux/vmalloc.h>
#include <linux/memblock.h>

+#include <asm/bootparam.h>
#include <asm/processor.h>
#include <asm/hardirq.h>
#include <asm/nmi.h>
diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index ccb0915e84e1..4962ec42dc68 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -9,6 +9,8 @@
* and is included directly into both code-bases.
*/

+#include <asm/setup_data.h>
+
#ifndef __BOOT_COMPRESSED
#define error(v) pr_err(v)
#define has_cpuflag(f) boot_cpu_has(f)
diff --git a/arch/x86/platform/pvh/enlighten.c b/arch/x86/platform/pvh/enlighten.c
index 00a92cb2c814..944e0290f2c0 100644
--- a/arch/x86/platform/pvh/enlighten.c
+++ b/arch/x86/platform/pvh/enlighten.c
@@ -3,6 +3,7 @@

#include <xen/hvc-console.h>

+#include <asm/bootparam.h>
#include <asm/io_apic.h>
#include <asm/hypervisor.h>
#include <asm/e820/api.h>
diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c
index ada3868c02c2..9e9db601bd52 100644
--- a/arch/x86/xen/enlighten_pvh.c
+++ b/arch/x86/xen/enlighten_pvh.c
@@ -4,6 +4,7 @@

#include <xen/hvc-console.h>

+#include <asm/bootparam.h>
#include <asm/io_apic.h>
#include <asm/hypervisor.h>
#include <asm/e820/api.h>
diff --git a/arch/x86/xen/vga.c b/arch/x86/xen/vga.c
index d97adab8420f..f7547807b0bd 100644
--- a/arch/x86/xen/vga.c
+++ b/arch/x86/xen/vga.c
@@ -2,7 +2,6 @@
#include <linux/screen_info.h>
#include <linux/init.h>

-#include <asm/bootparam.h>
#include <asm/setup.h>

#include <xen/interface/xen.h>
--
2.43.0

2024-01-12 17:29:21

by Ard Biesheuvel

[permalink] [raw]

Subject: Re: [PATCH v5 0/4] arch/x86: Remove unnecessary dependencies on bootparam.h

On Fri, 12 Jan 2024 at 10:50, Thomas Zimmermann <[email protected]> wrote:
>
> Reduce build time in some cases by removing unnecessary include statements
> for <asm/bootparam.h>. Reorganize some header files accordingly.
>
> While working on the kernel's boot-up graphics, I noticed that touching
> include/linux/screen_info.h triggers a complete rebuild of the kernel
> on x86. It turns out that the architecture's PCI and EFI headers include
> <asm/bootparam.h>, which depends on <linux/screen_info.h>. But none of
> the drivers have any business with boot parameters or the screen_info
> state.
>
> The patchset moves code from bootparam.h and efi.h into separate header
> files and removes obsolete include statements on x86. I did
>
> make allmodconfig
> make -j28
> touch include/linux/screen_info.h
> time make -j28
>
> to measure the time it takes to rebuild. Results without the patchset
> are around 20 minutes.
>
> real 20m46,705s
> user 354m29,166s
> sys 28m27,359s
>
> And with the patchset applied it goes down to less than one minute.
>
> real 0m56,643s
> user 4m0,661s
> sys 0m32,956s
>
> The test system is an Intel i5-13500.
>
> v5:
> * silence clang warnings for real-mode code (Nathan)
> * revert boot/compressed/misc.h (kernel test robot)
> v4:
> * fix fwd declaration in compressed/misc.h (Ard)
> v3:
> * keep setup_header in bootparam.h (Ard)
> * implement arch_ima_efi_boot_mode() in source file (Ard)
> v2:
> * only keep struct boot_params in bootparam.h (Ard)
> * simplify arch_ima_efi_boot_mode define (Ard)
> * updated cover letter
>
> Thomas Zimmermann (4):
> arch/x86: Move UAPI setup structures into setup_data.h
> arch/x86: Move internal setup_data structures into setup_data.h
> arch/x86: Implement arch_ima_efi_boot_mode() in source file
> arch/x86: Do not include <asm/bootparam.h> in several files
>

This looks ok to me, thanks for sticking with it.

For the series,

Reviewed-by: Ard Biesheuvel <[email protected]>

2024-01-15 07:59:25

by Thomas Zimmermann

[permalink] [raw]

Subject: Re: [PATCH v5 0/4] arch/x86: Remove unnecessary dependencies on bootparam.h

Hi

Am 12.01.24 um 18:28 schrieb Ard Biesheuvel:
> On Fri, 12 Jan 2024 at 10:50, Thomas Zimmermann <[email protected]> wrote:
>>
>> Reduce build time in some cases by removing unnecessary include statements
>> for <asm/bootparam.h>. Reorganize some header files accordingly.
>>
>> While working on the kernel's boot-up graphics, I noticed that touching
>> include/linux/screen_info.h triggers a complete rebuild of the kernel
>> on x86. It turns out that the architecture's PCI and EFI headers include
>> <asm/bootparam.h>, which depends on <linux/screen_info.h>. But none of
>> the drivers have any business with boot parameters or the screen_info
>> state.
>>
>> The patchset moves code from bootparam.h and efi.h into separate header
>> files and removes obsolete include statements on x86. I did
>>
>> make allmodconfig
>> make -j28
>> touch include/linux/screen_info.h
>> time make -j28
>>
>> to measure the time it takes to rebuild. Results without the patchset
>> are around 20 minutes.
>>
>> real 20m46,705s
>> user 354m29,166s
>> sys 28m27,359s
>>
>> And with the patchset applied it goes down to less than one minute.
>>
>> real 0m56,643s
>> user 4m0,661s
>> sys 0m32,956s
>>
>> The test system is an Intel i5-13500.
>>
>> v5:
>> * silence clang warnings for real-mode code (Nathan)
>> * revert boot/compressed/misc.h (kernel test robot)
>> v4:
>> * fix fwd declaration in compressed/misc.h (Ard)
>> v3:
>> * keep setup_header in bootparam.h (Ard)
>> * implement arch_ima_efi_boot_mode() in source file (Ard)
>> v2:
>> * only keep struct boot_params in bootparam.h (Ard)
>> * simplify arch_ima_efi_boot_mode define (Ard)
>> * updated cover letter
>>
>> Thomas Zimmermann (4):
>> arch/x86: Move UAPI setup structures into setup_data.h
>> arch/x86: Move internal setup_data structures into setup_data.h
>> arch/x86: Implement arch_ima_efi_boot_mode() in source file
>> arch/x86: Do not include <asm/bootparam.h> in several files
>>
>
> This looks ok to me, thanks for sticking with it.
>
> For the series,
>
> Reviewed-by: Ard Biesheuvel <[email protected]>

Thank you so much. Can this series go through the x86 tree?

Best regards
Thomas

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Attachments:

OpenPGP_signature.asc (855.00 B)
OpenPGP digital signature

2024-01-15 10:56:01

by Ard Biesheuvel

[permalink] [raw]

Subject: Re: [PATCH v5 0/4] arch/x86: Remove unnecessary dependencies on bootparam.h

On Mon, 15 Jan 2024 at 08:58, Thomas Zimmermann <[email protected]> wrote:
>
> Hi
>
> Am 12.01.24 um 18:28 schrieb Ard Biesheuvel:
> > On Fri, 12 Jan 2024 at 10:50, Thomas Zimmermann <[email protected]> wrote:
> >>
> >> Reduce build time in some cases by removing unnecessary include statements
> >> for <asm/bootparam.h>. Reorganize some header files accordingly.
> >>
> >> While working on the kernel's boot-up graphics, I noticed that touching
> >> include/linux/screen_info.h triggers a complete rebuild of the kernel
> >> on x86. It turns out that the architecture's PCI and EFI headers include
> >> <asm/bootparam.h>, which depends on <linux/screen_info.h>. But none of
> >> the drivers have any business with boot parameters or the screen_info
> >> state.
> >>
> >> The patchset moves code from bootparam.h and efi.h into separate header
> >> files and removes obsolete include statements on x86. I did
> >>
> >> make allmodconfig
> >> make -j28
> >> touch include/linux/screen_info.h
> >> time make -j28
> >>
> >> to measure the time it takes to rebuild. Results without the patchset
> >> are around 20 minutes.
> >>
> >> real 20m46,705s
> >> user 354m29,166s
> >> sys 28m27,359s
> >>
> >> And with the patchset applied it goes down to less than one minute.
> >>
> >> real 0m56,643s
> >> user 4m0,661s
> >> sys 0m32,956s
> >>
> >> The test system is an Intel i5-13500.
> >>
> >> v5:
> >> * silence clang warnings for real-mode code (Nathan)
> >> * revert boot/compressed/misc.h (kernel test robot)
> >> v4:
> >> * fix fwd declaration in compressed/misc.h (Ard)
> >> v3:
> >> * keep setup_header in bootparam.h (Ard)
> >> * implement arch_ima_efi_boot_mode() in source file (Ard)
> >> v2:
> >> * only keep struct boot_params in bootparam.h (Ard)
> >> * simplify arch_ima_efi_boot_mode define (Ard)
> >> * updated cover letter
> >>
> >> Thomas Zimmermann (4):
> >> arch/x86: Move UAPI setup structures into setup_data.h
> >> arch/x86: Move internal setup_data structures into setup_data.h
> >> arch/x86: Implement arch_ima_efi_boot_mode() in source file
> >> arch/x86: Do not include <asm/bootparam.h> in several files
> >>
> >
> > This looks ok to me, thanks for sticking with it.
> >
> > For the series,
> >
> > Reviewed-by: Ard Biesheuvel <[email protected]>
>
> Thank you so much. Can this series go through the x86 tree?
>

Yes, this should be taken through the -tip tree. But I am not a -tip maintainer.

But please be aware that we are in the middle of the merge window
right now, and I suspect that the -tip maintainers may have some
feedback of their own. So give it at least a week or so, and ping this
thread again to ask how to proceed.

Also, please trim the cc list a bit when you do - this is mostly a x86
specific reshuffle of headers so no need to keep all the other
subsystem maintainers on cc while we finish up the discussion.

2024-01-15 11:00:40

by Borislav Petkov

[permalink] [raw]

Subject: Re: [PATCH v5 0/4] arch/x86: Remove unnecessary dependencies on bootparam.h

On Mon, Jan 15, 2024 at 11:55:36AM +0100, Ard Biesheuvel wrote:
> But please be aware that we are in the middle of the merge window

Yes, and the merge window has been suspended too:

https://lore.kernel.org/r/CAHk-=wjMWpmXtKeiN__vnNO4TcttZR-8dVvd_oBq%[email protected]

> right now, and I suspect that the -tip maintainers may have some
> feedback of their own. So give it at least a week or so, and ping this
> thread again to ask how to proceed.

From: Documentation/process/maintainer-tip.rst

"Merge window
^^^^^^^^^^^^

Please do not expect large patch series to be handled during the merge
window or even during the week before. Such patches should be submitted in
mergeable state *at* *least* a week before the merge window opens.
Exceptions are made for bug fixes and *sometimes* for small standalone
drivers for new hardware or minimally invasive patches for hardware
enablement.

During the merge window, the maintainers instead focus on following the
upstream changes, fixing merge window fallout, collecting bug fixes, and
allowing themselves a breath. Please respect that.

The release candidate -rc1 is the starting point for new patches to be
applied which are targeted for the next merge window."

So pls be patient.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2024-01-30 18:36:52

by tip-bot2 for Alexey Makhalov

[permalink] [raw]

Subject: [tip: x86/build] x86/efi: Implement arch_ima_efi_boot_mode() in source file

The following commit has been merged into the x86/build branch of tip:

Commit-ID: 785ddc8b6bebd958a5b6fb7b6b4aa6584c2f0cb2
Gitweb: https://git.kernel.org/tip/785ddc8b6bebd958a5b6fb7b6b4aa6584c2f0cb2
Author: Thomas Zimmermann <[email protected]>
AuthorDate: Fri, 12 Jan 2024 10:44:38 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Tue, 30 Jan 2024 15:17:17 +01:00

x86/efi: Implement arch_ima_efi_boot_mode() in source file

The x86 implementation of arch_ima_efi_boot_mode() uses the global
boot_param state. Move it into a source file to clean up the header.
Avoid potential rebuilds of unrelated source files if boot_params
changes.

Suggested-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/efi.h | 5 +++--
arch/x86/platform/efi/efi.c | 5 +++++
2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index a5d7a83..1dc600f 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -409,8 +409,9 @@ extern int __init efi_memmap_split_count(efi_memory_desc_t *md,
extern void __init efi_memmap_insert(struct efi_memory_map *old_memmap,
void *buf, struct efi_mem_range *mem);

-#define arch_ima_efi_boot_mode \
- ({ extern struct boot_params boot_params; boot_params.secure_boot; })
+extern enum efi_secureboot_mode __x86_ima_efi_boot_mode(void);
+
+#define arch_ima_efi_boot_mode __x86_ima_efi_boot_mode()

#ifdef CONFIG_EFI_RUNTIME_MAP
int efi_get_runtime_map_size(void);
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index e9f99c5..f090ec9 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -950,3 +950,8 @@ umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n)
}
return attr->mode;
}
+
+enum efi_secureboot_mode __x86_ima_efi_boot_mode(void)
+{
+ return boot_params.secure_boot;
+}

2024-01-30 18:36:55

by tip-bot2 for Alexey Makhalov

[permalink] [raw]

Subject: [tip: x86/build] x86: Do not include <asm/bootparam.h> in several files

The following commit has been merged into the x86/build branch of tip:

Commit-ID: 103bf75fc928d16185feb216bda525b5aaca0b18
Gitweb: https://git.kernel.org/tip/103bf75fc928d16185feb216bda525b5aaca0b18
Author: Thomas Zimmermann <[email protected]>
AuthorDate: Fri, 12 Jan 2024 10:44:39 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Tue, 30 Jan 2024 15:17:24 +01:00

x86: Do not include <asm/bootparam.h> in several files

Remove the include statement for <asm/bootparam.h> from several files
that don't require it and limit the exposure of those definitions within
the Linux kernel code.

[ bp: Massage commit message. ]

Signed-off-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
Acked-by: Ard Biesheuvel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/boot/compressed/acpi.c | 2 ++
arch/x86/boot/compressed/cmdline.c | 2 ++
arch/x86/boot/compressed/efi.c | 2 ++
arch/x86/boot/compressed/pgtable_64.c | 1 +
arch/x86/boot/compressed/sev.c | 1 +
arch/x86/include/asm/kexec.h | 1 -
arch/x86/include/asm/mem_encrypt.h | 2 +-
arch/x86/include/asm/sev.h | 3 ++-
arch/x86/include/asm/x86_init.h | 2 --
arch/x86/kernel/crash.c | 1 +
arch/x86/kernel/sev-shared.c | 2 ++
arch/x86/platform/pvh/enlighten.c | 1 +
arch/x86/xen/enlighten_pvh.c | 1 +
arch/x86/xen/vga.c | 1 -
14 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 18d15d1..f196b1d 100644
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -5,6 +5,8 @@
#include "../string.h"
#include "efi.h"

+#include <asm/bootparam.h>
+
#include <linux/numa.h>

/*
diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c
index c1bb180..e162d7f 100644
--- a/arch/x86/boot/compressed/cmdline.c
+++ b/arch/x86/boot/compressed/cmdline.c
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: GPL-2.0
#include "misc.h"

+#include <asm/bootparam.h>
+
static unsigned long fs;
static inline void set_fs(unsigned long seg)
{
diff --git a/arch/x86/boot/compressed/efi.c b/arch/x86/boot/compressed/efi.c
index 6edd034..f2e50f9 100644
--- a/arch/x86/boot/compressed/efi.c
+++ b/arch/x86/boot/compressed/efi.c
@@ -7,6 +7,8 @@

#include "misc.h"

+#include <asm/bootparam.h>
+
/**
* efi_get_type - Given a pointer to boot_params, determine the type of EFI environment.
*
diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
index 51f957b..c882e1f 100644
--- a/arch/x86/boot/compressed/pgtable_64.c
+++ b/arch/x86/boot/compressed/pgtable_64.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
#include "misc.h"
+#include <asm/bootparam.h>
#include <asm/e820/types.h>
#include <asm/processor.h>
#include "pgtable.h"
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index 454acd7..13beae7 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -12,6 +12,7 @@
*/
#include "misc.h"

+#include <asm/bootparam.h>
#include <asm/pgtable_types.h>
#include <asm/sev.h>
#include <asm/trapnr.h>
diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
index c9f6a6c..91ca9a9 100644
--- a/arch/x86/include/asm/kexec.h
+++ b/arch/x86/include/asm/kexec.h
@@ -25,7 +25,6 @@

#include <asm/page.h>
#include <asm/ptrace.h>
-#include <asm/bootparam.h>

struct kimage;

diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
index 359ada4..c1a8a34 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -15,7 +15,7 @@
#include <linux/init.h>
#include <linux/cc_platform.h>

-#include <asm/bootparam.h>
+struct boot_params;

#ifdef CONFIG_X86_MEM_ENCRYPT
void __init mem_encrypt_init(void);
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 5b4a1ce..8dad8b1 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -13,7 +13,6 @@

#include <asm/insn.h>
#include <asm/sev-common.h>
-#include <asm/bootparam.h>
#include <asm/coco.h>

#define GHCB_PROTOCOL_MIN 1ULL
@@ -22,6 +21,8 @@

#define VMGEXIT() { asm volatile("rep; vmmcall\n\r"); }

+struct boot_params;
+
enum es_result {
ES_OK, /* All good */
ES_UNSUPPORTED, /* Requested operation not supported */
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index c878616..f062715 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -2,8 +2,6 @@
#ifndef _ASM_X86_PLATFORM_H
#define _ASM_X86_PLATFORM_H

-#include <asm/bootparam.h>
-
struct ghcb;
struct mpc_bus;
struct mpc_cpu;
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index b6b0443..fe404c1 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -26,6 +26,7 @@
#include <linux/vmalloc.h>
#include <linux/memblock.h>

+#include <asm/bootparam.h>
#include <asm/processor.h>
#include <asm/hardirq.h>
#include <asm/nmi.h>
diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index 1d24ec6..fcc6d83 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -9,6 +9,8 @@
* and is included directly into both code-bases.
*/

+#include <asm/setup_data.h>
+
#ifndef __BOOT_COMPRESSED
#define error(v) pr_err(v)
#define has_cpuflag(f) boot_cpu_has(f)
diff --git a/arch/x86/platform/pvh/enlighten.c b/arch/x86/platform/pvh/enlighten.c
index 00a92cb..944e029 100644
--- a/arch/x86/platform/pvh/enlighten.c
+++ b/arch/x86/platform/pvh/enlighten.c
@@ -3,6 +3,7 @@

#include <xen/hvc-console.h>

+#include <asm/bootparam.h>
#include <asm/io_apic.h>
#include <asm/hypervisor.h>
#include <asm/e820/api.h>
diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c
index ada3868..9e9db60 100644
--- a/arch/x86/xen/enlighten_pvh.c
+++ b/arch/x86/xen/enlighten_pvh.c
@@ -4,6 +4,7 @@

#include <xen/hvc-console.h>

+#include <asm/bootparam.h>
#include <asm/io_apic.h>
#include <asm/hypervisor.h>
#include <asm/e820/api.h>
diff --git a/arch/x86/xen/vga.c b/arch/x86/xen/vga.c
index d97adab..f754780 100644
--- a/arch/x86/xen/vga.c
+++ b/arch/x86/xen/vga.c
@@ -2,7 +2,6 @@
#include <linux/screen_info.h>
#include <linux/init.h>

-#include <asm/bootparam.h>
#include <asm/setup.h>

#include <xen/interface/xen.h>

2024-01-30 18:37:01

by tip-bot2 for Alexey Makhalov

[permalink] [raw]

Subject: [tip: x86/build] x86/setup: Move internal setup_data structures into setup_data.h

The following commit has been merged into the x86/build branch of tip:

Commit-ID: 2afa7994f794016d117b192e36b856df66d71172
Gitweb: https://git.kernel.org/tip/2afa7994f794016d117b192e36b856df66d71172
Author: Thomas Zimmermann <[email protected]>
AuthorDate: Fri, 12 Jan 2024 10:44:37 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Tue, 30 Jan 2024 15:17:12 +01:00

x86/setup: Move internal setup_data structures into setup_data.h

Move struct_efi_setup_data in order to unify duplicated definition of
the data structure in a single place. Also silence clang's warnings
about GNU extensions in real-mode code which might occur from the
changed includes.

[ bp: Massage commit message. ]

Signed-off-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/Makefile | 3 +++-
arch/x86/boot/compressed/efi.h | 9 +--------
arch/x86/include/asm/efi.h | 9 +--------
arch/x86/include/asm/pci.h | 13 +------------
arch/x86/include/asm/setup_data.h | 32 ++++++++++++++++++++++++++++++-
5 files changed, 35 insertions(+), 31 deletions(-)
create mode 100644 arch/x86/include/asm/setup_data.h

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 1a068de..24076db 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -53,6 +53,9 @@ REALMODE_CFLAGS += -fno-stack-protector
REALMODE_CFLAGS += -Wno-address-of-packed-member
REALMODE_CFLAGS += $(cc_stack_align4)
REALMODE_CFLAGS += $(CLANG_FLAGS)
+ifdef CONFIG_CC_IS_CLANG
+REALMODE_CFLAGS += -Wno-gnu
+endif
export REALMODE_CFLAGS

# BITS is used as extension for files which are available in a 32 bit
diff --git a/arch/x86/boot/compressed/efi.h b/arch/x86/boot/compressed/efi.h
index 866c0af..b223009 100644
--- a/arch/x86/boot/compressed/efi.h
+++ b/arch/x86/boot/compressed/efi.h
@@ -97,15 +97,6 @@ typedef struct {
u32 tables;
} efi_system_table_32_t;

-/* kexec external ABI */
-struct efi_setup_data {
- u64 fw_vendor;
- u64 __unused;
- u64 tables;
- u64 smbios;
- u64 reserved[8];
-};
-
struct efi_unaccepted_memory {
u32 version;
u32 unit_size;
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index c4555b2..a5d7a83 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -143,15 +143,6 @@ extern void efi_free_boot_services(void);
void arch_efi_call_virt_setup(void);
void arch_efi_call_virt_teardown(void);

-/* kexec external ABI */
-struct efi_setup_data {
- u64 fw_vendor;
- u64 __unused;
- u64 tables;
- u64 smbios;
- u64 reserved[8];
-};
-
extern u64 efi_setup;

#ifdef CONFIG_EFI
diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index f6100df..b3ab80a 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -10,7 +10,6 @@
#include <linux/numa.h>
#include <asm/io.h>
#include <asm/memtype.h>
-#include <asm/setup_data.h>

struct pci_sysdata {
int domain; /* PCI domain */
@@ -124,16 +123,4 @@ cpumask_of_pcibus(const struct pci_bus *bus)
}
#endif

-struct pci_setup_rom {
- struct setup_data data;
- uint16_t vendor;
- uint16_t devid;
- uint64_t pcilen;
- unsigned long segment;
- unsigned long bus;
- unsigned long device;
- unsigned long function;
- uint8_t romdata[];
-};
-
#endif /* _ASM_X86_PCI_H */
diff --git a/arch/x86/include/asm/setup_data.h b/arch/x86/include/asm/setup_data.h
new file mode 100644
index 0000000..77c5111
--- /dev/null
+++ b/arch/x86/include/asm/setup_data.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_SETUP_DATA_H
+#define _ASM_X86_SETUP_DATA_H
+
+#include <uapi/asm/setup_data.h>
+
+#ifndef __ASSEMBLY__
+
+struct pci_setup_rom {
+ struct setup_data data;
+ uint16_t vendor;
+ uint16_t devid;
+ uint64_t pcilen;
+ unsigned long segment;
+ unsigned long bus;
+ unsigned long device;
+ unsigned long function;
+ uint8_t romdata[];
+};
+
+/* kexec external ABI */
+struct efi_setup_data {
+ u64 fw_vendor;
+ u64 __unused;
+ u64 tables;
+ u64 smbios;
+ u64 reserved[8];
+};
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _ASM_X86_SETUP_DATA_H */

2024-01-30 18:37:11

by tip-bot2 for Alexey Makhalov

[permalink] [raw]

Subject: [tip: x86/build] x86/setup: Move UAPI setup structures into setup_data.h

The following commit has been merged into the x86/build branch of tip:

Commit-ID: efd7def00406ac57400501cdc76d0d95d4691927
Gitweb: https://git.kernel.org/tip/efd7def00406ac57400501cdc76d0d95d4691927
Author: Thomas Zimmermann <[email protected]>
AuthorDate: Fri, 12 Jan 2024 10:44:36 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Tue, 30 Jan 2024 15:17:06 +01:00

x86/setup: Move UAPI setup structures into setup_data.h

The type definition of struct pci_setup_rom in <asm/pci.h> requires
struct setup_data from <asm/bootparam.h>. Many drivers include
<linux/pci.h>, but do not use boot parameters. Changes to bootparam.h or
its included header files could easily trigger a large, unnecessary
rebuild of the kernel.

Moving struct setup_data and related code into its own header file
avoids including <asm/bootparam.h> in <asm/pci.h>. Instead include the
new header <asm/screen_data.h> and remove the include statement for
x86_init.h, which is unnecessary but pulls in bootparams.h.

Suggested-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/pci.h | 2 +-
arch/x86/include/uapi/asm/bootparam.h | 72 +----------------------
arch/x86/include/uapi/asm/setup_data.h | 83 +++++++++++++++++++++++++-
3 files changed, 85 insertions(+), 72 deletions(-)
create mode 100644 arch/x86/include/uapi/asm/setup_data.h

diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index b40c462..f6100df 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -10,7 +10,7 @@
#include <linux/numa.h>
#include <asm/io.h>
#include <asm/memtype.h>
-#include <asm/x86_init.h>
+#include <asm/setup_data.h>

struct pci_sysdata {
int domain; /* PCI domain */
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index 01d19fc..4a38e79 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -2,21 +2,7 @@
#ifndef _ASM_X86_BOOTPARAM_H
#define _ASM_X86_BOOTPARAM_H

-/* setup_data/setup_indirect types */
-#define SETUP_NONE 0
-#define SETUP_E820_EXT 1
-#define SETUP_DTB 2
-#define SETUP_PCI 3
-#define SETUP_EFI 4
-#define SETUP_APPLE_PROPERTIES 5
-#define SETUP_JAILHOUSE 6
-#define SETUP_CC_BLOB 7
-#define SETUP_IMA 8
-#define SETUP_RNG_SEED 9
-#define SETUP_ENUM_MAX SETUP_RNG_SEED
-
-#define SETUP_INDIRECT (1<<31)
-#define SETUP_TYPE_MAX (SETUP_ENUM_MAX | SETUP_INDIRECT)
+#include <asm/setup_data.h>

/* ram_size flags */
#define RAMDISK_IMAGE_START_MASK 0x07FF
@@ -48,22 +34,6 @@
#include <asm/ist.h>
#include <video/edid.h>

-/* extensible setup data list node */
-struct setup_data {
- __u64 next;
- __u32 type;
- __u32 len;
- __u8 data[];
-};
-
-/* 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;
@@ -137,50 +107,10 @@ struct efi_info {
#define E820_MAX_ENTRIES_ZEROPAGE 128

/*
- * The E820 memory region entry of the boot protocol ABI:
- */
-struct boot_e820_entry {
- __u64 addr;
- __u64 size;
- __u32 type;
-} __attribute__((packed));
-
-/*
* Smallest compatible version of jailhouse_setup_data required by this kernel.
*/
#define JAILHOUSE_SETUP_REQUIRED_VERSION 1

-/*
- * The boot loader is passing platform information via this Jailhouse-specific
- * setup data structure.
- */
-struct jailhouse_setup_data {
- struct {
- __u16 version;
- __u16 compatible_version;
- } __attribute__((packed)) hdr;
- struct {
- __u16 pm_timer_address;
- __u16 num_cpus;
- __u64 pci_mmconfig_base;
- __u32 tsc_khz;
- __u32 apic_khz;
- __u8 standard_ioapic;
- __u8 cpu_ids[255];
- } __attribute__((packed)) v1;
- struct {
- __u32 flags;
- } __attribute__((packed)) v2;
-} __attribute__((packed));
-
-/*
- * IMA buffer setup data information from the previous kernel during kexec
- */
-struct ima_setup_data {
- __u64 addr;
- __u64 size;
-} __attribute__((packed));
-
/* The so-called "zeropage" */
struct boot_params {
struct screen_info screen_info; /* 0x000 */
diff --git a/arch/x86/include/uapi/asm/setup_data.h b/arch/x86/include/uapi/asm/setup_data.h
new file mode 100644
index 0000000..b111b0c
--- /dev/null
+++ b/arch/x86/include/uapi/asm/setup_data.h
@@ -0,0 +1,83 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_ASM_X86_SETUP_DATA_H
+#define _UAPI_ASM_X86_SETUP_DATA_H
+
+/* setup_data/setup_indirect types */
+#define SETUP_NONE 0
+#define SETUP_E820_EXT 1
+#define SETUP_DTB 2
+#define SETUP_PCI 3
+#define SETUP_EFI 4
+#define SETUP_APPLE_PROPERTIES 5
+#define SETUP_JAILHOUSE 6
+#define SETUP_CC_BLOB 7
+#define SETUP_IMA 8
+#define SETUP_RNG_SEED 9
+#define SETUP_ENUM_MAX SETUP_RNG_SEED
+
+#define SETUP_INDIRECT (1<<31)
+#define SETUP_TYPE_MAX (SETUP_ENUM_MAX | SETUP_INDIRECT)
+
+#ifndef __ASSEMBLY__
+
+#include <linux/types.h>
+
+/* extensible setup data list node */
+struct setup_data {
+ __u64 next;
+ __u32 type;
+ __u32 len;
+ __u8 data[];
+};
+
+/* extensible setup indirect data node */
+struct setup_indirect {
+ __u32 type;
+ __u32 reserved; /* Reserved, must be set to zero. */
+ __u64 len;
+ __u64 addr;
+};
+
+/*
+ * The E820 memory region entry of the boot protocol ABI:
+ */
+struct boot_e820_entry {
+ __u64 addr;
+ __u64 size;
+ __u32 type;
+} __attribute__((packed));
+
+/*
+ * The boot loader is passing platform information via this Jailhouse-specific
+ * setup data structure.
+ */
+struct jailhouse_setup_data {
+ struct {
+ __u16 version;
+ __u16 compatible_version;
+ } __attribute__((packed)) hdr;
+ struct {
+ __u16 pm_timer_address;
+ __u16 num_cpus;
+ __u64 pci_mmconfig_base;
+ __u32 tsc_khz;
+ __u32 apic_khz;
+ __u8 standard_ioapic;
+ __u8 cpu_ids[255];
+ } __attribute__((packed)) v1;
+ struct {
+ __u32 flags;
+ } __attribute__((packed)) v2;
+} __attribute__((packed));
+
+/*
+ * IMA buffer setup data information from the previous kernel during kexec
+ */
+struct ima_setup_data {
+ __u64 addr;
+ __u64 size;
+} __attribute__((packed));
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _UAPI_ASM_X86_SETUP_DATA_H */