readOnly access (register and field level) blocks backdoor writes, contradicting documented behavior
- Dominant language
- Dart
- Stars
- 115
- Forks
- 39
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 1
Description
`doc/components/csr.md` documents that register-level access rules
(`CsrAccess`) only apply to frontdoor accesses (this line has been present
since the original CSR implementation, commit dd22bf51e / #151):
> Note that access rules only apply to frontdoor accesses. I.e., a read
> only register is still backdoor writeable if the configuration
> indicates as such.
It further documents that backdoor writes exist specifically so hardware
can update a register directly, and that "a status register typically
must be backdoor writeable" (status registers are commonly
`CsrAccess.readOnly` from the frontdoor/software perspective).
However, `Csr.getWriteData()` (`lib/src/memory/csr/csr.dart`) has:
```dart
Logic getWriteData(Logic wd, {bool isBackdoorWrite = false}) {
// if the whole register is ready only, return the current value
if (access == CsrAccess.readOnly) {
return this;
}
...
```
This check has no `isBackdoorWrite` condition at all -- it unconditionally
discards *any* write, including backdoor writes, whenever the register's
`CsrAccess` is `readOnly`. As a result, a `CsrAccess.readOnly` register can
never be updated by anything, including the backdoor/hardware path,
directly contradicting the documented behavior and defeating the common
"read-only status register that hardware updates via backdoor" use case.
## Field-level readOnly should behave the same way
The same problem exists one level down for `CsrFieldAccess.readOnly`:
```dart
final chk2 = fields[currField].access == CsrFieldAccess.readOnly;
if (chk2) {
finalWd = finalWd.withSet(currIdx, elements[i]);
...
```
This also unconditionally discards writes to a `readOnly` field regardless
of `isBackdoorWrite`, meaning a `readOnly` *field* can never be updated
either -- even via the backdoor. Given the register-level intent (backdoor
writes should bypass frontdoor-only access restrictions so hardware can
drive status content), `CsrFieldAccess.readOnly` should follow the same
rule: only block frontdoor writes, and let backdoor writes pass through
unrestricted. (Today, `doc/components/csr.md` states the opposite -- that
"field access rules apply to both frontdoor and backdoor accesses of the
register" -- but that line should be corrected/updated as part of this
fix, since it conflicts with the register-level behavior documented for
`CsrAccess.readOnly` and the underlying "status register updated via
backdoor" use case.)
## Reproduction
Configure a `CsrInstanceConfig`/`CsrConfig` with `access:
CsrAccess.readOnly` (or a field with `CsrFieldAccess.readOnly`) and
`isBackdoorWritable: true`. Attempt a backdoor write via
`CsrBackdoorInterface.wrEn`/`wrData`. The register's (or field's) value
never changes, even though the configuration and documentation say it
should be backdoor writeable.
## Suggested fix
In `Csr.getWriteData()`:
- Only short-circuit to the current value for `CsrAccess.readOnly` (whole
register) when `isBackdoorWrite` is `false`.
- Only short-circuit to the current value for `CsrFieldAccess.readOnly`
(individual field) when `isBackdoorWrite` is `false`.
- In both cases, pass the write through unrestricted for backdoor writes.
This mirrors the `isBackdoorWrite` parameter recently added for
`CsrFieldAccess.writeOnesClear` in #3, and update
`doc/components/csr.md` accordingly to reflect that readOnly access (at
both register and field granularity) only restricts frontdoor accesses.
Contributor guide
Research direction
Start in lib/src/memory/csr/csr.dart at Csr.getWriteData() and trace the register- and field-level access checks, then reproduce the behavior through CsrBackdoorInterface.wrEn and wrData. Update doc/components/csr.md to match the implemented access rules, and verify that readOnly registers and fields configured as backdoor-writable accept backdoor writes while frontdoor writes remain restricted.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- dart
- Domain
- embedded-iot
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 74/100