Skip to content

PC translation alerts show only when unhandled in simulated code#106

Open
rosenbergm wants to merge 2 commits into
masterfrom
fix/riscv-fetch-alert
Open

PC translation alerts show only when unhandled in simulated code#106
rosenbergm wants to merge 2 commits into
masterfrom
fix/riscv-fetch-alert

Conversation

@rosenbergm

Copy link
Copy Markdown
Collaborator

No description provided.

@rosenbergm rosenbergm marked this pull request as ready for review May 8, 2026 14:36
@rosenbergm rosenbergm requested a review from vhotspur May 8, 2026 14:36
alert("Fetching from unconvertable address!");
if (machine_trace) {
// rv32_idump(cpu, cpu->pc, (rv_instr_t) 0U);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the dump should remain there if we eventually decide to revive that support?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I added the commented code to system.c

Comment thread CHANGELOG.md

### Fixed

* PC translation alerts show only when unhandled in simulated code (see #106, @rosenbergm)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder: can we test this?

Or rather: when this really happens, it is game over, right? Because no one would be able to change the page tables, then?

Shouldn't we abort somehow? Maybe this would deserve a separate PR but it is probably something worth thinking about...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Or rather: when this really happens, it is game over, right? Because no one would be able to change the page tables, then?

Yes, this is IMO true. Right now (before this PR), the simulator is just stuck in a loop and prints out the error message (if not handled, that was the case we saw int the NSWI199 lab).

Shouldn't we abort somehow? Maybe this would deserve a separate PR but it is probably something worth thinking about...

Yeah, I guess aborting is the best we can do here.

Comment thread src/device/cpu/riscv_rv64ima/cpu.h
if (cpu->csr.sepc == cpu->pending_fetch_fault_pc) {
ptr36_t phys;
if (rv_convert_addr(cpu, cpu->csr.sepc, &phys, false, true, false) != rv_exc_none) {
alert("Fetching from unconvertable address 0x%" RV_PRIXLEN "!", (uxlen_t) cpu->pending_fetch_fault_pc);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know long messages are harder to read but telling the user "hey, there is no code for your exception handler" might be worth the effort to re-word the message?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I reworded the message.

From now on, alerts when translating PC fire only when there was no
meaningful handling of the exception.

Update changelog
@rosenbergm rosenbergm force-pushed the fix/riscv-fetch-alert branch from a6fc0ea to c0f7ad1 Compare May 20, 2026 11:25
@rosenbergm rosenbergm requested a review from vhotspur May 20, 2026 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants