[sw] Change the encoding of `status_t` and remove the module IDs
- Dominant language
- SystemVerilog
- Stars
- 3.6k
- Forks
- 1.1k
- Avg merge
- 2d 22h
- Merged PRs (30d)
- 141
Description
### Description
We currently use the `status_t` type in a lot of code. This type [encodes](https://github.com/lowRISC/opentitan/blob/master/sw/device/lib/base/status.h#L26) the module ID and line number on errors. Unfortunately, since its introduction, t
he number of files in the codebase has grown considerably, leading to many conflicts in the module ID names. Besides, the encoding is not particular efficient in its present form since it uses 26 bits to encode a module + line number.
A proposal would be to change the encoding to make it more efficient and more precise. One possibility would be to reuse those 26 bits to store the PC (caveat, see below) where the error is handled. This presents certain advanatages and inconvenients:
- more precise: with the PC, we know exactly where the error occured
- more efficient: we don't even need 26-bits to store that, we could even have some spare bits (see below)
- less friendly: the PC cannot be read by a human easily, but with the proper tooling, it's easy to map the PC to the code from the ELF file
- breaking change: it's unclear from current documentation if the fact that we can reuse the line number to store a "value" is supported or not, and used or not in the codebase
**Prelude: memory map**
The `status_t` test is for tests only, it is never used by the ROM. The only exception to that is potentially the test_rom. This means that there really are only 2 (or 3) memory ranges of interesting:
- the ROM
- the SRAM
- the Flash
Out of these three, the flash is the biggest by far at the moment (1 MiB in Earlgrey). This means that we only need 20 bits to store any address in the flash (though this may change with integrated tops).
**Encoding (option 1)**: the most straighforward approach is to directly take the PC and store its lowest 26-bits in place of the module ID and line number. In doing so, we potentially loose the type (ROM, SRAM, Flash) or the memory so a smarter encoding would squeese those 26-bits in less (21 bits?). The downside of doing this is that we loose or reduce the size of the "line number" field. This field can be overwritten by software by doing e.g. `DATA_LOSS(value)`. It is unclear however if this feature is used a lot in the codebase, and we never guaranteed any storage size in this case (we currently allocate 11 bits).
**Encoding (option 2)**: a less straightforward but more efficient encoding would be to store an index in a table. This table would live in the ELF file but in a section which is loaded in memory not on the device. On error, the host code would need to look at the ELF to figure out the PC that it corresponds to. The main advantage of this approach is that it is *very* efficient: we only need `log2()` bits. If we reuse the 15 bits, that's more than enough. Another advantage of this approach is that (at the expense of burning some SRAM, but we have quite a lot of it in tests anyway) we could even load the table in the device memory and let the device code do the lookup itself.
Another advantage of this approach is that we currently [already have a table](https://github.com/lowRISC/opentitan/blob/master/sw/device/lib/base/status.h#L97) very close to the above in the ELF! Therefore it would just be a matter of tweaking the code to store the index/offset in the table.
Contributor guide
Research direction
Start with sw/device/lib/base/status.h, including the current status_t encoding and the existing ELF-related table. Search the codebase for status values such as DATA_LOSS and inspect test_rom usage to determine compatibility requirements. Done means agreeing on an encoding, updating its consumers and documentation, and covering the resulting behavior with tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- embedded-iot
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100