Skip to content

Commit 2efa4e4

Browse files
adbridgeadbridge
authored
Merge pull request #7952 from offirko/offir-mbed-spif
Prevent sector-unaligned erase
2 parents c2fdc0d + df36f0e commit 2efa4e4

File tree

3 files changed

+81
-82
lines changed

3 files changed

+81
-82
lines changed

‎components/storage/blockdevice/COMPONENT_SPIF/SPIFBlockDevice.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -339,12 +339,22 @@ int SPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
339339
int size = (int)in_size;
340340
bool erase_failed = false;
341341
int status = SPIF_BD_ERROR_OK;
342-
// Find region of erased address
342+
// Find region of erased address
343343
int region = _utils_find_addr_region(addr);
344344
// Erase Types of selected region
345345
uint8_t bitfield = _region_erase_types_bitfield[region];
346346

347-
tr_error("DEBUG: erase - addr: %llu, in_size: %llu", addr, in_size);
347+
tr_info("DEBUG: erase - addr: %llu, in_size: %llu", addr, in_size);
348+
349+
if ((addr + in_size) > _device_size_bytes) {
350+
tr_error("ERROR: erase exceeds flash device size");
351+
return SPIF_BD_ERROR_INVALID_ERASE_PARAMS;
352+
}
353+
354+
if ( ((addr % get_erase_size(addr)) != 0 ) || (((addr + in_size) % get_erase_size(addr + in_size - 1)) != 0 ) ) {
355+
tr_error("ERROR: invalid erase - unaligned address and size");
356+
return SPIF_BD_ERROR_INVALID_ERASE_PARAMS;
357+
}
348358

