From 8f34fdfab3ac0ec76e7600222310afb0b18c26d0 Mon Sep 17 00:00:00 2001 From: Julius Werner Date: Tue, 9 Dec 2025 11:34:18 -0800 Subject: [PATCH] Remove and swabXX() functions GCC generates correct code for __builtin_bswapXX() on all architectures, including ArmV4. It seems that whatever bug caused this to not work back in commit 879ea7fce8a2 ("endian: Replace explicit byte swapping with compiler builtin") has been fixed now. We can eliminate the swabXX() functions and simplify the code. All instances that had been calling these functions directly should have been using real endianness conversions anyway. Change-Id: I19713fd009aa5c0e01c4a42e0cf012364d6bed60 Signed-off-by: Julius Werner Reviewed-on: https://review.coreboot.org/c/coreboot/+/90438 Reviewed-by: Maximilian Brune Reviewed-by: Yu-Ping Wu Tested-by: build bot (Jenkins) Reviewed-by: Frans Hendriks --- payloads/libpayload/include/endian.h | 4 -- .../bsd/include/commonlib/bsd/_endian.h | 16 +++--- src/ec/clevo/it5570e/commands.c | 4 +- src/include/endian.h | 1 - src/include/swab.h | 55 ------------------- src/mainboard/prodrive/hermes/eeprom.c | 3 +- .../amd/common/psp_verstage/vboot_crypto.c | 6 +- src/vendorcode/eltan/security/mboot/mboot.c | 3 +- src/vendorcode/eltan/security/mboot/mboot.h | 1 - 9 files changed, 17 insertions(+), 76 deletions(-) delete mode 100644 src/include/swab.h diff --git a/payloads/libpayload/include/endian.h b/payloads/libpayload/include/endian.h index 46e5dfe6eb..31d5f4f900 100644 --- a/payloads/libpayload/include/endian.h +++ b/payloads/libpayload/include/endian.h @@ -34,10 +34,6 @@ #include #include -#define swab16(x) __builtin_bswap16(x) -#define swab32(x) __builtin_bswap32(x) -#define swab64(x) __builtin_bswap64(x) - #if CONFIG(LP_BIG_ENDIAN) #define __BIG_ENDIAN #elif CONFIG(LP_LITTLE_ENDIAN) diff --git a/src/commonlib/bsd/include/commonlib/bsd/_endian.h b/src/commonlib/bsd/include/commonlib/bsd/_endian.h index d0d5be9f9f..d3fa661060 100644 --- a/src/commonlib/bsd/include/commonlib/bsd/_endian.h +++ b/src/commonlib/bsd/include/commonlib/bsd/_endian.h @@ -5,8 +5,8 @@ /* * This header should not be included directly. Including source must define - * prerequisites like uintXX_t types, the byteswap functions swabXX(), - * __BIG_ENDIAN or __LITTLE_ENDIAN and I/O accessors readXX()/writeXX(). + * prerequisites like uintXX_t types, __BIG_ENDIAN or __LITTLE_ENDIAN and + * I/O accessors readXX()/writeXX(). */ /* Endian functions from glibc 2.9 / BSD "endian.h" */ @@ -15,13 +15,13 @@ #define htobe16(in) (in) #define htobe32(in) (in) #define htobe64(in) (in) - #define htole16(in) ((uint16_t)swab16(in)) - #define htole32(in) ((uint32_t)swab32(in)) - #define htole64(in) ((uint64_t)swab64(in)) + #define htole16(in) ((uint16_t)__builtin_bswap16(in)) + #define htole32(in) ((uint32_t)__builtin_bswap32(in)) + #define htole64(in) ((uint64_t)__builtin_bswap64(in)) #elif defined(__LITTLE_ENDIAN) - #define htobe16(in) ((uint16_t)swab16(in)) - #define htobe32(in) ((uint32_t)swab32(in)) - #define htobe64(in) ((uint64_t)swab64(in)) + #define htobe16(in) ((uint16_t)__builtin_bswap16(in)) + #define htobe32(in) ((uint32_t)__builtin_bswap32(in)) + #define htobe64(in) ((uint64_t)__builtin_bswap64(in)) #define htole16(in) (in) #define htole32(in) (in) #define htole64(in) (in) diff --git a/src/ec/clevo/it5570e/commands.c b/src/ec/clevo/it5570e/commands.c index 8f53bb18c8..d5f08a4a23 100644 --- a/src/ec/clevo/it5570e/commands.c +++ b/src/ec/clevo/it5570e/commands.c @@ -3,7 +3,7 @@ #include #include #include -#include +#include #include #include @@ -102,7 +102,7 @@ void ec_set_kbled_timeout(uint16_t timeout) printk(BIOS_DEBUG, "EC: set keyboard backlight timeout to %us\n", timeout); write8p(ECRAM + FDAT, timeout ? 0xff : 0x00); - write16p(ECRAM + FBUF, swab16(timeout)); + write16p(ECRAM + FBUF, htobe16(timeout)); ec_fcmd(FCMD_SET_KBLED_TIMEOUT); } diff --git a/src/include/endian.h b/src/include/endian.h index efc56dd7be..7300ae839f 100644 --- a/src/include/endian.h +++ b/src/include/endian.h @@ -6,7 +6,6 @@ #include #include #include -#include /* This include depends on previous ones, do not reorder. */ #include diff --git a/src/include/swab.h b/src/include/swab.h deleted file mode 100644 index eadf293745..0000000000 --- a/src/include/swab.h +++ /dev/null @@ -1,55 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -/* - * linux/byteorder/swab.h - * Byte-swapping, independently from CPU endianness - * swabXX[ps]?(foo) - * - * Francois-Rene Rideau 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 - -#if ENV_ARMV4 -#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))) -#else /* ENV_ARMV4 */ -#define swab16(x) ((uint16_t)__builtin_bswap16(x)) -#define swab32(x) ((uint32_t)__builtin_bswap32(x)) -#define swab64(x) ((uint64_t)__builtin_bswap64(x)) -#endif /* !ENV_ARMV4 */ - -#endif /* _SWAB_H */ diff --git a/src/mainboard/prodrive/hermes/eeprom.c b/src/mainboard/prodrive/hermes/eeprom.c index c0e9cb6648..685b7a6372 100644 --- a/src/mainboard/prodrive/hermes/eeprom.c +++ b/src/mainboard/prodrive/hermes/eeprom.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -163,7 +164,7 @@ bool eeprom_read_buffer(void *blob, size_t read_offset, size_t size) u8 tmp[2] = {0}; ret = do_smbus_process_call(SMBUS_IO_BASE, I2C_ADDR_EEPROM, 0, - swab16(read_offset + i), (uint16_t *)&tmp[0]); + htobe16(read_offset + i), (uint16_t *)&tmp[0]); if (ret < 0) break; diff --git a/src/soc/amd/common/psp_verstage/vboot_crypto.c b/src/soc/amd/common/psp_verstage/vboot_crypto.c index 370416f8f7..bc094349e3 100644 --- a/src/soc/amd/common/psp_verstage/vboot_crypto.c +++ b/src/soc/amd/common/psp_verstage/vboot_crypto.c @@ -5,11 +5,11 @@ #include #include #include +#include #include "psp_verstage.h" #include #include #include -#include #include #include @@ -161,7 +161,7 @@ vb2_error_t vb2ex_hwcrypto_modexp(const struct vb2_public_key *key, return VB2_ERROR_WORKBUF_SMALL; for (i = 0; i < key->arrsize; i++) - sig_swapped[i] = swab32(inout_32[key->arrsize - i - 1]); + sig_swapped[i] = be32toh(inout_32[key->arrsize - i - 1]); mod_exp_param.pExponent = (char *)&exp; mod_exp_param.ExpSize = sizeof(exp); @@ -179,7 +179,7 @@ vb2_error_t vb2ex_hwcrypto_modexp(const struct vb2_public_key *key, /* vboot expects results in *inout with BE, so copy & convert. */ for (i = 0; i < key->arrsize; i++) - inout_32[i] = swab32(output_buffer[key->arrsize - i - 1]); + inout_32[i] = htobe32(output_buffer[key->arrsize - i - 1]); return VB2_SUCCESS; } diff --git a/src/vendorcode/eltan/security/mboot/mboot.c b/src/vendorcode/eltan/security/mboot/mboot.c index 9cdd0de2fe..fa513007cc 100644 --- a/src/vendorcode/eltan/security/mboot/mboot.c +++ b/src/vendorcode/eltan/security/mboot/mboot.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -87,7 +88,7 @@ tpm_result_t tpm2_get_capability_pcrs(TPML_PCR_SELECTION *Pcrs) 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); + be16toh(TpmCap.data.assignedPCR.pcrSelections[index].hash); printk(BIOS_DEBUG, "Pcrs->pcrSelections[%d].hash = %#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 20333fc1b1..b8510ac47e 100644 --- a/src/vendorcode/eltan/security/mboot/mboot.h +++ b/src/vendorcode/eltan/security/mboot/mboot.h @@ -12,7 +12,6 @@ #include #include #include -#include /* TPM2 interface */ #define EFI_TPM2_ACPI_TABLE_START_METHOD_TIS 6