Skip to content

Support 128-bit flash programming units #524

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

Open
aidangarske wants to merge 3 commits into
base: main
Choose a base branch
from
+642 −133
Open

Support 128-bit flash programming units #524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
97dbbd7
Support 128-bit flash programming units
aidangarske Sep 18, 2026
91818d9
Refine configurable flash unit layout
aidangarske Sep 22, 2026
cb127cb
Remove unused flash unit accessor
aidangarske Sep 22, 2026
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
5 changes: 5 additions & 0 deletions .github/workflows/build-and-test-refactor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@ jobs:
- name: Build and test refactor NVM flash CRC ASAN
run: cd test-refactor/posix && make clean && make -j NVM_FLASH_CRC=1 ASAN=1 WOLFSSL_DIR=../../wolfssl && make run

# Build and test with 16-byte flash programming units
- name: Build and test refactor 16-byte NVM flash units
if: matrix.group == 'base'
run: cd test-refactor/posix && make clean && make -j FLASH_UNIT_SIZE=16 NVM_FLASH_CRC=1 ASAN=1 WOLFSSL_DIR=../../wolfssl && make run

# Build and test with LMS and XMSS both in verify-only mode
- name: Build and test refactor DMA ASAN LMS/XMSS verify-only
if: matrix.group == 'pq-dma'
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/build-and-test-whnvmtool.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,13 @@ jobs:
- name: Build and test NVM tool with NVM flash CRC ASAN
run: cd tools/whnvmtool && make clean && make check WOLFSSL_DIR=../../wolfssl NVM_FLASH_CRC=1 ASAN=1

- name: Build and test NVM tool with 16-byte flash units
run: cd tools/whnvmtool && make clean && make check WOLFSSL_DIR=../../wolfssl FLASH_UNIT_SIZE=16 NVM_FLASH_CRC=1 ASAN=1

# Build and test with DEBUG=1
- name: Build and test NVM tool with DEBUG
run: cd tools/whnvmtool && make clean && make check WOLFSSL_DIR=../../wolfssl DEBUG=1

# Build and test with DEBUG_VERBOSE=1 (includes DEBUG)
- name: Build and test NVM tool with DEBUG_VERBOSE
run: cd tools/whnvmtool && make clean && make check WOLFSSL_DIR=../../wolfssl DEBUG_VERBOSE=1

5 changes: 5 additions & 0 deletions .github/workflows/build-and-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@ jobs:
- name: Build and test NVM flash CRC ASAN
run: cd test && make clean && make -j NVM_FLASH_CRC=1 ASAN=1 WOLFSSL_DIR=../wolfssl && make run

# Build and test with 16-byte flash programming units
- name: Build and test 16-byte NVM flash units
if: matrix.group == 'base'
run: cd test && make clean && make -j FLASH_UNIT_SIZE=16 NVM_FLASH_CRC=1 ASAN=1 WOLFSSL_DIR=../wolfssl && make run

# Build and test with LMS and XMSS both in verify-only mode
- name: Build and test DMA ASAN LMS/XMSS verify-only
if: matrix.group == 'pq-dma'
Expand Down
147 changes: 101 additions & 46 deletions src/wh_nvm_flash.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,22 +43,32 @@ enum {
NF_COPY_OBJECT_BUFFER_LEN = 8 * WHFU_BYTES_PER_UNIT,
};

/* MSW of state variables (nfState) must be set to this pattern when written
* to flash to prevent hardware on certain chipsets from confusing zero values
* with erased flash */
static const whFlashUnit BASE_STATE = 0x1234567800000000ULL;
/* Preserve the existing uint64_t state layout on both endian orders. */
#if defined(BIG_ENDIAN_ORDER) || defined(__BIG_ENDIAN__) || \
(defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && \
(__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__))
#define WHFU_STATE_MAGIC(_unit) ((_unit).u32[0])
#define WHFU_STATE_VALUE(_unit) ((_unit).u32[1])
#define WHFU_STATE_MAGIC16(_unit) ((_unit).u16[0])
#define WHFU_STATE_CRC16(_unit) ((_unit).u16[1])
#else
#define WHFU_STATE_MAGIC(_unit) ((_unit).u32[1])
#define WHFU_STATE_VALUE(_unit) ((_unit).u32[0])
#define WHFU_STATE_MAGIC16(_unit) ((_unit).u16[3])
#define WHFU_STATE_CRC16(_unit) ((_unit).u16[2])
#endif

#define NF_STATE_MAGIC_VALUE 0x12345678U

#ifdef WOLFHSM_CFG_NVM_FLASH_CRC16
/* With CRC16 enabled, the object start and count state words carry a CRC in
* bits [47:32], replacing the low half of the magic:
* start word: [63:48]=0x1234 [47:32]=CRC16(metadata) [31:0]=start
* count word: [63:48]=0x1234 [47:32]=CRC16(data) [31:0]=count
* The epoch word and all partition state words keep the full BASE_STATE
* The epoch word and all partition state words keep the full state
* magic. The remaining 0x12/0x34 bytes still keep every state word distinct
* from erased flash. */
static const whFlashUnit CRC_BASE_STATE = 0x1234000000000000ULL;
#define NF_STATE_CRC_PACK(_crc) (((whFlashUnit)(uint16_t)(_crc)) << 32)
#define NF_STATE_CRC_EXTRACT(_unit) ((uint16_t)(((_unit) >> 32) & 0xFFFFULL))
#define NF_STATE_CRC_MAGIC_VALUE 0x1234U
#endif

/* On-flash layout of the state of an Object or Directory*/
Expand Down Expand Up @@ -164,6 +174,24 @@ static int nfIdList_Contains(whNvmId list_count, const whNvmId* id_list,
whNvmId id);


static void nfStateUnit_Set(whFlashUnit* unit, uint32_t value)
{
memset(unit, 0, sizeof(*unit));
WHFU_STATE_MAGIC(*unit) = NF_STATE_MAGIC_VALUE;
WHFU_STATE_VALUE(*unit) = value;
}

#ifdef WOLFHSM_CFG_NVM_FLASH_CRC16
static void nfStateUnit_SetCrc(whFlashUnit* unit, uint32_t value, uint16_t crc)
{
memset(unit, 0, sizeof(*unit));
WHFU_STATE_MAGIC16(*unit) = NF_STATE_CRC_MAGIC_VALUE;
WHFU_STATE_CRC16(*unit) = crc;
WHFU_STATE_VALUE(*unit) = value;
}
#endif


