-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Conversation
} | ||
target_ulong prv = value & 0b11; | ||
bool v = (value & 0b100) != 0; | ||
switch (prv) { |
There was a problem hiding this comment.
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.
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 |
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. |
No worry. I appreciate your contributions anyway.
You could enable actions in your fork to reduce noise but it's okay to trial-and-error here anyway.
As said, the root cause I think is using |
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.
ed0d87c
to
90bdb39
Compare
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 The code that handles the other registers here also uses |
Thanks for your efforts and patience =). |
…#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.
…#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.
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.)