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

Added constraint for SXLEN and UXLEN #530

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

trdthg
Copy link
Contributor

@trdthg trdthg commented Aug 29, 2024

According to the spec:

  • Added the constraint that SXLEN≥UXLEN.
  • When MXLEN=64, if S-mode is not supported, then SXL is read-only zero.
  • When MXLEN=64, if U-mode is not supported, then UXL is read-only zero.

This PR adds assertion and some conditional statements to satisfy the above constraints.

@jrtc27
Copy link
Collaborator

jrtc27 commented Aug 29, 2024

I'm not sure what point this is when they're all hard-coded to the same value? If this is part of a larger series to do dynamic XLEN then sure, but that is a quite large undertaking.

0b01 => 32,
0b10 => 64,
0b11 => 128,
0b00 => 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tab not spaces

Copy link
Collaborator

Choose a reason for hiding this comment

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

And this should really be an abort, not silently give a junk result...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tab not spaces

sorry about that, fixed

And this should really be an abort, not silently give a junk result...

replace with Option(int)

@trdthg
Copy link
Contributor Author

trdthg commented Aug 29, 2024

I'm not sure what point this is when they're all hard-coded to the same value?

Agree, It's really of no use if you can make sure of it. The biggest use maybe to reassure beginners who don’t know much about model.

If this is part of a larger series to do dynamic XLEN then sure, but that is a quite large undertaking.

Not yet. But I discussed the issue of dynamic xlen with some people, about usage scenarios and requirements. Here are some conclusions:

  • Dynamic XLEN is mainly used for compatibility, such as having RV64 run RV32 programs.
  • One of implementation difficulty is that the rv32 rv64 shift instructions are conflicting from dec
  • The released K230 chip supports modifying sstatus.uxl, and the mainline kernel also supports it.
  • Technical issues outweigh requirements

Personally, I would do a little research on how to implement this in the model

@Alasdair
Copy link
Collaborator

Alasdair commented Aug 29, 2024

I have thought quite a bit about dynamic XLEN, and the biggest problem is the CSRs that change width.

Essentially what happens is as soon as you change the appropriate bits to change UXLEN or SXLEN, then any UXLEN or SXLEN register gets width-modulated down (or up) to the new size, according to the CSR width modulation algorithm in the spec, and what this means is a field specified in a register at UXLEN - N really does 'move' in some sense when UXLEN changes. The width-modulation algorithm (at least as I read it) is destructive and appears to allow producing illegal values in registers, so presumably some magic needs to happen to ensure that the width-modulated register remains legal.

Other architectures do the sensible thing of saying registers are exactly 64-bits and then defining the 32-bit view of the register when executing 32-bit code, or providing two registers for the lower and upper 32-bits (RISC-V does this for some registers).

There are also some CSRs that are defined as current XLEN-width, so presumably they need to be width-modulated every time you change modes that have a different XLEN?

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.

3 participants