ot_flash: Add program/erase, memory protection, multi-word ops, FIFO watermarks & fixes #137
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `mp_region_cfg` registers were missing an initial "Enable" shared field, which also caused the corresponding configuration feature enabling fields to all be at the incorrect offsets. Also now resets interrupt state so that the `prog_lvl` and `prog_empty` Program FIFO interrupts are set by default. Signed-off-by: Alex Jones <[email protected]>
The write to the `INIT` register needs to be stored so that it can be read out by software, so that software can poll this register to determine if controller initialization has already been requested. Signed-off-by: Alex Jones <[email protected]>
AlexJones0
force-pushed
the
eg_flash_ctrl
branch
2 times, most recently
from
84e62cb to
136082b
Compare
jwnrt
approved these changes
Feb 7, 2025
AlexJones0
force-pushed
the
eg_flash_ctrl
branch
from
136082b to
c5ecee9
Compare
rivos-eblot
requested changes
Feb 7, 2025
AlexJones0
changed the title
Implements functionality related to register writes that did not already exist, based on the register description in the specification. Also, a couple of changes to literals in the code for consistent styling. Signed-off-by: Alex Jones <[email protected]>
Previously, the INTR_STATE register was modelled as being entirely RW1C. This is not correct, as only the `corr_err` and `op_done` interrupts are RW1C. All other interrupts are R/O, and must be cleared by the hardware. Signed-off-by: Alex Jones <[email protected]>
The existing implementation sets the error code / address at the same time as completing the operation, in the same function. In practice, these functions need to be separate. Whilst operations with errors should still complete, you are able to error without completing. Example: a multi-word program (write) encounters an error, but the program FIFO has not been filled enough for it to have programmed all its data. At this point, the flash controller should display an error, but that the program transaction is not done. Signed-off-by: Alex Jones <[email protected]>
It is likely that many signals (and associated behavior) are missing at the moment. |
AlexJones0
force-pushed
the
eg_flash_ctrl
branch
from
c5ecee9 to
669d264
Compare
|
I believe my latest force push should have addressed all of the outstanding comments. The test results remain the same as the original post. Thanks for the reviews and detailed comments - I appreciate that this is quite a large PR to review. |
This commit implements some of the initial types & control register logic required for implementing the program & erase commands, which will be added later. Signed-off-by: Alex Jones <[email protected]>
AlexJones0
force-pushed
the
eg_flash_ctrl
branch
from
669d264 to
27464ba
Compare
Models the behaviour of OpenTitan's RTL: at the start of any (single- or multi-word) program transaction, two checks are performed. 1. Check that the start of the program beat is in the same program window as the end beat. This window boundary is typically, though not always, nicely aligned in memory. 2. Check that the selected type of program command is enabled. If either of these fail, the transaction is aborted before it starts. Signed-off-by: Alex Jones <[email protected]>
AlexJones0
force-pushed
the
eg_flash_ctrl
branch
from
27464ba to
6c5652d
Compare
jwnrt
approved these changes
Feb 7, 2025
In the existing flash, if you write to the `DIS` register to disable the flash, it completely removes the ability to read from or write to any of the CSRs. Referencing the documentation: https://opentitan.org/book/hw/top_earlgrey/ip_autogen/flash_ctrl/doc/theory_of_operation.html#flash-access-disable Upon disabling the flash, it should instead finish existing stateful operations, and then give a memory protection error for any further flash protocol controller initiated operations. That is; you should still be able to interact with the registers, but no new flash transactions can be started via the Flash Controller. The host-facing TL-UL adapter errors back all host initiated operations when disabled, so the disabling the relevant mem region is kept. Signed-off-by: Alex Jones <[email protected]>
This commit introduces the `prog_fifo` counterpart to the existing `rd_fifo`, and adds additional functionality relating to the two FIFOs, whilst fixing some existing issues. This includes properly resetting related status and interrupt state values when FIFOs are reset. Basic functionality for writing to the prog FIFO is added, though the corresponding program operation is not yet implemented. Watermarks have been added via the use of the FIFO_LVL register, and these are hooked up to the corresponding interrupts. Signed-off-by: Alex Jones <[email protected]>
This commit refactors `ot_flash_op_read` so that it can properly support multi-word reads. Two fields: `remaining`/`error` are introduced to the operation to count the number of processed/remaining words in the current transaction, and to track errors in multi-word transactions. These fields are used to allow us to retain the initial operation configuration and calculate the current address at any time. Under previous operation, if the read FIFO filled, the re-execution of the `ot_flash_op_read` function read from the initial `op.address`, thus reading repeated (garbage) data. This new logic fixes the issue and allows multi-word reads, whilst remaining generic for future operations. Calculation of the address has moved inside of the iteration, because this functionality is needed for the later addition of memory protection region functionality, and because it makes it easier to implement correct errors for individual operations in multi-word transactions. It also better decouples the transaction's memory accesses from our flash backend structure; it is possible for multi-word operations to occur over some boundary (e.g. read over a page boundary). Signed-off-by: Alex Jones <[email protected]>
Adds functionality that allows single/multi-word program (write) operations. This is similar to reading, but must also propagate changes back to our flash backend via the `blk_pwrite` API. The existing address calculation functionality from the flash read is re-used, as the flash controller transaction arbitration logic remains common across these operations. Upon encountering an error in a multi-word program transaction, the flash controller will continue to empty the program FIFO, but not perform any further writes. Writes before the error are valid. Signed-off-by: Alex Jones <[email protected]>
Adds flash erase operation functionality, which works at either page or bank granularities. Erase ops hence do not have lengths/counts, and the area to be erased is determine based on the address and partition selections. Successful erases are then propagated to the flash backend. Page erases use similar functionality to read/programs to calculate the page address and erase the page. Bank erases are more involved: they bypass regular memory protection mechanisms. This commit implements these bank protection mechanisms, as well as the logic for erasing just the data partition of the bank, or both data and info partitions. Signed-off-by: Alex Jones <[email protected]>
Adds the functionality for the flash controller's Memory Protection, for both info and data partitions. For access to info partitions, each page has a separate register `BANKX_INFOY_PAGE_CFG_Z` which controls the permissions for that specific page. For access to data partitions, there are a sequence of 10 `MP_REGION_X` and `MP_REGION_CFG_X` registers which define the size/bounds and permissions of 10 different regions of flash memory respectively. Data MP regions are defined by a base page and a size in base pages, used to calculate their relative bounds. Any data page accesses that do not fall under any of these 10 (enabled) regions will instead use the memory protection permissions of the default region, which is configured through the DEFAULT_REGION register. All MP configuration registers (except the DEFAULT_REGION) can be enabled/disabled. If enabled, they will match and their permissions apply. For data pages, if multiple regions match a given address, the region with the lowest matching index is applied. Implemented MP permissions include read/program/erase. This commit also introduces a `no-protection` property to allow the optional disabling of these MP checks if not needed. Although manual testing suggests low overheads, this avoids the overhead introduced by querying the ranges of all 10 regions for every data word read/progs. Signed-off-by: Alex Jones <[email protected]>
This commit adds a "partway" fix for erase suspend implementation, immediately clearing the request to simulate the response of "no erase ongoing", as currently QEMU flash erases are modelled as synchronous (i.e. the entire erase happens at once, and cannot be suspended). Also updates the author information in the file prologue, and introduces some additional documentation about the limitations of the current QEMU model, and a couple of relevant TODOs. Signed-off-by: Alex Jones <[email protected]>
AlexJones0
force-pushed
the
eg_flash_ctrl
branch
from
6c5652d to
7706195
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains a lot of changes - I recommend reviewing by commit. More details about each change are in the commit messages & comments. A list of remaining unimplemented features has been added to the documentation on the file itself.
This PR does most of the work to get Earlgrey's Flash Controller to a full implementation; the main missing features now are scrambling/ECC/ICVs and alerts. In summary, this PR:
This PR has been tested against
masteron OpenTitan via Bazel by overriding the QEMU binary:ot-earlgrey-9.2.0resultThe failure in
flash_ctrl_idle_low_power_testis because the reset reason is low power instead of POR; this is probably an issue in another block (rstmgr/pwrmgr etc.). I'm not sure where the failure inflash_ctrl_info_access_lc_devis. The test just fails to boot, so this might be an OTP issue or maybe this test needs additional flags/parameters that I'm not aware of.