Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix flash_cache_table allocation failures; clean up code #9169

Merged
merged 2 commits into from
Apr 12, 2024
Merged

fix flash_cache_table allocation failures; clean up code #9169

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2fa97f3
fix flash_cache_table allocation failures; clean up code
dhalbert Apr 11, 2024
98957cc
change unneeded uint8_t decls to size_t or uint32_t
dhalbert Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 63 additions & 64 deletions supervisor/shared/external_flash/external_flash.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ static const external_flash_device *flash_device = NULL;
// cache.
static uint32_t dirty_mask;

// Table of pointers to each cached block
// Table of pointers to each cached block. Should be zero'd after allocation.
#define BLOCKS_PER_SECTOR (SPI_FLASH_ERASE_SIZE / FILESYSTEM_BLOCK_SIZE)
#define PAGES_PER_BLOCK (FILESYSTEM_BLOCK_SIZE / SPI_FLASH_PAGE_SIZE)
#define FLASH_CACHE_TABLE_NUM_ENTRIES (BLOCKS_PER_SECTOR * PAGES_PER_BLOCK)
#define FLASH_CACHE_TABLE_SIZE (FLASH_CACHE_TABLE_NUM_ENTRIES * sizeof (uint8_t *))
static uint8_t **flash_cache_table = NULL;

