Skip to content

NCSDK-32779: Improve SUIT cache performance #21650

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

Closed
wants to merge 4 commits into from
Closed

NCSDK-32779: Improve SUIT cache performance #21650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bc25089
suit: Add missing conditions
tomchy Apr 9, 2025
533f2a6
suit: Ensure that cache IPUCs have no copies
tomchy Apr 9, 2025
addfc90
suit: Optimize SUIT DFU cache initialization
tomchy Apr 8, 2025
1f61216
suit: Move to the new flash driver
tomchy Apr 8, 2025
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
37 changes: 35 additions & 2 deletions drivers/flash/flash_ipuc/flash_ipuc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
#ifdef CONFIG_FLASH_SIMULATOR
#include <zephyr/drivers/flash/flash_simulator.h>
#endif /* CONFIG_FLASH_SIMULATOR */
#ifdef CONFIG_SOC_FLASH_NRF_RADIO_SYNC_RPC_HOST
#include "soc_flash_nrf.h"
#endif /* CONFIG_SOC_FLASH_NRF_RADIO_SYNC_RPC_HOST */

/* __ALIGNED macro is not defined on NATIVE POSIX. This platform uses __aligned macro. */
#ifndef __ALIGNED
Expand Down Expand Up @@ -634,6 +636,35 @@ bool flash_component_ipuc_check(struct zcbor_string *component_id)
return false;
}

static struct device *replace_if_duplicate(struct device *ipuc_dev)
{
struct ipuc_context *ipuc_ctx = NULL;
struct ipuc_context *ctx = NULL;
struct device *dev = NULL;

ipuc_ctx = (struct ipuc_context *)ipuc_dev->data;

for (size_t i = 0; i < ARRAY_SIZE(ipuc_devs); i++) {
dev = (struct device *)ipuc_devs[i];
ctx = (struct ipuc_context *)dev->data;
if ((ctx->component_id.value == NULL) || (dev == ipuc_dev)) {
continue;
}

if ((ctx->component_id.len == ipuc_ctx->component_id.len) &&
(memcmp(ctx->component_id.value, ipuc_ctx->component_id.value,
ctx->component_id.len) == 0) &&
(ctx->read_access == ipuc_ctx->read_access)) {
LOG_DBG("Remove duplicate for IPUC (0x%lx, 0x%x)", ctx->address,
ctx->size);
flash_ipuc_release(ipuc_dev);
return dev;
}
}

return ipuc_dev;
}

static struct device *flash_cache_ipuc(uintptr_t min_address, uintptr_t *ipuc_address,
size_t *ipuc_size, bool dry_run)
{
Expand Down Expand Up @@ -722,8 +753,10 @@ static struct device *flash_cache_ipuc(uintptr_t min_address, uintptr_t *ipuc_ad
}

if (!dry_run) {
LOG_INF("Cache IPUC at idx %d for address range (0x%lx, 0x%x) created", i_max,
ctx->address, ctx->size);
dev = replace_if_duplicate(dev);
ctx = (struct ipuc_context *)dev->data;
LOG_INF("Cache IPUC for 0x%lx at idx %d for address range (0x%lx, 0x%x) created",
min_address, i_max, ctx->address, ctx->size);
}

ctx->setup_pending = true;
Expand Down
42 changes: 0 additions & 42 deletions samples/suit/flash_companion/boards/nrf54h20dk_nrf54h20_cpuapp.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,48 +53,6 @@
};
};

/* Legacy exmif driver overlay - currently needed for DFU from extmem */

/delete-node/ &exmif;

&global_peripherals {
exmif: spi@95000 {
compatible = "nordic,nrf-exmif-spi", "snps,designware-spi";
pinctrl-0 = <&exmif_default>;
pinctrl-names = "default";
#address-cells = <1>;
#size-cells = <0>;
reg = <0x95000 0x500 0x95500 0xb00>;
reg-names = "wrapper", "core";
interrupts = <149 NRF_DEFAULT_IRQ_PRIORITY>;
power-domains = <&gpd NRF_GPD_FAST_ACTIVE0>;
clock-frequency = <DT_FREQ_M(400)>;
fifo-depth = <32>;
max-xfer-size = <16>;
cs-gpios = <&gpio6 3 GPIO_ACTIVE_LOW>;
};
};

