Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30116
Change subject: [WIP]pci_drivers/cpu_drivers: Fix constructed arrays on 64bit platforms ......................................................................
[WIP]pci_drivers/cpu_drivers: Fix constructed arrays on 64bit platforms
The __pci_driver and __cpu_driver uses variable length arrays which are constructed by the linker at build-time.
The linker always place the structs at 16-byte boundary, as per "System V ABI". That's not a problem on x86, as the struct is exactly 16 Bytes in size. On other platforms, like x86_64 it breaks, because the runtime iterator isn't aware of that alignment.
Make both linker and compiler aware of the used alignment. Fixes broken __pci_driver and __cpu_driver on x86_64 (and possibly aarch64).
* Introduce ARCH_VLA_ALIGN_SIZE which defaults to 16 * Set ARCH_VLA_ALIGN_SIZE to 32 on x86_64 and aarch64 * Align struct pci_driver and struct cpu_driver to ARCH_VLA_ALIGN_SIZE * Align __pci_driver and __cpu_driver to ARCH_VLA_ALIGN_SIZE
Change-Id: I2491d47ed03dcfd8db110dfb181b2c5281449591 Signed-off-by: Patrick Rudolph siro@das-labor.org --- M src/arch/arm/include/armv7/arch/cpu.h M src/arch/arm64/include/arch/memlayout.h M src/arch/arm64/include/armv8/arch/cpu.h M src/arch/mips/include/arch/cpu.h M src/arch/ppc64/include/arch/cpu.h M src/arch/riscv/include/arch/cpu.h M src/arch/x86/include/arch/cpu.h M src/arch/x86/include/arch/memlayout.h M src/include/device/pci.h M src/include/memlayout.h M src/lib/program.ld 11 files changed, 28 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/30116/1
diff --git a/src/arch/arm/include/armv7/arch/cpu.h b/src/arch/arm/include/armv7/arch/cpu.h index 22b3fb4..e0948cf 100644 --- a/src/arch/arm/include/armv7/arch/cpu.h +++ b/src/arch/arm/include/armv7/arch/cpu.h @@ -22,11 +22,12 @@
#if !defined(__PRE_RAM__) #include <device/device.h> +#include <memlayout.h>
struct cpu_driver { struct device_operations *ops; const struct cpu_device_id *id_table; -}; +} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE)));
struct cpuinfo_arm { uint8_t arm; /* CPU family */ diff --git a/src/arch/arm64/include/arch/memlayout.h b/src/arch/arm64/include/arch/memlayout.h index 7fce9aa..a7977d3 100644 --- a/src/arch/arm64/include/arch/memlayout.h +++ b/src/arch/arm64/include/arch/memlayout.h @@ -37,4 +37,6 @@ REGION(stack, addr, size, 16) \ _ = ASSERT(size >= 2K, "stack should be >= 2K, see toolchain.inc");
+#define ARCH_VLA_ALIGN_SIZE 32 + #endif /* __ARCH_MEMLAYOUT_H */ diff --git a/src/arch/arm64/include/armv8/arch/cpu.h b/src/arch/arm64/include/armv8/arch/cpu.h index 6e096cc..6c9498c 100644 --- a/src/arch/arm64/include/armv8/arch/cpu.h +++ b/src/arch/arm64/include/armv8/arch/cpu.h @@ -22,7 +22,9 @@
#if !defined(__PRE_RAM__) -struct cpu_driver { }; +#include <memlayout.h> + +struct cpu_driver { } __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE))); #endif
#endif /* __ARCH_CPU_H__ */ diff --git a/src/arch/mips/include/arch/cpu.h b/src/arch/mips/include/arch/cpu.h index 8e35908..16244cb 100644 --- a/src/arch/mips/include/arch/cpu.h +++ b/src/arch/mips/include/arch/cpu.h @@ -21,11 +21,12 @@ #ifndef __PRE_RAM__
#include <device/device.h> +#include <memlayout.h>
struct cpu_driver { struct device_operations *ops; const struct cpu_device_id *id_table; -}; +} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE)));
struct thread;
diff --git a/src/arch/ppc64/include/arch/cpu.h b/src/arch/ppc64/include/arch/cpu.h index 3238bfb..bd9b8ca 100644 --- a/src/arch/ppc64/include/arch/cpu.h +++ b/src/arch/ppc64/include/arch/cpu.h @@ -20,11 +20,12 @@
#if !defined(__PRE_RAM__) #include <device/device.h> +#include <memlayout.h>
struct cpu_driver { struct device_operations *ops; const struct cpu_device_id *id_table; -}; +} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE)));
struct thread;
diff --git a/src/arch/riscv/include/arch/cpu.h b/src/arch/riscv/include/arch/cpu.h index 4ee580c..db3c6b2 100644 --- a/src/arch/riscv/include/arch/cpu.h +++ b/src/arch/riscv/include/arch/cpu.h @@ -22,11 +22,12 @@
#if !defined(__PRE_RAM__) #include <device/device.h> +#include <memlayout.h>
struct cpu_driver { struct device_operations *ops; const struct cpu_device_id *id_table; -}; +} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE)));
struct thread;
diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h index 99d1000..00c7b7f 100644 --- a/src/arch/x86/include/arch/cpu.h +++ b/src/arch/x86/include/arch/cpu.h @@ -17,6 +17,7 @@ #include <stdint.h> #include <stddef.h> #include <rules.h> +#include <memlayout.h>
/* * EFLAGS bits @@ -180,7 +181,7 @@ struct device_operations *ops; const struct cpu_device_id *id_table; struct acpi_cstate *cstates; -}; +} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE)));
struct cpu_driver *find_cpu_driver(struct device *cpu);
diff --git a/src/arch/x86/include/arch/memlayout.h b/src/arch/x86/include/arch/memlayout.h index 83e5b90..fa4db03 100644 --- a/src/arch/x86/include/arch/memlayout.h +++ b/src/arch/x86/include/arch/memlayout.h @@ -28,4 +28,8 @@ # error "CONFIG_RAMTOP not configured" #endif
+#ifdef __ARCH_x86_64__ +#define ARCH_VLA_ALIGN_SIZE 32 +#endif + #endif /* __ARCH_MEMLAYOUT_H */ diff --git a/src/include/device/pci.h b/src/include/device/pci.h index f1ab91b..32e79ac 100644 --- a/src/include/device/pci.h +++ b/src/include/device/pci.h @@ -26,6 +26,7 @@ #include <device/device.h> #include <device/pci_ops.h> #include <device/pci_rom.h> +#include <memlayout.h>
/* Common pci operations without a standard interface */ @@ -54,7 +55,7 @@ unsigned short vendor; unsigned short device; const unsigned short *devices; -}; +} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE)));
struct msix_entry { union { diff --git a/src/include/memlayout.h b/src/include/memlayout.h index 5de2370..f02f97d 100644 --- a/src/include/memlayout.h +++ b/src/include/memlayout.h @@ -26,6 +26,11 @@ #define ARCH_POINTER_ALIGN_SIZE 8 #endif
+/* System V ABI alignment of Variable length arrays */ +#ifndef ARCH_VLA_ALIGN_SIZE +#define ARCH_VLA_ALIGN_SIZE 16 +#endif + #ifndef ARCH_CACHELINE_ALIGN_SIZE #define ARCH_CACHELINE_ALIGN_SIZE 64 #endif diff --git a/src/lib/program.ld b/src/lib/program.ld index 156b862..4e28958 100644 --- a/src/lib/program.ld +++ b/src/lib/program.ld @@ -56,11 +56,11 @@ _ersbe_init_begin = .;
#if ENV_RAMSTAGE - . = ALIGN(ARCH_POINTER_ALIGN_SIZE); + . = ALIGN(ARCH_VLA_ALIGN_SIZE); _pci_drivers = .; KEEP(*(.rodata.pci_driver)); _epci_drivers = .; - . = ALIGN(ARCH_POINTER_ALIGN_SIZE); + . = ALIGN(ARCH_VLA_ALIGN_SIZE); _cpu_drivers = .; KEEP(*(.rodata.cpu_driver)); _ecpu_drivers = .;