Re: [PATCH 1/3] arch/x86: Move struct pci_setup_rom into pci_setup.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ard

Am 07.12.23 um 16:35 schrieb Ard Biesheuvel:
Hello Thomas,

On Wed, 6 Dec 2023 at 13:54, Thomas Zimmermann <tzimmermann@xxxxxxx> 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 pci_setup_rom into its own header file avoid including
<asm/bootparam.h> in <asm/pci.h>. Update the only two users of the
struct in the x86 PCI code and in the EFI code. Also remove the include
statement for x86_init.h, which is unnecessary but pulls in bootparams.h.

Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
---
  arch/x86/include/asm/pci.h              | 13 -------------
  arch/x86/include/asm/pci_setup.h        | 19 +++++++++++++++++++
  arch/x86/pci/common.c                   |  1 +
  drivers/firmware/efi/libstub/x86-stub.c |  1 +
  4 files changed, 21 insertions(+), 13 deletions(-)
  create mode 100644 arch/x86/include/asm/pci_setup.h


Thanks for cleaning this up.

Would it be more appropriate to move all setup_data related
definitions into a separate header entirely?

- the SETUP_ defines
- struct setup_data
- struct pci_setup_rom
- struct   jailhouse_setup_data
etc etc

struct setup_header has a setup_data field which is the root of the
setup_data linked list, but it is typed as __u64 so it doesn't
actually need to know the real type of the associated structs.

That way, you can avoid creating a special asm/pci_setup.h that only
covers this one particular definition.

Thanks for reviewing.

I've now moved everything from <asm/bootparam.h> except struct bootparams into its own header file <asm/setup_data.h>. struct pci_setup_rom remains in pci.h. And most headers now include setup_data.h, while a few source files still require bootparam.h. I'll send this out in the next iteration.

Time for recompiling goes down from 58 sec to 56 sec. It's mostly bound by the linker now.

Best regards
Thomas




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

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

-struct pci_setup_rom {
-       struct setup_data data;
-       uint16_t vendor;
-       uint16_t devid;
-       uint64_t pcilen;
-       unsigned long segment;
-       unsigned long bus;
-       unsigned long device;
-       unsigned long function;
-       uint8_t romdata[];
-};
-
  #endif /* _ASM_X86_PCI_H */
diff --git a/arch/x86/include/asm/pci_setup.h b/arch/x86/include/asm/pci_setup.h
new file mode 100644
index 000000000000..b4b246ef6f2b
--- /dev/null
+++ b/arch/x86/include/asm/pci_setup.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_PCI_SETUP_H
+#define _ASM_X86_PCI_SETUP_H
+
+#include <asm/bootparam.h>
+
+struct pci_setup_rom {
+       struct setup_data data;
+       uint16_t vendor;
+       uint16_t devid;
+       uint64_t pcilen;
+       unsigned long segment;
+       unsigned long bus;
+       unsigned long device;
+       unsigned long function;
+       uint8_t romdata[];
+};
+
+#endif /* _ASM_X86_PCI_SETUP_H */
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index ddb798603201..c6cbb9182160 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -17,6 +17,7 @@
  #include <asm/segment.h>
  #include <asm/io.h>
  #include <asm/smp.h>
+#include <asm/pci_setup.h>
  #include <asm/pci_x86.h>
  #include <asm/setup.h>
  #include <asm/irqdomain.h>
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 1bfdae34df39..0c878ebe5257 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -17,6 +17,7 @@
  #include <asm/boot.h>
  #include <asm/kaslr.h>
  #include <asm/sev.h>
+#include <asm/pci_setup.h>

  #include "efistub.h"
  #include "x86-stub.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)

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux