From 6bb23815c5d3ac0c852505e3a2d56532e7355a19 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 | 138 ++++++- .../tests/UNITTESTS/SFDP/test_sfdp.cpp | 338 +++++++++++++++++- 2 files changed, 464 insertions(+), 12 deletions(-) diff --git a/storage/blockdevice/source/SFDP.cpp b/storage/blockdevice/source/SFDP.cpp index e737057cc0..e523c3449c 100644 --- a/storage/blockdevice/source/SFDP.cpp +++ b/storage/blockdevice/source/SFDP.cpp @@ -245,6 +245,110 @@ int sfdp_parse_headers(Callback sfdp_reader, + sfdp_hdr_info &sfdp_info, + uint8_t *&descriptor, + const uint8_t *table_end, + uint8_t &config) +{ + config = 0; + + // If the table starts with a sector map descriptor instead of a configuration + // detection command descriptor, this device has only one configuration (i.e. is + // not configurable) with ID equal to 0. + if (is_sector_map_descriptor(descriptor)) { + return 0; + } + + // Loop through all configuration detection descriptors and run detection commands + while (!is_sector_map_descriptor(descriptor) && (descriptor + min_descriptor_size <= table_end)) { + uint8_t instruction = descriptor[1]; + uint8_t dummy_cycles = descriptor[2] & 0x0F; + auto addr_size = static_cast(descriptor[2] >> 6); + uint8_t mask = descriptor[3]; + uint32_t cmd_addr; + memcpy(&cmd_addr, &descriptor[4], sizeof(cmd_addr)); // last 32 bits of the descriptor + + uint8_t rx; + int 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 + config <<= 1; + + // The mask may apply to any bit of rx, so we can't directly combine + // (rx & mask) with config. Instead, treat (rx & mask) as a boolean. + if (rx & mask) { + config |= 0x01; + } + + if (is_last_descriptor(descriptor)) { + // We've processed the last configuration detection command descriptor + descriptor += min_descriptor_size; // Increment the descriptor for the caller + return 0; + } + descriptor += min_descriptor_size; // next descriptor + } + + tr_error("Sector Map: Incomplete configuration detection command descriptors"); + return -1; +} + +static int sfdp_locate_sector_map_by_config( + const uint8_t config, + sfdp_hdr_info &sfdp_info, + uint8_t *&descriptor, + const uint8_t *table_end) +{ + // The size of a sector map descriptor depends on the number of regions. Before + // the number of regions is calculated, use the minimum possible size in the a loop condition. + while (is_sector_map_descriptor(descriptor) && (descriptor + min_descriptor_size <= table_end)) { + size_t regions = descriptor[2] + 1; // Region ID starts at 0 + size_t current_descriptor_size = (1 /*header*/ + regions) * 4 /*DWORD size*/; + if (descriptor + current_descriptor_size > table_end) { + tr_error("Sector Map: Incomplete sector map descriptor at the end of the table"); + return -1; + } + + if (descriptor[1] == config) { + // matching sector map found + return 0; + } + + if (is_last_descriptor(descriptor)) { + // We've processed the last sector map descriptor + tr_error("Sector Map: Failed to find a sector map that matches the current configuration"); + return -1; + } + + descriptor += current_descriptor_size; // next descriptor + } + + tr_error("Sector Map: Incomplete sector map descriptors"); + return -1; +} + int sfdp_parse_sector_map_table(Callback sfdp_reader, sfdp_hdr_info &sfdp_info) { uint32_t tmp_region_size = 0; @@ -268,7 +372,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::unique_ptr(new (std::nothrow) uint8_t[sfdp_info.smptbl.size]); if (!smptbl_buff) { @@ -291,13 +395,26 @@ int sfdp_parse_sector_map_table(Callback 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, @@ -305,13 +422,13 @@ int sfdp_parse_sector_map_table(Callback> 8) & 0x00FFFFFF; // bits 9-32 + tmp_region_size = ((*((uint32_t *)&descriptor[(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] = descriptor[(idx + 1) * 4] & 0x0F; // bits 1-4 min_common_erase_type_bits &= sfdp_info.smptbl.region_erase_types_bitfld[idx]; @@ -335,7 +452,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,291 @@ 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]); +} + +/** + * When a Sector Map Parameter Table has multiple configuration detection + * commands and sector maps, but one of the detection commands returns + * an error (e.g. due to a bus fault). + */ +TEST_F(TestSFDP, TestConfigDetectCmdFailure) +{ + 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 (failed) + 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(-1)); // Emulate command failure + + // No further calls after failure + Expectation call_4 = EXPECT_CALL( + sfdp_reader_mock, + Call(_, _, _, _, _, _) + ).Times(0).After(call_3); + + 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, but no detection command is declared as the + * last command. + * Note: This means either reading went wrong, or the SFDP data is inconsistent + * possibly due to hardware manufactured with wrong data. When the latter happens in + * practice, the solution is to let the block device apply a device-specific quirk + * and supply "corrected" SFDP data in its callback. + */ +TEST_F(TestSFDP, TestConfigIncompleteDetectCommands) +{ + const uint8_t table_incomplete_detect_commands[] = { + // Detect 1 + 0xFC, 0x65, 0xFF, 0x08, + 0x04, 0x00, 0x00, 0x00, + + // Detect 2 + 0xFC, 0x65, 0xFF, 0x04, + 0x02, 0x00, 0x00, 0x00, + + // Detect 3 + // Removed to trigger a parsing error + + // Config 1 + 0xFE, 0x01, 0x02, 0xFF, // header + 0xF1, 0x7F, 0x00, 0x00, // region 0 + 0xF4, 0x7F, 0x03, 0x00, // region 1 + 0xF4, 0xFF, 0xFB, 0x03, // region 2 + + // No Config 2 + + // Config 3 + 0xFE, 0x03, 0x02, 0xFF, // header + 0xF4, 0xFF, 0xFB, 0x03, // region 0 + 0xF4, 0x7F, 0x03, 0x00, // region 1 + 0xF1, 0x7F, 0x00, 0x00, // region 2 + + // Config 4 + 0xFF, 0x05, 0x00, 0xFF, // header + 0xF4, 0xFF, 0xFF, 0x03 // region 0 + }; + + mbed::sfdp_hdr_info header_info; + set_sector_map_param_table( + header_info.smptbl, + table_incomplete_detect_commands, + sizeof(table_incomplete_detect_commands) + ); + + // 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(table_incomplete_detect_commands) + ) + ).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)); + + // No further calls - incomplete detect command + Expectation call_4 = EXPECT_CALL( + sfdp_reader_mock, + Call(_, _, _, _, _, _) + ).Times(0).After(call_3); + + 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, but no sector map matches the active + * configuration. + * Note: This means either detection went wrong, or the SFDP data is inconsistent + * possibly due to hardware manufactured with wrong data. When the latter happens in + * practice, the solution is to let the block device apply a device-specific quirk + * and supply "corrected" SFDP data in its callback. + */ +TEST_F(TestSFDP, TestConfigNoMatchingSectorMap) +{ + const uint8_t table_no_matching_sector_map[] = { + // Detect 1 + 0xFC, 0x65, 0xFF, 0x08, + 0x04, 0x00, 0x00, 0x00, + + // Detect 2 + 0xFC, 0x65, 0xFF, 0x04, + 0x02, 0x00, 0x00, 0x00, + + // Detect 3 + 0xFD, 0x65, 0xFF, 0x02, + 0x04, 0x00, 0x00, 0x00, + + // Config 1 + 0xFE, 0x01, 0x02, 0xFF, // header + 0xF1, 0x7F, 0x00, 0x00, // region 0 + 0xF4, 0x7F, 0x03, 0x00, // region 1 + 0xF4, 0xFF, 0xFB, 0x03, // region 2 + + // No Config 2 + + // Config 3 + // The active configuration (for test purpose) is 0x03 which should match header[1], + // but we change the latter to 0x02 to trigger a parsing error. + 0xFE, 0x02, 0x02, 0xFF, // header + 0xF4, 0xFF, 0xFB, 0x03, // region 0 + 0xF4, 0x7F, 0x03, 0x00, // region 1 + 0xF1, 0x7F, 0x00, 0x00, // region 2 + + // Config 4 + 0xFF, 0x05, 0x00, 0xFF, // header + 0xF4, 0xFF, 0xFF, 0x03 // region 0 + }; + + mbed::sfdp_hdr_info header_info; + set_sector_map_param_table( + header_info.smptbl, + table_no_matching_sector_map, + sizeof(table_no_matching_sector_map) + ); + + // 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(table_no_matching_sector_map) + ) + ).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)); + + // Failed to find a sector map for the active configuration. + EXPECT_EQ(-1, sfdp_parse_sector_map_table(sfdp_reader_callback, header_info)); +}