&exmif {
mx25uw63: mx25uw6345g@0 {
compatible = "jedec,spi-nor";
reg = <0>;
spi-max-frequency = <DT_FREQ_M(48)>;
jedec-id = [c2 84 37];
sfdp-bfp = [
e5 20 8a ff ff ff ff 03 00 ff 00 ff 00 ff 00 ff
ee ff ff ff ff ff 00 ff ff ff 00 ff 0c 20 10 d8
00 ff 00 ff 87 79 01 00 84 12 00 c4 cc 04 67 46
30 b0 30 b0 f4 bd d5 5c 00 00 00 ff 10 10 00 20
00 00 00 00 00 00 7c 23 48 00 00 00 00 00 88 88
];
size = <67108864>;
has-dpd;
t-enter-dpd = <10000>;
t-exit-dpd = <30000>;
};
};

&mx25uw63 {
status = "okay";
};
7 changes: 7 additions & 0 deletions samples/suit/flash_companion/prj.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ CONFIG_NUM_COOP_PRIORITIES=16
CONFIG_NUM_METAIRQ_PRIORITIES=0
CONFIG_SCHED_DUMB=y
CONFIG_WAITQ_DUMB=y
CONFIG_TICKLESS_KERNEL=n
CONFIG_SSF_DEVICE_INFO_SERVICE_ENABLED=n

# Enable peripherals needed by the flash companion firmware
CONFIG_GPIO=y
Expand Down Expand Up @@ -45,6 +47,7 @@ CONFIG_NCS_BOOT_BANNER=n
CONFIG_BOOT_DELAY=0

# Disable serial communication
CONFIG_SERIAL=n
CONFIG_CONSOLE=n
CONFIG_UART_CONSOLE=n
CONFIG_STDOUT_CONSOLE=n
Expand Down Expand Up @@ -78,3 +81,7 @@ CONFIG_CLOCK_CONTROL=n
# Set flash base address to zero to ensure that the executable
# code is placed in CPUAPP RAM by the linker.
CONFIG_FLASH_BASE_ADDRESS=0x0
CONFIG_COMMON_LIBC_MALLOC=n
CONFIG_COMMON_LIBC_CALLOC=n
CONFIG_COMMON_LIBC_REALLOCARRAY=n
CONFIG_PICOLIBC_IO_LONG_LONG=n
51 changes: 0 additions & 51 deletions samples/suit/smp_transfer/boards/nrf54h20dk_nrf54h20_cpuapp_extflash.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,57 +10,6 @@
};
};

/* Legacy exmif driver overlay - currently needed for DFU from extmem */

&exmif_default {
group1 {
psels = <NRF_PSEL(EXMIF_CK, 6, 0)>,
<NRF_PSEL(EXMIF_DQ0, 6, 7)>,
<NRF_PSEL(EXMIF_DQ1, 6, 5)>;
nordic,drive-mode = <NRF_DRIVE_H0H1>;
};
};

/delete-node/ &exmif;

&global_peripherals {
exmif: spi@95000 {
compatible = "nordic,nrf-exmif-spi", "snps,designware-spi";
pinctrl-0 = <&exmif_default>;
pinctrl-names = "default";
#address-cells = <1>;
#size-cells = <0>;
reg = <0x95000 0x500 0x95500 0xb00>;
reg-names = "wrapper", "core";
interrupts = <149 NRF_DEFAULT_IRQ_PRIORITY>;
power-domains = <&gpd NRF_GPD_FAST_ACTIVE0>;
clock-frequency = <DT_FREQ_M(400)>;
fifo-depth = <32>;
max-xfer-size = <16>;
cs-gpios = <&gpio6 3 GPIO_ACTIVE_LOW>;
};
};

&exmif {
mx25uw63: mx25uw6345g@0 {
compatible = "jedec,spi-nor";
reg = <0>;
spi-max-frequency = <DT_FREQ_M(48)>;
jedec-id = [c2 84 37];
sfdp-bfp = [
e5 20 8a ff ff ff ff 03 00 ff 00 ff 00 ff 00 ff
ee ff ff ff ff ff 00 ff ff ff 00 ff 0c 20 10 d8
00 ff 00 ff 87 79 01 00 84 12 00 c4 cc 04 67 46
30 b0 30 b0 f4 bd d5 5c 00 00 00 ff 10 10 00 20
00 00 00 00 00 00 7c 23 48 00 00 00 00 00 88 88
];
size = <67108864>;
has-dpd;
t-enter-dpd = <10000>;
t-exit-dpd = <30000>;
};
};