// Wait until both the write enable and write in progress bits have cleared.
Expand Down Expand Up @@ -207,7 +211,7 @@ void supervisor_flash_init(void) {
// Delay to give the SPI Flash time to get going.
// TODO(tannewt): Only do this when we know power was applied vs a reset.
uint16_t max_start_up_delay_us = 0;
for (uint8_t i = 0; i < EXTERNAL_FLASH_DEVICE_COUNT; i++) {
for (size_t i = 0; i < EXTERNAL_FLASH_DEVICE_COUNT; i++) {
if (possible_devices[i].start_up_time_us > max_start_up_delay_us) {
max_start_up_delay_us = possible_devices[i].start_up_time_us;
}
Expand All @@ -219,7 +223,7 @@ void supervisor_flash_init(void) {
#ifdef EXTERNAL_FLASH_NO_JEDEC
// For NVM that don't have JEDEC response
spi_flash_command(CMD_WAKE);
for (uint8_t i = 0; i < EXTERNAL_FLASH_DEVICE_COUNT; i++) {
for (size_t i = 0; i < EXTERNAL_FLASH_DEVICE_COUNT; i++) {
const external_flash_device *possible_device = &possible_devices[i];
flash_device = possible_device;
break;
Expand All @@ -234,7 +238,7 @@ void supervisor_flash_init(void) {
while ((count-- > 0) && (jedec_id_response[0] == 0xff || jedec_id_response[2] == 0x00)) {
spi_flash_read_command(CMD_READ_JEDEC_ID, jedec_id_response, 3);
}
for (uint8_t i = 0; i < EXTERNAL_FLASH_DEVICE_COUNT; i++) {
for (size_t i = 0; i < EXTERNAL_FLASH_DEVICE_COUNT; i++) {
const external_flash_device *possible_device = &possible_devices[i];
if (jedec_id_response[0] == possible_device->manufacturer_id &&
jedec_id_response[1] == possible_device->memory_type &&
Expand Down Expand Up @@ -323,7 +327,7 @@ static bool flush_scratch_flash(void) {
// cached.
bool copy_to_scratch_ok = true;
uint32_t scratch_sector = flash_device->total_size - SPI_FLASH_ERASE_SIZE;
for (uint8_t i = 0; i < SPI_FLASH_ERASE_SIZE / FILESYSTEM_BLOCK_SIZE; i++) {
for (size_t i = 0; i < BLOCKS_PER_SECTOR; i++) {
if ((dirty_mask & (1 << i)) == 0) {
copy_to_scratch_ok = copy_to_scratch_ok &&
copy_block(current_sector + i * FILESYSTEM_BLOCK_SIZE,
Expand All @@ -338,72 +342,70 @@ static bool flush_scratch_flash(void) {
// Second, erase the current sector.
erase_sector(current_sector);
// Finally, copy the new version into it.
for (uint8_t i = 0; i < SPI_FLASH_ERASE_SIZE / FILESYSTEM_BLOCK_SIZE; i++) {
for (size_t i = 0; i < BLOCKS_PER_SECTOR; i++) {
copy_block(scratch_sector + i * FILESYSTEM_BLOCK_SIZE,
current_sector + i * FILESYSTEM_BLOCK_SIZE);
}
return true;
}

// Free all entries in the partially or completely filled flash_cache_table, and then free the table itself.
static void release_ram_cache(void) {
if (flash_cache_table == NULL) {
return;
}

for (size_t i = 0; i < FLASH_CACHE_TABLE_NUM_ENTRIES; i++) {
// Table may not be completely full. Stop at first NULL entry.
if (flash_cache_table[i] == NULL) {
break;
}
port_free(flash_cache_table[i]);
}
port_free(flash_cache_table);
flash_cache_table = NULL;
}

// Attempts to allocate a new set of page buffers for caching a full sector in
// ram. Each page is allocated separately so that the GC doesn't need to provide
// one huge block. We can free it as we write if we want to also.
static bool allocate_ram_cache(void) {
uint8_t blocks_per_sector = SPI_FLASH_ERASE_SIZE / FILESYSTEM_BLOCK_SIZE;
uint8_t pages_per_block = FILESYSTEM_BLOCK_SIZE / SPI_FLASH_PAGE_SIZE;

uint32_t table_size = blocks_per_sector * pages_per_block * sizeof(size_t);
// Attempt to allocate outside the heap first.
flash_cache_table = port_malloc(table_size, false);

// Declare i and j outside the loops in case we fail to allocate everything
// we need. In that case we'll give it back.
uint8_t i = 0;
uint8_t j = 0;
bool success = flash_cache_table != NULL;
for (i = 0; i < blocks_per_sector && success; i++) {
for (j = 0; j < pages_per_block && success; j++) {
flash_cache_table = port_malloc(FLASH_CACHE_TABLE_SIZE, false);
if (flash_cache_table == NULL) {
// Not enough space even for the cache table.
return false;
}

// Clear all the entries so it's easy to find the last entry.
memset(flash_cache_table, 0, FLASH_CACHE_TABLE_SIZE);

bool success = true;
for (size_t i = 0; i < BLOCKS_PER_SECTOR && success; i++) {
for (size_t j = 0; j < PAGES_PER_BLOCK && success; j++) {
uint8_t *page_cache = port_malloc(SPI_FLASH_PAGE_SIZE, false);
if (page_cache == NULL) {
success = false;
break;
}
flash_cache_table[i * pages_per_block + j] = page_cache;
flash_cache_table[i * PAGES_PER_BLOCK + j] = page_cache;
}
}

// We couldn't allocate enough so give back what we got.
if (!success) {
// We add 1 so that we delete 0 when i is 1. Going to zero (i >= 0)
// would never stop because i is unsigned.
i++;
for (; i > 0; i--) {
for (; j > 0; j--) {
port_free(flash_cache_table[(i - 1) * pages_per_block + (j - 1)]);
}
j = pages_per_block;
}
port_free(flash_cache_table);
flash_cache_table = NULL;
release_ram_cache();
}
return success;
}

static void release_ram_cache(void) {
uint8_t blocks_per_sector = SPI_FLASH_ERASE_SIZE / FILESYSTEM_BLOCK_SIZE;
uint8_t pages_per_block = FILESYSTEM_BLOCK_SIZE / SPI_FLASH_PAGE_SIZE;
for (uint8_t i = 0; i < blocks_per_sector; i++) {
for (uint8_t j = 0; j < pages_per_block; j++) {
uint32_t offset = i * pages_per_block + j;
port_free(flash_cache_table[offset]);
}
}
port_free(flash_cache_table);
flash_cache_table = NULL;
}

// Flush the cached sector from ram onto the flash. We'll free the cache unless
// keep_cache is true.
static bool flush_ram_cache(bool keep_cache) {
if (flash_cache_table == NULL) {
// Nothing to flush because there is no cache.
return true;
}

if (current_sector == NO_SECTOR_LOADED) {
if (!keep_cache) {
release_ram_cache();
Expand All @@ -414,13 +416,12 @@ static bool flush_ram_cache(bool keep_cache) {
// we've cached. If we don't do this we'll erase the data during the sector
// erase below.
bool copy_to_ram_ok = true;
uint8_t pages_per_block = FILESYSTEM_BLOCK_SIZE / SPI_FLASH_PAGE_SIZE;
for (uint8_t i = 0; i < SPI_FLASH_ERASE_SIZE / FILESYSTEM_BLOCK_SIZE; i++) {
for (size_t i = 0; i < BLOCKS_PER_SECTOR; i++) {
if ((dirty_mask & (1 << i)) == 0) {
for (uint8_t j = 0; j < pages_per_block; j++) {
for (size_t j = 0; j < PAGES_PER_BLOCK; j++) {
copy_to_ram_ok = read_flash(
current_sector + (i * pages_per_block + j) * SPI_FLASH_PAGE_SIZE,
flash_cache_table[i * pages_per_block + j],
current_sector + (i * PAGES_PER_BLOCK + j) * SPI_FLASH_PAGE_SIZE,
flash_cache_table[i * PAGES_PER_BLOCK + j],
SPI_FLASH_PAGE_SIZE);
if (!copy_to_ram_ok) {
break;
Expand All @@ -438,10 +439,10 @@ static bool flush_ram_cache(bool keep_cache) {
// Second, erase the current sector.
erase_sector(current_sector);
// Lastly, write all the data in ram that we've cached.
for (uint8_t i = 0; i < SPI_FLASH_ERASE_SIZE / FILESYSTEM_BLOCK_SIZE; i++) {
for (uint8_t j = 0; j < pages_per_block; j++) {
write_flash(current_sector + (i * pages_per_block + j) * SPI_FLASH_PAGE_SIZE,
flash_cache_table[i * pages_per_block + j],
for (size_t i = 0; i < BLOCKS_PER_SECTOR; i++) {
for (size_t j = 0; j < PAGES_PER_BLOCK; j++) {
write_flash(current_sector + (i * PAGES_PER_BLOCK + j) * SPI_FLASH_PAGE_SIZE,
flash_cache_table[i * PAGES_PER_BLOCK + j],
SPI_FLASH_PAGE_SIZE);
}
}
Expand Down Expand Up @@ -496,15 +497,14 @@ static bool external_flash_read_block(uint8_t *dest, uint32_t block) {

// Mask out the lower bits that designate the address within the sector.
uint32_t this_sector = address & (~(SPI_FLASH_ERASE_SIZE - 1));
uint8_t block_index = (address / FILESYSTEM_BLOCK_SIZE) % (SPI_FLASH_ERASE_SIZE / FILESYSTEM_BLOCK_SIZE);
uint8_t mask = 1 << (block_index);
size_t block_index = (address / FILESYSTEM_BLOCK_SIZE) % BLOCKS_PER_SECTOR;
uint32_t mask = 1 << (block_index);
// We're reading from the currently cached sector.
if (current_sector == this_sector && (mask & dirty_mask) > 0) {
if (flash_cache_table != NULL) {
uint8_t pages_per_block = FILESYSTEM_BLOCK_SIZE / SPI_FLASH_PAGE_SIZE;
for (int i = 0; i < pages_per_block; i++) {
for (int i = 0; i < PAGES_PER_BLOCK; i++) {
memcpy(dest + i * SPI_FLASH_PAGE_SIZE,
flash_cache_table[block_index * pages_per_block + i],
flash_cache_table[block_index * PAGES_PER_BLOCK + i],
SPI_FLASH_PAGE_SIZE);
}
return true;
Expand All @@ -527,8 +527,8 @@ static bool external_flash_write_block(const uint8_t *data, uint32_t block) {
wait_for_flash_ready();
// Mask out the lower bits that designate the address within the sector.
uint32_t this_sector = address & (~(SPI_FLASH_ERASE_SIZE - 1));
uint8_t block_index = (address / FILESYSTEM_BLOCK_SIZE) % (SPI_FLASH_ERASE_SIZE / FILESYSTEM_BLOCK_SIZE);
uint8_t mask = 1 << (block_index);
size_t block_index = (address / FILESYSTEM_BLOCK_SIZE) % BLOCKS_PER_SECTOR;
uint32_t mask = 1 << (block_index);
// Flush the cache if we're moving onto a sector or we're writing the
// same block again.
if (current_sector != this_sector || (mask & dirty_mask) > 0) {
Expand All @@ -550,9 +550,8 @@ static bool external_flash_write_block(const uint8_t *data, uint32_t block) {
dirty_mask |= mask;
// Copy the block to the appropriate cache.
if (flash_cache_table != NULL) {
uint8_t pages_per_block = FILESYSTEM_BLOCK_SIZE / SPI_FLASH_PAGE_SIZE;
for (int i = 0; i < pages_per_block; i++) {
memcpy(flash_cache_table[block_index * pages_per_block + i],
for (int i = 0; i < PAGES_PER_BLOCK; i++) {
memcpy(flash_cache_table[block_index * PAGES_PER_BLOCK + i],
data + i * SPI_FLASH_PAGE_SIZE,
SPI_FLASH_PAGE_SIZE);
}
Expand Down