Add callbacks and logging for page table walks

[Arielfoever]

Add callbacks for each step of the page table walk process, and an option to log them.

Fixes #1356

[github-actions]

Test Results

2 115 tests  ±0   2 115 ✅ ±0   20m 59s ⏱️ -9s
    1 suites ±0       0 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 48410f5. ± Comparison against base commit 2eafe6d.

♻️ This comment has been updated with latest results.

[Timmmm]

Looks pretty good. I think we probably don't want to convert things to strings though. Now that we use a generated header maybe we can use the actual generated types? zPrivilege and so on.

[arichardson]

Adding these makes a lot of sense. A few comments

[Arielfoever]

I think we need to take into meeting due to divergence from #1372

[Arielfoever]

We will add an option ptw-trace.

[jordancarlin]

We will add an option ptw-trace.

It should be --trace-ptw to be consistent with the other trace related flags.

[Arielfoever]

We will add an option ptw-trace.

It should be --trace-ptw to be consistent with the other trace related flags.

I really hate add a lot of options for trace. we should have some modern log system that support subscript.

Go #1204

[Arielfoever]

Looks pretty good. I think we probably don't want to convert things to strings though. Now that we use a generated header maybe we can use the actual generated types? zPrivilege and so on.

A little off topic - do we have a comparison table between C and sail and sail-riscv? like int in sail for sail-int for C.

[Timmmm]

do we have a comparison table between C and sail and sail-riscv? like int in sail for sail-int for C.

There are three options:

• fbits (alias for uint64_t) - when the type's size is statically known.
• sbits - when it's not statically known but is statically known to be <= 64 bits.
• lbits - if neither of the above applies.

[Timmmm]

Looks pretty good. It's a shame we don't get the full address in these callbacks, but maybe those should be separate callbacks (translation_start_callback etc.), in a future PR.

[Timmmm]

LGTM, thanks for this!

[arichardson]

Should we print the level as well? Otherwise LGTM.

[nadime15]

Instead of using int for level, couldnt this be a range(0, 4)? If so, would that be a different type than sail_int, allowing you to keep fprintf instead of switching to gmp_fprintf? Anyway, this is probably a minor detail and not really important.

Suggested change

	val ptw_start_callback = pure {c: "ptw_start_callback"} : (/* vpn */ bits(64), /* access type */ MemoryAccessType(ext_access_type), /* privilege */ Privilege) -> unit
	function ptw_start_callback(_) = ()
	val ptw_step_callback = pure {c: "ptw_step_callback"} : (/* level */ int, /* pte_addr */ physaddrbits, /* pte */ bits(64)) -> unit
	function ptw_step_callback(_) = ()
	val ptw_success_callback = pure {c: "ptw_success_callback"} : (/* final_ppn */ bits(64), /* level */ int) -> unit
	function ptw_success_callback(_) = ()
	val ptw_fail_callback = pure {c: "ptw_fail_callback"} : (/* error_type */ PTW_Error, /* level */ int, /* pte_addr */ physaddrbits) -> unit
	function ptw_fail_callback(_) = ()
	val ptw_start_callback = pure {c: "ptw_start_callback"} : (/* vpn */ bits(64), /* access type */ MemoryAccessType(ext_access_type), /* privilege */ Privilege) -> unit
	function ptw_start_callback(_) = ()
	val ptw_step_callback = pure {c: "ptw_step_callback"} : (/* level */ int, /* pte_addr */ physaddrbits, /* pte */ bits(64)) -> unit
	function ptw_step_callback(_) = ()
	val ptw_success_callback = pure {c: "ptw_success_callback"} : (/* final_ppn */ bits(64), /* level */ int) -> unit
	function ptw_success_callback(_) = ()
	val ptw_fail_callback = pure {c: "ptw_fail_callback"} : (/* error_type */ PTW_Error, /* level */ range(0, 4), /* pte_addr */ physaddrbits) -> unit
	function ptw_fail_callback(_) = ()

[arichardson]

Using range(0, 4) sounds good to me. Should we hold of on this until #1274 is merged?