Julius Werner would like Patrick Georgi to review this change.

View Change

endian: Replace explicit byte swapping with compiler builtin

gcc seems to have some stupid problem with deciding when to inline byte
swapping functions (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92716).
Using the compiler builtin instead seems to solve the problem.

(This doesn't yet solve the issue for the read_be32()-family of
functions, which we should maybe just get rid of at some point?)

Change-Id: Ia2a6d8ea98987266ccc32ffaa0a7f78965fca1cd
Signed-off-by: Julius Werner <jwerner@chromium.org>
---
M payloads/libpayload/include/endian.h
D payloads/libpayload/include/swab.h
M src/include/endian.h
D src/include/swab.h
M src/vendorcode/eltan/security/mboot/mboot.c
M src/vendorcode/eltan/security/mboot/mboot.h
6 files changed, 20 insertions(+), 130 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/37343/1
diff --git a/payloads/libpayload/include/endian.h b/payloads/libpayload/include/endian.h
index b387e66..7f8b93a 100644
--- a/payloads/libpayload/include/endian.h
+++ b/payloads/libpayload/include/endian.h
@@ -34,23 +34,6 @@
#include <arch/types.h>
#include <libpayload-config.h>

-static inline uint16_t swap_bytes16(uint16_t in)
-{
- return ((in & 0xFF) << 8) | ((in & 0xFF00) >> 8);
-}
-
-static inline uint32_t swap_bytes32(uint32_t in)
-{
- return ((in & 0xFF) << 24) | ((in & 0xFF00) << 8) |
- ((in & 0xFF0000) >> 8) | ((in & 0xFF000000) >> 24);
-}
-
-static inline uint64_t swap_bytes64(uint64_t in)
-{
- return ((uint64_t)swap_bytes32((uint32_t)in) << 32) |
- ((uint64_t)swap_bytes32((uint32_t)(in >> 32)));
-}
-
/* Endian functions from glibc 2.9 / BSD "endian.h" */

#if CONFIG(LP_BIG_ENDIAN)
@@ -59,15 +42,15 @@
#define htobe32(in) (in)
#define htobe64(in) (in)

-#define htole16(in) swap_bytes16(in)
-#define htole32(in) swap_bytes32(in)
-#define htole64(in) swap_bytes64(in)
+#define htole16(in) __builtin_bswap16(in)
+#define htole32(in) __builtin_bswap32(in)
+#define htole64(in) __builtin_bswap64(in)

#elif CONFIG(LP_LITTLE_ENDIAN)

-#define htobe16(in) swap_bytes16(in)
-#define htobe32(in) swap_bytes32(in)
-#define htobe64(in) swap_bytes64(in)
+#define htobe16(in) __builtin_bswap16(in)
+#define htobe32(in) __builtin_bswap32(in)
+#define htobe64(in) __builtin_bswap64(in)