static int nfMemState_Read(whNvmFlashContext* context, uint32_t offset,
nfMemState* state)
{
Expand Down Expand Up @@ -221,12 +249,12 @@ static int nfMemState_Read(whNvmFlashContext* context, uint32_t offset,
return ret;
}

state->epoch = buffer.epoch;
state->start = buffer.start;
state->count = buffer.count;
state->epoch = WHFU_STATE_VALUE(buffer.epoch);
state->start = WHFU_STATE_VALUE(buffer.start);
state->count = WHFU_STATE_VALUE(buffer.count);
#ifdef WOLFHSM_CFG_NVM_FLASH_CRC16
state->crc_meta = NF_STATE_CRC_EXTRACT(buffer.start);
state->crc_data = NF_STATE_CRC_EXTRACT(buffer.count);
state->crc_meta = WHFU_STATE_CRC16(buffer.start);
state->crc_data = WHFU_STATE_CRC16(buffer.count);
#endif

/* Used */
Expand All @@ -242,10 +270,10 @@ static int nfMemState_Read(whNvmFlashContext* context, uint32_t offset,
return ret;
}

state->epoch = buffer.epoch;
state->start = buffer.start;
state->epoch = WHFU_STATE_VALUE(buffer.epoch);
state->start = WHFU_STATE_VALUE(buffer.start);
#ifdef WOLFHSM_CFG_NVM_FLASH_CRC16
state->crc_meta = NF_STATE_CRC_EXTRACT(buffer.start);
state->crc_meta = WHFU_STATE_CRC16(buffer.start);
#endif
state->status = NF_STATUS_DATA_BAD;
} else if (blank_epoch == WH_ERROR_NOTBLANK) {
Expand Down Expand Up @@ -441,12 +469,14 @@ static int nfPartition_ReadParseMemDirectory(whNvmFlashContext* context, int par
static int nfPartition_ProgramEpoch(whNvmFlashContext* context,
int partition, uint32_t epoch)
{
whFlashUnit unit = BASE_STATE | epoch;
whFlashUnit unit;

if ((context == NULL) || (context->cb == NULL)) {
return WH_ERROR_BADARGS;
}

nfStateUnit_Set(&unit, epoch);

return wh_FlashUnit_Program(
context->cb,
context->flash,
Expand All @@ -459,12 +489,14 @@ static int nfPartition_ProgramEpoch(whNvmFlashContext* context,
static int nfPartition_ProgramStart(whNvmFlashContext* context,
int partition, uint32_t start)
{
whFlashUnit unit = BASE_STATE | start;
whFlashUnit unit;

if ((context == NULL) || (context->cb == NULL)) {
return WH_ERROR_BADARGS;
}

nfStateUnit_Set(&unit, start);

return wh_FlashUnit_Program(
context->cb,
context->flash,
Expand All @@ -477,12 +509,14 @@ static int nfPartition_ProgramStart(whNvmFlashContext* context,
static int nfPartition_ProgramCount(whNvmFlashContext* context,
int partition, uint32_t count)
{
whFlashUnit unit = BASE_STATE | count;
whFlashUnit unit;

if ((context == NULL) || (context->cb == NULL)) {
return WH_ERROR_BADARGS;
}

nfStateUnit_Set(&unit, count);

return wh_FlashUnit_Program(
context->cb,
context->flash,
Expand Down Expand Up @@ -588,19 +622,21 @@ static int nfObject_ProgramBegin(whNvmFlashContext* context, int partition,
{
int rc = 0;
uint32_t object_offset = 0;
whFlashUnit state_epoch = BASE_STATE | epoch;
whFlashUnit state_start = BASE_STATE | start;
whFlashUnit state_epoch;
whFlashUnit state_start;

if ( (context == NULL) ||
(context->cb == NULL) ||
(meta == NULL)) {
return WH_ERROR_BADARGS;
}

nfStateUnit_Set(&state_epoch, epoch);

#ifdef WOLFHSM_CFG_NVM_FLASH_CRC16
/* Start word carries the metadata CRC in place of the low magic half */
state_start = CRC_BASE_STATE | NF_STATE_CRC_PACK(crc_meta) | start;
nfStateUnit_SetCrc(&state_start, start, crc_meta);
#else
nfStateUnit_Set(&state_start, start);
(void)crc_meta;
#endif

Expand Down Expand Up @@ -672,17 +708,16 @@ static int nfObject_ProgramFinish(whNvmFlashContext* context, int partition,
{
int rc;
uint32_t object_offset = 0;
whFlashUnit state_count = BASE_STATE | WHFU_BYTES2UNITS(byte_count);
whFlashUnit state_count;

if ((context == NULL) || (context->cb == NULL)) {
return WH_ERROR_BADARGS;
}

#ifdef WOLFHSM_CFG_NVM_FLASH_CRC16
/* Count word carries the data CRC in place of the low magic half */
state_count = CRC_BASE_STATE | NF_STATE_CRC_PACK(crc_data) |
WHFU_BYTES2UNITS(byte_count);
nfStateUnit_SetCrc(&state_count, WHFU_BYTES2UNITS(byte_count), crc_data);
#else
nfStateUnit_Set(&state_count, WHFU_BYTES2UNITS(byte_count));
(void)crc_data;
#endif

Expand Down Expand Up @@ -1012,13 +1047,12 @@ static int nfIdList_Contains(whNvmId list_count, const whNvmId* id_list,

int wh_NvmFlash_Init(void* c, const void* cf)
{
whNvmFlashContext* context = c;
const whNvmFlashConfig* config = cf;
int ret = WH_ERROR_OK;
whNvmFlashContext* context = c;
const whNvmFlashConfig* config = cf;
uint32_t partition_size = 0;
int ret = WH_ERROR_OK;

if ( (context == NULL) ||
(config == NULL) ||
(config->cb == NULL)) {
if ((context == NULL) || (config == NULL) || (config->cb == NULL)) {
return WH_ERROR_BADARGS;
}

Expand All @@ -1028,14 +1062,27 @@ int wh_NvmFlash_Init(void* c, const void* cf)
if (ret == WH_ERROR_OK) {
/* Initialize and setup context */
memset(context, 0, sizeof(*context));
context->cb = config->cb;
context->cb = config->cb;
context->flash = config->context;

/* Get partition size from flash device */
if (context->cb->PartitionSize != NULL) {
context->partition_units =
context->cb->PartitionSize(context->flash) /
WHFU_BYTES_PER_UNIT;
if (context->cb->PartitionSize == NULL) {
ret = WH_ERROR_BADARGS;
}
else {
partition_size = context->cb->PartitionSize(context->flash);
if (((partition_size % WHFU_BYTES_PER_UNIT) != 0) ||
((partition_size / WHFU_BYTES_PER_UNIT) <

@padelsbach padelsbach Sep 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use <= here to allow the full sized partition?

@aidangarske aidangarske Sep 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the current < comparison already allows equality. Changing it to <= would reject a partition exactly NF_PARTITION_DATA_OFFSET units long, so I left it unchanged.

NF_PARTITION_DATA_OFFSET)) {
ret = WH_ERROR_BADARGS;
}
else {
context->partition_units = partition_size / WHFU_BYTES_PER_UNIT;
}
}

if (ret != WH_ERROR_OK) {

@padelsbach padelsbach Sep 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition of this cleanup. But it seems to be missing from the failure paths below too. Could you move this to the bottom of the function to handle them all?

And then in the tests, add calls to wh_NvmFlash_Cleanup on cases where this function returns error.

@aidangarske aidangarske Sep 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I moved failed initialization cleanup to the common exit path and added cleanup-count checks after each error case in both test suites.

goto exit;
}

/* Unlock the both partitions */
Expand All @@ -1051,22 +1098,25 @@ int wh_NvmFlash_Init(void* c, const void* cf)
(void)nfPartition_ReadMemState(context, 1, &part_states[1]);

/* Decide which directory should be active */
if ( (part_states[0].status == NF_STATUS_USED) &&
(part_states[1].status != NF_STATUS_USED)) {
if ((part_states[0].status == NF_STATUS_USED) &&
(part_states[1].status != NF_STATUS_USED)) {
context->active = 0;
context->state = part_states[context->active];
} else if ( (part_states[0].status != NF_STATUS_USED) &&
(part_states[1].status == NF_STATUS_USED)) {
}
else if ((part_states[0].status != NF_STATUS_USED) &&
(part_states[1].status == NF_STATUS_USED)) {
context->active = 1;
context->state = part_states[context->active];
} else if ( (part_states[0].status == NF_STATUS_USED) &&
(part_states[1].status == NF_STATUS_USED)) {
}
else if ((part_states[0].status == NF_STATUS_USED) &&

@padelsbach padelsbach Sep 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The both-UNUSED case is not exercised with the new tests. Can you add it?

Also, the preexisting both corrupted case is not covered if you want to add that too

@aidangarske aidangarske Sep 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, added explicit coverage for both free/erased partitions and both corrupted partitions in the legacy and refactored suites, at both 8-byte and 16-byte unit sizes.

(part_states[1].status == NF_STATUS_USED)) {
/* Check which has larger epoch */
context->active =
(part_states[1].epoch > part_states[0].epoch);
context->state = part_states[context->active];
} else if ( (part_states[0].status == NF_STATUS_FREE) &&
(part_states[1].status == NF_STATUS_FREE)) {
}
else if ((part_states[0].status == NF_STATUS_FREE) &&
(part_states[1].status == NF_STATUS_FREE)) {
/* Both are blank. Set active to 0 and initialize */
context->active = 0;
ret = nfPartition_ProgramInit(context, context->active);
Expand All @@ -1089,6 +1139,11 @@ int wh_NvmFlash_Init(void* c, const void* cf)
}
}
}

exit:
if ((ret != WH_ERROR_OK) && (context->cb->Cleanup != NULL)) {
(void)context->cb->Cleanup(context->flash);
}
}
return ret;
}
Expand Down
4 changes: 4 additions & 0 deletions test-refactor/posix/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ ifeq ($(NVM_FLASH_CRC),1)
DEF += -DWOLFHSM_CFG_NVM_FLASH_CRC16
endif

ifneq ($(FLASH_UNIT_SIZE),)
DEF += -DWOLFHSM_CFG_FLASH_UNIT_SIZE=$(FLASH_UNIT_SIZE)
endif

# Build LMS/XMSS in verify-only mode (omits private-key, sign, and keygen
# paths). May be combined to exercise the mixed (one verify-only) case.
ifeq ($(LMS_VERIFY_ONLY),1)
Expand Down
Loading
Loading