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

rust: Add RISCV CSR registers #1504

Merged
merged 1 commit into from
Dec 1, 2021

Conversation

Kritzefitz
Copy link
Contributor

The addition of these registers in the C base caused the rust values
for all floating point registers and the PC to point to some of the
CSR registers instead.

The addition of these registers in the C base caused the rust values
for all floating point registers and the PC to point to some of the
CSR registers instead.
F29 = 62,
F30 = 63,
F31 = 64,
PC = 65,
Copy link
Member

Choose a reason for hiding this comment

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

Why not add the variables after PC=65?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The conversion of RegisterRISCV in rust to uc_riscv_reg in C goes via an intermediate int. This means the conversion of PC should looks something like this:

RegisterRISCV::PC -> 190 -> UC_RISCV_REG_PC

However, this assumes that the mapping between RegisterRISCV and ints int rust (which is defined explicitly) matches that between uc_riscv_reg and ints in C (which is implicitly defined by the order of the enum). When the CSR registers were added to uc_riscv_reg the mapping in C became out of sync with the mapping in rust, so the conversion of PC from rust to C currently looks something like this:

RegisterRISCV::PC -> 65 -> UC_RISCV_REG_HPMCOUNTER21

As you see, the C side thinks we want to read HPMCOUNTER21, when we actually want to read the PC. This is obviously not good.

To fix this, we need to change the mappings in rust for all registers that have been shifted by the addition of the CSR registers in the C code.

As a side note: keeping the mappings between registers in C and rust synced by hand seems pretty error prone to me. Unfortunately I don't know a better alternative, because rust's FFI to my knowledge doesn't provide a mechanism to access definitions of enums in C.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry I realize it's my mistake here. The variables are in the right place.

@wtdcode wtdcode merged commit c190069 into unicorn-engine:dev Dec 1, 2021
@wtdcode
Copy link
Member

wtdcode commented Dec 1, 2021

Merged, thanks.

@Kritzefitz Kritzefitz deleted the rust-riscv-registers branch December 1, 2021 12:45
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