Add mtopi CSR #347

KushalMeghani1644 · 2025-09-24T11:53:37Z

This PR adds support for the mtopi (Machine Top Priority Interrupt) CSR, which is part of the RISC-V Advanced Interrupt Architecture (AIA) extension.

Changes:

Add mtopi.rs register implementation at address 0x7C0
Export mtopi module in register.rs
Read-only CSR with proper bit field definitions:
- iid (Interrupt ID): bits 16-27
- iprio (Interrupt Priority ID): bits 0-7
Convenience methods for interrupt detection and priority handling

Features:

Memory-safe read-only access
Comprehensive documentation with usage examples
Full test coverage
Follows existing CSR implementation patterns

Testing:

All existing tests continue to pass
New tests cover bit field extraction and edge cases
Documentation examples compile successfully

AI Assistance:
Parts of the explanation and understanding of the mtopi CSR were assisted using an AI tool to summarize its behavior in the context of the RISC-V Advanced Interrupt Architecture (AIA). All code and implementation decisions were written and verified manually. NO CODES WERE GENERATED WITH THE HELP OF AI.

riscv/src/register/mtopi.rs

rmsyn

Thanks for your contribution!

AI Assistance:
Parts of the explanation and understanding of the mtopi CSR were assisted using an AI tool to summarize its behavior in the context of the RISC-V Advanced Interrupt Architecture (AIA). All code and implementation decisions were written and verified manually. NO CODES WERE GENERATED WITH THE HELP OF AI.

Thank you for explicitly stating your use of AI tool assistance. AFAIK, we currently do not have a policy on AI contributions. IMO, we should ban AI contributions, even for "just documentation/explanation". Such contributions have deep issues with plagiarism, and we cannot possibly verify the provenance of of the training data.

"Manually" rewriting suggestions from AI is not the same as manually implementing the changes. You're already contradicting yourself by saying that AI only helped by providing explanations/documentation, and that you manually rewrote code suggested by AI.

@romancardenas what are your thoughts on adding a section on AI to our CODE_OF_CONDUCT.md, or creating a CONTRIBUTING.md? After your recent back and forth on @KushalMeghani1644's contributions, how do you feel about their usage of AI?

KushalMeghani1644 · 2025-09-24T12:43:54Z

Hey @rmsyn thanks for putting time to my PR! I feel there is maybe a little misunderstanding upon the "AI DISCLOSURE" part, I have clearly stated in the PR desc that:

"Parts of the explanation and understanding of the mtopi CSR were assisted using an AI tool to summarize its behavior in the context of the RISC-V Advanced Interrupt Architecture (AIA). All code and implementation decisions were written and verified manually. NO CODES WERE GENERATED WITH THE HELP OF AI."

See the "parts" word! I meant that only a few little queries and all were asked from AI to understand how Mtopi actually works since I haven't studied much on this CSR, and the codes WERE NOT generated by AI, I had indeed went through the official docs of https://tools.cloudbear.ru/docs/riscv-aia-1.0-20230630.pdf to understand mtopi well. And also this line by you, ""Manually" rewriting suggestions from AI is not the same as manually implementing the changes. You're already contradicting yourself by saying that AI only helped by providing explanations/documentation, and that you manually rewrote code suggested by AI." I hadn't not manually rewritter what AI gave me, I stated in the "AI DISCLOSURE" section in capital words that, "NO CODES WERE GENERATED WITH THE HELP OF AI."

Edit: Also just to be transparent, my previous PRs that were merged had 0 use of AI in the codes! Those PRs were purely made by my brain + hands :D

Thank you :)

KushalMeghani1644 · 2025-09-24T13:00:51Z

Hey @rmsynI've updated the mtopi tests to use test helper macros as requested. Created a test_ro_csr_field! macro that follows the same pattern as test_csr_field! from macros.rs but adapted for read-only CSRs. The macro uses identical bit manipulation logic and replaces all the manual assertions. Thanks for the guidance!

rmsyn · 2025-09-24T13:15:53Z

Hey @rmsyn thanks for putting time to my PR! I feel there is maybe a little misunderstanding upon the "AI DISCLOSURE" part, I have clearly stated in the PR desc that:

"Parts of the explanation and understanding of the mtopi CSR were assisted using an AI tool to summarize its behavior in the context of the RISC-V Advanced Interrupt Architecture (AIA). All code and implementation decisions were written and verified manually. NO CODES WERE GENERATED WITH THE HELP OF AI."

See the "parts" word! I meant that only a few little queries and all were asked from AI to understand how Mtopi actually works since I haven't studied much on this CSR, and the codes WERE NOT generated by AI, I had indeed went through the official docs of https://tools.cloudbear.ru/docs/riscv-aia-1.0-20230630.pdf to understand mtopi well. And also this line by you, ""Manually" rewriting suggestions from AI is not the same as manually implementing the changes. You're already contradicting yourself by saying that AI only helped by providing explanations/documentation, and that you manually rewrote code suggested by AI." I hadn't not manually rewritter what AI gave me, I stated in the "AI DISCLOSURE" section in capital words that, "NO CODES WERE GENERATED WITH THE HELP OF AI."

Edit: Also just to be transparent, my previous PRs that were merged had 0 use of AI in the codes! Those PRs were purely made by my brain + hands :D

Thank you :)

The official link to the AIA specification (from the RISC-V Atlassian repo) is a Google Docs link RISC-V AIA], maybe the cloudbear.ru link you posted is a mirror of that document?

Regarding your previous contributions, that's fine if you indeed contributed all those changes yourself. I was asking for @romancardenas to give feedback on allowing your AI contirbutions after the experience of reviewing your previous contribution.

KushalMeghani1644 · 2025-09-24T13:23:34Z

The official link to the AIA specification (from the RISC-V Atlassian repo) is a Google Docs link RISC-V AIA], maybe the cloudbear.ru link you posted is a mirror of that document?

Regarding your previous contributions, that's fine if you indeed contributed all those changes yourself. I was asking for @romancardenas to give feedback on allowing your AI contirbutions after the experience of reviewing your previous contribution.

Hey! sorry if I was too harsh in my previous comments. I was just trying to be as transparent as I can! cause I always find it fun to contribute to OSS projects, especially bare metal codes like RISCV. Apologize for the inconveniences caused by me here. I hope that I have cleared out that everything is fine now :) And yes the cloudbear.ru link is a mirror, I for some reason wasn't able to find the official docs from RISCV, thanks for providing the links though.

KushalMeghani1644 · 2025-09-24T13:33:28Z

Hey! @rmsyn (sorry for pinging if you were busy) I've added a new arm to the existing test_csr_field! macro instead of creating a separate one. The macro now supports read-only CSR fields with the pattern [start:literal, end:literal] and mtopi tests now use the standard macro. Thanks for the clarification!

riscv/src/register/mtopi.rs

rmsyn

Hey! @rmsyn (sorry for pinging if you were busy) I've added a new arm to the existing test_csr_field! macro instead of creating a separate one. The macro now supports read-only CSR fields with the pattern [start:literal, end:literal] and mtopi tests now use the standard macro. Thanks for the clarification!

No worries, and thanks for making the changes. I have a couple other suggestions, otherwise LGTM outside the question of AI contributions.

KushalMeghani1644 · 2025-09-24T14:04:36Z

Hi @rmsyn I have removed the helper functiosn as requested! I hope the changes are fine :) if there are more improvements remaining, I'd be happy to fix them :D

KushalMeghani1644 · 2025-10-06T10:50:48Z

Hey @rmsyn and @romancardenas sorry for disturbing but can I please get any updates on what do I have to do with this PR next?

riscv/src/register/mtopi.rs

KushalMeghani1644 · 2025-10-08T11:48:05Z

A unrelated change @romancardenas (maybe) but I have fixed the spelling in aclint.rs, I hope you are fine with that :) if you think its unrelated to this PR I can also remove the commits.

riscv/src/register/macros.rs

riscv/src/register/mtopi.rs

KushalMeghani1644 · 2025-10-20T10:22:14Z

I am sorry, but seems like most the things @romancardenas has asked are fixed, if there are more changes required, I would definitely make them but I'll be slow to respond just for 2 or so days. Either way I am sorry for the other hurdles that were caused... I would wait for a response :) thanks

romancardenas · 2025-10-22T12:07:09Z

@KushalMeghani1644 please add a reference value to the test macro branch, so the macro actually checks against an expected value

KushalMeghani1644 · 2025-10-23T13:04:34Z

