Pointer Masking SAIL

[jjscheel]

Technical Group

Pointer Masking TG

ratification-pkg

Pointer Masking

Technical Liaison

Martin Maas, Adam Zabrocki

Task Category

SAIL model

Task Sub Category

• gcc
• binutils
• gdb
• intrinsics
• Java
• KVM
• ld
• llvm
• Linux kernel
• QEMU
• Spike

Ratification Target

3Q2023

Statement of Work (SOW)

SOW: link

SOW Signoffs: (delete those not needed)

• Task group liaison sign-off date:
• Development partner sign-off date:

Waiver

• Freeze
• Ratification

Pull Request Details

Sail PR (closed): riscv/sail-riscv #452
Sail PR: riscv/sail-riscv #454

[jjscheel]

This work is not yet staffed.

[jjscheel]

@mustafa7755, @Muhammad-Saadi-Abbasi, it sounds like you guys will be working on this. I've assigned it to both of you not knowing if you're sharing or working on separate pieces. Feel free to unassign yourselves as-appropriate based on assignments.

[jjscheel]

If possible, I'd like an update in the April 25th meeting on when you think you'll have the following dates:

• Development of code complete (ready to test)
• Testing complete (ready to submit first PR)
• Last PR accepted (done)

Comments in the issue with these dates would be great.

[jjscheel]

BTW, I should note for this issue that it's marked "At Risk" due to the fact that we had no one assigned.

Given that we have assignees, I'd like their comfort with the SOW and have a basic plan before we move back to "As Planned" status. My hope is that we'll be able to do this in the April 25th meeting at the latest.

[UmerShahidengr]

Update Apr 25th, 2023 => Status changed to "As planned". @mustafa7755 and @Muhammad-Saadi-Abbasi have been assigned to this task, on-boarding is complete.
Next week task => Start of the developement phase

[mustafa7755]

If possible, I'd like an update in the April 25th meeting on when you think you'll have the following dates:

• Development of code complete (ready to test)
• Testing complete (ready to submit first PR)
• Last PR accepted (done)

Comments in the issue with these dates would be great.

Before giving any dates, there are some concerns:

• SOW is outdated(spec is changed a lot as compared to the initial proposal), we have created test plan in accordance with the new spec
• When is the architectural review of pointer masking spec be finalized? A lot of issues are open related to this, is there any ETA for this? For example, Opcodes and addresses of CSR's are missing from spec
• Spike and QeMU implementations are outdated, they are implemented wrt to the initial proposal. How we are going to compare our results with spike then?

Once these concerns are resolved, we can then give final dates.

[Adam-pi3]

@mustafa7755:

SOW is outdated

SOW is update - I modified it on 4/20/2023 but forgot to change the "Last modified Date". I've just updated it as well.

When is the architectural review of pointer masking spec be finalized?

The spec is very close to be final. We still modified minor details (like which section should include usage of PM). However, it should not affect the functionality of the current proposal.

Opcodes and addresses of CSR's are missing from spec

@martinmaas can help with that. We do have allocated temporary CSR addresses but for the older spec with more CSRs than we need today:
https://github.com/riscv/riscv-j-extension/blob/master/pointer-masking-proposal.adoc

How we are going to compare our results with spike then?

I'm starting the discussion to update QEMU implementation.

[UmerShahidengr]

Update May 7th, 2023 => Not much update on this task. Possibly it will get deferred to Q3, 2023 because of upcoming new work (debug native triggers)

[jjscheel]

@martinmaas, @Adam-pi3, you folks should be aware that given the challenges we've had with Code Size Reduction, I've got 1 resource to spend on either Pointer Masking and/or Debug. So, barring some explicit priority guidance, they will be started first-come, first serve. I'll be in touch with what and how we decide.

[mustafa7755]

I'm starting the discussion to update QEMU implementation.

@Adam-pi3, any update regarding this?

[UmerShahidengr]

Are we talking about QEMU implementation or Spike implementation?
Spike is compatible with riscof, so adding support for current specs in Spike will be ideal for us.

[jjscheel]

The discussion this week in the Pointer Masking TG was that @martinmaas would request the Architecture Review Committee to prioritize closure of the items preventing SAIL and ACT work in hopes of keeping this work moving forward.

@martinmaas, please provide an update when available.

[UmerShahidengr]