&mx25uw63 {
status = "okay";
partitions {
Expand Down
8 changes: 8 additions & 0 deletions subsys/suit/cache/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ config SUIT_CACHE_RW
depends on SUIT_UTILS
depends on !SUIT_PLATFORM_VARIANT_SDFW

config SUIT_CACHE_RW_READ_BUF_SIZE
int "Size of a stack-allocated buffer, used while checking DFU cache partition contents"
depends on SUIT_CACHE_RW
default 256
help
Increasing this size will speed up the SUIT DFU module initialization.
Reducing this size will allow to use smaller stack size.

config SUIT_CACHE_SDFW_IPUC
bool "Enable IPUC-based partition, capable of performing SDFW updates"
depends on SUIT_CACHE_RW
Expand Down
35 changes: 0 additions & 35 deletions subsys/suit/cache/src/suit_dfu_cache_helpers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,41 +190,6 @@ suit_plat_err_t suit_dfu_cache_partition_is_initialized(struct dfu_cache_pool *p
return SUIT_PLAT_ERR_INVAL;
}

suit_plat_err_t suit_dfu_cache_partition_is_empty(struct dfu_cache_pool *cache_pool)
{
uint8_t *address = cache_pool->address;
size_t remaining = cache_pool->size;
uint8_t buffer[128];
const size_t chunk_size = sizeof(buffer);
suit_plat_err_t ret = SUIT_PLAT_SUCCESS;

#ifdef CONFIG_FLASH_IPUC
if (is_cache_ipuc_uninitialized(cache_pool)) {
return SUIT_PLAT_SUCCESS;
}
#endif /* CONFIG_FLASH_IPUC */

while (remaining > 0) {
size_t read_size = MIN(chunk_size, remaining);

ret = suit_dfu_cache_memcpy(buffer, (uintptr_t)address, read_size);
if (ret != SUIT_PLAT_SUCCESS) {
break;
}

for (int i = 0; i < read_size; i++) {
if (buffer[i] != 0xFF) {
return SUIT_PLAT_ERR_NOMEM;
}
}

address += read_size;
remaining -= read_size;
}

return ret;
}

static bool find_free_address(struct dfu_cache_pool *cache_pool, zcbor_state_t *state,
const struct zcbor_string *uri, uintptr_t payload_offset,
size_t payload_size, void *ctx)
Expand Down
10 changes: 0 additions & 10 deletions subsys/suit/cache/src/suit_dfu_cache_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,6 @@ suit_plat_err_t suit_dfu_cache_partition_slot_foreach(struct dfu_cache_pool *cac
*/
suit_plat_err_t suit_dfu_cache_partition_is_initialized(struct dfu_cache_pool *cache_pool);

/**
* @brief Check if cache partition content is erased.
*
* @param cache_pool Pointer to the SUIT cache structure.
*
* @return SUIT_PLAT_SUCCESS in case of success, SUIT_PLAT_ERR_NOMEM if partition is not empty,
* otherwise an error code
*/
suit_plat_err_t suit_dfu_cache_partition_is_empty(struct dfu_cache_pool *cache_pool);

/**
* @brief Find a free slot in the cache partition.
*
Expand Down
97 changes: 80 additions & 17 deletions subsys/suit/cache/src/suit_dfu_cache_rw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,82 @@ static void erase_cache_partition(struct dfu_cache_partition_ext *partition)
}
#endif /* CONFIG_SUIT_CACHE_APP_IPUC_ID */

erase_on_sink(partition->address, partition->size);
(void)flash_erase(partition->fdev, partition->offset, partition->size);
}

#ifdef CONFIG_FLASH_IPUC
/* This function returns true if the given cache pool is an IPUC-based cache
* but is not initialized yet.
*/
static bool is_cache_ipuc_uninitialized(struct dfu_cache_pool *cache_pool)
{
uintptr_t ipuc_address;
size_t ipuc_size;
struct device *ipuc_dev = flash_ipuc_find((uintptr_t)cache_pool->address, cache_pool->size,
&ipuc_address, &ipuc_size);

if ((ipuc_dev != NULL) && (flash_ipuc_setup_pending(ipuc_dev))) {
return true;
}

return false;
}
#endif /* CONFIG_FLASH_IPUC */

static suit_plat_err_t suit_dfu_cache_partition_is_empty(struct dfu_cache_partition_ext *partition)
{
uint8_t buffer[CONFIG_SUIT_CACHE_RW_READ_BUF_SIZE];
const size_t chunk_size = sizeof(buffer);
suit_plat_err_t ret = SUIT_PLAT_SUCCESS;
size_t offset = partition->offset;
size_t remaining = partition->size;
#ifdef CONFIG_FLASH_IPUC
struct dfu_cache_pool cache_pool = {
.address = partition->address,
.size = partition->size,
};

#ifdef CONFIG_SUIT_CACHE_SDFW_IPUC_ID
if (partition->id == CONFIG_SUIT_CACHE_SDFW_IPUC_ID) {
if (is_cache_ipuc_uninitialized(&cache_pool)) {
return SUIT_PLAT_SUCCESS;
}
}
#else
(void)cache_pool;
#endif /* CONFIG_SUIT_CACHE_SDFW_IPUC_ID */

#ifdef CONFIG_SUIT_CACHE_APP_IPUC_ID
if (partition->id == CONFIG_SUIT_CACHE_APP_IPUC_ID) {
if (is_cache_ipuc_uninitialized(&cache_pool)) {
return SUIT_PLAT_SUCCESS;
}
}
#endif /* CONFIG_SUIT_CACHE_APP_IPUC_ID */
#endif /* CONFIG_FLASH_IPUC */

while (remaining > 0) {
size_t read_size = MIN(chunk_size, remaining);

/* Note: reading memory directly is allowed only if access to
* the external memory is not proxied through IPC.
*/
if (flash_read(partition->fdev, offset, buffer, read_size) != 0) {
ret = SUIT_PLAT_ERR_IO;
break;
}

for (int i = 0; i < read_size; i++) {
if (buffer[i] != 0xFF) {
return SUIT_PLAT_ERR_NOMEM;
}
}

offset += read_size;
remaining -= read_size;
}

return ret;
}

/**
Expand All @@ -260,7 +335,7 @@ static suit_plat_err_t cache_free_space_check(struct dfu_cache_partition_ext *pa

if ((part != NULL) && (slot != NULL)) {
part_tmp_address = (uintptr_t)part->address;
if (suit_dfu_cache_partition_is_empty(&cache_pool) != SUIT_PLAT_SUCCESS) {
if (suit_dfu_cache_partition_is_empty(part) != SUIT_PLAT_SUCCESS) {
ret = suit_dfu_cache_partition_find_free_space(
&cache_pool, &part_tmp_address, &needs_erase);
if (ret == SUIT_PLAT_ERR_CBOR_DECODING) {
Expand Down Expand Up @@ -450,13 +525,7 @@ static suit_plat_err_t cache_0_resize(bool drop_content)
(void *)partition->address, partition->size);

if (drop_content) {
struct dfu_cache_pool cache_pool = {
.address = partition->address,
.size = partition->size,
};

suit_plat_err_t err =
suit_dfu_cache_partition_is_empty(&cache_pool);
suit_plat_err_t err = suit_dfu_cache_partition_is_empty(partition);
if (err == SUIT_PLAT_ERR_NOMEM) {
LOG_INF("DFU Cache pool 0 is not empty... Erasing");
erase_on_sink(partition->address, partition->size);
Expand Down Expand Up @@ -502,7 +571,7 @@ suit_plat_err_t suit_dfu_cache_validate_content(void)
LOG_INF("DFU Cache pool, id: %d does not contain valid content",
partition->id);

err = suit_dfu_cache_partition_is_empty(&cache_pool);
err = suit_dfu_cache_partition_is_empty(partition);
if (err == SUIT_PLAT_ERR_NOMEM) {
LOG_INF("DFU Cache pool, id: %d is not empty... Erasing",
partition->id);
Expand All @@ -526,13 +595,7 @@ suit_plat_err_t suit_dfu_cache_drop_content(void)
struct dfu_cache_partition_ext *partition = &dfu_partitions_ext[i];

if (partition->address && partition->size) {

struct dfu_cache_pool cache_pool = {
.address = partition->address,
.size = partition->size,
};

suit_plat_err_t err = suit_dfu_cache_partition_is_empty(&cache_pool);
suit_plat_err_t err = suit_dfu_cache_partition_is_empty(partition);

if (err == SUIT_PLAT_ERR_NOMEM) {
LOG_INF("DFU Cache pool, id: %d is not empty... Dropping",
Expand Down
Loading
Loading