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

riscv: Expose privilege level as pseudo-register "priv" #1989

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

apparentlymart
Copy link
Contributor

Unlike some other architectures, RISC-V does not expose the current privilege mode in any architecturally-defined register. That is intentional to make it easier to implement virtualization in software, but a Unicorn caller operates outside of the emulated hart and so it can and should be able to observe and change the current privilege mode in order to properly emulate certain behaviors of a real CPU.

This PR therefore now exposes a new pseudo-register using the name "priv", defined to match the virtual register "priv" defined in the RISC-V Debug Specification, section 4.10.1. This design assumes that the Unicorn API is providing an abstraction similar to that of a debugger and so it's reasonable to take design cues from the Debug specification, rather than inventing something entirely Unicorn-specific.

The register read/write is currently implemented directly inside the Unicorn code because QEMU doesn't have explicit support for the CSRs from the debug specification. If a future QEMU release supports "dcsr" then this implementation could potentially change to wrap reading and writing that CSR and then projecting the "prv" and "v" bitfields into the correct locations for the virtual register, but for now I prioritize pragmatism since Unicorn previously offered no way to directly get/set the privilege mode from outside the emulated hart.

I've updated all of the bindings that already had the RISC-V register constants, but some of them do not currently have any defined and so I left them alone: vb6 and haskell. Of the remaining ones I only know how to test the rust bindings, and I've confirmed that this works when called from my own Rust-based Unicorn client that motivated this addition.

(I discovered a need for this while I was investigating over in #1988, but this is an orthogonal change and so submitted as a separate PR.)

}
target_ulong prv = value & 0b11;
bool v = (value & 0b100) != 0;
switch (prv) {
Copy link
Contributor Author

@apparentlymart apparentlymart Aug 27, 2024

Choose a reason for hiding this comment

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

The PRV_U, PRV_S, and PRV_M constants do happen to match this encoding and so I originally had this written to just accept values directly, but I decided to write this out more explicitly as a switch because it seemed unwise to expose the assumption that QEMU's priv field will always match the debug spec's encoding in future versions, and also unwise to expose the possibility of setting the reserved value 2 that might take on a different meaning in a future QEMU version.

@wtdcode
Copy link
Member

wtdcode commented Oct 30, 2024

Sorry for being late on this.

Your changes look good to me but it's failing CI. Looks like you need to fix mingw32? I think it's probably due to using target_ulong instead of uint32_t.

@apparentlymart
Copy link
Contributor Author

Thanks for the feedback!

I'm sorry I didn't notice those tests failing when I first submitted this; I think perhaps the checks were paused until a maintainer approved them?

Unfortunately my attention has now moved elsewhere so I won't be able to work on this immediately but I'll try to fix all of the tests soon. I don't have a Windows system to test with myself so I may need to rely on pushing commits here until all of the tests pass, and so I'm sorry in advance for any notification noise that might cause.

I also see some failures that are not for mingw32, so I guess I will see if fixing for Windows also fixes the others, or if I also need to do some other work to make all of the other platforms pass.

@wtdcode
Copy link
Member

wtdcode commented Oct 30, 2024

Unfortunately my attention has now moved elsewhere so I won't be able to work on this immediately

No worry. I appreciate your contributions anyway.

I don't have a Windows system to test with myself so I may need to rely on pushing commits here until all of the tests pass, and so I'm sorry in advance for any notification noise that might cause

You could enable actions in your fork to reduce noise but it's okay to trial-and-error here anyway.

I also see some failures that are not for mingw32, so I guess I will see if fixing for Windows also fixes the others, or if I also need to do some other work to make all of the other platforms pass.

As said, the root cause I think is using target_ulong which has a variable length on different platforms.

Unlike some other architectures, RISC-V does not expose the current
privilege mode in any architecturally-defined register. That is intentional
to make it easier to implement virtualization in software, but a Unicorn
caller operates outside of the emulated hart and so it can and should be
able to observe and change the current privilege mode in order to properly
emulate certain behaviors of a real CPU.

The current privilege level is therefore now exposed as a new
pseudo-register using the name "priv", which matches the name of the
virtual register used by RISC-V's debug extension to allow the debugger
to read and change the privilege mode while the hart is halted. Unicorn's
use of it is conceptually similar to a debugger.

The bit encoding of this register is the same as specified in RISC-V Debug
Specification v1.0-rc3 Section 4.10.1. It's defined as a "virtual"
register exposing a subset of fields from the dcsr register, although here
it's implemented directly inside the Unicorn code because QEMU doesn't
currently have explicit support for the CSRs from the debug specification.
If it supports "dcsr" in a future release then this implementation could
change to wrap reading and writing that CSR and then projecting the "prv"
and "v" bitfields into the correct locations for the virtual register.
@apparentlymart
Copy link
Contributor Author

Hello again, @wtdcode. Thank you for the additional help.

It seems that the main problem was that I forgot (again 🙄) that standard c99 does not allow a declaration immediately after a label, and so I was accidentally relying on a modern GCC extension that isn't available on all of the compilers used across all of the platforms in the GitHub Actions tests.

I have used an extra semicolon to introduce a null statement because that is the workaround I usually use for this situation in my own projects, but I know different projects have different preferences for this and so if you would prefer to resolve this a different way please tell me and I will do it.

There was also a problem with one of the x86-target unit tests, but rebasing on the latest dev branch fixed that so I guess I just forked at a time when there was an unrelated bug on the dev branch that would only reproduce on mingw32 for some reason.

The code that handles the other registers here also uses target_ulong and so it seems like that is the correct type to use in this situation. I think the various #ifdef TARGET_RISCV64 in the code (including a new one I added here) are handling the target_ulong size difference between riscv32 and riscv64. Please tell me if I misunderstand and it would be better to use uint32_t for this case instead.

@wtdcode wtdcode merged commit 7d8fe2a into unicorn-engine:dev Nov 11, 2024
36 checks passed
@wtdcode
Copy link
Member

wtdcode commented Nov 11, 2024

Thanks for your efforts and patience =).

