Skip to content

Conversation

@sudo-apt-abdullah
Copy link
Contributor

@sudo-apt-abdullah sudo-apt-abdullah commented Aug 4, 2025

Closes #560

Copy link
Collaborator

@ThinkOpenly ThinkOpenly left a comment

Choose a reason for hiding this comment

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

just nits. Thanks for your efforts!

Copy link
Collaborator

@ThinkOpenly ThinkOpenly left a comment

Choose a reason for hiding this comment

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

I'm glad you like feedback, because I have a couple more nitpicks. :-)

@ThinkOpenly
Copy link
Collaborator

The validation code seems to not like this combo:

priv_mode: U
length: XLEN

...but I'm not smart enough to know how to resolve that. @dhower-qc ?

@dhower-qc
Copy link
Collaborator

dhower-qc commented Aug 5, 2025

The validation code seems to not like this combo:

priv_mode: U
length: XLEN

...but I'm not smart enough to know how to resolve that. @dhower-qc ?

This is just an unhandled case by the pretty printer; it doesn't know how to say that the location of ssp.VALUE changes with the current XLEN.

Currently, it assumes that if the location changes, it's based on a particular mode (e.g., the CSR width is MXLEN or SXLEN). It needs to be extended to handle when length is XLEN.

Fixed in commit aacade9

@dhower-qc dhower-qc closed this Aug 5, 2025
@dhower-qc
Copy link
Collaborator

oops, pressed the wrong button

@dhower-qc dhower-qc reopened this Aug 5, 2025
@sudo-apt-abdullah
Copy link
Contributor Author

Added the SSE field to the xenvcfg CSRs to fix CI failures caused by its absence.

@sudo-apt-abdullah sudo-apt-abdullah changed the title docs(ssp): shadow stack pointer - issue #560 docs(ssp): shadow stack pointer - Closes #560 Aug 8, 2025
@dhower-qc dhower-qc changed the title docs(ssp): shadow stack pointer - Closes #560 feat: shadow stack pointer - Closes #560 Aug 8, 2025
@dhower-qc
Copy link
Collaborator

Do you also want to try to implement the shadow stack instructions (by filling out the operation() field of instructions in spec/std/isa/inst/Zicfiss/*)?

You don't have to, but it might make sense since you already in here.

@sudo-apt-abdullah
Copy link
Contributor Author

sudo-apt-abdullah commented Aug 8, 2025

Do you also want to try to implement the shadow stack instructions (by filling out the operation() field of instructions in spec/std/isa/inst/Zicfiss/*)?

You don't have to, but it might make sense since you already in here.

I'll be happy doing this and open a separate PR for that if it's okay.

@ThinkOpenly ThinkOpenly changed the title feat: shadow stack pointer - Closes #560 feat: add ssp (shadow stack pointer) extension Aug 12, 2025
@ThinkOpenly
Copy link
Collaborator

Do you also want to try to implement the shadow stack instructions (by filling out the operation() field of instructions in spec/std/isa/inst/Zicfiss/*)?
You don't have to, but it might make sense since you already in here.

I'll be happy doing this and open a separate PR for that if it's okay.

Feel free to open a sub-issue for #560 (there's a "create sub-issue") button in the original text there, so we don't lose track.

@sudo-apt-abdullah
Copy link
Contributor Author

Feel free to open a sub-issue for #560 (there's a "create sub-issue") button in the original text there, so we don't lose track.

Please assign #995 to me, thanks.

@ThinkOpenly
Copy link
Collaborator

I think this PR is waiting on #891, right? Just adding this comment for context.

@sudo-apt-abdullah
Copy link
Contributor Author

I think this PR is waiting on #891, right?

Yes, it's waiting for #891 to be merged.

@ThinkOpenly ThinkOpenly changed the title feat: add ssp (shadow stack pointer) extension feat: add Zicfiss (shadow stack) CSR support Oct 15, 2025
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.

Add Zicfiss CSRs

3 participants