#define htole16(in) (in)
#define htole32(in) (in)
diff --git a/payloads/libpayload/include/swab.h b/payloads/libpayload/include/swab.h
deleted file mode 100644
index 2198077..0000000
--- a/payloads/libpayload/include/swab.h
+++ /dev/null
@@ -1,44 +0,0 @@
-#ifndef _SWAB_H
-#define _SWAB_H
-
-/*
- * linux/byteorder/swab.h
- * Byte-swapping, independently from CPU endianness
- * swabXX[ps]?(foo)
- *
- * Francois-Rene Rideau <fare@tunes.org> 19971205
- * separated swab functions from cpu_to_XX,
- * to clean up support for bizarre-endian architectures.
- *
- * See asm-i386/byteorder.h and suches for examples of how to provide
- * architecture-dependent optimized versions
- *
- */
-
-/* casts are necessary for constants, because we never know for sure
- * how U/UL/ULL map to __u16, __u32, __u64. At least not in a portable way.
- */
-#define swab16(x) \
- ((unsigned short)( \
- (((unsigned short)(x) & (unsigned short)0x00ffU) << 8) | \
- (((unsigned short)(x) & (unsigned short)0xff00U) >> 8)))
-
-#define swab32(x) \
- ((unsigned int)( \
- (((unsigned int)(x) & (unsigned int)0x000000ffUL) << 24) | \
- (((unsigned int)(x) & (unsigned int)0x0000ff00UL) << 8) | \
- (((unsigned int)(x) & (unsigned int)0x00ff0000UL) >> 8) | \
- (((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24)))
-
-#define swab64(x) \
- ((uint64_t)( \
- (((uint64_t)(x) & (uint64_t)0x00000000000000ffULL) << 56) | \
- (((uint64_t)(x) & (uint64_t)0x000000000000ff00ULL) << 40) | \
- (((uint64_t)(x) & (uint64_t)0x0000000000ff0000ULL) << 24) | \
- (((uint64_t)(x) & (uint64_t)0x00000000ff000000ULL) << 8) | \
- (((uint64_t)(x) & (uint64_t)0x000000ff00000000ULL) >> 8) | \
- (((uint64_t)(x) & (uint64_t)0x0000ff0000000000ULL) >> 24) | \
- (((uint64_t)(x) & (uint64_t)0x00ff000000000000ULL) >> 40) | \
- (((uint64_t)(x) & (uint64_t)0xff00000000000000ULL) >> 56)))
-
-#endif /* _SWAB_H */
diff --git a/src/include/endian.h b/src/include/endian.h
index 8dc1854..d4b1707 100644
--- a/src/include/endian.h
+++ b/src/include/endian.h
@@ -17,7 +17,6 @@

#include <arch/byteorder.h>
#include <stdint.h>
-#include <swab.h>

#if defined(__LITTLE_ENDIAN)
#define cpu_to_le64(x) ((uint64_t)(x))
@@ -26,19 +25,19 @@
#define le32_to_cpu(x) ((uint32_t)(x))
#define cpu_to_le16(x) ((uint16_t)(x))
#define le16_to_cpu(x) ((uint16_t)(x))
- #define cpu_to_be64(x) swab64(x)
- #define be64_to_cpu(x) swab64(x)
- #define cpu_to_be32(x) swab32(x)
- #define be32_to_cpu(x) swab32(x)
- #define cpu_to_be16(x) swab16(x)
- #define be16_to_cpu(x) swab16(x)
+ #define cpu_to_be64(x) __builtin_bswap64(x)
+ #define be64_to_cpu(x) __builtin_bswap64(x)
+ #define cpu_to_be32(x) __builtin_bswap32(x)
+ #define be32_to_cpu(x) __builtin_bswap32(x)
+ #define cpu_to_be16(x) __builtin_bswap16(x)
+ #define be16_to_cpu(x) __builtin_bswap16(x)
#elif defined(__BIG_ENDIAN)
- #define cpu_to_le64(x) swab64(x)
- #define le64_to_cpu(x) swab64(x)
- #define cpu_to_le32(x) swab32(x)
- #define le32_to_cpu(x) swab32(x)
- #define cpu_to_le16(x) swab16(x)
- #define le16_to_cpu(x) swab16(x)
+ #define cpu_to_le64(x) __builtin_bswap64(x)
+ #define le64_to_cpu(x) __builtin_bswap64(x)
+ #define cpu_to_le32(x) __builtin_bswap32(x)
+ #define le32_to_cpu(x) __builtin_bswap32(x)
+ #define cpu_to_le16(x) __builtin_bswap16(x)
+ #define le16_to_cpu(x) __builtin_bswap16(x)
#define cpu_to_be64(x) ((uint64_t)(x))
#define be64_to_cpu(x) ((uint64_t)(x))
#define cpu_to_be32(x) ((uint32_t)(x))
diff --git a/src/include/swab.h b/src/include/swab.h
deleted file mode 100644
index 956cfa5..0000000
--- a/src/include/swab.h
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- * linux/byteorder/swab.h
- * Byte-swapping, independently from CPU endianness
- * swabXX[ps]?(foo)
- *
- * Francois-Rene Rideau <fare@tunes.org> 19971205
- * separated swab functions from cpu_to_XX,
- * to clean up support for bizarre-endian architectures.
- *
- * See asm-i386/byteorder.h and such for examples of how to provide
- * architecture-dependent optimized versions
- *
- */
-
-/* casts are necessary for constants, because we never know how for sure
- * how U/UL/ULL map to __u16, __u32, __u64. At least not in a portable way.
- */
-
-#ifndef _SWAB_H
-#define _SWAB_H
-
-#include <stdint.h>
-
-#define swab16(x) \
- ((unsigned short)( \
- (((unsigned short)(x) & (unsigned short)0x00ffU) << 8) | \
- (((unsigned short)(x) & (unsigned short)0xff00U) >> 8)))
-
-#define swab32(x) \
- ((unsigned int)( \
- (((unsigned int)(x) & 0x000000ffUL) << 24) | \
- (((unsigned int)(x) & 0x0000ff00UL) << 8) | \
- (((unsigned int)(x) & 0x00ff0000UL) >> 8) | \
- (((unsigned int)(x) & 0xff000000UL) >> 24)))
-
-#define swab64(x) \
- ((uint64_t)( \
- (((uint64_t)(x) & (uint64_t)0x00000000000000ffULL) << 56) | \
- (((uint64_t)(x) & (uint64_t)0x000000000000ff00ULL) << 40) | \
- (((uint64_t)(x) & (uint64_t)0x0000000000ff0000ULL) << 24) | \
- (((uint64_t)(x) & (uint64_t)0x00000000ff000000ULL) << 8) | \
- (((uint64_t)(x) & (uint64_t)0x000000ff00000000ULL) >> 8) | \
- (((uint64_t)(x) & (uint64_t)0x0000ff0000000000ULL) >> 24) | \
- (((uint64_t)(x) & (uint64_t)0x00ff000000000000ULL) >> 40) | \
- (((uint64_t)(x) & (uint64_t)0xff00000000000000ULL) >> 56)))
-
-#endif /* _SWAB_H */
diff --git a/src/vendorcode/eltan/security/mboot/mboot.c b/src/vendorcode/eltan/security/mboot/mboot.c
index c5523a5..f47d74b 100644
--- a/src/vendorcode/eltan/security/mboot/mboot.c
+++ b/src/vendorcode/eltan/security/mboot/mboot.c
@@ -100,8 +100,8 @@
Pcrs->count = TpmCap.data.assignedPCR.count;
printk(BIOS_DEBUG, "Pcrs->count = %d\n", Pcrs->count);
for (index = 0; index < Pcrs->count; index++) {
- Pcrs->pcrSelections[index].hash =
- swab16(TpmCap.data.assignedPCR.pcrSelections[index].hash);
+ Pcrs->pcrSelections[index].hash = __builtin_bswap16(
+ TpmCap.data.assignedPCR.pcrSelections[index].hash);
printk(BIOS_DEBUG, "Pcrs->pcrSelections[%d].hash = 0x%x\n", index,
Pcrs->pcrSelections[index].hash);
Pcrs->pcrSelections[index].sizeofSelect =
diff --git a/src/vendorcode/eltan/security/mboot/mboot.h b/src/vendorcode/eltan/security/mboot/mboot.h
index 79f2308..3b08924 100644
--- a/src/vendorcode/eltan/security/mboot/mboot.h
+++ b/src/vendorcode/eltan/security/mboot/mboot.h
@@ -27,7 +27,6 @@
#include <boot/coreboot_tables.h>
#include <security/tpm/tss/tcg-2.0/tss_structures.h>
#include <security/tpm/tss.h>
-#include <swab.h>

/* TPM2 interface */
#define EFI_TPM2_ACPI_TABLE_START_METHOD_TIS 6

To view, visit change 37343. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia2a6d8ea98987266ccc32ffaa0a7f78965fca1cd
Gerrit-Change-Number: 37343
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-MessageType: newchange