PATCH: Don't need to set MSTATUS_UXL/SXL for RV32.

[mintow]

For RV32, mstatus will be set to 0x5 0000 0000 in processor_t::reset(). It's not reasonable. Refer to 3.1.8, Privileged Architecture Version 1.10: For RV32 systems, the SXL and UXL fields do not exist.

BTW, I found two lines of duplicate ‘MSTATUS_UXL’ code, so the patch deleted one.

--- a/riscv/processor.cc
+++ b/riscv/processor.cc
@@ -397,13 +397,13 @@ void processor_t::set_csr(int which, reg_t val)
       dirty |= (state.mstatus & MSTATUS_XS) == MSTATUS_XS;
       if (max_xlen == 32)
         state.mstatus = set_field(state.mstatus, MSTATUS32_SD, dirty);
-      else
+      else {
         state.mstatus = set_field(state.mstatus, MSTATUS64_SD, dirty);
+        state.mstatus = set_field(state.mstatus, MSTATUS_UXL, xlen_to_uxl(max_xlen));
+        state.mstatus = set_field(state.mstatus, MSTATUS_SXL, xlen_to_uxl(max_xlen));
+        // U-XLEN == S-XLEN == M-XLEN
+      }

-      state.mstatus = set_field(state.mstatus, MSTATUS_UXL, xlen_to_uxl(max_xlen));
-      state.mstatus = set_field(state.mstatus, MSTATUS_UXL, xlen_to_uxl(max_xlen));
-      state.mstatus = set_field(state.mstatus, MSTATUS_SXL, xlen_to_uxl(max_xlen));
-      // U-XLEN == S-XLEN == M-XLEN
       xlen = max_xlen;
       break;
     }

0001-Don-t-need-to-set-MSTATUS_UXL-SXL-for-RV32.txt

[aswaterman]

I don’t think this is actually a bug. When software attempts to read the CSR, the MSBs will be zapped, and the value written to rd will not contain UXL and SXL. This is because of the sext_xlen macro invocation: https://github.com/riscv/riscv-isa-sim/blob/master/riscv/insns/csrrc.h#L7

In other words, the state.mstatus implementation is partially microarchitectural, and as long as those fields can’t become architecturally visible, they don’t matter.

On Thu, May 23, 2019 at 5:08 AM Matteo notifications@github.com wrote:

For RV32, mstatus will be set to 0x5 0000 0000 in processor_t::reset(). It's not reasonable. Refer to 3.1.8, Privileged Architecture Version 1.10: For RV32 systems, the SXL and UXL fields do not exist.

And I found two lines of duplicate ‘MSTATUS_UXL’ code, the patch deleted one.

From 3abc8ac4f13ef0f86bb9f7ddb7155f427f55d107 Mon Sep 17 00:00:00 2001

From: matteo duanmintao@allwinnertech.com

Date: Thu, 23 May 2019 17:26:49 +0800

Subject: Don't need to set MSTATUS_UXL/SXL for RV32.

Refer to 3.1.8, Privileged Architecture Version 1.10: For RV32 systems, the SXL and UXL fields do not exist.

Signed-off-by: matteo duanmintao@allwinnertech.com

riscv/processor.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/riscv/processor.cc b/riscv/processor.cc index 189b7db..ee906ed 100644 --- a/riscv/processor.cc +++ b/riscv/processor.cc @@ -397,13 +397,13 @@ void processor_t::set_csr(int which, reg_t val) dirty |= (state.mstatus & MSTATUS_XS) == MSTATUS_XS; if (max_xlen == 32) state.mstatus = set_field(state.mstatus, MSTATUS32_SD, dirty);

• else

• else { state.mstatus = set_field(state.mstatus, MSTATUS64_SD, dirty);

• state.mstatus = set_field(state.mstatus, MSTATUS_UXL, xlen_to_uxl(max_xlen));

• state.mstatus = set_field(state.mstatus, MSTATUS_SXL, xlen_to_uxl(max_xlen));

• // U-XLEN == S-XLEN == M-XLEN

• }

• state.mstatus = set_field(state.mstatus, MSTATUS_UXL, xlen_to_uxl(max_xlen));

• state.mstatus = set_field(state.mstatus, MSTATUS_UXL, xlen_to_uxl(max_xlen));

• state.mstatus = set_field(state.mstatus, MSTATUS_SXL, xlen_to_uxl(max_xlen));

• // U-XLEN == S-XLEN == M-XLEN xlen = max_xlen; break; }

-- 2.17.1

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-isa-sim/issues/301?email_source=notifications&email_token=AAH3XQQUDCYHHDFDWSM3CDLPWZUK3A5CNFSM4HO5CLH2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GVNJRHA, or mute the thread https://github.com/notifications/unsubscribe-auth/AAH3XQS3NETXCABKRFA36R3PWZUK3ANCNFSM4HO5CLHQ .

[mintow]

@aswaterman Thank you for explaining！ Yes，it does not affect the function of spike. Maybe I can say that it is not logical enough. :)

[aswaterman]

That is a fair complaint. The code is seriously lacking in comments. Sorry about that.

[MDBrothers]

@aswaterman Why does line 504 duplicate line 503? CODE: state.mstatus = set_field(state.mstatus, MSTATUS_UXL, xlen_to_uxl(max_xlen));

[aswaterman]

Hmm, looks like I messed up. I'll fix that. Fortunately the duplicated statement has no effect.

[MDBrothers]

Thanks for taking a look at it.

On Fri, Aug 23, 2019, 6:31 PM Andrew Waterman notifications@github.com wrote:

Hmm, looks like I messed up. I'll fix that. Fortunately the duplicated statement has no effect.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-isa-sim/issues/301?email_source=notifications&email_token=ABIH5GPS5RKEK5TU5CDILY3QGBXO7A5CNFSM4HO5CLH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5BR46Y#issuecomment-524492411, or mute the thread https://github.com/notifications/unsubscribe-auth/ABIH5GO7JAVJAI3TGPCIOKDQGBXO7ANCNFSM4HO5CLHQ .