I am quiet unsure if the CI failing is my fault... I would like some guidance here, but it doesn't seem like my changes broke anything here... I didn't even touch the riscv-rt crate?

romancardenas · 2025-10-23T15:55:36Z

Looks like the new version of syn bumped the MSRV to 1.68...

I was thinking on bumping MSRV to 1.81 with riscv-types, so it shouldn't be a big deal. Let put this PR in standby until I do all the required work.

KushalMeghani1644 · 2025-10-24T05:10:50Z

OK then, I'll wait :)

romancardenas · 2025-10-27T15:53:54Z

Rebase from master, fix conflicts (mainly you will need to remove your typo fixes in riscv-peripheral, as they are already fixed), and CI should pass

KushalMeghani1644 · 2025-10-28T09:46:41Z

I hope this was fine @romancardenas :)

riscv/src/register/macros.rs

riscv/src/register/mtopi.rs

romancardenas · 2025-10-28T15:09:05Z

riscv/src/register/mtopi.rs

+        test_csr_field!(mtopi, iid: [16, 27], 0x0);
+        test_csr_field!(mtopi, iprio: [0, 7], 0x0);
+
+        let mtopi = Mtopi::from_bits((11 << 16) | 5);


For clarity, so the numbers coincide with the next checks

Suggested change

let mtopi = Mtopi::from_bits((11 << 16) | 5);

let mtopi = Mtopi::from_bits((0xB << 16) | 0x5);

My bad, I added a bug to your tests! In any case, it served me to notice that tests are not currently being executed in the CI.

KushalMeghani1644 · 2025-10-29T10:41:08Z

Thanks for the suggestions + commits @romancardenas is there any other changes that I should make?

romancardenas

Please, rebase again from master. You will notice that CI does not pass. Did you try to run the tests on your machine? Because it does not even compile

romancardenas · 2025-10-29T11:34:46Z

riscv/src/register/mtopi.rs

+        test_csr_field!(mtopi, iid: [16, 27], 0x0);
+        test_csr_field!(mtopi, iprio: [0, 7], 0x0);
+
+        let mtopi = Mtopi::from_bits((11 << 16) | 5);


My bad, I added a bug to your tests! In any case, it served me to notice that tests are not currently being executed in the CI.

KushalMeghani1644 · 2025-10-30T09:40:45Z

I have messed up something here.... letme convert this into a draft, when its ready I'll mark it ready for review

…nto new test_ro_csr_field! macro

KushalMeghani1644 · 2025-10-30T10:24:26Z

OK finally, everything passes, besides 1 CI that doesn't seem my fault... it has troubles updating the rust toolchain. Either way, I apologize but while I was trying to force-push, rebase, and do all the git stuff, I had messed up allot leading to all my commit history lost. So I hope so you are fine with just 2 commits at the moment, the code is as-it-is besides the changes that I made to fix the CI failing... I am sorry for complicating this quiet allot.

romancardenas · 2025-10-30T10:44:51Z

There are too many changes in your PR that are not required. As reviewing this would take too long, I will close this PR. You can create a new PR from scratch, starting from the current master branch, with a clean commit history.

KushalMeghani1644 · 2025-10-30T10:50:09Z

I'll definitely re-work on this then :)

KushalMeghani1644 requested a review from a team as a code owner September 24, 2025 11:53

rmsyn reviewed Sep 24, 2025

View reviewed changes

riscv/src/register/mtopi.rs Outdated Show resolved Hide resolved

rmsyn requested changes Sep 24, 2025

View reviewed changes

KushalMeghani1644 requested a review from rmsyn September 24, 2025 13:00

rmsyn reviewed Sep 24, 2025

View reviewed changes

riscv/src/register/mtopi.rs Outdated Show resolved Hide resolved

rmsyn requested changes Sep 24, 2025

View reviewed changes

KushalMeghani1644 requested a review from rmsyn September 24, 2025 14:04

romancardenas requested changes Oct 7, 2025

View reviewed changes

riscv/src/register/mtopi.rs Outdated Show resolved Hide resolved

riscv/src/register/mtopi.rs Outdated Show resolved Hide resolved

riscv/src/register/mtopi.rs Outdated Show resolved Hide resolved

KushalMeghani1644 requested a review from romancardenas October 7, 2025 13:01

