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

Debug SAIL for Native Triggers #8

Open
13 tasks
jjscheel opened this issue Mar 16, 2023 · 54 comments
Open
13 tasks

Debug SAIL for Native Triggers #8

jjscheel opened this issue Mar 16, 2023 · 54 comments
Assignees

Comments

@jjscheel
Copy link
Contributor

jjscheel commented Mar 16, 2023

Technical Group

Debug TG

ratification-pkg

Debug

Technical Liaison

Tim Newsome

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

@jjscheel
Copy link
Contributor Author

@junambi, as you begin to start this work, I'd appreciate an outlook for when you think this work might be completed. I suggest we think about it in terms of steps and put dates for each:

  • Planning complete
  • Development completed (ready to submit first PR)
  • All PRs accepted (done)

@jjscheel jjscheel removed their assignment Apr 11, 2023
@junambi
Copy link

junambi commented Apr 25, 2023

Working on a flow for the implementation.

@jjscheel
Copy link
Contributor Author

@timsifive, @pdonahue-ventana, any volunteers to help here yet?

@timsifive
Copy link

I haven't heard from anyone.

@jjscheel jjscheel assigned jjscheel and unassigned junambi Jun 8, 2023
@jjscheel
Copy link
Contributor Author

jjscheel commented Jul 18, 2023

Comment from most recent ARC minutes (July 10) - link

  • Debug 1.0: Review has started of the latest submitted spec, but just review of all the changes since the last Debug 1.0 draft spec was submitted, reviewed, and approved. Review of this has started, with initial discussion of one of the more notable backward-incompatible changes (between v1.0 and the ratified v0.13). This relates with abstract commands and the data registers to support more efficient FPGA implementations. After further discussion in next week's ARC meeting, John will be providing ARC feedback and guidance to the Debug TG. Likely other items of feedback that arise will also be provided.
  • Review of the latest Debug and Pointer Masking specs (along with the S*deleg extensions) are expected to continue next week.

Also note, this remains on the list of future DevPartners activities. It will be addressed AFTER Pointer Masking and the Floating Point gap work work (Zdinx, Zhinx).

@jjscheel
Copy link
Contributor Author

jjscheel commented Feb 1, 2024

@UmerShahidengr, any chance that 10xEngineers has someone to pick this up and drive to conclusion?

@jjscheel
Copy link
Contributor Author

@UmerShahidengr, any feedback on your ability to start work here?

@UmerShahidengr
Copy link

@muhammad-maarij-zeeshan can you please look at it?
@jjscheel we will do it, you can change its status and set up its end date to be end of 2Q2024 (i.e, June 2024)

@jjscheel
Copy link
Contributor Author

jjscheel commented Mar 5, 2024

Thanks, @UmerShahidengr. Can you please make sure that @muhammad-maarij-zeeshan sends an email to [email protected] to request his account? Then, I'll take care of the other DevPartner enablement.

@jjscheel
Copy link
Contributor Author

jjscheel commented Apr 2, 2024

@UmerShahidengr, has work started here?

@jjscheel jjscheel moved this from Blocked to As-planned in RISC-V DevPartner Work Apr 2, 2024
@UmerShahidengr
Copy link

@jjscheel it has not been started yet because of resource limitation, but it is in my priority list. Keep it assigned to me, I will look into it soon.

@jjscheel
Copy link
Contributor Author

jjscheel commented Apr 3, 2024

Thanks, @UmerShahidengr. As we discussed, we will need to complete the code to the point of submitting a PR in order to be able to ratify the spec without a waiver from the TSC. Given that the spec has completed Public Review and is being updated, it could be ready for ratification within the next 1-2 months.

@jjscheel
Copy link
Contributor Author

@UmerShahidengr, any progress here?

@UmerShahidengr
Copy link

@jjscheel we will add the 1st PR within next 2 weeks.

@UmerShahidengr
Copy link

Update April 30th, 2024:
@HAMZA-AFZAL404 has been working on this SoW. We are still in developing phase.

@jjscheel
Copy link
Contributor Author

@UmerShahidengr, per my email, we need to understand who the trainee is and how soon they will be able to work with @billmcspadden-riscv to complete this work.

@jjscheel jjscheel moved this from As-planned to Blocked in RISC-V DevPartner Work Aug 21, 2024
@UmerShahidengr
Copy link

@jjscheel, after your email, we (Bilal and I) had conversation, one engineer will start working on this project full time from Sept 1st, we are estimating it to complete it till mid-October.

@UmerShahidengr
Copy link

@YazanHussnain-10x will be working on this SoW full time from Sept 1st.

CC: @jjscheel

@jjscheel
Copy link
Contributor Author

jjscheel commented Sep 3, 2024

Please ensure that Yazan applies for a RISC-V Portal id by sending an email to [email protected] with me on cc. I'll handle everything else after that. Welcome, YazanHussnain-10x!

@jjscheel
Copy link
Contributor Author

Ping: Please ensure that Yazan applies for a RISC-V Portal id by sending an email to [email protected] with me on cc. I'll handle everything else after that. Welcome, YazanHussnain-10x!

@YazanHussnain-10x
Copy link

Update: September 22nd, 2024

