Reduce built 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/linus/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.
v2:
* only keep struct boot_params in bootparam.h (Ard)
* simplify arch_ima_efi_boot_mode define (Ard)
* updated cover letter
Thomas Zimmermann (3):
arch/x86: Move UAPI setup structures into setup_data.h
arch/x86: Add <asm/ima-efi.h> for arch_ima_efi_boot_mode
arch/x86: Do not include <asm/bootparam.h> in several header 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/misc.h | 3 +-
arch/x86/boot/compressed/pgtable_64.c | 1 +
arch/x86/boot/compressed/sev.c | 1 +
arch/x86/include/asm/e820/types.h | 2 +-
arch/x86/include/asm/efi.h | 3 -
arch/x86/include/asm/ima-efi.h | 11 ++
arch/x86/include/asm/kexec.h | 1 -
arch/x86/include/asm/mem_encrypt.h | 2 +-
arch/x86/include/asm/pci.h | 2 +-
arch/x86/include/asm/sev.h | 3 +-
arch/x86/include/asm/x86_init.h | 2 -
arch/x86/include/uapi/asm/bootparam.h | 218 +----------------------
arch/x86/include/uapi/asm/setup_data.h | 229 +++++++++++++++++++++++++
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 -
include/asm-generic/Kbuild | 1 +
include/asm-generic/ima-efi.h | 16 ++
security/integrity/ima/ima_efi.c | 5 +-
24 files changed, 279 insertions(+), 233 deletions(-)
create mode 100644 arch/x86/include/asm/ima-efi.h
create mode 100644 arch/x86/include/uapi/asm/setup_data.h
create mode 100644 include/asm-generic/ima-efi.h
base-commit: 961ef418705ac5808345e883acd91f8ce167e00b
prerequisite-patch-id: 0aa359f6144c4015c140c8a6750be19099c676fb
prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
prerequisite-patch-id: cbc453ee02fae02af22fbfdce56ab732c7a88c36
prerequisite-patch-id: e7a5405fb48608e0c8e3b41bf983fefa2c8bd1f3
--
2.43.0
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 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]>
---
arch/x86/include/asm/pci.h | 2 +-
arch/x86/include/uapi/asm/bootparam.h | 218 +----------------------
arch/x86/include/uapi/asm/setup_data.h | 229 +++++++++++++++++++++++++
3 files changed, 231 insertions(+), 218 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..f6361eb792fd 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -2,42 +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)
-
-/* ram_size flags */
-#define RAMDISK_IMAGE_START_MASK 0x07FF
-#define RAMDISK_PROMPT_FLAG 0x8000
-#define RAMDISK_LOAD_FLAG 0x4000
-
-/* loadflags */
-#define LOADED_HIGH (1<<0)
-#define KASLR_FLAG (1<<1)
-#define QUIET_FLAG (1<<5)
-#define KEEP_SEGMENTS (1<<6)
-#define CAN_USE_HEAP (1<<7)
-
-/* xloadflags */
-#define XLF_KERNEL_64 (1<<0)
-#define XLF_CAN_BE_LOADED_ABOVE_4G (1<<1)
-#define XLF_EFI_HANDOVER_32 (1<<2)
-#define XLF_EFI_HANDOVER_64 (1<<3)
-#define XLF_EFI_KEXEC (1<<4)
-#define XLF_5LEVEL (1<<5)
-#define XLF_5LEVEL_ENABLED (1<<6)
+#include <asm/setup_data.h>
#ifndef __ASSEMBLY__
@@ -48,139 +13,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;
- __u32 syssize;
- __u16 ram_size;
- __u16 vid_mode;
- __u16 root_dev;
- __u16 boot_flag;
- __u16 jump;
- __u32 header;
- __u16 version;
- __u32 realmode_swtch;
- __u16 start_sys_seg;
- __u16 kernel_version;
- __u8 type_of_loader;
- __u8 loadflags;
- __u16 setup_move_size;
- __u32 code32_start;
- __u32 ramdisk_image;
- __u32 ramdisk_size;
- __u32 bootsect_kludge;
- __u16 heap_end_ptr;
- __u8 ext_loader_ver;
- __u8 ext_loader_type;
- __u32 cmd_line_ptr;
- __u32 initrd_addr_max;
- __u32 kernel_alignment;
- __u8 relocatable_kernel;
- __u8 min_alignment;
- __u16 xloadflags;
- __u32 cmdline_size;
- __u32 hardware_subarch;
- __u64 hardware_subarch_data;
- __u32 payload_offset;
- __u32 payload_length;
- __u64 setup_data;
- __u64 pref_address;
- __u32 init_size;
- __u32 handover_offset;
- __u32 kernel_info_offset;
-} __attribute__((packed));
-
-struct sys_desc_table {
- __u16 length;
- __u8 table[14];
-};
-
-/* Gleaned from OFW's set-parameters in cpu/x86/pc/linux.fth */
-struct olpc_ofw_header {
- __u32 ofw_magic; /* OFW signature */
- __u32 ofw_version;
- __u32 cif_handler; /* callback into OFW */
- __u32 irq_desc_table;
-} __attribute__((packed));
-
-struct efi_info {
- __u32 efi_loader_signature;
- __u32 efi_systab;
- __u32 efi_memdesc_size;
- __u32 efi_memdesc_version;
- __u32 efi_memmap;
- __u32 efi_memmap_size;
- __u32 efi_systab_hi;
- __u32 efi_memmap_hi;
-};
-
-/*
- * This is the maximum number of entries in struct boot_params::e820_table
- * (the zeropage), which is part of the x86 boot protocol ABI:
- */
-#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 */
@@ -231,54 +63,6 @@ struct boot_params {
__u8 _pad9[276]; /* 0xeec */
} __attribute__((packed));
-/**
- * enum x86_hardware_subarch - x86 hardware subarchitecture
- *
- * The x86 hardware_subarch and hardware_subarch_data were added as of the x86
- * boot protocol 2.07 to help distinguish and support custom x86 boot
- * sequences. This enum represents accepted values for the x86
- * hardware_subarch. Custom x86 boot sequences (not X86_SUBARCH_PC) do not
- * have or simply *cannot* make use of natural stubs like BIOS or EFI, the
- * hardware_subarch can be used on the Linux entry path to revector to a
- * subarchitecture stub when needed. This subarchitecture stub can be used to
- * set up Linux boot parameters or for special care to account for nonstandard
- * handling of page tables.
- *
- * These enums should only ever be used by x86 code, and the code that uses
- * it should be well contained and compartmentalized.
- *
- * KVM and Xen HVM do not have a subarch as these are expected to follow
- * standard x86 boot entries. If there is a genuine need for "hypervisor" type
- * that should be considered separately in the future. Future guest types
- * should seriously consider working with standard x86 boot stubs such as
- * the BIOS or EFI boot stubs.
- *
- * WARNING: this enum is only used for legacy hacks, for platform features that
- * are not easily enumerated or discoverable. You should not ever use
- * this for new features.
- *
- * @X86_SUBARCH_PC: Should be used if the hardware is enumerable using standard
- * PC mechanisms (PCI, ACPI) and doesn't need a special boot flow.
- * @X86_SUBARCH_LGUEST: Used for x86 hypervisor demo, lguest, deprecated
- * @X86_SUBARCH_XEN: Used for Xen guest types which follow the PV boot path,
- * which start at asm startup_xen() entry point and later jump to the C
- * xen_start_kernel() entry point. Both domU and dom0 type of guests are
- * currently supported through this PV boot path.
- * @X86_SUBARCH_INTEL_MID: Used for Intel MID (Mobile Internet Device) platform
- * systems which do not have the PCI legacy interfaces.
- * @X86_SUBARCH_CE4100: Used for Intel CE media processor (CE4100) SoC
- * for settop boxes and media devices, the use of a subarch for CE4100
- * is more of a hack...
- */
-enum x86_hardware_subarch {
- X86_SUBARCH_PC = 0,
- X86_SUBARCH_LGUEST,
- X86_SUBARCH_XEN,
- X86_SUBARCH_INTEL_MID,
- X86_SUBARCH_CE4100,
- X86_NR_SUBARCHS,
-};
-
#endif /* __ASSEMBLY__ */
#endif /* _ASM_X86_BOOTPARAM_H */
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..e1396e1bf048
--- /dev/null
+++ b/arch/x86/include/uapi/asm/setup_data.h
@@ -0,0 +1,229 @@
+/* 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)
+
+/* ram_size flags */
+#define RAMDISK_IMAGE_START_MASK 0x07FF
+#define RAMDISK_PROMPT_FLAG 0x8000
+#define RAMDISK_LOAD_FLAG 0x4000
+
+/* loadflags */
+#define LOADED_HIGH (1<<0)
+#define KASLR_FLAG (1<<1)
+#define QUIET_FLAG (1<<5)
+#define KEEP_SEGMENTS (1<<6)
+#define CAN_USE_HEAP (1<<7)
+
+/* xloadflags */
+#define XLF_KERNEL_64 (1<<0)
+#define XLF_CAN_BE_LOADED_ABOVE_4G (1<<1)
+#define XLF_EFI_HANDOVER_32 (1<<2)
+#define XLF_EFI_HANDOVER_64 (1<<3)
+#define XLF_EFI_KEXEC (1<<4)
+#define XLF_5LEVEL (1<<5)
+#define XLF_5LEVEL_ENABLED (1<<6)
+
+#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;
+};
+
+struct setup_header {
+ __u8 setup_sects;
+ __u16 root_flags;
+ __u32 syssize;
+ __u16 ram_size;
+ __u16 vid_mode;
+ __u16 root_dev;
+ __u16 boot_flag;
+ __u16 jump;
+ __u32 header;
+ __u16 version;
+ __u32 realmode_swtch;
+ __u16 start_sys_seg;
+ __u16 kernel_version;
+ __u8 type_of_loader;
+ __u8 loadflags;
+ __u16 setup_move_size;
+ __u32 code32_start;
+ __u32 ramdisk_image;
+ __u32 ramdisk_size;
+ __u32 bootsect_kludge;
+ __u16 heap_end_ptr;
+ __u8 ext_loader_ver;
+ __u8 ext_loader_type;
+ __u32 cmd_line_ptr;
+ __u32 initrd_addr_max;
+ __u32 kernel_alignment;
+ __u8 relocatable_kernel;
+ __u8 min_alignment;
+ __u16 xloadflags;
+ __u32 cmdline_size;
+ __u32 hardware_subarch;
+ __u64 hardware_subarch_data;
+ __u32 payload_offset;
+ __u32 payload_length;
+ __u64 setup_data;
+ __u64 pref_address;
+ __u32 init_size;
+ __u32 handover_offset;
+ __u32 kernel_info_offset;
+} __attribute__((packed));
+
+struct sys_desc_table {
+ __u16 length;
+ __u8 table[14];
+};
+
+/* Gleaned from OFW's set-parameters in cpu/x86/pc/linux.fth */
+struct olpc_ofw_header {
+ __u32 ofw_magic; /* OFW signature */
+ __u32 ofw_version;
+ __u32 cif_handler; /* callback into OFW */
+ __u32 irq_desc_table;
+} __attribute__((packed));
+
+struct efi_info {
+ __u32 efi_loader_signature;
+ __u32 efi_systab;
+ __u32 efi_memdesc_size;
+ __u32 efi_memdesc_version;
+ __u32 efi_memmap;
+ __u32 efi_memmap_size;
+ __u32 efi_systab_hi;
+ __u32 efi_memmap_hi;
+};
+
+/*
+ * This is the maximum number of entries in struct boot_params::e820_table
+ * (the zeropage), which is part of the x86 boot protocol ABI:
+ */
+#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));
+
+/**
+ * enum x86_hardware_subarch - x86 hardware subarchitecture
+ *
+ * The x86 hardware_subarch and hardware_subarch_data were added as of the x86
+ * boot protocol 2.07 to help distinguish and support custom x86 boot
+ * sequences. This enum represents accepted values for the x86
+ * hardware_subarch. Custom x86 boot sequences (not X86_SUBARCH_PC) do not
+ * have or simply *cannot* make use of natural stubs like BIOS or EFI, the
+ * hardware_subarch can be used on the Linux entry path to revector to a
+ * subarchitecture stub when needed. This subarchitecture stub can be used to
+ * set up Linux boot parameters or for special care to account for nonstandard
+ * handling of page tables.
+ *
+ * These enums should only ever be used by x86 code, and the code that uses
+ * it should be well contained and compartmentalized.
+ *
+ * KVM and Xen HVM do not have a subarch as these are expected to follow
+ * standard x86 boot entries. If there is a genuine need for "hypervisor" type
+ * that should be considered separately in the future. Future guest types
+ * should seriously consider working with standard x86 boot stubs such as
+ * the BIOS or EFI boot stubs.
+ *
+ * WARNING: this enum is only used for legacy hacks, for platform features that
+ * are not easily enumerated or discoverable. You should not ever use
+ * this for new features.
+ *
+ * @X86_SUBARCH_PC: Should be used if the hardware is enumerable using standard
+ * PC mechanisms (PCI, ACPI) and doesn't need a special boot flow.
+ * @X86_SUBARCH_LGUEST: Used for x86 hypervisor demo, lguest, deprecated
+ * @X86_SUBARCH_XEN: Used for Xen guest types which follow the PV boot path,
+ * which start at asm startup_xen() entry point and later jump to the C
+ * xen_start_kernel() entry point. Both domU and dom0 type of guests are
+ * currently supported through this PV boot path.
+ * @X86_SUBARCH_INTEL_MID: Used for Intel MID (Mobile Internet Device) platform
+ * systems which do not have the PCI legacy interfaces.
+ * @X86_SUBARCH_CE4100: Used for Intel CE media processor (CE4100) SoC
+ * for settop boxes and media devices, the use of a subarch for CE4100
+ * is more of a hack...
+ */
+enum x86_hardware_subarch {
+ X86_SUBARCH_PC = 0,
+ X86_SUBARCH_LGUEST,
+ X86_SUBARCH_XEN,
+ X86_SUBARCH_INTEL_MID,
+ X86_SUBARCH_CE4100,
+ X86_NR_SUBARCHS,
+};
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _UAPI_ASM_X86_SETUP_DATA_H */
--
2.43.0
The header file <asm/efi.h> contains the macro arch_ima_efi_boot_mode,
which expands to use struct boot_params from <asm/bootparams.h>. Many
drivers include <linux/efi.h>, but do not use boot parameters. Changes
to bootparam.h or its included headers can easily trigger large,
unnessary rebuilds of the kernel.
Moving x86's arch_ima_efi_boot_mode to <asm/ima-efi.h> and including
<asm/setup.h> separates that dependency from the rest of the EFI
interfaces. The only user is in ima_efi.c. As the file already declares
a default value for arch_ima_efi_boot_mode, move this define into
asm-generic for all other architectures.
With arch_ima_efi_boot_mode removed from efi.h, <asm/bootparam.h> can
later be removed from further x86 header files.
v2:
* remove extra declaration of boot_params (Ard)
Signed-off-by: Thomas Zimmermann <[email protected]>
---
arch/x86/include/asm/efi.h | 3 ---
arch/x86/include/asm/ima-efi.h | 11 +++++++++++
include/asm-generic/Kbuild | 1 +
include/asm-generic/ima-efi.h | 16 ++++++++++++++++
security/integrity/ima/ima_efi.c | 5 +----
5 files changed, 29 insertions(+), 7 deletions(-)
create mode 100644 arch/x86/include/asm/ima-efi.h
create mode 100644 include/asm-generic/ima-efi.h
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index c4555b269a1b..99f31176c892 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -418,9 +418,6 @@ 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; })
-
#ifdef CONFIG_EFI_RUNTIME_MAP
int efi_get_runtime_map_size(void);
int efi_get_runtime_map_desc_size(void);
diff --git a/arch/x86/include/asm/ima-efi.h b/arch/x86/include/asm/ima-efi.h
new file mode 100644
index 000000000000..b4d904e66b39
--- /dev/null
+++ b/arch/x86/include/asm/ima-efi.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_IMA_EFI_H
+#define _ASM_X86_IMA_EFI_H
+
+#include <asm/setup.h>
+
+#define arch_ima_efi_boot_mode boot_params.secure_boot
+
+#include <asm-generic/ima-efi.h>
+
+#endif /* _ASM_X86_IMA_EFI_H */
diff --git a/include/asm-generic/Kbuild b/include/asm-generic/Kbuild
index def242528b1d..4fd16e71e8cd 100644
--- a/include/asm-generic/Kbuild
+++ b/include/asm-generic/Kbuild
@@ -26,6 +26,7 @@ mandatory-y += ftrace.h
mandatory-y += futex.h
mandatory-y += hardirq.h
mandatory-y += hw_irq.h
+mandatory-y += ima-efi.h
mandatory-y += io.h
mandatory-y += irq.h
mandatory-y += irq_regs.h
diff --git a/include/asm-generic/ima-efi.h b/include/asm-generic/ima-efi.h
new file mode 100644
index 000000000000..f87f5edef440
--- /dev/null
+++ b/include/asm-generic/ima-efi.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef __ASM_GENERIC_IMA_EFI_H_
+#define __ASM_GENERIC_IMA_EFI_H_
+
+#include <linux/efi.h>
+
+/*
+ * Only include this header file from your architecture's <asm/ima-efi.h>.
+ */
+
+#ifndef arch_ima_efi_boot_mode
+#define arch_ima_efi_boot_mode efi_secureboot_mode_unset
+#endif
+
+#endif /* __ASM_GENERIC_FB_H_ */
diff --git a/security/integrity/ima/ima_efi.c b/security/integrity/ima/ima_efi.c
index 138029bfcce1..56bbee271cec 100644
--- a/security/integrity/ima/ima_efi.c
+++ b/security/integrity/ima/ima_efi.c
@@ -6,10 +6,7 @@
#include <linux/module.h>
#include <linux/ima.h>
#include <asm/efi.h>
-
-#ifndef arch_ima_efi_boot_mode
-#define arch_ima_efi_boot_mode efi_secureboot_mode_unset
-#endif
+#include <asm/ima-efi.h>
static enum efi_secureboot_mode get_sb_mode(void)
{
--
2.43.0
Remove the include statement for <asm/bootparam.h> from several header
files that don't require it. Limits the exposure of the boot parameters
within the Linux kernel code. Update several source files that require
code from bootparam.h.
v2:
* clean up misc.h and e820/types.h
* include bootparam.h in several source files
Signed-off-by: Thomas Zimmermann <[email protected]>
Acked-by: Ard Biesheuvel <[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/misc.h | 3 ++-
arch/x86/boot/compressed/pgtable_64.c | 1 +
arch/x86/boot/compressed/sev.c | 1 +
arch/x86/include/asm/e820/types.h | 2 +-
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 -
16 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 55c98fdd67d2..4db998a8d8f0 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/misc.h b/arch/x86/boot/compressed/misc.h
index c0d502bd8716..01c89c410efd 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -33,7 +33,6 @@
#include <linux/elf.h>
#include <asm/page.h>
#include <asm/boot.h>
-#include <asm/bootparam.h>
#include <asm/desc_defs.h>
#include "tdx.h"
@@ -53,6 +52,8 @@
#define memptr unsigned
#endif
+struct boot_param;
+
/* boot/compressed/vmlinux start and end markers */
extern char _head[], _end[];
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/e820/types.h b/arch/x86/include/asm/e820/types.h
index 314f75d886d0..47695db18246 100644
--- a/arch/x86/include/asm/e820/types.h
+++ b/arch/x86/include/asm/e820/types.h
@@ -2,7 +2,7 @@
#ifndef _ASM_E820_TYPES_H
#define _ASM_E820_TYPES_H
-#include <uapi/asm/bootparam.h>
+#include <asm/setup_data.h>
/*
* These are the E820 types known to the kernel:
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
Hi Thomas,
On Fri, 15 Dec 2023 at 13:26, Thomas Zimmermann <[email protected]> wrote:
>
> 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 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]>
> ---
> arch/x86/include/asm/pci.h | 2 +-
> arch/x86/include/uapi/asm/bootparam.h | 218 +----------------------
> arch/x86/include/uapi/asm/setup_data.h | 229 +++++++++++++++++++++++++
> 3 files changed, 231 insertions(+), 218 deletions(-)
> create mode 100644 arch/x86/include/uapi/asm/setup_data.h
>
This is an improvement but not quite what I had in mind: setup_data is
a x86 specific linked list that is only referred to via a u64 in
setup_header.
So setup_data and all the specializations belong in setup_data.h.
OTOH, setup_header, the XLF load flags, efi_info, the e820 related
definitions etc are not related to setup_data at all but to
setup_header. Whether or not setup_header could live in its own header
too (along with those related definitions) is a separate question imo,
but I don't think they belong in 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..f6361eb792fd 100644
> --- a/arch/x86/include/uapi/asm/bootparam.h
> +++ b/arch/x86/include/uapi/asm/bootparam.h
> @@ -2,42 +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)
> -
> -/* ram_size flags */
> -#define RAMDISK_IMAGE_START_MASK 0x07FF
> -#define RAMDISK_PROMPT_FLAG 0x8000
> -#define RAMDISK_LOAD_FLAG 0x4000
> -
> -/* loadflags */
> -#define LOADED_HIGH (1<<0)
> -#define KASLR_FLAG (1<<1)
> -#define QUIET_FLAG (1<<5)
> -#define KEEP_SEGMENTS (1<<6)
> -#define CAN_USE_HEAP (1<<7)
> -
> -/* xloadflags */
> -#define XLF_KERNEL_64 (1<<0)
> -#define XLF_CAN_BE_LOADED_ABOVE_4G (1<<1)
> -#define XLF_EFI_HANDOVER_32 (1<<2)
> -#define XLF_EFI_HANDOVER_64 (1<<3)
> -#define XLF_EFI_KEXEC (1<<4)
> -#define XLF_5LEVEL (1<<5)
> -#define XLF_5LEVEL_ENABLED (1<<6)
> +#include <asm/setup_data.h>
>
> #ifndef __ASSEMBLY__
>
> @@ -48,139 +13,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;
> - __u32 syssize;
> - __u16 ram_size;
> - __u16 vid_mode;
> - __u16 root_dev;
> - __u16 boot_flag;
> - __u16 jump;
> - __u32 header;
> - __u16 version;
> - __u32 realmode_swtch;
> - __u16 start_sys_seg;
> - __u16 kernel_version;
> - __u8 type_of_loader;
> - __u8 loadflags;
> - __u16 setup_move_size;
> - __u32 code32_start;
> - __u32 ramdisk_image;
> - __u32 ramdisk_size;
> - __u32 bootsect_kludge;
> - __u16 heap_end_ptr;
> - __u8 ext_loader_ver;
> - __u8 ext_loader_type;
> - __u32 cmd_line_ptr;
> - __u32 initrd_addr_max;
> - __u32 kernel_alignment;
> - __u8 relocatable_kernel;
> - __u8 min_alignment;
> - __u16 xloadflags;
> - __u32 cmdline_size;
> - __u32 hardware_subarch;
> - __u64 hardware_subarch_data;
> - __u32 payload_offset;
> - __u32 payload_length;
> - __u64 setup_data;
> - __u64 pref_address;
> - __u32 init_size;
> - __u32 handover_offset;
> - __u32 kernel_info_offset;
> -} __attribute__((packed));
> -
> -struct sys_desc_table {
> - __u16 length;
> - __u8 table[14];
> -};
> -
> -/* Gleaned from OFW's set-parameters in cpu/x86/pc/linux.fth */
> -struct olpc_ofw_header {
> - __u32 ofw_magic; /* OFW signature */
> - __u32 ofw_version;
> - __u32 cif_handler; /* callback into OFW */
> - __u32 irq_desc_table;
> -} __attribute__((packed));
> -
> -struct efi_info {
> - __u32 efi_loader_signature;
> - __u32 efi_systab;
> - __u32 efi_memdesc_size;
> - __u32 efi_memdesc_version;
> - __u32 efi_memmap;
> - __u32 efi_memmap_size;
> - __u32 efi_systab_hi;
> - __u32 efi_memmap_hi;
> -};
> -
> -/*
> - * This is the maximum number of entries in struct boot_params::e820_table
> - * (the zeropage), which is part of the x86 boot protocol ABI:
> - */
> -#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 */
> @@ -231,54 +63,6 @@ struct boot_params {
> __u8 _pad9[276]; /* 0xeec */
> } __attribute__((packed));
>
> -/**
> - * enum x86_hardware_subarch - x86 hardware subarchitecture
> - *
> - * The x86 hardware_subarch and hardware_subarch_data were added as of the x86
> - * boot protocol 2.07 to help distinguish and support custom x86 boot
> - * sequences. This enum represents accepted values for the x86
> - * hardware_subarch. Custom x86 boot sequences (not X86_SUBARCH_PC) do not
> - * have or simply *cannot* make use of natural stubs like BIOS or EFI, the
> - * hardware_subarch can be used on the Linux entry path to revector to a
> - * subarchitecture stub when needed. This subarchitecture stub can be used to
> - * set up Linux boot parameters or for special care to account for nonstandard
> - * handling of page tables.
> - *
> - * These enums should only ever be used by x86 code, and the code that uses
> - * it should be well contained and compartmentalized.
> - *
> - * KVM and Xen HVM do not have a subarch as these are expected to follow
> - * standard x86 boot entries. If there is a genuine need for "hypervisor" type
> - * that should be considered separately in the future. Future guest types
> - * should seriously consider working with standard x86 boot stubs such as
> - * the BIOS or EFI boot stubs.
> - *
> - * WARNING: this enum is only used for legacy hacks, for platform features that
> - * are not easily enumerated or discoverable. You should not ever use
> - * this for new features.
> - *
> - * @X86_SUBARCH_PC: Should be used if the hardware is enumerable using standard
> - * PC mechanisms (PCI, ACPI) and doesn't need a special boot flow.
> - * @X86_SUBARCH_LGUEST: Used for x86 hypervisor demo, lguest, deprecated
> - * @X86_SUBARCH_XEN: Used for Xen guest types which follow the PV boot path,
> - * which start at asm startup_xen() entry point and later jump to the C
> - * xen_start_kernel() entry point. Both domU and dom0 type of guests are
> - * currently supported through this PV boot path.
> - * @X86_SUBARCH_INTEL_MID: Used for Intel MID (Mobile Internet Device) platform
> - * systems which do not have the PCI legacy interfaces.
> - * @X86_SUBARCH_CE4100: Used for Intel CE media processor (CE4100) SoC
> - * for settop boxes and media devices, the use of a subarch for CE4100
> - * is more of a hack...
> - */
> -enum x86_hardware_subarch {
> - X86_SUBARCH_PC = 0,
> - X86_SUBARCH_LGUEST,
> - X86_SUBARCH_XEN,
> - X86_SUBARCH_INTEL_MID,
> - X86_SUBARCH_CE4100,
> - X86_NR_SUBARCHS,
> -};
> -
> #endif /* __ASSEMBLY__ */
>
> #endif /* _ASM_X86_BOOTPARAM_H */
> 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..e1396e1bf048
> --- /dev/null
> +++ b/arch/x86/include/uapi/asm/setup_data.h
> @@ -0,0 +1,229 @@
> +/* 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)
> +
> +/* ram_size flags */
> +#define RAMDISK_IMAGE_START_MASK 0x07FF
> +#define RAMDISK_PROMPT_FLAG 0x8000
> +#define RAMDISK_LOAD_FLAG 0x4000
> +
> +/* loadflags */
> +#define LOADED_HIGH (1<<0)
> +#define KASLR_FLAG (1<<1)
> +#define QUIET_FLAG (1<<5)
> +#define KEEP_SEGMENTS (1<<6)
> +#define CAN_USE_HEAP (1<<7)
> +
> +/* xloadflags */
> +#define XLF_KERNEL_64 (1<<0)
> +#define XLF_CAN_BE_LOADED_ABOVE_4G (1<<1)
> +#define XLF_EFI_HANDOVER_32 (1<<2)
> +#define XLF_EFI_HANDOVER_64 (1<<3)
> +#define XLF_EFI_KEXEC (1<<4)
> +#define XLF_5LEVEL (1<<5)
> +#define XLF_5LEVEL_ENABLED (1<<6)
> +
> +#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;
> +};
> +
> +struct setup_header {
> + __u8 setup_sects;
> + __u16 root_flags;
> + __u32 syssize;
> + __u16 ram_size;
> + __u16 vid_mode;
> + __u16 root_dev;
> + __u16 boot_flag;
> + __u16 jump;
> + __u32 header;
> + __u16 version;
> + __u32 realmode_swtch;
> + __u16 start_sys_seg;
> + __u16 kernel_version;
> + __u8 type_of_loader;
> + __u8 loadflags;
> + __u16 setup_move_size;
> + __u32 code32_start;
> + __u32 ramdisk_image;
> + __u32 ramdisk_size;
> + __u32 bootsect_kludge;
> + __u16 heap_end_ptr;
> + __u8 ext_loader_ver;
> + __u8 ext_loader_type;
> + __u32 cmd_line_ptr;
> + __u32 initrd_addr_max;
> + __u32 kernel_alignment;
> + __u8 relocatable_kernel;
> + __u8 min_alignment;
> + __u16 xloadflags;
> + __u32 cmdline_size;
> + __u32 hardware_subarch;
> + __u64 hardware_subarch_data;
> + __u32 payload_offset;
> + __u32 payload_length;
> + __u64 setup_data;
> + __u64 pref_address;
> + __u32 init_size;
> + __u32 handover_offset;
> + __u32 kernel_info_offset;
> +} __attribute__((packed));
> +
> +struct sys_desc_table {
> + __u16 length;
> + __u8 table[14];
> +};
> +
> +/* Gleaned from OFW's set-parameters in cpu/x86/pc/linux.fth */
> +struct olpc_ofw_header {
> + __u32 ofw_magic; /* OFW signature */
> + __u32 ofw_version;
> + __u32 cif_handler; /* callback into OFW */
> + __u32 irq_desc_table;
> +} __attribute__((packed));
> +
> +struct efi_info {
> + __u32 efi_loader_signature;
> + __u32 efi_systab;
> + __u32 efi_memdesc_size;
> + __u32 efi_memdesc_version;
> + __u32 efi_memmap;
> + __u32 efi_memmap_size;
> + __u32 efi_systab_hi;
> + __u32 efi_memmap_hi;
> +};
> +
> +/*
> + * This is the maximum number of entries in struct boot_params::e820_table
> + * (the zeropage), which is part of the x86 boot protocol ABI:
> + */
> +#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));
> +
> +/**
> + * enum x86_hardware_subarch - x86 hardware subarchitecture
> + *
> + * The x86 hardware_subarch and hardware_subarch_data were added as of the x86
> + * boot protocol 2.07 to help distinguish and support custom x86 boot
> + * sequences. This enum represents accepted values for the x86
> + * hardware_subarch. Custom x86 boot sequences (not X86_SUBARCH_PC) do not
> + * have or simply *cannot* make use of natural stubs like BIOS or EFI, the
> + * hardware_subarch can be used on the Linux entry path to revector to a
> + * subarchitecture stub when needed. This subarchitecture stub can be used to
> + * set up Linux boot parameters or for special care to account for nonstandard
> + * handling of page tables.
> + *
> + * These enums should only ever be used by x86 code, and the code that uses
> + * it should be well contained and compartmentalized.
> + *
> + * KVM and Xen HVM do not have a subarch as these are expected to follow
> + * standard x86 boot entries. If there is a genuine need for "hypervisor" type
> + * that should be considered separately in the future. Future guest types
> + * should seriously consider working with standard x86 boot stubs such as
> + * the BIOS or EFI boot stubs.
> + *
> + * WARNING: this enum is only used for legacy hacks, for platform features that
> + * are not easily enumerated or discoverable. You should not ever use
> + * this for new features.
> + *
> + * @X86_SUBARCH_PC: Should be used if the hardware is enumerable using standard
> + * PC mechanisms (PCI, ACPI) and doesn't need a special boot flow.
> + * @X86_SUBARCH_LGUEST: Used for x86 hypervisor demo, lguest, deprecated
> + * @X86_SUBARCH_XEN: Used for Xen guest types which follow the PV boot path,
> + * which start at asm startup_xen() entry point and later jump to the C
> + * xen_start_kernel() entry point. Both domU and dom0 type of guests are
> + * currently supported through this PV boot path.
> + * @X86_SUBARCH_INTEL_MID: Used for Intel MID (Mobile Internet Device) platform
> + * systems which do not have the PCI legacy interfaces.
> + * @X86_SUBARCH_CE4100: Used for Intel CE media processor (CE4100) SoC
> + * for settop boxes and media devices, the use of a subarch for CE4100
> + * is more of a hack...
> + */
> +enum x86_hardware_subarch {
> + X86_SUBARCH_PC = 0,
> + X86_SUBARCH_LGUEST,
> + X86_SUBARCH_XEN,
> + X86_SUBARCH_INTEL_MID,
> + X86_SUBARCH_CE4100,
> + X86_NR_SUBARCHS,
> +};
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* _UAPI_ASM_X86_SETUP_DATA_H */
> --
> 2.43.0
>
>
Hi Thomas,
On Fri, 15 Dec 2023 at 13:26, Thomas Zimmermann <[email protected]> wrote:
>
> The header file <asm/efi.h> contains the macro arch_ima_efi_boot_mode,
> which expands to use struct boot_params from <asm/bootparams.h>. Many
> drivers include <linux/efi.h>, but do not use boot parameters. Changes
> to bootparam.h or its included headers can easily trigger large,
> unnessary rebuilds of the kernel.
>
> Moving x86's arch_ima_efi_boot_mode to <asm/ima-efi.h> and including
> <asm/setup.h> separates that dependency from the rest of the EFI
> interfaces. The only user is in ima_efi.c. As the file already declares
> a default value for arch_ima_efi_boot_mode, move this define into
> asm-generic for all other architectures.
>
> With arch_ima_efi_boot_mode removed from efi.h, <asm/bootparam.h> can
> later be removed from further x86 header files.
>
Apologies if I missed this in v1 but is the new asm-generic header
really necessary? Could we instead turn arch_ima_efi_boot_mode into a
function that is a static inline { return unset; } by default, but can
be emitted out of line in one of the x86/platform/efi.c source files,
where referring to boot_params is fine?
> v2:
> * remove extra declaration of boot_params (Ard)
>
Please don't put the revision log here, but below the --- so that 'git
am' will ignore it.
> Signed-off-by: Thomas Zimmermann <[email protected]>
> ---
> arch/x86/include/asm/efi.h | 3 ---
> arch/x86/include/asm/ima-efi.h | 11 +++++++++++
> include/asm-generic/Kbuild | 1 +
> include/asm-generic/ima-efi.h | 16 ++++++++++++++++
> security/integrity/ima/ima_efi.c | 5 +----
> 5 files changed, 29 insertions(+), 7 deletions(-)
> create mode 100644 arch/x86/include/asm/ima-efi.h
> create mode 100644 include/asm-generic/ima-efi.h
>
> diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
> index c4555b269a1b..99f31176c892 100644
> --- a/arch/x86/include/asm/efi.h
> +++ b/arch/x86/include/asm/efi.h
> @@ -418,9 +418,6 @@ 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; })
> -
> #ifdef CONFIG_EFI_RUNTIME_MAP
> int efi_get_runtime_map_size(void);
> int efi_get_runtime_map_desc_size(void);
> diff --git a/arch/x86/include/asm/ima-efi.h b/arch/x86/include/asm/ima-efi.h
> new file mode 100644
> index 000000000000..b4d904e66b39
> --- /dev/null
> +++ b/arch/x86/include/asm/ima-efi.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_X86_IMA_EFI_H
> +#define _ASM_X86_IMA_EFI_H
> +
> +#include <asm/setup.h>
> +
> +#define arch_ima_efi_boot_mode boot_params.secure_boot
> +
> +#include <asm-generic/ima-efi.h>
> +
> +#endif /* _ASM_X86_IMA_EFI_H */
> diff --git a/include/asm-generic/Kbuild b/include/asm-generic/Kbuild
> index def242528b1d..4fd16e71e8cd 100644
> --- a/include/asm-generic/Kbuild
> +++ b/include/asm-generic/Kbuild
> @@ -26,6 +26,7 @@ mandatory-y += ftrace.h
> mandatory-y += futex.h
> mandatory-y += hardirq.h
> mandatory-y += hw_irq.h
> +mandatory-y += ima-efi.h
> mandatory-y += io.h
> mandatory-y += irq.h
> mandatory-y += irq_regs.h
> diff --git a/include/asm-generic/ima-efi.h b/include/asm-generic/ima-efi.h
> new file mode 100644
> index 000000000000..f87f5edef440
> --- /dev/null
> +++ b/include/asm-generic/ima-efi.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#ifndef __ASM_GENERIC_IMA_EFI_H_
> +#define __ASM_GENERIC_IMA_EFI_H_
> +
> +#include <linux/efi.h>
> +
> +/*
> + * Only include this header file from your architecture's <asm/ima-efi.h>.
> + */
> +
> +#ifndef arch_ima_efi_boot_mode
> +#define arch_ima_efi_boot_mode efi_secureboot_mode_unset
> +#endif
> +
> +#endif /* __ASM_GENERIC_FB_H_ */
> diff --git a/security/integrity/ima/ima_efi.c b/security/integrity/ima/ima_efi.c
> index 138029bfcce1..56bbee271cec 100644
> --- a/security/integrity/ima/ima_efi.c
> +++ b/security/integrity/ima/ima_efi.c
> @@ -6,10 +6,7 @@
> #include <linux/module.h>
> #include <linux/ima.h>
> #include <asm/efi.h>
> -
> -#ifndef arch_ima_efi_boot_mode
> -#define arch_ima_efi_boot_mode efi_secureboot_mode_unset
> -#endif
> +#include <asm/ima-efi.h>
>
> static enum efi_secureboot_mode get_sb_mode(void)
> {
> --
> 2.43.0
>
>
Hi Ard
Am 19.12.23 um 11:50 schrieb Ard Biesheuvel:
> Hi Thomas,
>
> On Fri, 15 Dec 2023 at 13:26, Thomas Zimmermann <[email protected]> wrote:
>>
>> 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 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]>
>> ---
>> arch/x86/include/asm/pci.h | 2 +-
>> arch/x86/include/uapi/asm/bootparam.h | 218 +----------------------
>> arch/x86/include/uapi/asm/setup_data.h | 229 +++++++++++++++++++++++++
>> 3 files changed, 231 insertions(+), 218 deletions(-)
>> create mode 100644 arch/x86/include/uapi/asm/setup_data.h
>>
>
> This is an improvement but not quite what I had in mind: setup_data is
> a x86 specific linked list that is only referred to via a u64 in
> setup_header.
>
> So setup_data and all the specializations belong in setup_data.h.
> OTOH, setup_header, the XLF load flags, efi_info, the e820 related
> definitions etc are not related to setup_data at all but to
> setup_header. Whether or not setup_header could live in its own header
> too (along with those related definitions) is a separate question imo,
> but I don't think they belong in setup_data.h
Thanks for the feedback. I'll send out an update to address this and
additional changes for PCI and EFI.
So fa, the only reason for a setup_header header file would be
e820/types.h which includes bootparam.h for E820_MAX_ENTRIES_ZEROPAGE.
It doesn't make a difference in build time, tough.
Best regards
Thomas
>
>
>
>> 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..f6361eb792fd 100644
>> --- a/arch/x86/include/uapi/asm/bootparam.h
>> +++ b/arch/x86/include/uapi/asm/bootparam.h
>> @@ -2,42 +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)
>> -
>> -/* ram_size flags */
>> -#define RAMDISK_IMAGE_START_MASK 0x07FF
>> -#define RAMDISK_PROMPT_FLAG 0x8000
>> -#define RAMDISK_LOAD_FLAG 0x4000
>> -
>> -/* loadflags */
>> -#define LOADED_HIGH (1<<0)
>> -#define KASLR_FLAG (1<<1)
>> -#define QUIET_FLAG (1<<5)
>> -#define KEEP_SEGMENTS (1<<6)
>> -#define CAN_USE_HEAP (1<<7)
>> -
>> -/* xloadflags */
>> -#define XLF_KERNEL_64 (1<<0)
>> -#define XLF_CAN_BE_LOADED_ABOVE_4G (1<<1)
>> -#define XLF_EFI_HANDOVER_32 (1<<2)
>> -#define XLF_EFI_HANDOVER_64 (1<<3)
>> -#define XLF_EFI_KEXEC (1<<4)
>> -#define XLF_5LEVEL (1<<5)
>> -#define XLF_5LEVEL_ENABLED (1<<6)
>> +#include <asm/setup_data.h>
>>
>> #ifndef __ASSEMBLY__
>>
>> @@ -48,139 +13,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;
>> - __u32 syssize;
>> - __u16 ram_size;
>> - __u16 vid_mode;
>> - __u16 root_dev;
>> - __u16 boot_flag;
>> - __u16 jump;
>> - __u32 header;
>> - __u16 version;
>> - __u32 realmode_swtch;
>> - __u16 start_sys_seg;
>> - __u16 kernel_version;
>> - __u8 type_of_loader;
>> - __u8 loadflags;
>> - __u16 setup_move_size;
>> - __u32 code32_start;
>> - __u32 ramdisk_image;
>> - __u32 ramdisk_size;
>> - __u32 bootsect_kludge;
>> - __u16 heap_end_ptr;
>> - __u8 ext_loader_ver;
>> - __u8 ext_loader_type;
>> - __u32 cmd_line_ptr;
>> - __u32 initrd_addr_max;
>> - __u32 kernel_alignment;
>> - __u8 relocatable_kernel;
>> - __u8 min_alignment;
>> - __u16 xloadflags;
>> - __u32 cmdline_size;
>> - __u32 hardware_subarch;
>> - __u64 hardware_subarch_data;
>> - __u32 payload_offset;
>> - __u32 payload_length;
>> - __u64 setup_data;
>> - __u64 pref_address;
>> - __u32 init_size;
>> - __u32 handover_offset;
>> - __u32 kernel_info_offset;
>> -} __attribute__((packed));
>> -
>> -struct sys_desc_table {
>> - __u16 length;
>> - __u8 table[14];
>> -};
>> -
>> -/* Gleaned from OFW's set-parameters in cpu/x86/pc/linux.fth */
>> -struct olpc_ofw_header {
>> - __u32 ofw_magic; /* OFW signature */
>> - __u32 ofw_version;
>> - __u32 cif_handler; /* callback into OFW */
>> - __u32 irq_desc_table;
>> -} __attribute__((packed));
>> -
>> -struct efi_info {
>> - __u32 efi_loader_signature;
>> - __u32 efi_systab;
>> - __u32 efi_memdesc_size;
>> - __u32 efi_memdesc_version;
>> - __u32 efi_memmap;
>> - __u32 efi_memmap_size;
>> - __u32 efi_systab_hi;
>> - __u32 efi_memmap_hi;
>> -};
>> -
>> -/*
>> - * This is the maximum number of entries in struct boot_params::e820_table
>> - * (the zeropage), which is part of the x86 boot protocol ABI:
>> - */
>> -#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 */
>> @@ -231,54 +63,6 @@ struct boot_params {
>> __u8 _pad9[276]; /* 0xeec */
>> } __attribute__((packed));
>>
>> -/**
>> - * enum x86_hardware_subarch - x86 hardware subarchitecture
>> - *
>> - * The x86 hardware_subarch and hardware_subarch_data were added as of the x86
>> - * boot protocol 2.07 to help distinguish and support custom x86 boot
>> - * sequences. This enum represents accepted values for the x86
>> - * hardware_subarch. Custom x86 boot sequences (not X86_SUBARCH_PC) do not
>> - * have or simply *cannot* make use of natural stubs like BIOS or EFI, the
>> - * hardware_subarch can be used on the Linux entry path to revector to a
>> - * subarchitecture stub when needed. This subarchitecture stub can be used to
>> - * set up Linux boot parameters or for special care to account for nonstandard
>> - * handling of page tables.
>> - *
>> - * These enums should only ever be used by x86 code, and the code that uses
>> - * it should be well contained and compartmentalized.
>> - *
>> - * KVM and Xen HVM do not have a subarch as these are expected to follow
>> - * standard x86 boot entries. If there is a genuine need for "hypervisor" type
>> - * that should be considered separately in the future. Future guest types
>> - * should seriously consider working with standard x86 boot stubs such as
>> - * the BIOS or EFI boot stubs.
>> - *
>> - * WARNING: this enum is only used for legacy hacks, for platform features that
>> - * are not easily enumerated or discoverable. You should not ever use
>> - * this for new features.
>> - *
>> - * @X86_SUBARCH_PC: Should be used if the hardware is enumerable using standard
>> - * PC mechanisms (PCI, ACPI) and doesn't need a special boot flow.
>> - * @X86_SUBARCH_LGUEST: Used for x86 hypervisor demo, lguest, deprecated
>> - * @X86_SUBARCH_XEN: Used for Xen guest types which follow the PV boot path,
>> - * which start at asm startup_xen() entry point and later jump to the C
>> - * xen_start_kernel() entry point. Both domU and dom0 type of guests are
>> - * currently supported through this PV boot path.
>> - * @X86_SUBARCH_INTEL_MID: Used for Intel MID (Mobile Internet Device) platform
>> - * systems which do not have the PCI legacy interfaces.
>> - * @X86_SUBARCH_CE4100: Used for Intel CE media processor (CE4100) SoC
>> - * for settop boxes and media devices, the use of a subarch for CE4100
>> - * is more of a hack...
>> - */
>> -enum x86_hardware_subarch {
>> - X86_SUBARCH_PC = 0,
>> - X86_SUBARCH_LGUEST,
>> - X86_SUBARCH_XEN,
>> - X86_SUBARCH_INTEL_MID,
>> - X86_SUBARCH_CE4100,
>> - X86_NR_SUBARCHS,
>> -};
>> -
>> #endif /* __ASSEMBLY__ */
>>
>> #endif /* _ASM_X86_BOOTPARAM_H */
>> 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..e1396e1bf048
>> --- /dev/null
>> +++ b/arch/x86/include/uapi/asm/setup_data.h
>> @@ -0,0 +1,229 @@
>> +/* 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)
>> +
>> +/* ram_size flags */
>> +#define RAMDISK_IMAGE_START_MASK 0x07FF
>> +#define RAMDISK_PROMPT_FLAG 0x8000
>> +#define RAMDISK_LOAD_FLAG 0x4000
>> +
>> +/* loadflags */
>> +#define LOADED_HIGH (1<<0)
>> +#define KASLR_FLAG (1<<1)
>> +#define QUIET_FLAG (1<<5)
>> +#define KEEP_SEGMENTS (1<<6)
>> +#define CAN_USE_HEAP (1<<7)
>> +
>> +/* xloadflags */
>> +#define XLF_KERNEL_64 (1<<0)
>> +#define XLF_CAN_BE_LOADED_ABOVE_4G (1<<1)
>> +#define XLF_EFI_HANDOVER_32 (1<<2)
>> +#define XLF_EFI_HANDOVER_64 (1<<3)
>> +#define XLF_EFI_KEXEC (1<<4)
>> +#define XLF_5LEVEL (1<<5)
>> +#define XLF_5LEVEL_ENABLED (1<<6)
>> +
>> +#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;
>> +};
>> +
>> +struct setup_header {
>> + __u8 setup_sects;
>> + __u16 root_flags;
>> + __u32 syssize;
>> + __u16 ram_size;
>> + __u16 vid_mode;
>> + __u16 root_dev;
>> + __u16 boot_flag;
>> + __u16 jump;
>> + __u32 header;
>> + __u16 version;
>> + __u32 realmode_swtch;
>> + __u16 start_sys_seg;
>> + __u16 kernel_version;
>> + __u8 type_of_loader;
>> + __u8 loadflags;
>> + __u16 setup_move_size;
>> + __u32 code32_start;
>> + __u32 ramdisk_image;
>> + __u32 ramdisk_size;
>> + __u32 bootsect_kludge;
>> + __u16 heap_end_ptr;
>> + __u8 ext_loader_ver;
>> + __u8 ext_loader_type;
>> + __u32 cmd_line_ptr;
>> + __u32 initrd_addr_max;
>> + __u32 kernel_alignment;
>> + __u8 relocatable_kernel;
>> + __u8 min_alignment;
>> + __u16 xloadflags;
>> + __u32 cmdline_size;
>> + __u32 hardware_subarch;
>> + __u64 hardware_subarch_data;
>> + __u32 payload_offset;
>> + __u32 payload_length;
>> + __u64 setup_data;
>> + __u64 pref_address;
>> + __u32 init_size;
>> + __u32 handover_offset;
>> + __u32 kernel_info_offset;
>> +} __attribute__((packed));
>> +
>> +struct sys_desc_table {
>> + __u16 length;
>> + __u8 table[14];
>> +};
>> +
>> +/* Gleaned from OFW's set-parameters in cpu/x86/pc/linux.fth */
>> +struct olpc_ofw_header {
>> + __u32 ofw_magic; /* OFW signature */
>> + __u32 ofw_version;
>> + __u32 cif_handler; /* callback into OFW */
>> + __u32 irq_desc_table;
>> +} __attribute__((packed));
>> +
>> +struct efi_info {
>> + __u32 efi_loader_signature;
>> + __u32 efi_systab;
>> + __u32 efi_memdesc_size;
>> + __u32 efi_memdesc_version;
>> + __u32 efi_memmap;
>> + __u32 efi_memmap_size;
>> + __u32 efi_systab_hi;
>> + __u32 efi_memmap_hi;
>> +};
>> +
>> +/*
>> + * This is the maximum number of entries in struct boot_params::e820_table
>> + * (the zeropage), which is part of the x86 boot protocol ABI:
>> + */
>> +#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));
>> +
>> +/**
>> + * enum x86_hardware_subarch - x86 hardware subarchitecture
>> + *
>> + * The x86 hardware_subarch and hardware_subarch_data were added as of the x86
>> + * boot protocol 2.07 to help distinguish and support custom x86 boot
>> + * sequences. This enum represents accepted values for the x86
>> + * hardware_subarch. Custom x86 boot sequences (not X86_SUBARCH_PC) do not
>> + * have or simply *cannot* make use of natural stubs like BIOS or EFI, the
>> + * hardware_subarch can be used on the Linux entry path to revector to a
>> + * subarchitecture stub when needed. This subarchitecture stub can be used to
>> + * set up Linux boot parameters or for special care to account for nonstandard
>> + * handling of page tables.
>> + *
>> + * These enums should only ever be used by x86 code, and the code that uses
>> + * it should be well contained and compartmentalized.
>> + *
>> + * KVM and Xen HVM do not have a subarch as these are expected to follow
>> + * standard x86 boot entries. If there is a genuine need for "hypervisor" type
>> + * that should be considered separately in the future. Future guest types
>> + * should seriously consider working with standard x86 boot stubs such as
>> + * the BIOS or EFI boot stubs.
>> + *
>> + * WARNING: this enum is only used for legacy hacks, for platform features that
>> + * are not easily enumerated or discoverable. You should not ever use
>> + * this for new features.
>> + *
>> + * @X86_SUBARCH_PC: Should be used if the hardware is enumerable using standard
>> + * PC mechanisms (PCI, ACPI) and doesn't need a special boot flow.
>> + * @X86_SUBARCH_LGUEST: Used for x86 hypervisor demo, lguest, deprecated
>> + * @X86_SUBARCH_XEN: Used for Xen guest types which follow the PV boot path,
>> + * which start at asm startup_xen() entry point and later jump to the C
>> + * xen_start_kernel() entry point. Both domU and dom0 type of guests are
>> + * currently supported through this PV boot path.
>> + * @X86_SUBARCH_INTEL_MID: Used for Intel MID (Mobile Internet Device) platform
>> + * systems which do not have the PCI legacy interfaces.
>> + * @X86_SUBARCH_CE4100: Used for Intel CE media processor (CE4100) SoC
>> + * for settop boxes and media devices, the use of a subarch for CE4100
>> + * is more of a hack...
>> + */
>> +enum x86_hardware_subarch {
>> + X86_SUBARCH_PC = 0,
>> + X86_SUBARCH_LGUEST,
>> + X86_SUBARCH_XEN,
>> + X86_SUBARCH_INTEL_MID,
>> + X86_SUBARCH_CE4100,
>> + X86_NR_SUBARCHS,
>> +};
>> +
>> +#endif /* __ASSEMBLY__ */
>> +
>> +#endif /* _UAPI_ASM_X86_SETUP_DATA_H */
>> --
>> 2.43.0
>>
>>
--
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)
Hii Ard
Am 19.12.23 um 12:38 schrieb Ard Biesheuvel:
> Hi Thomas,
>
> On Fri, 15 Dec 2023 at 13:26, Thomas Zimmermann <[email protected]> wrote:
>>
>> The header file <asm/efi.h> contains the macro arch_ima_efi_boot_mode,
>> which expands to use struct boot_params from <asm/bootparams.h>. Many
>> drivers include <linux/efi.h>, but do not use boot parameters. Changes
>> to bootparam.h or its included headers can easily trigger large,
>> unnessary rebuilds of the kernel.
>>
>> Moving x86's arch_ima_efi_boot_mode to <asm/ima-efi.h> and including
>> <asm/setup.h> separates that dependency from the rest of the EFI
>> interfaces. The only user is in ima_efi.c. As the file already declares
>> a default value for arch_ima_efi_boot_mode, move this define into
>> asm-generic for all other architectures.
>>
>> With arch_ima_efi_boot_mode removed from efi.h, <asm/bootparam.h> can
>> later be removed from further x86 header files.
>>
>
> Apologies if I missed this in v1 but is the new asm-generic header
> really necessary? Could we instead turn arch_ima_efi_boot_mode into a
> function that is a static inline { return unset; } by default, but can
> be emitted out of line in one of the x86/platform/efi.c source files,
> where referring to boot_params is fine?
I cannot figure out how to do this without *something* in asm-generic or
adding if-CONFIG_X86 guards in ima-efi.c.
But I noticed that linux/efi.h already contains 2 or 3 ifdef branches
for x86. Would it be an option to move this code into asm/efi.h
(including a header file in asm-generic for the non-x86 variants) and
add the arch_ima_efi_boot_mode() helper there as well? At least that
wouldn't be a header for only a single define.
Best regards
Thomas
>
>
>
>
>
>> v2:
>> * remove extra declaration of boot_params (Ard)
>>
>
> Please don't put the revision log here, but below the --- so that 'git
> am' will ignore it.
>
>
>> Signed-off-by: Thomas Zimmermann <[email protected]>
>> ---
>> arch/x86/include/asm/efi.h | 3 ---
>> arch/x86/include/asm/ima-efi.h | 11 +++++++++++
>> include/asm-generic/Kbuild | 1 +
>> include/asm-generic/ima-efi.h | 16 ++++++++++++++++
>> security/integrity/ima/ima_efi.c | 5 +----
>> 5 files changed, 29 insertions(+), 7 deletions(-)
>> create mode 100644 arch/x86/include/asm/ima-efi.h
>> create mode 100644 include/asm-generic/ima-efi.h
>>
>> diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
>> index c4555b269a1b..99f31176c892 100644
>> --- a/arch/x86/include/asm/efi.h
>> +++ b/arch/x86/include/asm/efi.h
>> @@ -418,9 +418,6 @@ 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; })
>> -
>> #ifdef CONFIG_EFI_RUNTIME_MAP
>> int efi_get_runtime_map_size(void);
>> int efi_get_runtime_map_desc_size(void);
>> diff --git a/arch/x86/include/asm/ima-efi.h b/arch/x86/include/asm/ima-efi.h
>> new file mode 100644
>> index 000000000000..b4d904e66b39
>> --- /dev/null
>> +++ b/arch/x86/include/asm/ima-efi.h
>> @@ -0,0 +1,11 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _ASM_X86_IMA_EFI_H
>> +#define _ASM_X86_IMA_EFI_H
>> +
>> +#include <asm/setup.h>
>> +
>> +#define arch_ima_efi_boot_mode boot_params.secure_boot
>> +
>> +#include <asm-generic/ima-efi.h>
>> +
>> +#endif /* _ASM_X86_IMA_EFI_H */
>> diff --git a/include/asm-generic/Kbuild b/include/asm-generic/Kbuild
>> index def242528b1d..4fd16e71e8cd 100644
>> --- a/include/asm-generic/Kbuild
>> +++ b/include/asm-generic/Kbuild
>> @@ -26,6 +26,7 @@ mandatory-y += ftrace.h
>> mandatory-y += futex.h
>> mandatory-y += hardirq.h
>> mandatory-y += hw_irq.h
>> +mandatory-y += ima-efi.h
>> mandatory-y += io.h
>> mandatory-y += irq.h
>> mandatory-y += irq_regs.h
>> diff --git a/include/asm-generic/ima-efi.h b/include/asm-generic/ima-efi.h
>> new file mode 100644
>> index 000000000000..f87f5edef440
>> --- /dev/null
>> +++ b/include/asm-generic/ima-efi.h
>> @@ -0,0 +1,16 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +
>> +#ifndef __ASM_GENERIC_IMA_EFI_H_
>> +#define __ASM_GENERIC_IMA_EFI_H_
>> +
>> +#include <linux/efi.h>
>> +
>> +/*
>> + * Only include this header file from your architecture's <asm/ima-efi.h>.
>> + */
>> +
>> +#ifndef arch_ima_efi_boot_mode
>> +#define arch_ima_efi_boot_mode efi_secureboot_mode_unset
>> +#endif
>> +
>> +#endif /* __ASM_GENERIC_FB_H_ */
>> diff --git a/security/integrity/ima/ima_efi.c b/security/integrity/ima/ima_efi.c
>> index 138029bfcce1..56bbee271cec 100644
>> --- a/security/integrity/ima/ima_efi.c
>> +++ b/security/integrity/ima/ima_efi.c
>> @@ -6,10 +6,7 @@
>> #include <linux/module.h>
>> #include <linux/ima.h>
>> #include <asm/efi.h>
>> -
>> -#ifndef arch_ima_efi_boot_mode
>> -#define arch_ima_efi_boot_mode efi_secureboot_mode_unset
>> -#endif
>> +#include <asm/ima-efi.h>
>>
>> static enum efi_secureboot_mode get_sb_mode(void)
>> {
>> --
>> 2.43.0
>>
>>
--
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)
On Tue, 2 Jan 2024 at 15:07, Thomas Zimmermann <[email protected]> wrote:
>
> Hii Ard
>
> Am 19.12.23 um 12:38 schrieb Ard Biesheuvel:
> > Hi Thomas,
> >
> > On Fri, 15 Dec 2023 at 13:26, Thomas Zimmermann <[email protected]> wrote:
> >>
> >> The header file <asm/efi.h> contains the macro arch_ima_efi_boot_mode,
> >> which expands to use struct boot_params from <asm/bootparams.h>. Many
> >> drivers include <linux/efi.h>, but do not use boot parameters. Changes
> >> to bootparam.h or its included headers can easily trigger large,
> >> unnessary rebuilds of the kernel.
> >>
> >> Moving x86's arch_ima_efi_boot_mode to <asm/ima-efi.h> and including
> >> <asm/setup.h> separates that dependency from the rest of the EFI
> >> interfaces. The only user is in ima_efi.c. As the file already declares
> >> a default value for arch_ima_efi_boot_mode, move this define into
> >> asm-generic for all other architectures.
> >>
> >> With arch_ima_efi_boot_mode removed from efi.h, <asm/bootparam.h> can
> >> later be removed from further x86 header files.
> >>
> >
> > Apologies if I missed this in v1 but is the new asm-generic header
> > really necessary? Could we instead turn arch_ima_efi_boot_mode into a
> > function that is a static inline { return unset; } by default, but can
> > be emitted out of line in one of the x86/platform/efi.c source files,
> > where referring to boot_params is fine?
>
> I cannot figure out how to do this without *something* in asm-generic or
> adding if-CONFIG_X86 guards in ima-efi.c.
>
> But I noticed that linux/efi.h already contains 2 or 3 ifdef branches
> for x86. Would it be an option to move this code into asm/efi.h
> (including a header file in asm-generic for the non-x86 variants) and
> add the arch_ima_efi_boot_mode() helper there as well? At least that
> wouldn't be a header for only a single define.
>
Could we just move the x86 implementation out of line?
So something like this in arch/x86/include/asm/efi.h
enum efi_secureboot_mode x86_ima_efi_boot_mode(void);
#define arch_ima_efi_boot_mode x86_ima_efi_boot_mode()
and an implementation in one of the related .c files:
enum efi_secureboot_mode x86_ima_efi_boot_mode(void)
{
return boot_params.secure_boot;
}
?
Hi
Am 03.01.24 um 14:11 schrieb Ard Biesheuvel:
> On Tue, 2 Jan 2024 at 15:07, Thomas Zimmermann <[email protected]> wrote:
>>
>> Hii Ard
>>
>> Am 19.12.23 um 12:38 schrieb Ard Biesheuvel:
>>> Hi Thomas,
>>>
>>> On Fri, 15 Dec 2023 at 13:26, Thomas Zimmermann <[email protected]> wrote:
>>>>
>>>> The header file <asm/efi.h> contains the macro arch_ima_efi_boot_mode,
>>>> which expands to use struct boot_params from <asm/bootparams.h>. Many
>>>> drivers include <linux/efi.h>, but do not use boot parameters. Changes
>>>> to bootparam.h or its included headers can easily trigger large,
>>>> unnessary rebuilds of the kernel.
>>>>
>>>> Moving x86's arch_ima_efi_boot_mode to <asm/ima-efi.h> and including
>>>> <asm/setup.h> separates that dependency from the rest of the EFI
>>>> interfaces. The only user is in ima_efi.c. As the file already declares
>>>> a default value for arch_ima_efi_boot_mode, move this define into
>>>> asm-generic for all other architectures.
>>>>
>>>> With arch_ima_efi_boot_mode removed from efi.h, <asm/bootparam.h> can
>>>> later be removed from further x86 header files.
>>>>
>>>
>>> Apologies if I missed this in v1 but is the new asm-generic header
>>> really necessary? Could we instead turn arch_ima_efi_boot_mode into a
>>> function that is a static inline { return unset; } by default, but can
>>> be emitted out of line in one of the x86/platform/efi.c source files,
>>> where referring to boot_params is fine?
>>
>> I cannot figure out how to do this without *something* in asm-generic or
>> adding if-CONFIG_X86 guards in ima-efi.c.
>>
>> But I noticed that linux/efi.h already contains 2 or 3 ifdef branches
>> for x86. Would it be an option to move this code into asm/efi.h
>> (including a header file in asm-generic for the non-x86 variants) and
>> add the arch_ima_efi_boot_mode() helper there as well? At least that
>> wouldn't be a header for only a single define.
>>
>
> Could we just move the x86 implementation out of line?
>
> So something like this in arch/x86/include/asm/efi.h
>
> enum efi_secureboot_mode x86_ima_efi_boot_mode(void);
> #define arch_ima_efi_boot_mode x86_ima_efi_boot_mode()
>
> and an implementation in one of the related .c files:
>
> enum efi_secureboot_mode x86_ima_efi_boot_mode(void)
> {
> return boot_params.secure_boot;
> }
>
> ?
Well, that's just enough to avoid boot_params within the header file.
But it should work.
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)