Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

the exception priority for atomic instructions #1841

Open
NewPaulWalker opened this issue Oct 21, 2024 · 0 comments
Open

the exception priority for atomic instructions #1841

NewPaulWalker opened this issue Oct 21, 2024 · 0 comments

Comments

@NewPaulWalker
Copy link

For AMO instructions, such as amoadd, we should prioritize checking the load/store address triggers because BreakPoint exception prior to load/store address misaligned or access fault according to the privileged spec.

However, in Spike, for AMO instructions, the store_slow_path function is called once with actually_store set to false before executing the load. This means that if the address is misaligned and also a trigger address, Spike will raise a Store/AMO address misaligned or Store/AMO access fault first, rather than a BreakPoint exception.
image
image
Similarly, for store conditional instructions, the check_load_reservation function is called first. For misaligned addresses, it calls the store_slow_path function with the actually_store parameter set to false. This results in the Store/AMO address misaligned or Store/AMO access fault being raised before the BreakPoint exception caused by store address trigger.
image

I think we can prioritize the address trigger check in the store_slow_path, like this:

--- a/riscv/mmu.cc
+++ b/riscv/mmu.cc
@@ -285,6 +285,7 @@ void mmu_t::store_slow_path(reg_t original_addr, reg_t len, const uint8_t* bytes
 {
   auto access_info = generate_access_info(original_addr, STORE, xlate_flags);
   reg_t transformed_addr = access_info.transformed_vaddr;
+  check_triggers(triggers::OPERATION_STORE, transformed_addr, access_info.effective_virt);
   if (actually_store) {
     reg_t trig_len = len;
     const uint8_t* trig_bytes = bytes;

This would allow the store conditional instruction to prioritize raising a BreakPoint exception for a misaligned trigger address, instead of a Store/AMO address misaligned or Store/AMO access fault.
Additionally, for AMO instructions, we also need to perform the load address trigger check before the load, like this:

--- a/riscv/mmu.h
+++ b/riscv/mmu.h
@@ -194,6 +194,11 @@ public:
   template<typename T>
   T amo_compare_and_swap(reg_t addr, T comp, T swap) {
     convert_load_traps_to_store_traps({
+      xlate_flags_t xlate_flags = {};
+      auto access_info = generate_access_info(addr, LOAD, xlate_flags);
+      reg_t transformed_addr = access_info.transformed_vaddr;
+      check_triggers(triggers::OPERATION_LOAD, transformed_addr, access_info.effective_virt);
+
       store_slow_path(addr, sizeof(T), nullptr, {}, false, true);
       auto lhs = load<T>(addr);
       if (lhs == comp)

This allows us to prioritize triggering the load/store address trigger for AMO instructions.

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

No branches or pull requests

1 participant