<p>Patrick Rudolph has uploaded this change for <strong>review</strong>.</p><p><a href="https://review.coreboot.org/c/coreboot/+/30116">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">[WIP]pci_drivers/cpu_drivers: Fix constructed arrays on 64bit platforms<br><br>The __pci_driver and __cpu_driver uses variable length arrays which are<br>constructed by the linker at build-time.<br><br>The linker always place the structs at 16-byte boundary, as per<br>"System V ABI". That's not a problem on x86, as the struct is exactly<br>16 Bytes in size. On other platforms, like x86_64 it breaks, because the<br>runtime iterator isn't aware of that alignment.<br><br>Make both linker and compiler aware of the used alignment.<br>Fixes broken __pci_driver and __cpu_driver on x86_64 (and possibly aarch64).<br><br>* Introduce ARCH_VLA_ALIGN_SIZE which defaults to 16<br>* Set ARCH_VLA_ALIGN_SIZE to 32 on x86_64 and aarch64<br>* Align struct pci_driver and struct cpu_driver to ARCH_VLA_ALIGN_SIZE<br>* Align __pci_driver and __cpu_driver to ARCH_VLA_ALIGN_SIZE<br><br>Change-Id: I2491d47ed03dcfd8db110dfb181b2c5281449591<br>Signed-off-by: Patrick Rudolph <siro@das-labor.org><br>---<br>M src/arch/arm/include/armv7/arch/cpu.h<br>M src/arch/arm64/include/arch/memlayout.h<br>M src/arch/arm64/include/armv8/arch/cpu.h<br>M src/arch/mips/include/arch/cpu.h<br>M src/arch/ppc64/include/arch/cpu.h<br>M src/arch/riscv/include/arch/cpu.h<br>M src/arch/x86/include/arch/cpu.h<br>M src/arch/x86/include/arch/memlayout.h<br>M src/include/device/pci.h<br>M src/include/memlayout.h<br>M src/lib/program.ld<br>11 files changed, 28 insertions(+), 9 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/30116/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/arch/arm/include/armv7/arch/cpu.h b/src/arch/arm/include/armv7/arch/cpu.h</span><br><span>index 22b3fb4..e0948cf 100644</span><br><span>--- a/src/arch/arm/include/armv7/arch/cpu.h</span><br><span>+++ b/src/arch/arm/include/armv7/arch/cpu.h</span><br><span>@@ -22,11 +22,12 @@</span><br><span> </span><br><span> #if !defined(__PRE_RAM__)</span><br><span> #include <device/device.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <memlayout.h></span><br><span> </span><br><span> struct cpu_driver {</span><br><span> struct device_operations *ops;</span><br><span> const struct cpu_device_id *id_table;</span><br><span style="color: hsl(0, 100%, 40%);">-};</span><br><span style="color: hsl(120, 100%, 40%);">+} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE)));</span><br><span> </span><br><span> struct cpuinfo_arm {</span><br><span> uint8_t arm; /* CPU family */</span><br><span>diff --git a/src/arch/arm64/include/arch/memlayout.h b/src/arch/arm64/include/arch/memlayout.h</span><br><span>index 7fce9aa..a7977d3 100644</span><br><span>--- a/src/arch/arm64/include/arch/memlayout.h</span><br><span>+++ b/src/arch/arm64/include/arch/memlayout.h</span><br><span>@@ -37,4 +37,6 @@</span><br><span> REGION(stack, addr, size, 16) \</span><br><span> _ = ASSERT(size >= 2K, "stack should be >= 2K, see toolchain.inc");</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#define ARCH_VLA_ALIGN_SIZE 32</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #endif /* __ARCH_MEMLAYOUT_H */</span><br><span>diff --git a/src/arch/arm64/include/armv8/arch/cpu.h b/src/arch/arm64/include/armv8/arch/cpu.h</span><br><span>index 6e096cc..6c9498c 100644</span><br><span>--- a/src/arch/arm64/include/armv8/arch/cpu.h</span><br><span>+++ b/src/arch/arm64/include/armv8/arch/cpu.h</span><br><span>@@ -22,7 +22,9 @@</span><br><span> </span><br><span> </span><br><span> #if !defined(__PRE_RAM__)</span><br><span style="color: hsl(0, 100%, 40%);">-struct cpu_driver { };</span><br><span style="color: hsl(120, 100%, 40%);">+#include <memlayout.h></span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+struct cpu_driver { } __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE)));</span><br><span> #endif</span><br><span> </span><br><span> #endif /* __ARCH_CPU_H__ */</span><br><span>diff --git a/src/arch/mips/include/arch/cpu.h b/src/arch/mips/include/arch/cpu.h</span><br><span>index 8e35908..16244cb 100644</span><br><span>--- a/src/arch/mips/include/arch/cpu.h</span><br><span>+++ b/src/arch/mips/include/arch/cpu.h</span><br><span>@@ -21,11 +21,12 @@</span><br><span> #ifndef __PRE_RAM__</span><br><span> </span><br><span> #include <device/device.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <memlayout.h></span><br><span> </span><br><span> struct cpu_driver {</span><br><span> struct device_operations *ops;</span><br><span> const struct cpu_device_id *id_table;</span><br><span style="color: hsl(0, 100%, 40%);">-};</span><br><span style="color: hsl(120, 100%, 40%);">+} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE)));</span><br><span> </span><br><span> struct thread;</span><br><span> </span><br><span>diff --git a/src/arch/ppc64/include/arch/cpu.h b/src/arch/ppc64/include/arch/cpu.h</span><br><span>index 3238bfb..bd9b8ca 100644</span><br><span>--- a/src/arch/ppc64/include/arch/cpu.h</span><br><span>+++ b/src/arch/ppc64/include/arch/cpu.h</span><br><span>@@ -20,11 +20,12 @@</span><br><span> </span><br><span> #if !defined(__PRE_RAM__)</span><br><span> #include <device/device.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <memlayout.h></span><br><span> </span><br><span> struct cpu_driver {</span><br><span> struct device_operations *ops;</span><br><span> const struct cpu_device_id *id_table;</span><br><span style="color: hsl(0, 100%, 40%);">-};</span><br><span style="color: hsl(120, 100%, 40%);">+} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE)));</span><br><span> </span><br><span> struct thread;</span><br><span> </span><br><span>diff --git a/src/arch/riscv/include/arch/cpu.h b/src/arch/riscv/include/arch/cpu.h</span><br><span>index 4ee580c..db3c6b2 100644</span><br><span>--- a/src/arch/riscv/include/arch/cpu.h</span><br><span>+++ b/src/arch/riscv/include/arch/cpu.h</span><br><span>@@ -22,11 +22,12 @@</span><br><span> </span><br><span> #if !defined(__PRE_RAM__)</span><br><span> #include <device/device.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <memlayout.h></span><br><span> </span><br><span> struct cpu_driver {</span><br><span> struct device_operations *ops;</span><br><span> const struct cpu_device_id *id_table;</span><br><span style="color: hsl(0, 100%, 40%);">-};</span><br><span style="color: hsl(120, 100%, 40%);">+} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE)));</span><br><span> </span><br><span> struct thread;</span><br><span> </span><br><span>diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h</span><br><span>index 99d1000..00c7b7f 100644</span><br><span>--- a/src/arch/x86/include/arch/cpu.h</span><br><span>+++ b/src/arch/x86/include/arch/cpu.h</span><br><span>@@ -17,6 +17,7 @@</span><br><span> #include <stdint.h></span><br><span> #include <stddef.h></span><br><span> #include <rules.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <memlayout.h></span><br><span> </span><br><span> /*</span><br><span> * EFLAGS bits</span><br><span>@@ -180,7 +181,7 @@</span><br><span> struct device_operations *ops;</span><br><span> const struct cpu_device_id *id_table;</span><br><span> struct acpi_cstate *cstates;</span><br><span style="color: hsl(0, 100%, 40%);">-};</span><br><span style="color: hsl(120, 100%, 40%);">+} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE)));</span><br><span> </span><br><span> struct cpu_driver *find_cpu_driver(struct device *cpu);</span><br><span> </span><br><span>diff --git a/src/arch/x86/include/arch/memlayout.h b/src/arch/x86/include/arch/memlayout.h</span><br><span>index 83e5b90..fa4db03 100644</span><br><span>--- a/src/arch/x86/include/arch/memlayout.h</span><br><span>+++ b/src/arch/x86/include/arch/memlayout.h</span><br><span>@@ -28,4 +28,8 @@</span><br><span> # error "CONFIG_RAMTOP not configured"</span><br><span> #endif</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#ifdef __ARCH_x86_64__</span><br><span style="color: hsl(120, 100%, 40%);">+#define ARCH_VLA_ALIGN_SIZE 32</span><br><span style="color: hsl(120, 100%, 40%);">+#endif</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #endif /* __ARCH_MEMLAYOUT_H */</span><br><span>diff --git a/src/include/device/pci.h b/src/include/device/pci.h</span><br><span>index f1ab91b..32e79ac 100644</span><br><span>--- a/src/include/device/pci.h</span><br><span>+++ b/src/include/device/pci.h</span><br><span>@@ -26,6 +26,7 @@</span><br><span> #include <device/device.h></span><br><span> #include <device/pci_ops.h></span><br><span> #include <device/pci_rom.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <memlayout.h></span><br><span> </span><br><span> </span><br><span> /* Common pci operations without a standard interface */</span><br><span>@@ -54,7 +55,7 @@</span><br><span> unsigned short vendor;</span><br><span> unsigned short device;</span><br><span> const unsigned short *devices;</span><br><span style="color: hsl(0, 100%, 40%);">-};</span><br><span style="color: hsl(120, 100%, 40%);">+} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE)));</span><br><span> </span><br><span> struct msix_entry {</span><br><span> union {</span><br><span>diff --git a/src/include/memlayout.h b/src/include/memlayout.h</span><br><span>index 5de2370..f02f97d 100644</span><br><span>--- a/src/include/memlayout.h</span><br><span>+++ b/src/include/memlayout.h</span><br><span>@@ -26,6 +26,11 @@</span><br><span> #define ARCH_POINTER_ALIGN_SIZE 8</span><br><span> #endif</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/* System V ABI alignment of Variable length arrays */</span><br><span style="color: hsl(120, 100%, 40%);">+#ifndef ARCH_VLA_ALIGN_SIZE</span><br><span style="color: hsl(120, 100%, 40%);">+#define ARCH_VLA_ALIGN_SIZE 16</span><br><span style="color: hsl(120, 100%, 40%);">+#endif</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #ifndef ARCH_CACHELINE_ALIGN_SIZE</span><br><span> #define ARCH_CACHELINE_ALIGN_SIZE 64</span><br><span> #endif</span><br><span>diff --git a/src/lib/program.ld b/src/lib/program.ld</span><br><span>index 156b862..4e28958 100644</span><br><span>--- a/src/lib/program.ld</span><br><span>+++ b/src/lib/program.ld</span><br><span>@@ -56,11 +56,11 @@</span><br><span> _ersbe_init_begin = .;</span><br><span> </span><br><span> #if ENV_RAMSTAGE</span><br><span style="color: hsl(0, 100%, 40%);">- . = ALIGN(ARCH_POINTER_ALIGN_SIZE);</span><br><span style="color: hsl(120, 100%, 40%);">+ . = ALIGN(ARCH_VLA_ALIGN_SIZE);</span><br><span> _pci_drivers = .;</span><br><span> KEEP(*(.rodata.pci_driver));</span><br><span> _epci_drivers = .;</span><br><span style="color: hsl(0, 100%, 40%);">- . = ALIGN(ARCH_POINTER_ALIGN_SIZE);</span><br><span style="color: hsl(120, 100%, 40%);">+ . = ALIGN(ARCH_VLA_ALIGN_SIZE);</span><br><span> _cpu_drivers = .;</span><br><span> KEEP(*(.rodata.cpu_driver));</span><br><span> _ecpu_drivers = .;</span><br><span></span><br></pre><p>To view, visit <a href="https://review.coreboot.org/c/coreboot/+/30116">change 30116</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/c/coreboot/+/30116"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I2491d47ed03dcfd8db110dfb181b2c5281449591 </div>
<div style="display:none"> Gerrit-Change-Number: 30116 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer@gmx.net> </div>
<div style="display:none"> Gerrit-Reviewer: Julius Werner <jwerner@chromium.org> </div>
<div style="display:none"> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> </div>
<div style="display:none"> Gerrit-Reviewer: Philipp Hug <philipp@hug.cx> </div>
<div style="display:none"> Gerrit-Reviewer: ron minnich <rminnich@gmail.com> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>