Update May 23rd, 2023 => Not much update on this task yet.

[jjscheel]

Understand. Thanks, @UmerShahidengr.

Have not seen update from AR about this. @martinmaas, any updates?

[jjscheel]

Moving to Blocked due to dependency on AR closure of content.

[UmerShahidengr]

Update ⇾ December 12th, 2023
@HAMZA-AFZAL404 have started working on it, specs have been understood, Sail implementation is in process.

[HAMZA-AFZAL404]

Update -> February 14th, 2024

Up to this point, I have successfully implemented pointer masking support for the Base, Floating Point, and Compressed Instructions. However, work on other instructions is still pending, and I will be addressing those aspects before proceeding further.

To ensure the correctness of the implementation, I am seeking a review from someone from Sail. Currently, as there is no dev branch in the official RISC-V Sail repository, I have initiated a pull request to my own forked repository. You can review the changes and progress in the pull request here.

If you want me to submit the pull request to a specific branch in the official repository for a comprehensive review, kindly provide guidance on the recommended approach.

Thank you for your attention and support.

Best regards,
Hamza

[jjscheel]

Thanks, @HAMZA-AFZAL404!

@billmcspadden-riscv, can you work with Hamza to provide some guidance on how to solicit review of his work and where to build his PR? Thanks!

[jjscheel]

@martinmaas, @Adam-pi3, Bill has been unavailable to help. Any chance on of you could work with Hamza to ensure that his implementation is correct for this first piece of work? Thanks!

[UmerShahidengr]

Update ⇾ March 5th, 2024
Stalled.

[jjscheel]

Please note that ARC has approved Pointer Masking and the Committee Chairs signoff has completed. It has thus Frozen and will be entering public review.

@martinmaas and @Adam-pi3, please try to help @HAMZA-AFZAL404 so that we can ensure this doesn't hold up Ratification.

[martinmaas]

I am, unfortunately, entirely unfamiliar with Sail. I can take a look and see whether I can spot anything based on my limited understanding, but I don't think this can replace a proper review by someone familiar with Sail.

[UmerShahidengr]

Update ⇾ April 2nd, 2024
@HAMZA-AFZAL404 has completed the Sail model, it will be released for review soon.

[jjscheel]

@martinmaas, there's an excellent Sail overview video listed on the wiki.riscv.org page in the "Learn" column ("Explore Sail"). That would likely be the best overview.

[jjscheel]

@UmerShahidengr, any PR for the Sail code yet? If so, please provide a link.

[UmerShahidengr]

@jjscheel we have not added the PR yet as @billmcspadden-riscv asked to update some self checking tests at riscv-tests for the Sail CI, we have been working on those self checking tests, will deliver the model once tests for CI will be ready.
@billmcspadden-riscv we have not found any Priv Arch tests in assembly in riscv-tests, all tests are fairly basic ones and are written in C. If there exists any assembly test related to CSR or Priv Arch, please point us out to those tests as sample.

[billmcspadden-riscv]

Go to: https://github.com/riscv-software-src/riscv-tests Then look under the isa/ directory. All tests under this directory are assembly language tests, although they all make use of various macros for test expansion (which can make reading the test a little more complicated). Tests in isa/rv32si/ and isa/rv64si/ all are run in supervisor mode. I hope this helps. Bill Mc.

…

On Tue, Apr 16, 2024 at 9:09 AM Umer Shahid ***@***.***> wrote: @jjscheel <https://github.com/jjscheel> we have not added the PR yet as @billmcspadden-riscv <https://github.com/billmcspadden-riscv> asked to update some self checking tests at riscv-tests for the Sail CI, we have been working on those self checking tests, will deliver the model once tests for CI will be ready. @billmcspadden-riscv <https://github.com/billmcspadden-riscv> we have not found any Priv Arch tests in assembly in riscv-tests, all tests are fairly basic ones and are written in C. If there exists any assembly test related to CSR or Priv Arch, please point us out to those tests as sample. — Reply to this email directly, view it on GitHub <#14 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AXROLOCX7QZKVLRO6ERQW5DY5UWIJAVCNFSM6AAAAAAV5YT5U2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJZGE4DQNBUGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Bill McSpadden Formal Verification Engineer RISC-V International mobile: 503-807-9309

[UmerShahidengr]

