Linux-Integrity Archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] arch/x86: Remove unnecessary dependencies on bootparam.h
@ 2024-01-04  9:51 Thomas Zimmermann
  2024-01-04  9:51 ` [PATCH v3 1/4] arch/x86: Move UAPI setup structures into setup_data.h Thomas Zimmermann
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Thomas Zimmermann @ 2024-01-04  9:51 UTC (permalink / raw
  To: ardb, tglx, mingo, bp, dave.hansen, x86, hpa, bhelgaas, arnd,
	zohar, dmitry.kasatkin, paul, jmorris, serge, javierm
  Cc: linux-arch, linux-efi, linux-kernel, linux-pci, linux-integrity,
	linux-security-module, Thomas Zimmermann

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/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.

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/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/misc.h        |  3 +-
 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, 142 insertions(+), 111 deletions(-)
 create mode 100644 arch/x86/include/asm/setup_data.h
 create mode 100644 arch/x86/include/uapi/asm/setup_data.h


base-commit: 25232eb8a9ac7fa0dac7e846a4bf7fba2b6db39a
prerequisite-patch-id: 0aa359f6144c4015c140c8a6750be19099c676fb
prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
prerequisite-patch-id: cbc453ee02fae02af22fbfdce56ab732c7a88c36
prerequisite-patch-id: e7a5405fb48608e0c8e3b41bf983fefa2c8bd1f3
prerequisite-patch-id: f12b8b5465e519f8588e3743e70166be3294009b
prerequisite-patch-id: c3de42afb37f6240a840f8b12504262d4483873c
prerequisite-patch-id: 5f31d981a18037906b0e422c0a1031e7dff91a2d
prerequisite-patch-id: 2345c90842ae2a9117d21b9bd205fe3b89e89c20
-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/4] arch/x86: Move UAPI setup structures into setup_data.h
  2024-01-04  9:51 [PATCH v3 0/4] arch/x86: Remove unnecessary dependencies on bootparam.h Thomas Zimmermann
@ 2024-01-04  9:51 ` Thomas Zimmermann
  2024-01-04  9:51 ` [PATCH v3 2/4] arch/x86: Move internal setup_data " Thomas Zimmermann
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Thomas Zimmermann @ 2024-01-04  9:51 UTC (permalink / raw
  To: ardb, tglx, mingo, bp, dave.hansen, x86, hpa, bhelgaas, arnd,
	zohar, dmitry.kasatkin, paul, jmorris, serge, javierm
  Cc: linux-arch, linux-efi, linux-kernel, linux-pci, linux-integrity,
	linux-security-module, Thomas Zimmermann

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

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

Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

---

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

diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index b40c462b4af3..f6100df3652e 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -10,7 +10,7 @@
 #include <linux/numa.h>
 #include <asm/io.h>
 #include <asm/memtype.h>
-#include <asm/x86_init.h>
+#include <asm/setup_data.h>
 
 struct pci_sysdata {
 	int		domain;		/* PCI domain */
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index 01d19fc22346..4a38e7917756 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -2,21 +2,7 @@
 #ifndef _ASM_X86_BOOTPARAM_H
 #define _ASM_X86_BOOTPARAM_H
 
-/* setup_data/setup_indirect types */
-#define SETUP_NONE			0
-#define SETUP_E820_EXT			1
-#define SETUP_DTB			2
-#define SETUP_PCI			3
-#define SETUP_EFI			4
-#define SETUP_APPLE_PROPERTIES		5
-#define SETUP_JAILHOUSE			6
-#define SETUP_CC_BLOB			7
-#define SETUP_IMA			8
-#define SETUP_RNG_SEED			9
-#define SETUP_ENUM_MAX			SETUP_RNG_SEED
-
-#define SETUP_INDIRECT			(1<<31)
-#define SETUP_TYPE_MAX			(SETUP_ENUM_MAX | SETUP_INDIRECT)
+#include <asm/setup_data.h>
 
 /* ram_size flags */
 #define RAMDISK_IMAGE_START_MASK	0x07FF
@@ -48,22 +34,6 @@
 #include <asm/ist.h>
 #include <video/edid.h>
 
-/* extensible setup data list node */
-struct setup_data {
-	__u64 next;
-	__u32 type;
-	__u32 len;
-	__u8 data[];
-};
-
-/* extensible setup indirect data node */
-struct setup_indirect {
-	__u32 type;
-	__u32 reserved;  /* Reserved, must be set to zero. */
-	__u64 len;
-	__u64 addr;
-};
-
 struct setup_header {
 	__u8	setup_sects;
 	__u16	root_flags;
@@ -136,51 +106,11 @@ struct efi_info {
  */
 #define E820_MAX_ENTRIES_ZEROPAGE 128
 
-/*
- * The E820 memory region entry of the boot protocol ABI:
- */
-struct boot_e820_entry {
-	__u64 addr;
-	__u64 size;
-	__u32 type;
-} __attribute__((packed));
-
 /*
  * Smallest compatible version of jailhouse_setup_data required by this kernel.
  */
 #define JAILHOUSE_SETUP_REQUIRED_VERSION	1
 
-/*
- * The boot loader is passing platform information via this Jailhouse-specific
- * setup data structure.
- */
-struct jailhouse_setup_data {
-	struct {
-		__u16	version;
-		__u16	compatible_version;
-	} __attribute__((packed)) hdr;
-	struct {
-		__u16	pm_timer_address;
-		__u16	num_cpus;
-		__u64	pci_mmconfig_base;
-		__u32	tsc_khz;
-		__u32	apic_khz;
-		__u8	standard_ioapic;
-		__u8	cpu_ids[255];
-	} __attribute__((packed)) v1;
-	struct {
-		__u32	flags;
-	} __attribute__((packed)) v2;
-} __attribute__((packed));
-
-/*
- * IMA buffer setup data information from the previous kernel during kexec
- */
-struct ima_setup_data {
-	__u64 addr;
-	__u64 size;
-} __attribute__((packed));
-
 /* The so-called "zeropage" */
 struct boot_params {
 	struct screen_info screen_info;			/* 0x000 */
diff --git a/arch/x86/include/uapi/asm/setup_data.h b/arch/x86/include/uapi/asm/setup_data.h
new file mode 100644
index 000000000000..b111b0c18544
--- /dev/null
+++ b/arch/x86/include/uapi/asm/setup_data.h
@@ -0,0 +1,83 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_ASM_X86_SETUP_DATA_H
+#define _UAPI_ASM_X86_SETUP_DATA_H
+
+/* setup_data/setup_indirect types */
+#define SETUP_NONE			0
+#define SETUP_E820_EXT			1
+#define SETUP_DTB			2
+#define SETUP_PCI			3
+#define SETUP_EFI			4
+#define SETUP_APPLE_PROPERTIES		5
+#define SETUP_JAILHOUSE			6
+#define SETUP_CC_BLOB			7
+#define SETUP_IMA			8
+#define SETUP_RNG_SEED			9
+#define SETUP_ENUM_MAX			SETUP_RNG_SEED
+
+#define SETUP_INDIRECT			(1<<31)
+#define SETUP_TYPE_MAX			(SETUP_ENUM_MAX | SETUP_INDIRECT)
+
+#ifndef __ASSEMBLY__
+
+#include <linux/types.h>
+
+/* extensible setup data list node */
+struct setup_data {
+	__u64 next;
+	__u32 type;
+	__u32 len;
+	__u8 data[];
+};
+
+/* extensible setup indirect data node */
+struct setup_indirect {
+	__u32 type;
+	__u32 reserved;  /* Reserved, must be set to zero. */
+	__u64 len;
+	__u64 addr;
+};
+
+/*
+ * The E820 memory region entry of the boot protocol ABI:
+ */
+struct boot_e820_entry {
+	__u64 addr;
+	__u64 size;
+	__u32 type;
+} __attribute__((packed));
+
+/*
+ * The boot loader is passing platform information via this Jailhouse-specific
+ * setup data structure.
+ */
+struct jailhouse_setup_data {
+	struct {
+		__u16	version;
+		__u16	compatible_version;
+	} __attribute__((packed)) hdr;
+	struct {
+		__u16	pm_timer_address;
+		__u16	num_cpus;
+		__u64	pci_mmconfig_base;
+		__u32	tsc_khz;
+		__u32	apic_khz;
+		__u8	standard_ioapic;
+		__u8	cpu_ids[255];
+	} __attribute__((packed)) v1;
+	struct {
+		__u32	flags;
+	} __attribute__((packed)) v2;
+} __attribute__((packed));
+
+/*
+ * IMA buffer setup data information from the previous kernel during kexec
+ */
+struct ima_setup_data {
+	__u64 addr;
+	__u64 size;
+} __attribute__((packed));
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _UAPI_ASM_X86_SETUP_DATA_H */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/4] arch/x86: Move internal setup_data structures into setup_data.h
  2024-01-04  9:51 [PATCH v3 0/4] arch/x86: Remove unnecessary dependencies on bootparam.h Thomas Zimmermann
  2024-01-04  9:51 ` [PATCH v3 1/4] arch/x86: Move UAPI setup structures into setup_data.h Thomas Zimmermann
