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

When using DebugPlugin, don't halt CPU upon encountering EBREAK instruction #176

Closed
tomverbeure opened this issue Apr 20, 2021 · 9 comments

Comments

@tomverbeure
Copy link
Contributor

tomverbeure commented Apr 20, 2021

(Copying from this gitter discussion)

Tom:

In the current DebugPlugin implementation, when the CPU encounters an EBREAK instructions, the processor halts unconditionally, even if there's no debugger attached.

I think this is a bit too aggressive. By default, the CPU should treat an EBREAK as an illegal instruction and trap. And it should only halt the CPU when the debugger has been enabled. This way, a CPU doesn't hang upon encountering EBREAK.

The reason I would like this behavior is because it allows preventing CPU hangs due to semihosting calls, as implemented in this ARM semihosting implementation.

Doulu:

Yes that's right, that would be something to improve.

I think what we could do, is to add a register in the DebugPlugin which is set as soon one access is done on the debug port
If that register is set, then all ebreak will go to the DebugPlugin, else they are ignored
I think that would be the less intrusive and the lightest way to implement it.

Dolu1990 added a commit that referenced this issue Apr 20, 2021
@Dolu1990
Copy link
Member

Dolu1990 commented Apr 20, 2021

@tomverbeure There is one implementation of it. It is in a separated branch for now. Let's me know if you give a try ^^
bfe65da

@tomverbeure
Copy link
Contributor Author

tomverbeure commented Apr 21, 2021

After staring at code and waveforms the whole evening, I've come to the conclusion that the DebugPlugin was doing the right thing all along. The key issue is that I didn't set ebreakGen=true. That is what resulted in no trap being generated...

With the old DebugPlugin code, I've tried these combinations:

Situation 1: ebreakGen=false, no debugger attached

EBREAK -> DebugPlugin.haltedByBreak goes high. CPU hangs.

Situation 2: ebreakGen=true, no debugger attached

EBREAK -> DebugPlugin.haltedByBreak stays low. CPU traps due to exception code 3.

Situation 3: ebreakGen=true, debugger attached

EBREAK -> DebugPlugin.haltedByBreak goes high. CPU stops at the EBREAK instruction and doesn't trap.

The way I understand it, the haltIt control bit was already doing what I was asking for: it's set by the OpenOCD, and when it's not set, and ebreakGen=true, then that EBREAK instruction traps as it should.

Sorry for wasting your time on this!

@Dolu1990
Copy link
Member

The key issue is that I didn't set ebreakGen=true. That is what resulted in no trap being generated...

Hoo right, nearly all configuration do not enable CsrPlugin.ebreakGen XD

Sorry for wasting your time on this!

no worries, it is good that now it allow having both CsrPlugin.ebreak and DebugPlugin ebreak in the same design ^^

So all good right now ?

@tomverbeure
Copy link
Contributor Author

So all good right now ?

No need to make any changes to DebugPlugin.

However, for advanced VexRiscv demo configuration, I would set ebreakGen=true and ecallGen=true, especially for things like the Linux configuration etc. Because those are the kind of configuration where you'd expect people to write advanced trap routines for.

Closing.

@Dolu1990
Copy link
Member

No need to make any changes to DebugPlugin.

Ahh you mean "Do not need to merge this branch into dev" ? or "All good can merge it" ? XD

ebreakGen=true and ecallGen=true, for linux

Yes right, also, once i would like to try GDB debug a linux app via software ebreak :D

@tomverbeure
Copy link
Contributor Author

I mean: "Do not need to merge this branch into dev".

Tom

@Dolu1990
Copy link
Member

The thing is that without that patch, software ebreak aren't possible in machine mode if the DebugPlugin is in the design. As it will make the cpu hang, and jtag debug isn't possible in user/supervisor mode.

(Before this patch, the DebugPlugin listen to ebreak in machine mode only)

So as far as i can see, it is worth merging it into dev.

@tomverbeure
Copy link
Contributor Author

Ok. I have never used anything that exercises the different modes, so I can't give any feedback on this.

@tomverbeure
Copy link
Contributor Author

I understand things (even) better now...

In my earlier comment about things being fine, when the debugger was not enabled and ebreakGen=true, the CPU would indeed follow the trap, but my trap code was simply doing an endless loop. When the trap returns however (with MRET), it will still hang the CPU.

So your RTL fix is needed after all!

Dolu1990 added a commit that referenced this issue Apr 22, 2021
…ebreak. Also, DebugPlugin ebreak can be disabled via the debug bus.
Dolu1990 added a commit that referenced this issue Apr 22, 2021
ciniml pushed a commit to ciniml/VexRiscv that referenced this issue Apr 18, 2022
ciniml pushed a commit to ciniml/VexRiscv that referenced this issue Apr 18, 2022
…srPlugin ebreak. Also, DebugPlugin ebreak can be disabled via the debug bus.
ciniml pushed a commit to ciniml/VexRiscv that referenced this issue Apr 18, 2022
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

No branches or pull requests

2 participants