Antelox pushed a commit to Antelox/unicorn that referenced this pull request Nov 14, 2024
…#1989)

Unlike some other architectures, RISC-V does not expose the current
privilege mode in any architecturally-defined register. That is intentional
to make it easier to implement virtualization in software, but a Unicorn
caller operates outside of the emulated hart and so it can and should be
able to observe and change the current privilege mode in order to properly
emulate certain behaviors of a real CPU.

The current privilege level is therefore now exposed as a new
pseudo-register using the name "priv", which matches the name of the
virtual register used by RISC-V's debug extension to allow the debugger
to read and change the privilege mode while the hart is halted. Unicorn's
use of it is conceptually similar to a debugger.

The bit encoding of this register is the same as specified in RISC-V Debug
Specification v1.0-rc3 Section 4.10.1. It's defined as a "virtual"
register exposing a subset of fields from the dcsr register, although here
it's implemented directly inside the Unicorn code because QEMU doesn't
currently have explicit support for the CSRs from the debug specification.
If it supports "dcsr" in a future release then this implementation could
change to wrap reading and writing that CSR and then projecting the "prv"
and "v" bitfields into the correct locations for the virtual register.
hyunmin-furiosa pushed a commit to hyunmin-furiosa/unicorn that referenced this pull request Jan 17, 2025
…#1989)

Unlike some other architectures, RISC-V does not expose the current
privilege mode in any architecturally-defined register. That is intentional
to make it easier to implement virtualization in software, but a Unicorn
caller operates outside of the emulated hart and so it can and should be
able to observe and change the current privilege mode in order to properly
emulate certain behaviors of a real CPU.

The current privilege level is therefore now exposed as a new
pseudo-register using the name "priv", which matches the name of the
virtual register used by RISC-V's debug extension to allow the debugger
to read and change the privilege mode while the hart is halted. Unicorn's
use of it is conceptually similar to a debugger.

The bit encoding of this register is the same as specified in RISC-V Debug
Specification v1.0-rc3 Section 4.10.1. It's defined as a "virtual"
register exposing a subset of fields from the dcsr register, although here
it's implemented directly inside the Unicorn code because QEMU doesn't
currently have explicit support for the CSRs from the debug specification.
If it supports "dcsr" in a future release then this implementation could
change to wrap reading and writing that CSR and then projecting the "prv"
and "v" bitfields into the correct locations for the virtual register.
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.

2 participants