Skip to content

Commit 9bbb19a

Browse files
Damian-Nordicnordicjm
authored andcommitted
debug: coredump: fix CRC calcuation for changing data
Fix CRC calculation in the nRF internal flash core dump backend in the scenario where the data being dumped keeps changing. This occurs in one of the Core Dump modes that dumps also the IRQ stack. The fix is done by writing all data using an intermedate buffer and doing the CRC calculation on that buffer instead of input data. Signed-off-by: Damian Krolik <[email protected]>
1 parent a837eff commit 9bbb19a

File tree

2 files changed

+31
-37
lines changed

2 files changed

+31
-37
lines changed

subsys/debug/coredump/Kconfig

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
# SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
55
#
66

7-
config DEBUG_COREDUMP_BACKEND_NRF_FLASH_PARTITION
7+
menuconfig DEBUG_COREDUMP_BACKEND_NRF_FLASH_PARTITION
88
bool "Core dump backend using nRF internal flash/RRAM partition [EXPERIMENTAL]"
99
depends on DEBUG_COREDUMP_BACKEND_OTHER
1010
depends on DT_HAS_NORDIC_NRF51_FLASH_CONTROLLER_ENABLED || \
@@ -23,3 +23,11 @@ config DEBUG_COREDUMP_BACKEND_NRF_FLASH_PARTITION
2323
nrfx library directly. This minimizes the dependencies needed to write
2424
the core dump and bypasses synchronization primitives incorporated
2525
into the Zephyr flash drivers.
26+
27+
if DEBUG_COREDUMP_BACKEND_NRF_FLASH_PARTITION
28+
29+
config DEBUG_COREDUMP_BACKEND_NRF_FLASH_PARTITION_WRITE_BUF_SIZE
30+
int "Write buffer size"
31+
default 128
32+
33+
endif # DEBUG_COREDUMP_BACKEND_NRF_FLASH_PARTITION

subsys/debug/coredump/coredump_backend_nrf_flash_partition.c

Lines changed: 22 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,15 @@ static const uint8_t MAGIC[4] = {'C', 'D', '0', '1'};
6969

7070
enum {
7171
HEADER_SIZE = ROUND_UP(sizeof(struct header), FLASH_WRITE_SIZE),
72+
WRITE_BUF_SIZE = ROUND_UP(CONFIG_DEBUG_COREDUMP_BACKEND_NRF_FLASH_PARTITION_WRITE_BUF_SIZE,
73+
FLASH_WRITE_SIZE),
7274
};
7375

74-
static int write_error; /* Error occurred when writing a core dump */
75-
static uint8_t write_buf[FLASH_WRITE_SIZE]; /* Write buffer to assure aligned flash access */
76-
static size_t write_buf_pos; /* # of dump data bytes buffered in the write buffer */
77-
static size_t write_pos; /* # of dump data bytes already written to flash */
78-
static uint16_t dump_crc; /* CRC16 of already written or buffered bytes */
76+
static int write_error; /* Error occurred when writing a core dump */
77+
static uint8_t write_buf[WRITE_BUF_SIZE]; /* Write buffer to assure aligned flash access */
78+
static size_t write_buf_pos; /* # of dump data bytes buffered in the write buffer */
79+
static size_t write_pos; /* # of dump data bytes already written to flash */
80+
static uint16_t dump_crc; /* CRC16 of already written or buffered bytes */
7981

8082
static inline const struct header *get_stored_header(void)
8183
{
@@ -229,8 +231,8 @@ static void coredump_nrf_flash_backend_end(void)
229231

230232
if (write_buf_pos > 0) {
231233
/* Flush the write buffer */
232-
memset(&write_buf[write_buf_pos], 0, FLASH_WRITE_SIZE - write_buf_pos);
233-
write(HEADER_SIZE + write_pos, write_buf, FLASH_WRITE_SIZE);
234+
memset(&write_buf[write_buf_pos], 0, WRITE_BUF_SIZE - write_buf_pos);
235+
write(HEADER_SIZE + write_pos, write_buf, WRITE_BUF_SIZE);
234236
write_pos += write_buf_pos;
235237
}
236238

@@ -252,41 +254,25 @@ static void coredump_nrf_flash_backend_buffer_output(uint8_t *data, size_t size)
252254
{
253255
size_t chunk_size;
254256

255-
dump_crc = crc16_ccitt(dump_crc, data, size);
256-
257-
/* If the write buffer is non-empty, append it with new data. */
257+
/*
258+
* Write all data using intermediate buffer to assure proper write alignment,
259+
* and to calculate a valid CRC even if the data keeps changing during write
260+
* (for example, if the data is within the IRQ stack region).
261+
*/
258262

259-
if (write_buf_pos > 0) {
260-
chunk_size = MIN(size, FLASH_WRITE_SIZE - write_buf_pos);
263+
while (size > 0) {
264+
chunk_size = MIN(size, WRITE_BUF_SIZE - write_buf_pos);
261265
memcpy(&write_buf[write_buf_pos], data, chunk_size);
266+
dump_crc = crc16_ccitt(dump_crc, &write_buf[write_buf_pos], chunk_size);
262267
write_buf_pos += chunk_size;
263268
data += chunk_size;
264269
size -= chunk_size;
265-
}
266-
267-
/* Flush the write buffer if it's full. */
268-
269-
if (write_buf_pos == FLASH_WRITE_SIZE) {
270-
write(HEADER_SIZE + write_pos, write_buf, FLASH_WRITE_SIZE);
271-
write_pos += write_buf_pos;
272-
write_buf_pos = 0;
273-
}
274-
275-
/* Write the remaining full write blocks directly to flash. */
276-
277-
if (size >= FLASH_WRITE_SIZE) {
278-
chunk_size = ROUND_DOWN(size, FLASH_WRITE_SIZE);
279-
write(HEADER_SIZE + write_pos, data, chunk_size);
280-
write_pos += chunk_size;
281-
data += chunk_size;
282-
size -= chunk_size;
283-
}
284270

285-
/* Store the remaining data in the write buffer. */
286-
287-
if (size > 0) {
288-
memcpy(write_buf, data, size);
289-
write_buf_pos = size;
271+
if (write_buf_pos == WRITE_BUF_SIZE) {
272+
write(HEADER_SIZE + write_pos, write_buf, WRITE_BUF_SIZE);
273+
write_pos += WRITE_BUF_SIZE;
274+
write_buf_pos = 0;
275+
}
290276
}
291277
}
292278

0 commit comments

Comments
 (0)