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

Fix XLEN assumption in riscv_instr_pkg #925

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dpetrisko
Copy link
Contributor

This assumption causes an unaligned store on 64b processor in some cases. This fix should preserve behavior on 32b processors but allow for aligned 64b store

@dpetrisko
Copy link
Contributor Author

Hi can someone take a look at this?

Copy link

@talhashahzad12345 talhashahzad12345 left a comment

Choose a reason for hiding this comment

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

The tests I ran using RV64imc, ran into this issue where ecall would go to mmode handler and there unaligned store would restart mmode handler where mmode handler has no direct handling of unaligned stores. This fix has worked for me and makes sense.

@il-steffen
Copy link

Looks like there are more instances of this:

diff --git a/src/riscv_directed_instr_lib.sv b/src/riscv_directed_instr_lib.sv
index f3bc708..cd0e2b9 100644
--- a/src/riscv_directed_instr_lib.sv
+++ b/src/riscv_directed_instr_lib.sv
@@ -372,7 +372,7 @@ class riscv_pop_stack_instr extends riscv_rand_instr_stream;
   function void init();
     reserved_rd = {cfg.ra};
     num_of_reg_to_save = saved_regs.size();
-    if(num_of_reg_to_save * 4 > stack_len) begin
+    if(num_of_reg_to_save * XLEN/8 > stack_len) begin
       `uvm_fatal(get_full_name(), $sformatf("stack len [%0d] is not enough to store %d regs",
                  stack_len, num_of_reg_to_save))
     end
diff --git a/src/riscv_instr_pkg.sv b/src/riscv_instr_pkg.sv
index 9620766..c305485 100644
--- a/src/riscv_instr_pkg.sv
+++ b/src/riscv_instr_pkg.sv
@@ -1380,7 +1380,7 @@ package riscv_instr_pkg;
     string store_instr = (XLEN == 32) ? "sw" : "sd";
     if (scratch inside {implemented_csr}) begin
       // Push USP from gpr.SP onto the kernel stack
-      instr.push_back($sformatf("addi x%0d, x%0d, -4", tp, tp));
+      instr.push_back($sformatf("addi x%0d, x%0d, -%0d", tp, tp, (XLEN/8)));
       instr.push_back($sformatf("%0s  x%0d, (x%0d)", store_instr, sp, tp));
       // Move KSP to gpr.SP
       instr.push_back($sformatf("add x%0d, x%0d, zero", sp, tp));
@@ -1435,7 +1435,7 @@ package riscv_instr_pkg;
       instr.push_back($sformatf("add x%0d, x%0d, zero", tp, sp));
       // Pop USP from the kernel stack, move back to gpr.SP
       instr.push_back($sformatf("%0s  x%0d, (x%0d)", load_instr, sp, tp));
-      instr.push_back($sformatf("addi x%0d, x%0d, 4", tp, tp));
+      instr.push_back($sformatf("addi x%0d, x%0d, %0d", tp, tp, (XLEN/8)));
     end
   endfunction

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.

None yet

3 participants