-
Notifications
You must be signed in to change notification settings - Fork 159
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
Pmm support #452
Pmm support #452
Conversation
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.
Thank you for putting together this PR! I left a few small comments, but it looks good overall! I don't think it supports the full pointer masking standard yet, but it should be enough to run a large amount of real-world software or tests with pointer masking.
Missing features include, as far as I can tell:
- Some of the instructions in Section 2.6
- How MPRV and SPVP affect pointer masking
- 2-stage address translation (Section 3.5)
Thanks again for all the work on this!
(0b11) => transform_PA (vaddr,16), | ||
} | ||
}, | ||
(Sv48) => { |
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.
How will Sv39 be handled? Also, does the current code cover 2-stage address translation?
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.
At present, the sail model does not support Hyper Visor, and therefore, it does not cover the 2-stage address translation Yet. However, I am implementing the PM for Sv39 mode.
@@ -21,6 +21,7 @@ extern bool rv_enable_dirty_update; | |||
extern bool rv_enable_misaligned; | |||
extern bool rv_mtval_has_illegal_inst_bits; | |||
extern bool rv_enable_writable_fiom; | |||
extern bool rv_enable_pmm; |
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.
Nit: Here and throughout the PR, pmm is used to refer to pointer masking. I think "pm" or "zpm" would be a better naming (PMM stands for "pointer masking mode", which is only referring to the bits used to configure pointer masking).
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.
That’s a great suggestion. I will proceed with changing it to "zpm".
model/riscv_sys_regs.sail
Outdated
match cur_privilege { | ||
Machine => mseccfg.PMM(), | ||
Supervisor => menvcfg.PMM(), | ||
User => senvcfg.PMM(), |
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.
I think this is only the case if the system has all of these privilege modes. For example, if a system does not have a supervisor mode, then menvcfg controls the user mode pointer masking.
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.
I have updated the logic here as mentioned.
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.
Nit: Typo in the commit description (Atmoic -> Atomic)
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.
Noted
let XLEN = sizeof(xlen); | ||
let i = XLEN -(PMLEN +1); | ||
let effective_bit = effective_address[i..i]; | ||
let NVBITS = replicate_bits(effective_bit, PMLEN); |
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.
Nit: This naming isn't quite accurate – NVBITS/VBITS is independent of pointer masking. I think what these variables describe are masked_bits/unmasked_bits.
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.
I have modified the variable names as suggested.
NVBITS @ VBITS | ||
} | ||
|
||
function transform_effective_address(vaddr : xlenbits, pmm : bool) -> xlenbits = { |
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.
For misaligned addresses, will this be applied to each byte of the address or just the initial address? (the spec mandates the former.)
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.
Currently, the misaligned extension is not integrated into the Sail model. Therefore, I am applying Pointer Masking only to the initial address. However, once the integration is complete, we will need to apply Pointer Masking to each byte of the address.
Thank you, Martin, for reviewing this. I have responded to the issues you pointed out in the comments above. |
Actuall i have skipped the implementation of pointer masking on instructions which are not yet implemented in the sail model.The missing instructions are
|
|
Also, Martin, I wanted to inform you that the previous pull request was closed due to conflicts. Therefore, I've created a new pull request, which you can access here |
RISC-V Pointer Masking Extension Implementation
This PR features the implementation of the Pointer Masking Extension for the RISC-V architecture, enhancing the Base, Floating Point, Compressed, Vector and Atomics Instruction Set.
Changes
Implemented the
mseccfg
register with associated changes in riscv_sys_regs.sail ,riscv_insts_zicsr.sail ,riscv_sys_control.sail ,riscv_csr_map.sailUpdated the
leglize_envcfg
function and added functionsis_pmm_active
,legalize_mseccfg
andget_pmm
in riscv_sys_regs.sail.Introduced functions
transform_VA
andtransform_PA
in riscv_insts_base.sail.Implemented the function
transform_effective_address
. and Integratedtransform_effective_address
into the execution of LOAD and STORE functions of the targeted instruction files.Testing
The functionality has been verified with several tests. However, testing is still ongoing. Once completed, the repository link will be shared promptly.
The self-checking tests have not been written yet. However, work has commenced on them and they should be available shortly.