Open Question
If the trigger is configured to match and fire on the Load address, in that case, xepc is set to the virtual address of the instruction matched. When returning from the break-point exception trap, the instruction is re-executed, and the configured trigger matches and fires again. How to prevent it from firing again on the same instruction?

Update

  1. Sdtrig Extension CSRs have been implemented.
  2. Both solutions for the re-entrancy problem have been implemented.
  3. The Icount Trigger, itrigger, and etrigger have been implemented.
  4. The Match Control Type 6 Trigger remains to be implemented.

I verified the functionality with a basic test, but I need some ACTs for the Native trigger to verify it properly and to ensure that it should not behave strangely.

@pdonahue-ventana
Copy link

Disable the trigger in question, single step, reenable the trigger, resume.

@YazanHussnain-10x
Copy link

Thank, you @pdonahue-ventana

@rtwfroody
Copy link

Both solutions for the re-entrancy problem have been implemented.

Note that only one of the two solutions should be enabled at any one time. No real hardware will implement both.

@UmerShahidengr UmerShahidengr moved this from Blocked to Late in RISC-V DevPartner Work Oct 1, 2024
@UmerShahidengr
Copy link

Update October 1st, 2024:
Basic Implementation is done, but we are getting some issues in the trap handler when trigger is firing upon returning to the trap due to illegal instruction, trap handler is not reserving the mepc value to take to code back to the test. Trap Handler requires some updates to handle such nested exception cases.
Basic Implementation is done, and we are waiting for the ACTs to check the implementation

@rtwfroody
Copy link

Initial ACT tests are at riscv-non-isa/riscv-arch-test#487. They're not fully reviewed but they should work.

@YazanHussnain-10x
Copy link

YazanHussnain-10x commented Oct 4, 2024

Update October 4th, 2024:
Match control trigger implementation is done. I tested the functionality, with the basic test, both on RV32 and RV64. Now testing with the ACTs mentioned by @rtwfroody. I raised the PR on the sail-riscv repo for the initial review

@jjscheel
Copy link
Contributor Author

It appears that the new PR is here: riscv/sail-riscv#573

What are the plans for Mudassir's older PR? riscv/sail-riscv#486

@UmerShahidengr
Copy link

@jjscheel, the old one will be closed by Mudassir soon.

@UmerShahidengr
Copy link

Update October 29th, 2024:
The PR is in review, @rtwfroody has been reviewing the PR, @YazanHussnain-10x has been working on this one, review process is going smoothly.

@jjscheel
Copy link
Contributor Author

Old PR closed by @billmcspadden-riscv. THANKS!

@jjscheel
Copy link
Contributor Author

@rtwfroody and @pdonahue-ventana, thanks for your reviews and comments on the PR. If you've completed the review of all code in the PR, the first question we need to answer is whether we believe the test are functionally complete. If so, we can conclude the Freeze waiver for Sail. Can you share your thoughts/comments here? If your answer is no, please clearly identify what you'd like to see resolved to be considered complete.

@pdonahue-ventana
Copy link

Is the question about whether the Sail is complete (which I think is the topic here) or whether the tests (in riscv-non-isa/riscv-arch-test#487) are complete?

@pdonahue-ventana
Copy link

Answering my own question: I think that you just copied the text above from the ACT issue so I'll assume that the question here is about Sail.

I think that the Sail is as complete as necessary. Things like VS/VU mode support are known to be missing, but Sail doesn't support those modes in general so the Sdtrig support can't be expected to add full support for the hypervisor extension. There are a few other minor things like tmexttrigger (asynchronous triggers) that are also missing but I don't expect that Sail would ever support that. I'm satisfied with what's here.

@jjscheel
Copy link
Contributor Author

jjscheel commented Nov 1, 2024

@pdonahue-ventana, yes your assumptions and guess as to its motivation are correct. This was a copy and paster error. Indeed I was wondering if the Sail was complete as defined by this SOW (which is defined in the first entry of this issue), which in turn links to Tim's original SOW doc -- SOW link.

Please review it one final time and confirm your answer.

@timsifive, would love your review and thoughts too when you have a moment.

@pdonahue-ventana
Copy link

timsifive is now @rtwfroody

@jjscheel
Copy link
Contributor Author

jjscheel commented Nov 1, 2024

@rtwfroody, would love your review and thoughts too when you have a moment.

@pdonahue-ventana
Copy link

I believe that the Sail work meets the SOW and actually exceeds it in some ways (e.g. support for all match types, not just three of them).

I don't know whether to laugh or cry about this line from the SOW: "Needed for end of 2022 ratification"

@jjscheel
Copy link
Contributor Author

@rtwfroody, are you ready to sign-off that the Sail work is feature complete relative to the SOW? Note, this does not mean it's approval or merged: it means it's not missing significant function, just as we would review for Freeze signoff.

If you will approve this, I believe we can start Ratification-ready signoff this week.

@rtwfroody
Copy link

Yes, the SAIL work satisfies the SOW.

@jjscheel
Copy link
Contributor Author

So, @YazanHussnain-10x, you need to continue working on getting the PR merged. @rtwfroody and @pdonahue-ventana can now focus on the final activity for Ratification (email on the wire).

@YazanHussnain-10x
Copy link

Acknowledged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Late
Development

No branches or pull requests

8 participants