From f0d50aa404d10adeb7215478df9b13a697ef2888 Mon Sep 17 00:00:00 2001 From: Patrick Rudolph Date: Sun, 5 Jan 2025 15:16:33 +0100 Subject: [PATCH] commonlib/include/commonlib: Add volatile qualifier With the introduction of the stack canary breakpoint QEMU uncovered a different bug within coreboot. Currently the compiler optimizes over aggressively inline functions and memory stores. That also affects write_at_ble8(), which is supposed to store a single byte at time. The compiler however optimizes multiple byte stores into a single wider (and possibly unaligned) store operation. This can be seen in the emited assembly code of write_le16(), as used to install the EBDA: 401348a: 66 c7 04 25 13 04 00 movw $0x400,0x413 4013491: 00 00 04 Make sure that the compiler does not optimize multiple calls to write_at_ble8() by adding the volatile qualifier. The emitted assembly code of the same function changes to: 401394c: c6 04 25 13 04 00 00 movb $0x0,0x413 4013953: 00 4013954: c6 04 25 14 04 00 00 movb $0x4,0x414 401395b: 04 Fixes a strange bug in QEMU where it triggers the DEBUG breakpoint handler on unaligned 16-bit stores in the first 4KiB of memory. Aligned stores and store outside of the first 4KiB do not dispatch the DEBUG breakpoint handler. Change-Id: Ibbc661235a38c7f7540b656a67f067c3e51105d1 Signed-off-by: Patrick Rudolph Reviewed-on: https://review.coreboot.org/c/coreboot/+/85855 Tested-by: build bot (Jenkins) Reviewed-by: Angel Pons --- src/commonlib/include/commonlib/endian.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/commonlib/include/commonlib/endian.h b/src/commonlib/include/commonlib/endian.h index 84c90b4297..971854c2b3 100644 --- a/src/commonlib/include/commonlib/endian.h +++ b/src/commonlib/include/commonlib/endian.h @@ -11,7 +11,7 @@ static inline uint8_t read_ble8(const void *src) { - const uint8_t *s = src; + const volatile uint8_t *s = src; return *s; } @@ -24,7 +24,7 @@ static inline uint8_t read_at_ble8(const void *src, size_t offset) static inline void write_ble8(void *dest, uint8_t val) { - *(uint8_t *)dest = val; + *(volatile uint8_t *)dest = val; } static inline void write_at_ble8(void *dest, uint8_t val, size_t offset) @@ -58,7 +58,7 @@ static inline void write_at_be8(void *dest, uint8_t val, size_t offset) static inline uint16_t read_be16(const void *src) { - const uint8_t *s = src; + const volatile uint8_t *s = src; return (((uint16_t)s[0]) << 8) | (((uint16_t)s[1]) << 0); } @@ -84,7 +84,7 @@ static inline void write_at_be16(void *dest, uint16_t val, size_t offset) static inline uint32_t read_be32(const void *src) { - const uint8_t *s = src; + const volatile uint8_t *s = src; return (((uint32_t)s[0]) << 24) | (((uint32_t)s[1]) << 16) | (((uint32_t)s[2]) << 8) | (((uint32_t)s[3]) << 0); } @@ -162,7 +162,7 @@ static inline void write_at_le8(void *dest, uint8_t val, size_t offset) static inline uint16_t read_le16(const void *src) { - const uint8_t *s = src; + const volatile uint8_t *s = src; return (((uint16_t)s[1]) << 8) | (((uint16_t)s[0]) << 0); } @@ -188,7 +188,7 @@ static inline void write_at_le16(void *dest, uint16_t val, size_t offset) static inline uint32_t read_le32(const void *src) { - const uint8_t *s = src; + const volatile uint8_t *s = src; return (((uint32_t)s[3]) << 24) | (((uint32_t)s[2]) << 16) | (((uint32_t)s[1]) << 8) | (((uint32_t)s[0]) << 0); }