mbox series

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

Message ID 20240112095000.8952-1-tzimmermann@suse.de (mailing list archive)
Headers show
Series arch/x86: Remove unnecessary dependencies on bootparam.h | expand

Message

Thomas Zimmermann Jan. 12, 2024, 9:44 a.m. UTC
Reduce build time in some cases by removing unnecessary include statements
for <asm/bootparam.h>. Reorganize some header files accordingly.

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

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

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

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

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

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

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

The test system is an Intel i5-13500.

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

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

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

Comments

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

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

For the series,

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Thomas Zimmermann Jan. 15, 2024, 7:58 a.m. UTC | #2
Hi

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

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

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

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

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

Also, please trim the cc list a bit when you do - this is mostly a x86
specific reshuffle of headers so no need to keep all the other
subsystem maintainers on cc while we finish up the discussion.
Borislav Petkov Jan. 15, 2024, 11 a.m. UTC | #4
On Mon, Jan 15, 2024 at 11:55:36AM +0100, Ard Biesheuvel wrote:
> But please be aware that we are in the middle of the merge window

Yes, and the merge window has been suspended too:

https://lore.kernel.org/r/CAHk-=wjMWpmXtKeiN__vnNO4TcttZR-8dVvd_oBq%2BhjeSsWUwg@mail.gmail.com

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

From: Documentation/process/maintainer-tip.rst

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

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

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

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

So pls be patient.

Thx.