@ 2024-01-04  9:51 ` Thomas Zimmermann
  2024-01-04  9:51 ` [PATCH v3 3/4] arch/x86: Implement arch_ima_efi_boot_mode() in source file Thomas Zimmermann
  2024-01-04  9:51 ` [PATCH v3 4/4] arch/x86: Do not include <asm/bootparam.h> in several files Thomas Zimmermann
  3 siblings, 0 replies; 7+ messages in thread
From: Thomas Zimmermann @ 2024-01-04  9:51 UTC (permalink / raw
  To: ardb, tglx, mingo, bp, dave.hansen, x86, hpa, bhelgaas, arnd,
	zohar, dmitry.kasatkin, paul, jmorris, serge, javierm
  Cc: linux-arch, linux-efi, linux-kernel, linux-pci, linux-integrity,
	linux-security-module, Thomas Zimmermann

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 avoids including
<asm/bootparam.h> in <asm/pci.h>. Moving struct_efi_setup_data allows
to unify duplicated definition of the data structure in a single place.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 arch/x86/boot/compressed/efi.h    |  9 ---------
 arch/x86/include/asm/efi.h        |  9 ---------
 arch/x86/include/asm/pci.h        | 13 -------------
 arch/x86/include/asm/setup_data.h | 32 +++++++++++++++++++++++++++++++
 4 files changed, 32 insertions(+), 31 deletions(-)
 create mode 100644 arch/x86/include/asm/setup_data.h

diff --git a/arch/x86/boot/compressed/efi.h b/arch/x86/boot/compressed/efi.h
index 866c0af8b5b9..b22300970f97 100644
--- a/arch/x86/boot/compressed/efi.h
+++ b/arch/x86/boot/compressed/efi.h
@@ -97,15 +97,6 @@ typedef struct {
 	u32 tables;
 } efi_system_table_32_t;
 
-/* kexec external ABI */
-struct efi_setup_data {
-	u64 fw_vendor;
-	u64 __unused;
-	u64 tables;
-	u64 smbios;
-	u64 reserved[8];
-};
-
 struct efi_unaccepted_memory {
 	u32 version;
 	u32 unit_size;
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index c4555b269a1b..a5d7a83e4c2a 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -143,15 +143,6 @@ extern void efi_free_boot_services(void);
 void arch_efi_call_virt_setup(void);
 void arch_efi_call_virt_teardown(void);
 
-/* kexec external ABI */
-struct efi_setup_data {
-	u64 fw_vendor;
-	u64 __unused;
-	u64 tables;
-	u64 smbios;
-	u64 reserved[8];
-};
-
 extern u64 efi_setup;
 
 #ifdef CONFIG_EFI
diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index f6100df3652e..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/setup_data.h>
 
 struct pci_sysdata {
 	int		domain;		/* PCI domain */
@@ -124,16 +123,4 @@ cpumask_of_pcibus(const struct pci_bus *bus)
 }
 #endif
 
-struct pci_setup_rom {
-	struct setup_data data;
-	uint16_t vendor;
-	uint16_t devid;
-	uint64_t pcilen;
-	unsigned long segment;
-	unsigned long bus;
-	unsigned long device;
-	unsigned long function;
-	uint8_t romdata[];
-};
-
 #endif /* _ASM_X86_PCI_H */
diff --git a/arch/x86/include/asm/setup_data.h b/arch/x86/include/asm/setup_data.h
new file mode 100644
index 000000000000..77c51111a893
--- /dev/null
+++ b/arch/x86/include/asm/setup_data.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_SETUP_DATA_H
+#define _ASM_X86_SETUP_DATA_H
+
+#include <uapi/asm/setup_data.h>
+
+#ifndef __ASSEMBLY__
+
+struct pci_setup_rom {
+	struct setup_data data;
+	uint16_t vendor;
+	uint16_t devid;
+	uint64_t pcilen;
+	unsigned long segment;
+	unsigned long bus;
+	unsigned long device;
+	unsigned long function;
+	uint8_t romdata[];
+};
+
+/* kexec external ABI */
+struct efi_setup_data {
+	u64 fw_vendor;
+	u64 __unused;
+	u64 tables;
+	u64 smbios;
+	u64 reserved[8];
+};
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _ASM_X86_SETUP_DATA_H */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 3/4] arch/x86: Implement arch_ima_efi_boot_mode() in source file
  2024-01-04  9:51 [PATCH v3 0/4] arch/x86: Remove unnecessary dependencies on bootparam.h Thomas Zimmermann
  2024-01-04  9:51 ` [PATCH v3 1/4] arch/x86: Move UAPI setup structures into setup_data.h Thomas Zimmermann
  2024-01-04  9:51 ` [PATCH v3 2/4] arch/x86: Move internal setup_data " Thomas Zimmermann
@ 2024-01-04  9:51 ` Thomas Zimmermann
  2024-01-04  9:51 ` [PATCH v3 4/4] arch/x86: Do not include <asm/bootparam.h> in several files Thomas Zimmermann
  3 siblings, 0 replies; 7+ messages in thread
From: Thomas Zimmermann @ 2024-01-04  9:51 UTC (permalink / raw
  To: ardb, tglx, mingo, bp, dave.hansen, x86, hpa, bhelgaas, arnd,
	zohar, dmitry.kasatkin, paul, jmorris, serge, javierm
  Cc: linux-arch, linux-efi, linux-kernel, linux-pci, linux-integrity,
	linux-security-module, Thomas Zimmermann

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

Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 arch/x86/include/asm/efi.h  | 5 +++--
 arch/x86/platform/efi/efi.c | 5 +++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index a5d7a83e4c2a..1dc600fa3ba5 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -409,8 +409,9 @@ extern int __init efi_memmap_split_count(efi_memory_desc_t *md,
 extern void __init efi_memmap_insert(struct efi_memory_map *old_memmap,
 				     void *buf, struct efi_mem_range *mem);
 
-#define arch_ima_efi_boot_mode	\
-	({ extern struct boot_params boot_params; boot_params.secure_boot; })
+extern enum efi_secureboot_mode __x86_ima_efi_boot_mode(void);
+
+#define arch_ima_efi_boot_mode	__x86_ima_efi_boot_mode()
 
 #ifdef CONFIG_EFI_RUNTIME_MAP
 int efi_get_runtime_map_size(void);
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index e9f99c56f3ce..f090ec972d7b 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -950,3 +950,8 @@ umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n)
 	}
 	return attr->mode;
 }
+
+enum efi_secureboot_mode __x86_ima_efi_boot_mode(void)
+{
+	return boot_params.secure_boot;
+}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 4/4] arch/x86: Do not include <asm/bootparam.h> in several files
  2024-01-04  9:51 [PATCH v3 0/4] arch/x86: Remove unnecessary dependencies on bootparam.h Thomas Zimmermann
                   ` (2 preceding siblings ...)
  2024-01-04  9:51 ` [PATCH v3 3/4] arch/x86: Implement arch_ima_efi_boot_mode() in source file Thomas Zimmermann
@ 2024-01-04  9:51 ` Thomas Zimmermann
  2024-01-04 16:51   ` Ard Biesheuvel
  3 siblings, 1 reply; 7+ messages in thread
From: Thomas Zimmermann @ 2024-01-04  9:51 UTC (permalink / raw
  To: ardb, tglx, mingo, bp, dave.hansen, x86, hpa, bhelgaas, arnd,
	zohar, dmitry.kasatkin, paul, jmorris, serge, javierm
  Cc: linux-arch, linux-efi, linux-kernel, linux-pci, linux-integrity,
	linux-security-module, Thomas Zimmermann

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

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Ard Biesheuvel <ardb@kernel.org>

---

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

diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 18d15d1ce87d..f196b1d1ddf8 100644
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -5,6 +5,8 @@
 #include "../string.h"
 #include "efi.h"
 
+#include <asm/bootparam.h>
+
 #include <linux/numa.h>
 
 /*
diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c
index c1bb180973ea..e162d7f59cc5 100644
--- a/arch/x86/boot/compressed/cmdline.c
+++ b/arch/x86/boot/compressed/cmdline.c
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 #include "misc.h"
 
+#include <asm/bootparam.h>
+
 static unsigned long fs;
 static inline void set_fs(unsigned long seg)
 {
diff --git a/arch/x86/boot/compressed/efi.c b/arch/x86/boot/compressed/efi.c
index 6edd034b0b30..f2e50f9758e6 100644
--- a/arch/x86/boot/compressed/efi.c
+++ b/arch/x86/boot/compressed/efi.c
@@ -7,6 +7,8 @@
 
 #include "misc.h"
 
+#include <asm/bootparam.h>
+
 /**
  * efi_get_type - Given a pointer to boot_params, determine the type of EFI environment.
  *
diff --git a/arch/x86/boot/compressed/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/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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 4/4] arch/x86: Do not include <asm/bootparam.h> in several files
  2024-01-04  9:51 ` [PATCH v3 4/4] arch/x86: Do not include <asm/bootparam.h> in several files Thomas Zimmermann
