From 9ce29ebd77548796e3ed982b421ea2bfc05d3886 Mon Sep 17 00:00:00 2001 From: Lingkai Dong Date: Mon, 9 Aug 2021 11:27:08 +0100 Subject: [PATCH] SFDP: Add support for multiple configurations and sector maps A Sector Map Parameter Table contains a sequence of the following descriptors: * (Optional) configuration detection command descriptors, one for each command to run to determine the current configuration. This exists only if the flash layout is configurable. * Sector map descriptors, one for each possible configuration. On a flash device with a non-configurable layout, there is only one such descriptor. Previously we only supported the non-configurable case with a single descriptor. This commit adds support for multiple configurations. --- storage/blockdevice/source/SFDP.cpp | 98 ++++++++++++-- .../tests/UNITTESTS/SFDP/test_sfdp.cpp | 120 +++++++++++++++++- 2 files changed, 209 insertions(+), 9 deletions(-) diff --git a/storage/blockdevice/source/SFDP.cpp b/storage/blockdevice/source/SFDP.cpp index 112a6a355053..0bac4fb52856 100644 --- a/storage/blockdevice/source/SFDP.cpp +++ b/storage/blockdevice/source/SFDP.cpp @@ -268,7 +268,7 @@ int sfdp_parse_sector_map_table(Callback the size of this table is variable + * are variable -> the size of this table is variable */ auto smptbl_buff = std::make_unique(sfdp_info.smptbl.size); if (!smptbl_buff) { @@ -291,13 +291,96 @@ int sfdp_parse_sector_map_table(Callback(descriptor[2] >> 6); + uint8_t mask = descriptor[3]; + uint32_t cmd_addr; + memcpy(&cmd_addr, &descriptor[4], sizeof(cmd_addr)); // bytes 4-7 of the descriptor + + uint8_t rx; + status = sfdp_reader(cmd_addr, addr_size, instruction, dummy_cycles, &rx, sizeof(rx)); + if (status < 0) { + tr_error("Sector Map: Configuration detection command failed"); + return -1; + } + + // Shift existing bits to the left, so we can add the newly detected bit + active_config_id <<= 1; + + // The mask may apply to any bit of rx, so we can't directly combine + // (rx & mask) with active_config_id. Instead, treat (rx & mask) as a boolean. + if (rx & mask) { + active_config_id |= 0x01; + } + + // Each command descriptor has a fixed size of two DWORDS. + if (descriptor[0] & 0x01) { // the current command descriptor is the last one + descriptor += min_descriptor_size; // next descriptor + break; + } + descriptor += min_descriptor_size; // next descriptor + } + } + + // Part 2: sector map descriptors. + // Find the descriptor that matches the active configuration, or use the only one + // if the flash is not configurable. + // Each sector map descriptor is two DWORDs or larger. Use two DOWRDs as a loop condition + // to avoid buffer overflow, then deduce the actual descriptor size from the descriptor header. + uint8_t *enabled_map = nullptr; + while (descriptor + min_descriptor_size <= table + sfdp_info.smptbl.size) { // table boundary + if (!(descriptor[0] & 0x02)) { // not a sector map descriptor + tr_error("Sector Map: Expecting a sector map descriptor"); + return -1; + } + + size_t regions = descriptor[2] + 1; // Region ID starts at 0 + size_t current_descriptor_size = (1 /*header*/ + regions) * 4 /*DWORD*/; + if (descriptor + current_descriptor_size > table + sfdp_info.smptbl.size) { + tr_error("Sector Map: Incomplete sector map descriptor at the end of the table"); + return -1; + } + + if (descriptor[1] == active_config_id) { + // matching sector map found + enabled_map = descriptor; + break; + } + + if (descriptor[0] & 0x01) { + // last command descriptor: do not loop any further + break; + } + + descriptor += current_descriptor_size; // next descriptor + } + + if (!enabled_map) { + tr_error("Sector Map: No sector map matches the current configuration"); return -1; } - sfdp_info.smptbl.region_cnt = smptbl_buff[2] + 1; + // Process the currently active enabled map + sfdp_info.smptbl.region_cnt = enabled_map[2] + 1; if (sfdp_info.smptbl.region_cnt > SFDP_SECTOR_MAP_MAX_REGIONS) { tr_error("Sector Map: Supporting up to %d regions, current setup to %d regions - fail", SFDP_SECTOR_MAP_MAX_REGIONS, @@ -308,10 +391,10 @@ int sfdp_parse_sector_map_table(Callback> 8) & 0x00FFFFFF; // bits 9-32 + tmp_region_size = ((*((uint32_t *)&enabled_map[(idx + 1) * 4])) >> 8) & 0x00FFFFFF; // bits 9-32 sfdp_info.smptbl.region_size[idx] = (tmp_region_size + 1) * 256; // Region size is 0 based multiple of 256 bytes; - sfdp_info.smptbl.region_erase_types_bitfld[idx] = smptbl_buff[(idx + 1) * 4] & 0x0F; // bits 1-4 + sfdp_info.smptbl.region_erase_types_bitfld[idx] = enabled_map[(idx + 1) * 4] & 0x0F; // bits 1-4 min_common_erase_type_bits &= sfdp_info.smptbl.region_erase_types_bitfld[idx]; @@ -335,7 +418,6 @@ int sfdp_parse_sector_map_table(Callback(buff); + switch (addr) { + case sector_map_start_addr: + memcpy(buff, sector_descriptors, sector_descriptors_size); + break; + case register_CR1NV: + out[0] = 0x04; + break; + case register_CR3NV: + out[0] = 0x02; + break; + } return 0; }; @@ -291,3 +339,73 @@ TEST_F(TestSFDP, TestMoreRegionsThanSupported) EXPECT_EQ(-1, sfdp_parse_sector_map_table(sfdp_reader_callback, header_info)); } + +/** + * When a Sector Map Parameter Table has multiple configuration detection + * commands and sector maps, sfdp_parse_sector_map_table() runs all commands + * to find the active configuration and selects the matching sector map. + */ +TEST_F(TestSFDP, TestMultipleSectorConfigs) +{ + mbed::sfdp_hdr_info header_info; + set_sector_map_param_table( + header_info.smptbl, + sector_map_multiple_descriptors, + sizeof(sector_map_multiple_descriptors) + ); + + // First call: get all detection command and sector map descriptors + Expectation call_1 = EXPECT_CALL( + sfdp_reader_mock, + Call( + sector_map_start_addr, + mbed::SFDP_READ_CMD_ADDR_TYPE, + mbed::SFDP_READ_CMD_INST, + mbed::SFDP_READ_CMD_DUMMY_CYCLES, + _, + sizeof(sector_map_multiple_descriptors) + ) + ).Times(1).WillOnce(Return(0)); + + // Second call: detect bit-0 of configuration + Expectation call_2 = EXPECT_CALL( + sfdp_reader_mock, + Call(register_CR3NV, mbed::SFDP_CMD_ADDR_SIZE_VARIABLE, 0x65, mbed::SFDP_CMD_DUMMY_CYCLES_VARIABLE, _, 1) + ).Times(1).After(call_1).WillOnce(Return(0)); + + // Third call: detect bit-1 of configuration + Expectation call_3 = EXPECT_CALL( + sfdp_reader_mock, + Call(register_CR1NV, mbed::SFDP_CMD_ADDR_SIZE_VARIABLE, 0x65, mbed::SFDP_CMD_DUMMY_CYCLES_VARIABLE, _, 1) + ).Times(1).After(call_2).WillOnce(Return(0)); + + // Fourth call: detect bit-2 of configuration + Expectation call_4 = EXPECT_CALL( + sfdp_reader_mock, + Call(register_CR3NV, mbed::SFDP_CMD_ADDR_SIZE_VARIABLE, 0x65, mbed::SFDP_CMD_DUMMY_CYCLES_VARIABLE, _, 1) + ).Times(1).After(call_3).WillOnce(Return(0)); + + EXPECT_EQ(0, sfdp_parse_sector_map_table(sfdp_reader_callback, header_info)); + + // Expecting sector map for Configuration ID = 0x03: + // Three regions + EXPECT_EQ(3, header_info.smptbl.region_cnt); + + // Region 0: erase type 3 (256KB erase) + // Range: first 64 MB minus 256 KB + EXPECT_EQ(64_MB - 256_KB, header_info.smptbl.region_size[0]); + EXPECT_EQ(64_MB - 256_KB - 1_B, header_info.smptbl.region_high_boundary[0]); + EXPECT_EQ(1 << (3 - 1), header_info.smptbl.region_erase_types_bitfld[0]); + + // Region 1: erase type 3 (256KB erase, which also covers 32KB from Region 2) + // Range: between Region 0 and Region 2 + EXPECT_EQ(256_KB - 32_KB, header_info.smptbl.region_size[1]); + EXPECT_EQ(64_MB - 32_KB - 1_B, header_info.smptbl.region_high_boundary[1]); + EXPECT_EQ(1 << (3 - 1), header_info.smptbl.region_erase_types_bitfld[1]); + + // Region 2: erase type 1 (4KB erase) + // Range: last 32 KB + EXPECT_EQ(32_KB, header_info.smptbl.region_size[2]); + EXPECT_EQ(64_MB - 1_B, header_info.smptbl.region_high_boundary[2]); + EXPECT_EQ(1 << (1 - 1), header_info.smptbl.region_erase_types_bitfld[2]); +}