intel / intel/rohd-hcl

readOnly access (register and field level) blocks backdoor writes, contradicting documented behavior

Open
#297 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.