Skip to content

OpenTitan: Flash fixes#2765

Merged
bors[bot] merged 4 commits into
tock:masterfrom
alistair23:alistair/opentitan-fixes
Aug 16, 2021
Merged

OpenTitan: Flash fixes#2765
bors[bot] merged 4 commits into
tock:masterfrom
alistair23:alistair/opentitan-fixes

Conversation

@alistair23

Copy link
Copy Markdown
Contributor

Pull Request Overview

There was a bug in the OpenTitan flash controller where the app flash at and above 0x2004_0000 was erased (set to all 1s).

The problem is that this was really hard to fix.

It turns out that the flash on the Nexys Video FPGA board is cut in half (lowRISC/opentitan#7730) so the address would wrap around.

This PR fixes the flash controller to not access the high addresses of flash to avoid the wrap around issue in hardware. This fixes all OT release tests.

Testing Strategy

Run the release tests and kernel unit tests.

TODO or Help Wanted

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make prepush.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
@github-actions github-actions Bot added the WG-OpenTitan In the purview of the OpenTitan working group. label Aug 13, 2021
@alistair23 alistair23 mentioned this pull request Aug 13, 2021
23 tasks
Comment thread capsules/src/tickv.rs
if self
.flash
.write_page((0x20040000 + address) / 64, data_buf)
.write_page((0x20060000 + address) / 64, data_buf)

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.

I know this isn't relevant to this PR, but why does the tickv capsule have a hardcoded address?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it needs to be removed. I only just noticed it as well. That should be a separate PR though

@hudson-ayers hudson-ayers left a comment

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.

Good that this fixes the tests for now, but I assume some of the changes in the lowrisc/ folder are different than what you would want if you had actual hardware instead of this FPGA board?

@bradjc bradjc added the release-blocker Issue or PR that must be resolved before the next release label Aug 16, 2021
@bradjc

bradjc commented Aug 16, 2021

Copy link
Copy Markdown
Contributor

bors r+

@bors

bors Bot commented Aug 16, 2021

Copy link
Copy Markdown
Contributor

@bors bors Bot merged commit 3c0d323 into tock:master Aug 16, 2021
@alistair23 alistair23 deleted the alistair/opentitan-fixes branch August 16, 2021 22:58
sirchnik pushed a commit to sirchnik/tock that referenced this pull request May 12, 2026
2765: OpenTitan: Flash fixes r=bradjc a=alistair23

### Pull Request Overview

There was a bug in the OpenTitan flash controller where the app flash at and above 0x2004_0000 was erased (set to all 1s).

The problem is that this was really hard to fix.

It turns out that the flash on the Nexys Video FPGA board is cut in half (lowRISC/opentitan#7730) so the address would wrap around.

This PR fixes the flash controller to not access the high addresses of flash to avoid the wrap around issue in hardware. This fixes all OT release tests.

### Testing Strategy

Run the release tests and kernel unit tests.

### TODO or Help Wanted

### Documentation Updated

- [X] Updated the relevant files in `/docs`, or no updates are required.

### Formatting

- [X] Ran `make prepush`.


Co-authored-by: Alistair Francis <alistair.francis@wdc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-blocker Issue or PR that must be resolved before the next release WG-OpenTitan In the purview of the OpenTitan working group.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants