diff --git a/include/zephyr/fs/zms.h b/include/zephyr/fs/zms.h index 8b776b426f9..436f448f6b4 100644 --- a/include/zephyr/fs/zms.h +++ b/include/zephyr/fs/zms.h @@ -77,6 +77,17 @@ struct zms_fs { * @{ */ +/** + * @brief ID type used in the ZMS API. + * + * @note The width of this type depends on @kconfig{CONFIG_ZMS_ID_64BIT}. + */ +#if CONFIG_ZMS_ID_64BIT +typedef uint64_t zms_id_t; +#else +typedef uint32_t zms_id_t; +#endif + /** * @brief Mount a ZMS file system onto the device specified in `fs`. * @@ -127,7 +138,7 @@ int zms_clear(struct zms_fs *fs); * @retval -EINVAL if `len` is invalid. * @retval -ENOSPC if no space is left on the device. */ -ssize_t zms_write(struct zms_fs *fs, uint32_t id, const void *data, size_t len); +ssize_t zms_write(struct zms_fs *fs, zms_id_t id, const void *data, size_t len); /** * @brief Delete an entry from the file system @@ -140,7 +151,7 @@ ssize_t zms_write(struct zms_fs *fs, uint32_t id, const void *data, size_t len); * @retval -ENXIO if there is a device error. * @retval -EIO if there is a memory read/write error. */ -int zms_delete(struct zms_fs *fs, uint32_t id); +int zms_delete(struct zms_fs *fs, zms_id_t id); /** * @brief Read an entry from the file system. @@ -158,7 +169,7 @@ int zms_delete(struct zms_fs *fs, uint32_t id); * @retval -EIO if there is a memory read/write error. * @retval -ENOENT if there is no entry with the given `id`. */ -ssize_t zms_read(struct zms_fs *fs, uint32_t id, void *data, size_t len); +ssize_t zms_read(struct zms_fs *fs, zms_id_t id, void *data, size_t len); /** * @brief Read a history entry from the file system. @@ -178,7 +189,7 @@ ssize_t zms_read(struct zms_fs *fs, uint32_t id, void *data, size_t len); * @retval -EIO if there is a memory read/write error. * @retval -ENOENT if there is no entry with the given `id` and history counter. */ -ssize_t zms_read_hist(struct zms_fs *fs, uint32_t id, void *data, size_t len, uint32_t cnt); +ssize_t zms_read_hist(struct zms_fs *fs, zms_id_t id, void *data, size_t len, uint32_t cnt); /** * @brief Gets the length of the data that is stored in an entry with a given `id` @@ -193,7 +204,7 @@ ssize_t zms_read_hist(struct zms_fs *fs, uint32_t id, void *data, size_t len, ui * @retval -EIO if there is a memory read/write error. * @retval -ENOENT if there is no entry with the given id and history counter. */ -ssize_t zms_get_data_length(struct zms_fs *fs, uint32_t id); +ssize_t zms_get_data_length(struct zms_fs *fs, zms_id_t id); /** * @brief Calculate the available free space in the file system. diff --git a/subsys/fs/zms/Kconfig b/subsys/fs/zms/Kconfig index f371d75647f..ad424608e0e 100644 --- a/subsys/fs/zms/Kconfig +++ b/subsys/fs/zms/Kconfig @@ -15,6 +15,15 @@ config ZMS if ZMS +config ZMS_ID_64BIT + bool "64 bit ZMS IDs" + help + When this option is true, the `zms_id_t` values passed to ZMS APIs will be 64 bit, + as opposed to the default 32 bit. + This option will also change the format of allocation table entries (ATEs) in memory + to accommodate larger IDs. Currently, this will make ZMS unable to mount an existing + file system if it has been initialized with a different ATE format. + config ZMS_LOOKUP_CACHE bool "ZMS lookup cache" help @@ -33,6 +42,7 @@ config ZMS_LOOKUP_CACHE_SIZE config ZMS_DATA_CRC bool "ZMS data CRC" + depends on !ZMS_ID_64BIT config ZMS_CUSTOMIZE_BLOCK_SIZE bool "Customize the size of the buffer used internally for reads and writes" diff --git a/subsys/fs/zms/zms.c b/subsys/fs/zms/zms.c index 95c2ef82c3b..86a4d469f90 100644 --- a/subsys/fs/zms/zms.c +++ b/subsys/fs/zms/zms.c @@ -33,10 +33,8 @@ static int zms_ate_valid_different_sector(struct zms_fs *fs, const struct zms_at #ifdef CONFIG_ZMS_LOOKUP_CACHE -static inline size_t zms_lookup_cache_pos(uint32_t id) +static inline size_t zms_lookup_cache_pos(zms_id_t id) { - uint32_t hash = id; - #ifdef CONFIG_ZMS_LOOKUP_CACHE_FOR_SETTINGS #ifdef CONFIG_SETTINGS_ZMS_LEGACY /* @@ -89,6 +87,7 @@ static inline size_t zms_lookup_cache_pos(uint32_t id) uint32_t key_value_bit; uint32_t key_value_hash; uint32_t key_value_ll; + uint32_t hash; key_value_bit = (id >> LOG2(ZMS_DATA_ID_OFFSET)) & 1; key_value_hash = (id & ZMS_HASH_MASK) >> (CONFIG_SETTINGS_ZMS_MAX_COLLISIONS_BITS + 1); @@ -96,8 +95,19 @@ static inline size_t zms_lookup_cache_pos(uint32_t id) hash = (key_value_hash << 2) | (key_value_bit << 1) | key_value_ll; #endif /* CONFIG_SETTINGS_ZMS_LEGACY */ +#elif defined(CONFIG_ZMS_ID_64BIT) + /* 64-bit integer hash function found by https://github.com/skeeto/hash-prospector. */ + uint64_t hash = id; + + hash ^= hash >> 32; + hash *= 0x42ab4abe4c475039ULL; + hash ^= hash >> 31; + hash *= 0xfa90c4424c537791ULL; + hash ^= hash >> 32; #else /* 32-bit integer hash function found by https://github.com/skeeto/hash-prospector. */ + uint32_t hash = id; + hash ^= hash >> 16; hash *= 0x7feb352dU; hash ^= hash >> 15; @@ -277,7 +287,7 @@ static int zms_flash_ate_wrt(struct zms_fs *fs, const struct zms_ate *entry) goto end; } #ifdef CONFIG_ZMS_LOOKUP_CACHE - /* 0xFFFFFFFF is a special-purpose identifier. Exclude it from the cache */ + /* ZMS_HEAD_ID is a special-purpose identifier. Exclude it from the cache */ if (entry->id != ZMS_HEAD_ID) { fs->lookup_cache[zms_lookup_cache_pos(entry->id)] = fs->ate_wra; } @@ -525,7 +535,7 @@ static bool zms_close_ate_valid(struct zms_fs *fs, const struct zms_ate *entry) /* zms_empty_ate_valid validates an sector empty ate. * A valid sector empty ate should be: * - a valid ate - * - with len = 0xffff and id = 0xffffffff + * - with len = 0xffff and id = ZMS_HEAD_ID * return true if valid, false otherwise */ static bool zms_empty_ate_valid(struct zms_fs *fs, const struct zms_ate *entry) @@ -538,7 +548,7 @@ static bool zms_empty_ate_valid(struct zms_fs *fs, const struct zms_ate *entry) * Valid gc_done_ate: * - valid ate * - len = 0 - * - id = 0xffffffff + * - id = ZMS_HEAD_ID * return true if valid, false otherwise */ static bool zms_gc_done_ate_valid(struct zms_fs *fs, const struct zms_ate *entry) @@ -574,7 +584,7 @@ static int zms_validate_closed_sector(struct zms_fs *fs, uint64_t addr, struct z } /* store an entry in flash */ -static int zms_flash_write_entry(struct zms_fs *fs, uint32_t id, const void *data, size_t len) +static int zms_flash_write_entry(struct zms_fs *fs, zms_id_t id, const void *data, size_t len) { int rc; struct zms_ate entry; @@ -587,13 +597,13 @@ static int zms_flash_write_entry(struct zms_fs *fs, uint32_t id, const void *dat entry.cycle_cnt = fs->sector_cycle; if (len > ZMS_DATA_IN_ATE_SIZE) { - /* only compute CRC if len is greater than 8 bytes */ - if (IS_ENABLED(CONFIG_ZMS_DATA_CRC)) { - entry.data_crc = crc32_ieee(data, len); - } +#ifdef CONFIG_ZMS_DATA_CRC + /* only compute CRC if data is to be stored outside of entry */ + entry.data_crc = crc32_ieee(data, len); +#endif entry.offset = (uint32_t)SECTOR_OFFSET(fs->data_wra); } else if ((len > 0) && (len <= ZMS_DATA_IN_ATE_SIZE)) { - /* Copy data into entry for small data ( < 8B) */ + /* Copy data into entry if data is sufficiently small */ memcpy(&entry.data, data, len); } @@ -726,10 +736,12 @@ static int zms_sector_close(struct zms_fs *fs) struct zms_ate close_ate; struct zms_ate garbage_ate; + /* Initialize all members to 0xff */ + memset(&close_ate, 0xff, sizeof(struct zms_ate)); + close_ate.id = ZMS_HEAD_ID; close_ate.len = 0U; close_ate.offset = (uint32_t)SECTOR_OFFSET(fs->ate_wra + fs->ate_size); - close_ate.metadata = 0xffffffff; close_ate.cycle_cnt = fs->sector_cycle; /* When we close the sector, we must write all non used ATE with @@ -778,11 +790,13 @@ static int zms_add_gc_done_ate(struct zms_fs *fs) { struct zms_ate gc_done_ate; + /* Initialize all members to 0xff */ + memset(&gc_done_ate, 0xff, sizeof(struct zms_ate)); + LOG_DBG("Adding gc done ate at %llx", fs->ate_wra); gc_done_ate.id = ZMS_HEAD_ID; gc_done_ate.len = 0U; gc_done_ate.offset = (uint32_t)SECTOR_OFFSET(fs->data_wra); - gc_done_ate.metadata = 0xffffffff; gc_done_ate.cycle_cnt = fs->sector_cycle; zms_ate_crc8_update(&gc_done_ate); @@ -831,12 +845,14 @@ static int zms_add_empty_ate(struct zms_fs *fs, uint64_t addr) int rc = 0; uint64_t previous_ate_wra; + /* Initialize all members to 0 */ + memset(&empty_ate, 0, sizeof(struct zms_ate)); + addr &= ADDR_SECT_MASK; LOG_DBG("Adding empty ate at %llx", (uint64_t)(addr + fs->sector_size - fs->ate_size)); empty_ate.id = ZMS_HEAD_ID; empty_ate.len = 0xffff; - empty_ate.offset = 0U; empty_ate.metadata = FIELD_PREP(ZMS_MAGIC_NUMBER_MASK, ZMS_MAGIC_NUMBER) | ZMS_DEFAULT_VERSION; @@ -931,7 +947,7 @@ static int zms_get_sector_header(struct zms_fs *fs, uint64_t addr, struct zms_at * @retval 1 valid ATE with same ID found * @retval < 0 An error happened */ -static int zms_find_ate_with_id(struct zms_fs *fs, uint32_t id, uint64_t start_addr, +static int zms_find_ate_with_id(struct zms_fs *fs, zms_id_t id, uint64_t start_addr, uint64_t end_addr, struct zms_ate *ate, uint64_t *ate_addr) { int rc; @@ -1082,10 +1098,10 @@ static int zms_gc(struct zms_fs *fs) */ if (wlk_prev_addr == gc_prev_addr) { /* copy needed */ - LOG_DBG("Moving %d, len %d", gc_ate.id, gc_ate.len); + LOG_DBG("Moving %lld, len %d", (long long)gc_ate.id, gc_ate.len); if (gc_ate.len > ZMS_DATA_IN_ATE_SIZE) { - /* Copy Data only when len > 8 + /* Only copy Data with large enough len * Otherwise, Data is already inside ATE */ data_addr = (gc_prev_addr & ADDR_SECT_MASK); @@ -1496,7 +1512,7 @@ int zms_mount(struct zms_fs *fs) return 0; } -ssize_t zms_write(struct zms_fs *fs, uint32_t id, const void *data, size_t len) +ssize_t zms_write(struct zms_fs *fs, zms_id_t id, const void *data, size_t len) { int rc; size_t data_size; @@ -1636,12 +1652,12 @@ ssize_t zms_write(struct zms_fs *fs, uint32_t id, const void *data, size_t len) return rc; } -int zms_delete(struct zms_fs *fs, uint32_t id) +int zms_delete(struct zms_fs *fs, zms_id_t id) { return zms_write(fs, id, NULL, 0); } -ssize_t zms_read_hist(struct zms_fs *fs, uint32_t id, void *data, size_t len, uint32_t cnt) +ssize_t zms_read_hist(struct zms_fs *fs, zms_id_t id, void *data, size_t len, uint32_t cnt) { int rc; int prev_found = 0; @@ -1738,7 +1754,7 @@ ssize_t zms_read_hist(struct zms_fs *fs, uint32_t id, void *data, size_t len, ui return rc; } -ssize_t zms_read(struct zms_fs *fs, uint32_t id, void *data, size_t len) +ssize_t zms_read(struct zms_fs *fs, zms_id_t id, void *data, size_t len) { int rc; @@ -1751,7 +1767,7 @@ ssize_t zms_read(struct zms_fs *fs, uint32_t id, void *data, size_t len) return MIN(rc, len); } -ssize_t zms_get_data_length(struct zms_fs *fs, uint32_t id) +ssize_t zms_get_data_length(struct zms_fs *fs, zms_id_t id) { int rc; diff --git a/subsys/fs/zms/zms_priv.h b/subsys/fs/zms/zms_priv.h index 84f77296f4f..97e202c2d53 100644 --- a/subsys/fs/zms/zms_priv.h +++ b/subsys/fs/zms/zms_priv.h @@ -28,22 +28,21 @@ #endif #define ZMS_LOOKUP_CACHE_NO_ADDR GENMASK64(63, 0) -#define ZMS_HEAD_ID GENMASK(31, 0) #define ZMS_VERSION_MASK GENMASK(7, 0) #define ZMS_GET_VERSION(x) FIELD_GET(ZMS_VERSION_MASK, x) #define ZMS_DEFAULT_VERSION 1 -#define ZMS_MAGIC_NUMBER 0x42 /* murmur3a hash of "ZMS" (MSB) */ #define ZMS_MAGIC_NUMBER_MASK GENMASK(15, 8) #define ZMS_GET_MAGIC_NUMBER(x) FIELD_GET(ZMS_MAGIC_NUMBER_MASK, x) #define ZMS_MIN_ATE_NUM 5 #define ZMS_INVALID_SECTOR_NUM -1 -#define ZMS_DATA_IN_ATE_SIZE 8 /** * @ingroup zms_data_structures * ZMS Allocation Table Entry (ATE) structure + * + * @note This structure depends on @kconfig{CONFIG_ZMS_ID_64BIT}. */ struct zms_ate { /** crc8 check of the entry */ @@ -52,6 +51,8 @@ struct zms_ate { uint8_t cycle_cnt; /** data len within sector */ uint16_t len; + +#if !defined(CONFIG_ZMS_ID_64BIT) /** data id */ uint32_t id; union { @@ -75,6 +76,31 @@ struct zms_ate { }; }; }; +#else + /** data id */ + uint64_t id; + union { + /** data field used to store small sized data */ + uint8_t data[4]; + /** data offset within sector */ + uint32_t offset; + /** Used to store metadata information such as storage version. */ + uint32_t metadata; + }; +#endif /* CONFIG_ZMS_ID_64BIT */ + } __packed; +#define ZMS_DATA_IN_ATE_SIZE SIZEOF_FIELD(struct zms_ate, data) + +#if !defined(CONFIG_ZMS_ID_64BIT) +#define ZMS_HEAD_ID GENMASK(31, 0) +#define ZMS_MAGIC_NUMBER 0x42 /* murmur3a hash of "ZMS" (MSB) */ + +#else +#define ZMS_HEAD_ID GENMASK64(63, 0) +#define ZMS_MAGIC_NUMBER 0xb8 /* murmur3a hash of "ZMS64" (MSB) */ + +#endif /* CONFIG_ZMS_ID_64BIT */ + #endif /* __ZMS_PRIV_H_ */ diff --git a/tests/subsys/fs/zms/src/main.c b/tests/subsys/fs/zms/src/main.c index 14d6291970c..8dc8cadba34 100644 --- a/tests/subsys/fs/zms/src/main.c +++ b/tests/subsys/fs/zms/src/main.c @@ -578,17 +578,18 @@ ZTEST_F(zms, test_zms_gc_corrupt_close_ate) int err; Z_TEST_SKIP_IFNDEF(CONFIG_FLASH_SIMULATOR_DOUBLE_WRITES); - close_ate.id = 0xffffffff; + memset(&close_ate, 0xff, sizeof(struct zms_ate)); + close_ate.id = ZMS_HEAD_ID; close_ate.offset = fixture->fs.sector_size - sizeof(struct zms_ate) * 5; close_ate.len = 0; - close_ate.metadata = 0xffffffff; close_ate.cycle_cnt = 1; close_ate.crc8 = 0xff; /* Incorrect crc8 */ - empty_ate.id = 0xffffffff; - empty_ate.offset = 0; + memset(&empty_ate, 0, sizeof(struct zms_ate)); + empty_ate.id = ZMS_HEAD_ID; empty_ate.len = 0xffff; - empty_ate.metadata = 0x4201; + empty_ate.metadata = + FIELD_PREP(ZMS_MAGIC_NUMBER_MASK, ZMS_MAGIC_NUMBER) | ZMS_DEFAULT_VERSION; empty_ate.cycle_cnt = 1; empty_ate.crc8 = crc8_ccitt(0xff, (uint8_t *)&empty_ate + SIZEOF_FIELD(struct zms_ate, crc8), @@ -649,7 +650,7 @@ ZTEST_F(zms, test_zms_gc_corrupt_ate) struct zms_ate close_ate; int err; - close_ate.id = 0xffffffff; + close_ate.id = ZMS_HEAD_ID; close_ate.offset = fixture->fs.sector_size / 2; close_ate.len = 0; close_ate.crc8 = @@ -752,6 +753,8 @@ ZTEST_F(zms, test_zms_cache_init) num = num_matching_cache_entries(ate_addr, false, &fixture->fs); zassert_equal(num, 1, "invalid cache entry after restart"); +#else + ztest_test_skip(); #endif } @@ -780,6 +783,8 @@ ZTEST_F(zms, test_zms_cache_collission) zassert_equal(err, sizeof(data), "zms_read call failure: %d", err); zassert_equal(data, id, "incorrect data read"); } +#else + ztest_test_skip(); #endif } @@ -829,6 +834,8 @@ ZTEST_F(zms, test_zms_cache_gc) num = num_matching_cache_entries(2ULL << ADDR_SECT_SHIFT, true, &fixture->fs); zassert_equal(num, 2, "invalid cache content after gc"); +#else + ztest_test_skip(); #endif } @@ -886,6 +893,49 @@ ZTEST_F(zms, test_zms_cache_hash_quality) TC_PRINT("Cache occupancy: %u\n", (unsigned int)num); zassert_between_inclusive(num, MIN_CACHE_OCCUPANCY, CONFIG_ZMS_LOOKUP_CACHE_SIZE, "too low cache occupancy - poor hash quality"); - +#else + ztest_test_skip(); #endif } + +/* + * Test 64 bit ZMS ID support. + */ +ZTEST_F(zms, test_zms_id_64bit) +{ + int err; + ssize_t len; + uint64_t data; + uint64_t filling_id = 0xdeadbeefULL; + + Z_TEST_SKIP_IFNDEF(CONFIG_ZMS_ID_64BIT); + + err = zms_mount(&fixture->fs); + zassert_true(err == 0, "zms_mount call failure: %d", err); + + /* Fill the first sector with writes of different IDs */ + + while (fixture->fs.data_wra + sizeof(data) + sizeof(struct zms_ate) <= + fixture->fs.ate_wra) { + data = filling_id; + len = zms_write(&fixture->fs, (zms_id_t)filling_id, &data, sizeof(data)); + zassert_true(len == sizeof(data), "zms_write failed: %d", len); + + /* Choose the next ID so that its lower 32 bits stay invariant. + * The purpose is to test that ZMS doesn't mistakenly cast the + * 64 bit ID to a 32 bit one somewhere. + */ + filling_id += BIT64(32); + } + + /* Read back the written entries and check that they're all unique */ + + for (uint64_t id = 0xdeadbeefULL; id < filling_id; id += BIT64(32)) { + len = zms_read_hist(&fixture->fs, (zms_id_t)id, &data, sizeof(data), 0); + zassert_true(len == sizeof(data), "zms_read_hist unexpected failure: %d", len); + zassert_equal(data, id, "read unexpected data: %llx instead of %llx", data, id); + + len = zms_read_hist(&fixture->fs, (zms_id_t)id, &data, sizeof(data), 1); + zassert_true(len == -ENOENT, "zms_read_hist unexpected failure: %d", len); + } +} diff --git a/tests/subsys/fs/zms/testcase.yaml b/tests/subsys/fs/zms/testcase.yaml index bdee4529f2a..18886b0e23d 100644 --- a/tests/subsys/fs/zms/testcase.yaml +++ b/tests/subsys/fs/zms/testcase.yaml @@ -8,21 +8,29 @@ tests: extra_args: DTC_OVERLAY_FILE=boards/qemu_x86_ev_0x00.overlay platform_allow: qemu_x86 filesystem.zms.sim.no_erase: - extra_args: CONFIG_FLASH_SIMULATOR_EXPLICIT_ERASE=n + extra_configs: + - CONFIG_FLASH_SIMULATOR_EXPLICIT_ERASE=n platform_allow: qemu_x86 filesystem.zms.sim.corrupt_close: - extra_args: + extra_configs: - CONFIG_FLASH_SIMULATOR_EXPLICIT_ERASE=y - CONFIG_FLASH_SIMULATOR_DOUBLE_WRITES=y platform_allow: qemu_x86 filesystem.zms.cache: - extra_args: + extra_configs: - CONFIG_ZMS_LOOKUP_CACHE=y - CONFIG_ZMS_LOOKUP_CACHE_SIZE=64 platform_allow: native_sim filesystem.zms.data_crc: - extra_args: + extra_configs: - CONFIG_ZMS_DATA_CRC=y platform_allow: - native_sim - qemu_x86 + filesystem.zms.id_64bit: + extra_configs: + - CONFIG_ZMS_ID_64BIT=y + - CONFIG_ZMS_LOOKUP_CACHE=y + - CONFIG_ZMS_LOOKUP_CACHE_SIZE=64 + - CONFIG_FLASH_SIMULATOR_DOUBLE_WRITES=y + platform_allow: qemu_x86