@ 2024-01-04 16:51   ` Ard Biesheuvel
  2024-01-08  9:03     ` Thomas Zimmermann
  0 siblings, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2024-01-04 16:51 UTC (permalink / raw
  To: Thomas Zimmermann
  Cc: tglx, mingo, bp, dave.hansen, x86, hpa, bhelgaas, arnd, zohar,
	dmitry.kasatkin, paul, jmorris, serge, javierm, linux-arch,
	linux-efi, linux-kernel, linux-pci, linux-integrity,
	linux-security-module

On Thu, 4 Jan 2024 at 10:54, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Remove the include statement for <asm/bootparam.h> from several files
> that don't require it. Limits the exposure of the boot parameters
> within the Linux kernel code.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
>
> ---
>
> v3:
>         * revert of e820/types.h required
> v2:
>         * clean up misc.h and e820/types.h
>         * include bootparam.h in several source files
> ---
>  arch/x86/boot/compressed/acpi.c       | 2 ++
>  arch/x86/boot/compressed/cmdline.c    | 2 ++
>  arch/x86/boot/compressed/efi.c        | 2 ++
>  arch/x86/boot/compressed/misc.h       | 3 ++-
>  arch/x86/boot/compressed/pgtable_64.c | 1 +
>  arch/x86/boot/compressed/sev.c        | 1 +
>  arch/x86/include/asm/kexec.h          | 1 -
>  arch/x86/include/asm/mem_encrypt.h    | 2 +-
>  arch/x86/include/asm/sev.h            | 3 ++-
>  arch/x86/include/asm/x86_init.h       | 2 --
>  arch/x86/kernel/crash.c               | 1 +
>  arch/x86/kernel/sev-shared.c          | 2 ++
>  arch/x86/platform/pvh/enlighten.c     | 1 +
>  arch/x86/xen/enlighten_pvh.c          | 1 +
>  arch/x86/xen/vga.c                    | 1 -
>  15 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
> index 18d15d1ce87d..f196b1d1ddf8 100644
> --- a/arch/x86/boot/compressed/acpi.c
> +++ b/arch/x86/boot/compressed/acpi.c
> @@ -5,6 +5,8 @@
>  #include "../string.h"
>  #include "efi.h"
>
> +#include <asm/bootparam.h>
> +
>  #include <linux/numa.h>
>
>  /*
> diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c
> index c1bb180973ea..e162d7f59cc5 100644
> --- a/arch/x86/boot/compressed/cmdline.c
> +++ b/arch/x86/boot/compressed/cmdline.c
> @@ -1,6 +1,8 @@
>  // SPDX-License-Identifier: GPL-2.0
>  #include "misc.h"
>
> +#include <asm/bootparam.h>
> +
>  static unsigned long fs;
>  static inline void set_fs(unsigned long seg)
>  {
> diff --git a/arch/x86/boot/compressed/efi.c b/arch/x86/boot/compressed/efi.c
> index 6edd034b0b30..f2e50f9758e6 100644
> --- a/arch/x86/boot/compressed/efi.c
> +++ b/arch/x86/boot/compressed/efi.c
> @@ -7,6 +7,8 @@
>
>  #include "misc.h"
>
> +#include <asm/bootparam.h>
> +
>  /**
>   * efi_get_type - Given a pointer to boot_params, determine the type of EFI environment.
>   *
> diff --git a/arch/x86/boot/compressed/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;
> +

Typo?

Interestingly, it still builds fine for me without any warnings.


>  /* 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/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;
>

Unfortunately, the SEV/SNP code is a bit of a kludge given that it
declares routines in headers under arch/x86/include/asm, and defines
them in two different places (the decompressor and the kernel proper).

So while I feel that we should avoid relying on incomplete struct
definitions, this one (and the one below) seems fine to me for now.
If/when someone gets around to cleaning up the SEV/SNP header files,
to split the init code from the more widely used mm types etc, we can
revisit this.


>  #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
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 4/4] arch/x86: Do not include <asm/bootparam.h> in several files
  2024-01-04 16:51   ` Ard Biesheuvel
@ 2024-01-08  9:03     ` Thomas Zimmermann
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Zimmermann @ 2024-01-08  9:03 UTC (permalink / raw
  To: Ard Biesheuvel
  Cc: tglx, mingo, bp, dave.hansen, x86, hpa, bhelgaas, arnd, zohar,
	dmitry.kasatkin, paul, jmorris, serge, javierm, linux-arch,
	linux-efi, linux-kernel, linux-pci, linux-integrity,
	linux-security-module


[-- Attachment #1.1: Type: text/plain, Size: 8892 bytes --]

Hi

Am 04.01.24 um 17:51 schrieb Ard Biesheuvel:
> On Thu, 4 Jan 2024 at 10:54, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Remove the include statement for <asm/bootparam.h> from several files
>> that don't require it. Limits the exposure of the boot parameters
>> within the Linux kernel code.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Acked-by: Ard Biesheuvel <ardb@kernel.org>
>>
>> ---
>>
>> v3:
>>          * revert of e820/types.h required
>> v2:
>>          * clean up misc.h and e820/types.h
>>          * include bootparam.h in several source files
>> ---
>>   arch/x86/boot/compressed/acpi.c       | 2 ++
>>   arch/x86/boot/compressed/cmdline.c    | 2 ++
>>   arch/x86/boot/compressed/efi.c        | 2 ++
>>   arch/x86/boot/compressed/misc.h       | 3 ++-
>>   arch/x86/boot/compressed/pgtable_64.c | 1 +
>>   arch/x86/boot/compressed/sev.c        | 1 +
>>   arch/x86/include/asm/kexec.h          | 1 -
>>   arch/x86/include/asm/mem_encrypt.h    | 2 +-
>>   arch/x86/include/asm/sev.h            | 3 ++-
>>   arch/x86/include/asm/x86_init.h       | 2 --
>>   arch/x86/kernel/crash.c               | 1 +
>>   arch/x86/kernel/sev-shared.c          | 2 ++
>>   arch/x86/platform/pvh/enlighten.c     | 1 +
>>   arch/x86/xen/enlighten_pvh.c          | 1 +
>>   arch/x86/xen/vga.c                    | 1 -
>>   15 files changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
>> index 18d15d1ce87d..f196b1d1ddf8 100644
>> --- a/arch/x86/boot/compressed/acpi.c
>> +++ b/arch/x86/boot/compressed/acpi.c
>> @@ -5,6 +5,8 @@
>>   #include "../string.h"
>>   #include "efi.h"
>>
>> +#include <asm/bootparam.h>
>> +
>>   #include <linux/numa.h>
>>
>>   /*
>> diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c
>> index c1bb180973ea..e162d7f59cc5 100644
>> --- a/arch/x86/boot/compressed/cmdline.c
>> +++ b/arch/x86/boot/compressed/cmdline.c
>> @@ -1,6 +1,8 @@
>>   // SPDX-License-Identifier: GPL-2.0
>>   #include "misc.h"
>>
>> +#include <asm/bootparam.h>
>> +
>>   static unsigned long fs;
>>   static inline void set_fs(unsigned long seg)
>>   {
>> diff --git a/arch/x86/boot/compressed/efi.c b/arch/x86/boot/compressed/efi.c
>> index 6edd034b0b30..f2e50f9758e6 100644
>> --- a/arch/x86/boot/compressed/efi.c
>> +++ b/arch/x86/boot/compressed/efi.c
>> @@ -7,6 +7,8 @@
>>
>>   #include "misc.h"
>>
>> +#include <asm/bootparam.h>
>> +
>>   /**
>>    * efi_get_type - Given a pointer to boot_params, determine the type of EFI environment.
>>    *
>> diff --git a/arch/x86/boot/compressed/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;
>> +
> 
> Typo?

Indeed

> 
> Interestingly, it still builds fine for me without any warnings.
> 
> 
>>   /* 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/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;
>>
> 
> Unfortunately, the SEV/SNP code is a bit of a kludge given that it
> declares routines in headers under arch/x86/include/asm, and defines
> them in two different places (the decompressor and the kernel proper).
> 
> So while I feel that we should avoid relying on incomplete struct
> definitions, this one (and the one below) seems fine to me for now.
> If/when someone gets around to cleaning up the SEV/SNP header files,
> to split the init code from the more widely used mm types etc, we can
> revisit this.

Thanks

> 
> 
>>   #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
>>
>>

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-01-08  9:03 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-04  9:51 [PATCH v3 0/4] arch/x86: Remove unnecessary dependencies on bootparam.h Thomas Zimmermann
2024-01-04  9:51 ` [PATCH v3 1/4] arch/x86: Move UAPI setup structures into setup_data.h Thomas Zimmermann
2024-01-04  9:51 ` [PATCH v3 2/4] arch/x86: Move internal setup_data " Thomas Zimmermann
2024-01-04  9:51 ` [PATCH v3 3/4] arch/x86: Implement arch_ima_efi_boot_mode() in source file Thomas Zimmermann
2024-01-04  9:51 ` [PATCH v3 4/4] arch/x86: Do not include <asm/bootparam.h> in several files Thomas Zimmermann
2024-01-04 16:51   ` Ard Biesheuvel
2024-01-08  9:03     ` Thomas Zimmermann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).