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

registers: pmpaddrx: increase addr size to u128 on 64bit platform. #12

Open
wants to merge 1 commit into
base: rivos/dev
Choose a base branch
from

Conversation

beezow
Copy link
Collaborator

@beezow beezow commented Nov 23, 2022

Even though the spec says that the top 10bits of pmpaddrx must be zero, qemu lets you set these to one, allowing a region up to 2^67 in size. We initially removed this feature from #11 during the review process, but I think we should bring it back to allow supporting this qemu configuration. (in particular the SBI sets up the pmps this way...)

Even though the spec says that the top 10bits must be zero, qemu lets
you set these to one, allowing a region upto 2^67 in size.
@FawazTirmizi
Copy link
Collaborator

I am hesitant to have this crate support features that don't conform to the specs (Assuming our interpretation is correct). This sounds like a bug in the SBI implementation and QEMU as opposed to something the crate should be dealing with. If fixing those issues is too cumbersome for now then merging this change is fine and we can revert it later, but otherwise I think we're barking up the wrong tree.

@furquan-rivos
Copy link

furquan-rivos commented Nov 23, 2022

I am hesitant to have this crate support features that don't conform to the specs (Assuming our interpretation is correct). This sounds like a bug in the SBI implementation and QEMU as opposed to something the crate should be dealing with. If fixing those issues is too cumbersome for now then merging this change is fine and we can revert it later, but otherwise I think we're barking up the wrong tree.

+1. This seems to be a problem with QEMU and SBI implementation. RV Priv spec has bits 63:54 marked as WARL (0). So, these bits can be written as 0 or 1, but should always read back as 0.

image

Can we add a check in this crate to mask out those bits and not use those? If they are set to 1, probably we can return an error or throw out an error message and mask out those bits and continue decoding.

@beezow
Copy link
Collaborator Author

beezow commented Nov 29, 2022

Fair points, I made this internal PR for qemu.

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