erebion/coreboot
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

commonlib/include/commonlib: Add volatile qualifier

Browse source 
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 <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/85855
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
This commit is contained in:
Patrick Rudolph 2025-01-05 15:16:33 +01:00 • committed by Matt DeVillier
commit f0d50aa404
1 changed files with 6 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

12
src/commonlib/include/commonlib/endian.h
View file

@ -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);
}