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 rebuilt 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 a few limes from pci.h and efi.h into separate header
files and then removes the obsolete include statements on x86. I did
make allmodconfig
make -j28
touch include/linus/screen_info.h
time -j28 make
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 about a minute.
real 0m58,232s
user 4m37,617s
sys 0m34,993s
The test system was an Intel i5-13500.
Thomas Zimmermann (3):
arch/x86: Move struct pci_setup_rom into pci_setup.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/include/asm/efi.h | 3 ---
arch/x86/include/asm/ima-efi.h | 12 ++++++++++++
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/pci_setup.h | 19 +++++++++++++++++++
arch/x86/include/asm/sev.h | 3 ++-
arch/x86/include/asm/x86_init.h | 2 --
arch/x86/pci/common.c | 1 +
drivers/firmware/efi/libstub/x86-stub.c | 1 +
include/asm-generic/Kbuild | 1 +
include/asm-generic/ima-efi.h | 16 ++++++++++++++++
security/integrity/ima/ima_efi.c | 5 +----
13 files changed, 54 insertions(+), 25 deletions(-)
create mode 100644 arch/x86/include/asm/ima-efi.h
create mode 100644 arch/x86/include/asm/pci_setup.h
create mode 100644 include/asm-generic/ima-efi.h
base-commit: a9d99261a978835b02e248fe18af3026416af3e8
--
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.
Signed-off-by: Thomas Zimmermann <[email protected]>
---
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 --
4 files changed, 3 insertions(+), 5 deletions(-)
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;
--
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/bootparam.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.
Signed-off-by: Thomas Zimmermann <[email protected]>
---
arch/x86/include/asm/efi.h | 3 ---
arch/x86/include/asm/ima-efi.h | 12 ++++++++++++
include/asm-generic/Kbuild | 1 +
include/asm-generic/ima-efi.h | 16 ++++++++++++++++
security/integrity/ima/ima_efi.c | 5 +----
5 files changed, 30 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..3fe054937077
--- /dev/null
+++ b/arch/x86/include/asm/ima-efi.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_IMA_EFI_H
+#define _ASM_X86_IMA_EFI_H
+
+#include <asm/bootparam.h>
+
+#define arch_ima_efi_boot_mode \
+ ({ extern struct boot_params boot_params; 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
On Wed, 6 Dec 2023 at 13:54, 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/bootparam.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.
>
> Signed-off-by: Thomas Zimmermann <[email protected]>
> ---
> arch/x86/include/asm/efi.h | 3 ---
> arch/x86/include/asm/ima-efi.h | 12 ++++++++++++
> include/asm-generic/Kbuild | 1 +
> include/asm-generic/ima-efi.h | 16 ++++++++++++++++
> security/integrity/ima/ima_efi.c | 5 +----
> 5 files changed, 30 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..3fe054937077
> --- /dev/null
> +++ b/arch/x86/include/asm/ima-efi.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_X86_IMA_EFI_H
> +#define _ASM_X86_IMA_EFI_H
> +
> +#include <asm/bootparam.h>
> +
> +#define arch_ima_efi_boot_mode \
> + ({ extern struct boot_params boot_params; boot_params.secure_boot; })
> +
Could you please check whether this kludge is still needed now that we
no longer have conflicting declarations of boot_params? (i.e., drop
the ({ }) and the extern declaration)
> +#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
>
On Wed, 6 Dec 2023 at 13:54, Thomas Zimmermann <[email protected]> wrote:
>
> 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.
>
> Signed-off-by: Thomas Zimmermann <[email protected]>
Acked-by: Ard Biesheuvel <[email protected]>
> ---
> 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 --
> 4 files changed, 3 insertions(+), 5 deletions(-)
>
> 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;
> --
> 2.43.0
>
Hi
Am 07.12.23 um 16:37 schrieb Ard Biesheuvel:
> On Wed, 6 Dec 2023 at 13:54, 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/bootparam.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.
>>
>> Signed-off-by: Thomas Zimmermann <[email protected]>
>> ---
>> arch/x86/include/asm/efi.h | 3 ---
>> arch/x86/include/asm/ima-efi.h | 12 ++++++++++++
>> include/asm-generic/Kbuild | 1 +
>> include/asm-generic/ima-efi.h | 16 ++++++++++++++++
>> security/integrity/ima/ima_efi.c | 5 +----
>> 5 files changed, 30 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..3fe054937077
>> --- /dev/null
>> +++ b/arch/x86/include/asm/ima-efi.h
>> @@ -0,0 +1,12 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _ASM_X86_IMA_EFI_H
>> +#define _ASM_X86_IMA_EFI_H
>> +
>> +#include <asm/bootparam.h>
>> +
>> +#define arch_ima_efi_boot_mode \
>> + ({ extern struct boot_params boot_params; boot_params.secure_boot; })
>> +
>
> Could you please check whether this kludge is still needed now that we
> no longer have conflicting declarations of boot_params? (i.e., drop
> the ({ }) and the extern declaration)
I dropped this and include setup.h instead. Appears to work.
Best regards
Thomas
>
>
>
>> +#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)