[coreboot-gerrit] Change in ...coreboot[master]: [WIP]pci_drivers/cpu_drivers: Fix constructed arrays on 64bit platforms
Patrick Rudolph (Code Review)
gerrit at coreboot.org
Sun Dec 9 14:37:46 CET 2018
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 at 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 = .;
--
To view, visit https://review.coreboot.org/c/coreboot/+/30116
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2491d47ed03dcfd8db110dfb181b2c5281449591
Gerrit-Change-Number: 30116
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Rudolph <siro at das-labor.org>
Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer at gmx.net>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro at das-labor.org>
Gerrit-Reviewer: Philipp Hug <philipp at hug.cx>
Gerrit-Reviewer: ron minnich <rminnich at gmail.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20181209/f4d0ad06/attachment.html>
More information about the coreboot-gerrit
mailing list