349359
// For each iteration erase the largest section supported by current region
350360
while (size > 0) {
@@ -353,13 +363,12 @@ int SPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
353363
// find the matching instruction and erase size chunk for that type.
354364
type = _utils_iterate_next_largest_erase_type(bitfield, size, (unsigned int)addr, _region_high_boundary[region]);
355365
cur_erase_inst = _erase_type_inst_arr[type];
356-
//chunk = _erase_type_size_arr[type];
357366
offset = addr % _erase_type_size_arr[type];
358367
chunk = ( (offset + size) < _erase_type_size_arr[type]) ? size : (_erase_type_size_arr[type] - offset);
359368

360-
tr_error("DEBUG: erase - addr: %llu, size:%d, Inst: 0x%xh, chunk: %lu , ",
369+
tr_debug("DEBUG: erase - addr: %llu, size:%d, Inst: 0x%xh, chunk: %lu , ",
361370
addr, size, cur_erase_inst, chunk);
362-
tr_error("DEBUG: erase - Region: %d, Type:%d",
371+
tr_debug("DEBUG: erase - Region: %d, Type:%d",
363372
region, type);
364373

365374
_mutex->lock();

‎components/storage/blockdevice/COMPONENT_SPIF/SPIFBlockDevice.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,12 @@
2525
* @enum spif_bd_error
2626
*/
2727
enum spif_bd_error {
28-
SPIF_BD_ERROR_OK = 0, /*!< no error */
29-
SPIF_BD_ERROR_DEVICE_ERROR = BD_ERROR_DEVICE_ERROR, /*!< device specific error -4001 */
30-
SPIF_BD_ERROR_PARSING_FAILED = -4002, /* SFDP Parsing failed */
31-
SPIF_BD_ERROR_READY_FAILED = -4003, /* Wait for Mem Ready failed */
32-
SPIF_BD_ERROR_WREN_FAILED = -4004, /* Write Enable Failed */
28+
SPIF_BD_ERROR_OK = 0, /*!< no error */
29+
SPIF_BD_ERROR_DEVICE_ERROR = BD_ERROR_DEVICE_ERROR, /*!< device specific error -4001 */
30+
SPIF_BD_ERROR_PARSING_FAILED = -4002, /* SFDP Parsing failed */
31+
SPIF_BD_ERROR_READY_FAILED = -4003, /* Wait for Mem Ready failed */
32+
SPIF_BD_ERROR_WREN_FAILED = -4004, /* Write Enable Failed */
33+
SPIF_BD_ERROR_INVALID_ERASE_PARAMS = -4005, /* Erase command not on sector aligned addresses or exceeds device size */
3334
};
3435

3536

@@ -136,6 +137,7 @@ class SPIFBlockDevice : public BlockDevice {
136137
* SPIF_BD_ERROR_DEVICE_ERROR - device driver transaction failed
137138
* SPIF_BD_ERROR_READY_FAILED - Waiting for Memory ready failed or timed out
138139
* SPIF_BD_ERROR_WREN_FAILED - Write Enable failed
140+
* SPIF_BD_ERROR_INVALID_ERASE_PARAMS - Trying to erase unaligned address or size
139141
*/
140142
virtual int erase(bd_addr_t addr, bd_size_t size);
141143

‎components/storage/blockdevice/COMPONENT_SPIF/TESTS/block_device/spif/main.cpp

Lines changed: 60 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,13 @@ static SingletonPtr<PlatformMutex> _mutex;
4141
// Mutex is protecting rand() per srand for buffer writing and verification.
4242
// Mutex is also protecting printouts for clear logs.
4343
// Mutex is NOT protecting Block Device actions: erase/program/read - which is the purpose of the multithreaded test!
44-
void basic_erase_program_read_test(SPIFBlockDevice& blockD, bd_size_t block_size, uint8_t *write_block,
44+
void basic_erase_program_read_test(SPIFBlockDevice& block_device, bd_size_t block_size, uint8_t *write_block,
4545
uint8_t *read_block, unsigned addrwidth)
4646
{
4747
int err = 0;
4848
_mutex->lock();
4949
// Find a random block
50-
bd_addr_t block = (rand() * block_size) % blockD.size();
50+
bd_addr_t block = (rand() * block_size) % block_device.size();
5151

5252
// Use next random number as temporary seed to keep
5353
// the address progressing in the pseudorandom sequence
@@ -62,13 +62,13 @@ void basic_erase_program_read_test(SPIFBlockDevice& blockD, bd_size_t block_size
6262
utest_printf("\ntest %0*llx:%llu...", addrwidth, block, block_size);
6363
_mutex->unlock();
6464

65-
err = blockD.erase(block, block_size);
65+
err = block_device.erase(block, block_size);
6666
TEST_ASSERT_EQUAL(0, err);
6767

68-
err = blockD.program(write_block, block, block_size);
68+
err = block_device.program(write_block, block, block_size);
6969
TEST_ASSERT_EQUAL(0, err);
7070

71-
err = blockD.read(read_block, block, block_size);
71+
err = block_device.read(read_block, block, block_size);
7272
TEST_ASSERT_EQUAL(0, err);
7373

7474
_mutex->lock();
@@ -91,16 +91,17 @@ void test_spif_random_program_read_erase()
9191
{
9292
utest_printf("\nTest Random Program Read Erase Starts..\n");
9393

94-
SPIFBlockDevice blockD(MBED_CONF_SPIF_DRIVER_SPI_MOSI, MBED_CONF_SPIF_DRIVER_SPI_MISO, MBED_CONF_SPIF_DRIVER_SPI_CLK,
95-
MBED_CONF_SPIF_DRIVER_SPI_CS);
94+
SPIFBlockDevice block_device(MBED_CONF_SPIF_DRIVER_SPI_MOSI, MBED_CONF_SPIF_DRIVER_SPI_MISO,
95+
MBED_CONF_SPIF_DRIVER_SPI_CLK,
96+
MBED_CONF_SPIF_DRIVER_SPI_CS);
9697

97-
int err = blockD.init();
98+
int err = block_device.init();
9899
TEST_ASSERT_EQUAL(0, err);
99100

100101
for (unsigned atr = 0; atr < sizeof(ATTRS) / sizeof(ATTRS[0]); atr++) {
101102
static const char *prefixes[] = {"", "k", "M", "G"};
102103
for (int i_ind = 3; i_ind >= 0; i_ind--) {
103-
bd_size_t size = (blockD.*ATTRS[atr].method)();
104+
bd_size_t size = (block_device.*ATTRS[atr].method)();
104105
if (size >= (1ULL << 10 * i_ind)) {
105106
utest_printf("%s: %llu%sbytes (%llubytes)\n",
106107
ATTRS[atr].name, size >> 10 * i_ind, prefixes[i_ind], size);
@@ -109,8 +110,8 @@ void test_spif_random_program_read_erase()
109110
}
110111
}
111112

112-
bd_size_t block_size = blockD.get_erase_size();
113-
unsigned addrwidth = ceil(log(float(blockD.size() - 1)) / log(float(16))) + 1;
113+
bd_size_t block_size = block_device.get_erase_size();
114+
unsigned addrwidth = ceil(log(float(block_device.size() - 1)) / log(float(16))) + 1;
114115

115116
uint8_t *write_block = new (std::nothrow) uint8_t[block_size];
116117
uint8_t *read_block = new (std::nothrow) uint8_t[block_size];
@@ -120,31 +121,32 @@ void test_spif_random_program_read_erase()
120121
}
121122

122123
for (int b = 0; b < TEST_BLOCK_COUNT; b++) {
123-
basic_erase_program_read_test(blockD, block_size, write_block, read_block, addrwidth);
124+
basic_erase_program_read_test(block_device, block_size, write_block, read_block, addrwidth);
124125
}
125126

126-
err = blockD.deinit();
127+
err = block_device.deinit();
127128
TEST_ASSERT_EQUAL(0, err);
128129

129130
end:
130131
delete[] write_block;
131132
delete[] read_block;
132133
}
133134

134-
void test_spif_unaligned_program()
135+
void test_spif_unaligned_erase()
135136
{
136-
utest_printf("\nTest Unaligned Program Starts..\n");
137+
utest_printf("\nTest Unaligned Erase Starts..\n");
137138

138-
SPIFBlockDevice blockD(MBED_CONF_SPIF_DRIVER_SPI_MOSI, MBED_CONF_SPIF_DRIVER_SPI_MISO, MBED_CONF_SPIF_DRIVER_SPI_CLK,
139-
MBED_CONF_SPIF_DRIVER_SPI_CS);
139+
SPIFBlockDevice block_device(MBED_CONF_SPIF_DRIVER_SPI_MOSI, MBED_CONF_SPIF_DRIVER_SPI_MISO,
140+
MBED_CONF_SPIF_DRIVER_SPI_CLK,
141+
MBED_CONF_SPIF_DRIVER_SPI_CS);
140142

141-
int err = blockD.init();
143+
int err = block_device.init();
142144
TEST_ASSERT_EQUAL(0, err);
143145

144146
for (unsigned atr = 0; atr < sizeof(ATTRS) / sizeof(ATTRS[0]); atr++) {
145147
static const char *prefixes[] = {"", "k", "M", "G"};
146148
for (int i_ind = 3; i_ind >= 0; i_ind--) {
147-
bd_size_t size = (blockD.*ATTRS[atr].method)();
149+
bd_size_t size = (block_device.*ATTRS[atr].method)();
148150
if (size >= (1ULL << 10 * i_ind)) {
149151
utest_printf("%s: %llu%sbytes (%llubytes)\n",
150152
ATTRS[atr].name, size >> 10 * i_ind, prefixes[i_ind], size);
@@ -153,68 +155,53 @@ void test_spif_unaligned_program()
153155
}
154156
}
155157

156-
bd_size_t block_size = blockD.get_erase_size();
157-
unsigned addrwidth = ceil(log(float(blockD.size() - 1)) / log(float(16))) + 1;
158+
bd_addr_t addr = 0;
159+
bd_size_t sector_erase_size = block_device.get_erase_size(addr);
160+
unsigned addrwidth = ceil(log(float(block_device.size() - 1)) / log(float(16))) + 1;
158161

159-
uint8_t *write_block = new (std::nothrow) uint8_t[block_size];
160-
uint8_t *read_block = new (std::nothrow) uint8_t[block_size];
161-
if (!write_block || !read_block ) {
162-
utest_printf("\n Not enough memory for test");
163-
goto end;
164-
}
165-
166-
{
167-
bd_addr_t block = (rand() * block_size) % blockD.size() + 1;
162+
utest_printf("\ntest %0*llx:%llu...", addrwidth, addr, sector_erase_size);
168163

169-
// Use next random number as temporary seed to keep
170-
// the address progressing in the pseudorandom sequence
171-
unsigned seed = rand();
164+
//unaligned start address
165+
addr += 1;
166+
err = block_device.erase(addr, sector_erase_size - 1);
167+
TEST_ASSERT_EQUAL(SPIF_BD_ERROR_INVALID_ERASE_PARAMS, err);
172168

173-
// Fill with random sequence
174-
srand(seed);
175-
for (bd_size_t i_ind = 0; i_ind < block_size; i_ind++) {
176-
write_block[i_ind] = 0xff & rand();
177-
}
169+
err = block_device.erase(addr, sector_erase_size);
170+
TEST_ASSERT_EQUAL(SPIF_BD_ERROR_INVALID_ERASE_PARAMS, err);
178171

179-
// Write, sync, and read the block
180-
utest_printf("\ntest %0*llx:%llu...", addrwidth, block, block_size);
172+
err = block_device.erase(addr, 1);
173+
TEST_ASSERT_EQUAL(SPIF_BD_ERROR_INVALID_ERASE_PARAMS, err);
181174

182-
err = blockD.erase(block, block_size);
183-
TEST_ASSERT_EQUAL(0, err);
175+
//unaligned end address
176+
addr = 0;
184177

185-
//err = blockD.erase(block+4096, block_size);
186-
//TEST_ASSERT_EQUAL(0, err);
178+
err = block_device.erase(addr, 1);
179+
TEST_ASSERT_EQUAL(SPIF_BD_ERROR_INVALID_ERASE_PARAMS, err);
187180

188-
err = blockD.program(write_block, block, block_size);
189-
TEST_ASSERT_EQUAL(0, err);
181+
err = block_device.erase(addr, sector_erase_size + 1);
182+
TEST_ASSERT_EQUAL(SPIF_BD_ERROR_INVALID_ERASE_PARAMS, err);
190183

191-
err = blockD.read(read_block, block, block_size);
192-
TEST_ASSERT_EQUAL(0, err);
184+
//erase size exceeds flash device size
185+
err = block_device.erase(addr, block_device.size() + 1);
186+
TEST_ASSERT_EQUAL(SPIF_BD_ERROR_INVALID_ERASE_PARAMS, err);
193187

194-
// Check that the data was unmodified
195-
srand(seed);
196-
for (bd_size_t i_ind = 0; i_ind < block_size; i_ind++) {
197-
//printf("index %d\n", i_ind);
198-
TEST_ASSERT_EQUAL(0xff & rand(), read_block[i_ind]);
199-
}
188+
// Valid erase
189+
err = block_device.erase(addr, sector_erase_size);
190+
TEST_ASSERT_EQUAL(SPIF_BD_ERROR_OK, err);
200191

201-
err = blockD.deinit();
202-
TEST_ASSERT_EQUAL(0, err);
203-
}
204-
end:
205-
delete[] write_block;
206-
delete[] read_block;
192+
err = block_device.deinit();
193+
TEST_ASSERT_EQUAL(0, err);
207194
}
208195

209-
static void test_spif_thread_job(void *vBlockD/*, int thread_num*/)
196+
static void test_spif_thread_job(void *block_device_ptr/*, int thread_num*/)
210197
{
211198
static int thread_num = 0;
212199
thread_num++;
213-
SPIFBlockDevice *blockD = (SPIFBlockDevice *)vBlockD;
200+
SPIFBlockDevice *block_device = (SPIFBlockDevice *)block_device_ptr;
214201
utest_printf("\n Thread %d Started \n", thread_num);
215202

216-
bd_size_t block_size = blockD->get_erase_size();
217-
unsigned addrwidth = ceil(log(float(blockD->size() - 1)) / log(float(16))) + 1;
203+
bd_size_t block_size = block_device->get_erase_size();
204+
unsigned addrwidth = ceil(log(float(block_device->size() - 1)) / log(float(16))) + 1;
218205

219206
uint8_t *write_block = new (std::nothrow) uint8_t[block_size];
220207
uint8_t *read_block = new (std::nothrow) uint8_t[block_size];
@@ -224,7 +211,7 @@ static void test_spif_thread_job(void *vBlockD/*, int thread_num*/)
224211
}
225212

226213
for (int b = 0; b < TEST_BLOCK_COUNT; b++) {
227-
basic_erase_program_read_test((*blockD), block_size, write_block, read_block, addrwidth);
214+
basic_erase_program_read_test((*block_device), block_size, write_block, read_block, addrwidth);
228215
}
229216

230217
end:
@@ -236,16 +223,17 @@ void test_spif_multi_threads()
236223
{
237224
utest_printf("\nTest Multi Threaded Erase/Program/Read Starts..\n");
238225

239-
SPIFBlockDevice blockD(MBED_CONF_SPIF_DRIVER_SPI_MOSI, MBED_CONF_SPIF_DRIVER_SPI_MISO, MBED_CONF_SPIF_DRIVER_SPI_CLK,
240-
MBED_CONF_SPIF_DRIVER_SPI_CS);
226+
SPIFBlockDevice block_device(MBED_CONF_SPIF_DRIVER_SPI_MOSI, MBED_CONF_SPIF_DRIVER_SPI_MISO,
227+
MBED_CONF_SPIF_DRIVER_SPI_CLK,
228+
MBED_CONF_SPIF_DRIVER_SPI_CS);
241229

242-
int err = blockD.init();
230+
int err = block_device.init();
243231
TEST_ASSERT_EQUAL(0, err);
244232

245233
for (unsigned atr = 0; atr < sizeof(ATTRS) / sizeof(ATTRS[0]); atr++) {
246234
static const char *prefixes[] = {"", "k", "M", "G"};
247235
for (int i_ind = 3; i_ind >= 0; i_ind--) {
248-
bd_size_t size = (blockD.*ATTRS[atr].method)();
236+
bd_size_t size = (block_device.*ATTRS[atr].method)();
249237
if (size >= (1ULL << 10 * i_ind)) {
250238
utest_printf("%s: %llu%sbytes (%llubytes)\n",
251239
ATTRS[atr].name, size >> 10 * i_ind, prefixes[i_ind], size);
@@ -260,7 +248,7 @@ void test_spif_multi_threads()
260248
int i_ind;
261249

262250
for (i_ind = 0; i_ind < SPIF_TEST_NUM_OF_THREADS; i_ind++) {
263-
threadStatus = spif_bd_thread[i_ind].start(test_spif_thread_job, (void *)&blockD);
251+
threadStatus = spif_bd_thread[i_ind].start(test_spif_thread_job, (void *)&block_device);
264252
if (threadStatus != 0) {
265253
utest_printf("\n Thread %d Start Failed!", i_ind + 1);
266254
}
@@ -270,7 +258,7 @@ void test_spif_multi_threads()
270258
spif_bd_thread[i_ind].join();
271259
}
272260

273-
err = blockD.deinit();
261+
err = block_device.deinit();
274262
TEST_ASSERT_EQUAL(0, err);
275263
}
276264

@@ -282,7 +270,7 @@ utest::v1::status_t test_setup(const size_t number_of_cases)
282270
}
283271

284272
Case cases[] = {
285-
Case("Testing unaligned program blocks", test_spif_unaligned_program),
273+
Case("Testing unaligned erase blocks", test_spif_unaligned_erase),
286274
Case("Testing read write random blocks", test_spif_random_program_read_erase),
287275
Case("Testing Multi Threads Erase Program Read", test_spif_multi_threads)
288276
};

0 commit comments

Comments
 (0)