From 5fb9a957f0b8fc0f20d9df820f6987600848b4dd Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sat, 22 Jul 2023 14:39:13 +0100 Subject: [PATCH 1/6] crc32: Fixed a type oops in the header --- src/include/crc32.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/include/crc32.h b/src/include/crc32.h index 99b285af255..9521255504a 100644 --- a/src/include/crc32.h +++ b/src/include/crc32.h @@ -21,6 +21,9 @@ #ifndef INCLUDE_CRC32_H #define INCLUDE_CRC32_H -bool generic_crc32(target_s *t, uint32_t *crc, uint32_t base, int len); +#include +#include + +bool generic_crc32(target_s *target, uint32_t *crc, uint32_t base, size_t len); #endif /* INCLUDE_CRC32_H */ From 4305da9bb018d8f289267fc5697887c60a0d33b7 Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sat, 22 Jul 2023 14:39:30 +0100 Subject: [PATCH 2/6] crc32: Fixed the naming of the target parameter in both implementations --- src/crc32.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/crc32.c b/src/crc32.c index d5be271924f..342a83d0bf6 100644 --- a/src/crc32.c +++ b/src/crc32.c @@ -100,7 +100,7 @@ static uint32_t crc32_calc(const uint32_t crc, const uint8_t data) return (crc << 8U) ^ crc32_table[((crc >> 24U) ^ data) & 0xffU]; } -bool generic_crc32(target_s *const t, uint32_t *const crc_res, uint32_t base, size_t len) +bool generic_crc32(target_s *const target, uint32_t *const crc_res, uint32_t base, size_t len) { uint32_t crc = 0xffffffffU; #if PC_HOSTED == 1 @@ -124,7 +124,7 @@ bool generic_crc32(target_s *const t, uint32_t *const crc_res, uint32_t base, si gdb_if_putchar(0, true); } const size_t read_len = MIN(sizeof(bytes), len); - if (target_mem_read(t, bytes, base, (read_len + 3U) & ~3U)) { + if (target_mem_read(target, bytes, base, (read_len + 3U) & ~3U)) { DEBUG_ERROR("generic_crc32 error around address 0x%08" PRIx32 "\n", base); return false; } @@ -142,7 +142,7 @@ bool generic_crc32(target_s *const t, uint32_t *const crc_res, uint32_t base, si #else #include -bool generic_crc32(target_s *const t, uint32_t *const crc_res, uint32_t base, size_t len) +bool generic_crc32(target_s *const target, uint32_t *const crc_res, uint32_t base, size_t len) { uint8_t bytes[128]; @@ -156,7 +156,7 @@ bool generic_crc32(target_s *const t, uint32_t *const crc_res, uint32_t base, si gdb_if_putchar(0, true); } const size_t read_len = MIN(sizeof(bytes), len) & ~3U; - if (target_mem_read(t, bytes, base, read_len)) { + if (target_mem_read(target, bytes, base, read_len)) { DEBUG_ERROR("generic_crc32 error around address 0x%08" PRIx32 "\n", base); return false; } @@ -170,7 +170,7 @@ bool generic_crc32(target_s *const t, uint32_t *const crc_res, uint32_t base, si uint32_t crc = CRC_DR; - if (target_mem_read(t, bytes, base, len)) { + if (target_mem_read(target, bytes, base, len)) { DEBUG_ERROR("generic_crc32 error around address 0x%08" PRIx32 "\n", base); return false; } From c69987c4ebab89859ee4205be238ed1204e588b7 Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sat, 22 Jul 2023 14:45:46 +0100 Subject: [PATCH 3/6] crc32: Cleaned up the generic impementation --- src/crc32.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/crc32.c b/src/crc32.c index 342a83d0bf6..439b6dbc2a9 100644 --- a/src/crc32.c +++ b/src/crc32.c @@ -100,7 +100,7 @@ static uint32_t crc32_calc(const uint32_t crc, const uint8_t data) return (crc << 8U) ^ crc32_table[((crc >> 24U) ^ data) & 0xffU]; } -bool generic_crc32(target_s *const target, uint32_t *const crc_res, uint32_t base, size_t len) +bool generic_crc32(target_s *const target, uint32_t *const crc_res, const uint32_t base, const size_t len) { uint32_t crc = 0xffffffffU; #if PC_HOSTED == 1 @@ -108,32 +108,29 @@ bool generic_crc32(target_s *const target, uint32_t *const crc_res, uint32_t bas * Reading a 2 MByte on a H743 takes about 80 s@128, 28s @ 1k, * 22 s @ 4k and 21 s @ 64k */ - uint8_t bytes[0x1000]; + uint8_t bytes[4096U]; #else - uint8_t bytes[128]; + uint8_t bytes[128U]; #endif #if defined(ENABLE_DEBUG) const uint32_t start_time = platform_time_ms(); #endif uint32_t last_time = platform_time_ms(); - while (len) { + for (size_t offset = 0; offset < len; offset += sizeof(bytes)) { const uint32_t actual_time = platform_time_ms(); if (actual_time > last_time + 1000U) { last_time = actual_time; gdb_if_putchar(0, true); } - const size_t read_len = MIN(sizeof(bytes), len); - if (target_mem_read(target, bytes, base, (read_len + 3U) & ~3U)) { - DEBUG_ERROR("generic_crc32 error around address 0x%08" PRIx32 "\n", base); + const size_t read_len = MIN(sizeof(bytes), len - offset); + if (target_mem_read(target, bytes, base + offset, (read_len + 3U) & ~3U)) { + DEBUG_ERROR("generic_crc32 error around address 0x%08" PRIx32 "\n", (uint32_t)(base + offset)); return false; } for (size_t i = 0; i < read_len; i++) crc = crc32_calc(crc, bytes[i]); - - base += read_len; - len -= read_len; } DEBUG_WARN("%" PRIu32 " ms\n", platform_time_ms() - start_time); *crc_res = crc; From 5890ee9b9f29fa2ca0b09b975c3e84629e156a07 Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sat, 22 Jul 2023 14:55:38 +0100 Subject: [PATCH 4/6] crc32: Cleaned up the STM32-specific impementation --- src/crc32.c | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/crc32.c b/src/crc32.c index 439b6dbc2a9..f95a7a317e0 100644 --- a/src/crc32.c +++ b/src/crc32.c @@ -100,7 +100,7 @@ static uint32_t crc32_calc(const uint32_t crc, const uint8_t data) return (crc << 8U) ^ crc32_table[((crc >> 24U) ^ data) & 0xffU]; } -bool generic_crc32(target_s *const target, uint32_t *const crc_res, const uint32_t base, const size_t len) +bool generic_crc32(target_s *const target, uint32_t *const result, const uint32_t base, const size_t len) { uint32_t crc = 0xffffffffU; #if PC_HOSTED == 1 @@ -133,55 +133,55 @@ bool generic_crc32(target_s *const target, uint32_t *const crc_res, const uint32 crc = crc32_calc(crc, bytes[i]); } DEBUG_WARN("%" PRIu32 " ms\n", platform_time_ms() - start_time); - *crc_res = crc; + *result = crc; return true; } #else #include -bool generic_crc32(target_s *const target, uint32_t *const crc_res, uint32_t base, size_t len) +bool generic_crc32(target_s *const target, uint32_t *const result, const uint32_t base, const size_t len) { - uint8_t bytes[128]; + uint8_t bytes[128U]; CRC_CR |= CRC_CR_RESET; uint32_t last_time = platform_time_ms(); - while (len > 3U) { + const size_t adjusted_len = len & ~3U; + for (size_t offset = 0; offset < adjusted_len; offset += sizeof(bytes)) { const uint32_t actual_time = platform_time_ms(); if (actual_time > last_time + 1000U) { last_time = actual_time; gdb_if_putchar(0, true); } - const size_t read_len = MIN(sizeof(bytes), len) & ~3U; - if (target_mem_read(target, bytes, base, read_len)) { - DEBUG_ERROR("generic_crc32 error around address 0x%08" PRIx32 "\n", base); + const size_t read_len = MIN(sizeof(bytes), adjusted_len - offset); + if (target_mem_read(target, bytes, base + offset, read_len)) { + DEBUG_ERROR("generic_crc32 error around address 0x%08" PRIx32 "\n", (uint32_t)(base + offset)); return false; } for (size_t i = 0; i < read_len; i += 4U) CRC_DR = __builtin_bswap32(*(uint32_t *)(bytes + i)); - - base += read_len; - len -= read_len; } uint32_t crc = CRC_DR; - if (target_mem_read(target, bytes, base, len)) { - DEBUG_ERROR("generic_crc32 error around address 0x%08" PRIx32 "\n", base); - return false; - } - uint8_t *data = bytes; - while (len--) { - crc ^= *data++ << 24U; - for (size_t i = 0; i < 8U; i++) { - if (crc & 0x80000000U) - crc = (crc << 1U) ^ 0x4c11db7U; - else - crc <<= 1U; + const size_t remainder = len - adjusted_len; + if (remainder) { + if (target_mem_read(target, bytes, base + adjusted_len, remainder)) { + DEBUG_ERROR("generic_crc32 error around address 0x%08" PRIx32 "\n", (uint32_t)(base + adjusted_len)); + return false; + } + for (size_t offset = 0; offset < remainder; ++offset) { + crc ^= bytes[offset] << 24U; + for (size_t i = 0; i < 8U; i++) { + if (crc & 0x80000000U) + crc = (crc << 1U) ^ 0x4c11db7U; + else + crc <<= 1U; + } } } - *crc_res = crc; + *result = crc; return true; } #endif From 00d183bb1d2b78d9d5dfebeaad5290f29c73ab8e Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sat, 22 Jul 2023 14:59:52 +0100 Subject: [PATCH 5/6] buffer_utils: Implemented a big endian 32-bit read --- src/include/buffer_utils.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/include/buffer_utils.h b/src/include/buffer_utils.h index c8792be1b23..085d75af9ad 100644 --- a/src/include/buffer_utils.h +++ b/src/include/buffer_utils.h @@ -62,4 +62,10 @@ static inline uint32_t read_le4(const uint8_t *const buffer, const size_t offset ((uint32_t)buffer[offset + 3U] << 24U); } +static inline uint32_t read_be4(const uint8_t *const buffer, const size_t offset) +{ + return ((uint32_t)buffer[offset + 0U] << 24U) | ((uint32_t)buffer[offset + 1U] << 16U) | + ((uint32_t)buffer[offset + 2U] << 8U) | buffer[offset + 3U]; +} + #endif /*INCLUDE_BUFFER_UTILS_H*/ From bc03e437f87fb5287492c25904ca696fb62d4b13 Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sat, 22 Jul 2023 15:01:02 +0100 Subject: [PATCH 6/6] crc32: Replaced the `__builtin_swap32()` shenanigan with a proper big endian buffer reader function --- src/crc32.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/crc32.c b/src/crc32.c index f95a7a317e0..7b05155cc05 100644 --- a/src/crc32.c +++ b/src/crc32.c @@ -138,6 +138,7 @@ bool generic_crc32(target_s *const target, uint32_t *const result, const uint32_ } #else #include +#include "buffer_utils.h" bool generic_crc32(target_s *const target, uint32_t *const result, const uint32_t base, const size_t len) { @@ -160,7 +161,7 @@ bool generic_crc32(target_s *const target, uint32_t *const result, const uint32_ } for (size_t i = 0; i < read_len; i += 4U) - CRC_DR = __builtin_bswap32(*(uint32_t *)(bytes + i)); + CRC_DR = read_be4(bytes, i); } uint32_t crc = CRC_DR;