Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### 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.


### Added

### Changed
Expand Down
8 changes: 4 additions & 4 deletions src/device/cpu/riscv_rv32ima/cpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ void rv32_cpu_init(rv32_cpu_t *cpu, unsigned int procno)

cpu->priv_mode = rv_mmode;

cpu->pending_fetch_fault = false;

/* Breakpoints */
list_init(&cpu->bps);
}
Expand Down Expand Up @@ -725,10 +727,8 @@ static rv_exc_t execute(rv32_cpu_t *cpu)
rv_exc_t ex = rv_convert_addr(cpu, cpu->pc, &phys, false, true, true);

if (ex != rv_exc_none) {
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

cpu->pending_fetch_fault = true;
cpu->pending_fetch_fault_pc = cpu->pc;
return ex;
}

Expand Down
3 changes: 3 additions & 0 deletions src/device/cpu/riscv_rv32ima/cpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ typedef struct rv32_cpu {
/** breakpoints **/
list_t bps;

bool pending_fetch_fault;
uint32_t pending_fetch_fault_pc;

} rv32_cpu_t;

/** Basic CPU routines */
Expand Down
9 changes: 5 additions & 4 deletions src/device/cpu/riscv_rv64ima/cpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ void rv64_cpu_init(rv64_cpu_t *cpu, unsigned int procno)

rv64_tlb_init(&cpu->tlb, DEFAULT_RV64_TLB_SIZE);

cpu->pending_fetch_fault = false;

cpu->priv_mode = rv_mmode;
}

Expand Down Expand Up @@ -792,10 +794,9 @@ static rv_exc_t execute(rv64_cpu_t *cpu)
rv_exc_t ex = rv_convert_addr(cpu, cpu->pc, &phys, false, true, true);

if (ex != rv_exc_none) {
alert("Fetching from unconvertable address!");
// if (machine_trace) {
// rv64_idump(cpu, cpu->pc, (rv_instr_t) 0U);
// }
cpu->pending_fetch_fault = true;
cpu->pending_fetch_fault_pc = cpu->pc;

return ex;
}

Expand Down
3 changes: 3 additions & 0 deletions src/device/cpu/riscv_rv64ima/cpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ typedef struct rv64_cpu {
/** Translation Lookaside Buffer used for caching translated addresses */
rv64_tlb_t tlb;

bool pending_fetch_fault;
Comment thread
rosenbergm marked this conversation as resolved.
uint64_t pending_fetch_fault_pc;

} rv64_cpu_t;

/** Basic CPU routines */
Expand Down
30 changes: 30 additions & 0 deletions src/device/cpu/riscv_rv_ima/instructions/system.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,21 @@ static rv_exc_t rv_sret_instr(rv_cpu_t *cpu, rv_instr_t instr)
}

cpu->pc_next = cpu->csr.sepc;

if (cpu->pending_fetch_fault) {
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("Exception handler returned to 0x%" RV_PRIXLEN ", but code at that address is not mapped!", (uxlen_t) cpu->pending_fetch_fault_pc);

/* if (machine_trace) {
rv_idump(cpu, cpu->csr.sepc, (rv_instr_t) 0U);
} */
}
}
cpu->pending_fetch_fault = false;
}

return rv_exc_none;
}

Expand Down Expand Up @@ -169,6 +184,21 @@ static rv_exc_t rv_mret_instr(rv_cpu_t *cpu, rv_instr_t instr)
}

cpu->pc_next = cpu->csr.mepc;

if (cpu->pending_fetch_fault) {
if (cpu->csr.mepc == cpu->pending_fetch_fault_pc) {
ptr36_t phys;
if (rv_convert_addr(cpu, cpu->csr.mepc, &phys, false, true, false) != rv_exc_none) {
alert("Exception handler returned to 0x%" RV_PRIXLEN ", but code at that address is not mapped!", (uxlen_t) cpu->pending_fetch_fault_pc);

/* if (machine_trace) {
rv_idump(cpu, cpu->csr.mepc, (rv_instr_t) 0U);
} */
}
}
cpu->pending_fetch_fault = false;
}

return rv_exc_none;
}

Expand Down
2 changes: 2 additions & 0 deletions src/device/cpu/riscv_rv_ima/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@ typedef uint64_t virt_t;
typedef int64_t xlen_t;
typedef __int128 bigxlen_t;
typedef unsigned __int128 ubigxlen_t;
#define RV_PRIXLEN "016lx"
#elif XLEN == 32
typedef uint32_t uxlen_t;
typedef uint32_t virt_t;
typedef int32_t xlen_t;
typedef int64_t bigxlen_t;
typedef uint64_t ubigxlen_t;
#define RV_PRIXLEN "08x"
#else
#error "XLEN is not set to 32 or 64 bits"
#endif
Expand Down