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

EBREAK and mtval #600

Closed
aswaterman opened this issue Oct 12, 2020 · 10 comments · Fixed by #601
Closed

EBREAK and mtval #600

aswaterman opened this issue Oct 12, 2020 · 10 comments · Fixed by #601

Comments

@aswaterman
Copy link
Member

The spec remarks that mtval is written with the virtual address on hardware breakpoints. It is mum about the EBREAK instruction, which currently means mtval should be set to 0.

There are two questions:

  • Is this the right definition? Should EBREAK also be allowed to set mtval to the PC? (Note that making this change is backwards compatible, since zeroing mtval remains a legal implementation, at least at the ISA level.)
  • Do we need to clarify somewhere that EBREAK is not a hardware breakpoint?
@aswaterman
Copy link
Member Author

(FWIW, Spike and Rocket both set mtval to the PC when executing an EBREAK instruction, which if "hardware breakpoint" is defined to mean "not EBREAK" is a violation of the spec.)

@gfavor
Copy link
Collaborator

gfavor commented Oct 13, 2020 via email

@aswaterman
Copy link
Member Author

It turns out this sentence (and the corresponding one in the supervisor chapter) are the only places we were using the term "hardware breakpoint", so the clarification isn't needed if the sentence is reworded to say "breakpoint exception" (which of course includes both cases).

aswaterman added a commit that referenced this issue Oct 13, 2020
This is a normative change, but it is backwards compatible, since writing 0
to mtval remains a legal option.

Resolves #600
@gfavor
Copy link
Collaborator

gfavor commented Oct 13, 2020 via email

@allenjbaum
Copy link

That's odd - the Riscof folks just said that the newest version of spike does not populate mtval.
I would have expected it to act the same as Ecall. MTVAL is totally redundant with EPC, isn't it?

@allenjbaum
Copy link

Oh, and the SAIL model doesn't populate it either.

@aswaterman
Copy link
Member Author

It is redundant with epc--but only if the cause wasn't a hardware watchpoint (a.k.a. ld/st breakpoint). So I think there's utility in populating it, if only to help a tiny bit with low-level debugging. It is basically HW-cost neutral (especially since writing 0 is still a legal option).

@neelgala
Copy link
Collaborator

@allenjbaum the latest version of spike is indeed storing a 0.
Fix is to change the arguments to function in c_ebreak.h and ebreak.h to be pc instead of 0.

@allenjbaum
Copy link

allenjbaum commented Oct 13, 2020 via email

@aswaterman
Copy link
Member Author

I think whether the debug triggers are required to do something in particular w.r.t. mtval is a matter for the debug spec, right?

aswaterman added a commit that referenced this issue Oct 13, 2020
This is a normative change, but it is backwards compatible, since writing 0
to mtval remains a legal option.

Resolves #600
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 a pull request may close this issue.

4 participants