romancardenas requested changes Oct 13, 2025

View reviewed changes

riscv/src/register/macros.rs Outdated Show resolved Hide resolved

riscv/src/register/mtopi.rs Outdated Show resolved Hide resolved

riscv/src/register/mtopi.rs Outdated Show resolved Hide resolved

KushalMeghani1644 requested a review from romancardenas October 13, 2025 12:58

KushalMeghani1644 mentioned this pull request Oct 16, 2025

Rename all instances of riscv-pac to riscv-types besides docs #354

Closed

KushalMeghani1644 force-pushed the add_mtopi branch from 34efc27 to c646018 Compare October 28, 2025 09:46

romancardenas requested changes Oct 28, 2025

View reviewed changes

KushalMeghani1644 requested a review from romancardenas October 29, 2025 10:41

romancardenas mentioned this pull request Oct 29, 2025

riscv: Run tests on CI #357

Merged

romancardenas requested changes Oct 30, 2025

View reviewed changes

KushalMeghani1644 force-pushed the add_mtopi branch 3 times, most recently from b85fc8f to 6d75606 Compare October 30, 2025 09:39

KushalMeghani1644 marked this pull request as draft October 30, 2025 09:40

KushalMeghani1644 closed this Oct 30, 2025

KushalMeghani1644 force-pushed the add_mtopi branch from 6d75606 to 257c3d6 Compare October 30, 2025 09:45

Add MTOPI CSR

9a3b727

KushalMeghani1644 reopened this Oct 30, 2025

Fix macro pattern conflicts by separating read-only CSR field tests i…

3e38720

…nto new test_ro_csr_field! macro

KushalMeghani1644 marked this pull request as ready for review October 30, 2025 10:22

KushalMeghani1644 requested a review from romancardenas October 30, 2025 10:27

romancardenas closed this Oct 30, 2025

KushalMeghani1644 deleted the add_mtopi branch October 30, 2025 10:49

This was referenced Oct 30, 2025

Add MTOPI CSR #359

Closed

Add mtopi CSR #360

Merged

	let mtopi = Mtopi::from_bits((11 << 16) \| 5);
	let mtopi = Mtopi::from_bits((0xB << 16) \| 0x5);

Add mtopi CSR #347

Add mtopi CSR #347

Uh oh!

Conversation

KushalMeghani1644 commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

rmsyn left a comment

Choose a reason for hiding this comment

Uh oh!

KushalMeghani1644 commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KushalMeghani1644 commented Sep 24, 2025

Uh oh!

rmsyn commented Sep 24, 2025

Uh oh!

KushalMeghani1644 commented Sep 24, 2025

Uh oh!

KushalMeghani1644 commented Sep 24, 2025

Uh oh!

Uh oh!

rmsyn left a comment

Choose a reason for hiding this comment

Uh oh!

KushalMeghani1644 commented Sep 24, 2025

Uh oh!

KushalMeghani1644 commented Oct 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KushalMeghani1644 commented Oct 8, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KushalMeghani1644 commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

romancardenas commented Oct 22, 2025

Uh oh!

KushalMeghani1644 commented Oct 23, 2025

Uh oh!

romancardenas commented Oct 23, 2025

Uh oh!

KushalMeghani1644 commented Oct 24, 2025

Uh oh!

romancardenas commented Oct 27, 2025

Uh oh!

KushalMeghani1644 commented Oct 28, 2025

Uh oh!

Uh oh!

Uh oh!

romancardenas Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romancardenas Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

KushalMeghani1644 commented Oct 29, 2025

Uh oh!

romancardenas left a comment

Choose a reason for hiding this comment

Uh oh!

romancardenas Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

KushalMeghani1644 commented Oct 30, 2025

Uh oh!

KushalMeghani1644 commented Oct 30, 2025

Uh oh!

romancardenas commented Oct 30, 2025

Uh oh!

KushalMeghani1644 commented Oct 30, 2025

Uh oh!

Reviewers

Assignees

KushalMeghani1644 commented Sep 24, 2025 •

edited

Loading

KushalMeghani1644 commented Sep 24, 2025 •

edited

Loading

KushalMeghani1644 commented Oct 20, 2025 •

edited

Loading

romancardenas Oct 28, 2025 •

edited

Loading