@jjscheel , @billmcspadden-riscv Pointer Masking PR has been added, here is the link to it.

[martinmaas]

Thanks a lot for the PR, and for all the work on this! I added a review at

riscv/sail-riscv#452 (review)

[UmerShahidengr]

Thanks alot @martinmaas for the review. @HAMZA-AFZAL404 will reply to your remarks soon. Just one more thing, this PR, which you have reviewed, was closed due to the conflicts issues in Sail repo. @HAMZA-AFZAL404 has removed those conflict, and added a new PR which is here.

[jjscheel]

@martinmaas and @HAMZA-AFZAL404, thanks for all of your work here!

@martinmaas, to resolve the Freeze waiver for Sail, we need a functionally complete PR for Sail (withing the bounds of what's implementable). If you'd be so kind as to acknowledge that in this issue, when we've achieved that goal, it will help me in the broader RISC-V process.

[martinmaas]

Thanks @HAMZA-AFZAL404 for all the work on this. I believe there is one more change, which is a modification to the spec that happened during public review:

MPRV and SPVP affect pointer masking as well, causing the pointer masking settings of the effective privilege mode to be applied. When MXR is in effect at the effective privilege mode where explicit memory access is performed, pointer masking does not apply.

https://github.com/riscv/riscv-j-extension/pull/74/files

(Based on the comment thread for the PR, I believe that the MPRV and SPVP portion is already handled, but the MXR exception will likely still need to be implemented.)

[UmerShahidengr]

Update June 11th, 2024:
The updated PR is available in Sail repo, @martinmaas please review this one and let us know what further changes are required

[jjscheel]

@billmcspadden-riscv, I'd appreciate your review too with an eye toward completeness for "Freeze", not full PR acceptance.

[UmerShahidengr]

Update June 25th, 2024:
The PR is yet to be reviewed.

[billmcspadden-riscv]

We talked about this at the tech-golden-model meeting on Monday. We scanned through the PR (PR #454). It's fairly large: 18 files and roughly 325 lines of new code. We need to identify an SME to review this. There are 1 or 2 others who expressed an interest in reviewing the PR (from a Sail perspective). Bill Mc.

…

On Tue, Jun 25, 2024 at 7:54 AM Umer Shahid ***@***.***> wrote: Update June 25th, 2024: The PR is yet to be reviewed. — Reply to this email directly, view it on GitHub <#14 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AXROLOHIK5SWE2AEAQDJCBTZJFSBDAVCNFSM6AAAAAAV5YT5U2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBYHA3DAOBUGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Bill McSpadden Formal Verification Engineer RISC-V International mobile: 503-807-9309

[UmerShahidengr]

Update July 23rd, 2024:
Thanks to @Timmmm for reviewing the PR. @HAMZA-AFZAL404 has been working on rebasing the PR (as more than 20 PRs have been merged in Sail since this PR, and the latest changes are conflicting with this one). Hamza will update the PR as per Tim's comments.

[jjscheel]

@martinmaas and @billmcspadden-riscv, the outstanding question at this time is whether the PR is functionally complete enough to consider the Freeze waiver for Sail resolved. Please share your thoughts.

[martinmaas]

I took a look and believe the PR is functionally complete enough to consider the Freeze waiver for Sail resolved.

[billmcspadden-riscv]

We have had 2 SMEs review PR #454 (Martin and Adam Zabrocki). Both have blessed
the PR. So the subject matter looks good.

Tim Hutt has reviewed the PR. There are 8 github conversations that need to be resolved,
mostly concerning coding style and structure. And there is a change request.

Also, there are code conflicts that must be resolved.

Bill Mc.

[jjscheel]

Thanks, @martinmaas, @Adam-pi3, and @billmcspadden-riscv, for your reviews and feedback. Given the positive feedback, I'm comfortable that we've met the Freeze Milestone for Sail.

Certainly, we're not done here from an SOW process, but we're making great progress.

[HAMZA-AFZAL404]

Thank you, @jjscheel, @martinmaas , @Adam-pi3, and @billmcspadden-riscv , for your feedback.

I've addressed all the comments on the PR. The requested changes have been implemented, and the code conflicts have been resolved. The PR has been updated accordingly.

Please review the latest changes, and let me know if anything else needs attention.

Hamza

[Adam-pi3]

LGTM