Skip to content

Add keepalive feature.#592

Merged
Tim Newsome (timsifive) merged 5 commits into
masterfrom
keepalive
Nov 13, 2020
Merged

Add keepalive feature.#592
Tim Newsome (timsifive) merged 5 commits into
masterfrom
keepalive

Conversation

@timsifive
Copy link
Copy Markdown
Contributor

Addresses #519. Went with the simple version without reading the keepalive values.

This allows a debugger to request that a given hart stays available for
debugging (e.g. is not powered down). Fixes #519.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this simpler approach is the right thing to do. Just one minor nit...

Comment thread xml/dm_registers.xml Outdated
Co-authored-by: Ernie Edgar <43148441+ernie-sifive@users.noreply.github.com>
@timsifive
Copy link
Copy Markdown
Contributor Author

Paul Donahue (@pdonahue-ventana) does this look OK?

@pdonahue-ventana
Copy link
Copy Markdown
Collaborator

Robert Chyla (@RobertChyla): You made the original request so what are your thoughts?

This allows a debugger to request that harts should stay available but there's no feedback about whether this feature is implemented or if it will be respected. As such, I think that debuggers are not simplified since they'll still have to deal with things going unavailable unexpectedly. At least now with stickyunavail they know that this happened. But it's a nice feature to at least allow the debugger to make the request.

I think that some sort of discovery mechanism would be an improvement to at least know if the DM supports making the request (and whether the power controller will honor it may be beyond the scope of the DM spec).

@timsifive
Copy link
Copy Markdown
Contributor Author

I contacted Robert by e-mail, and he suggested the slight rewording, and said "it is OK as it is now."

Even if the feature is supported, some hardware will not be able to guarantee that the hart will remain available, so debuggers will still need to deal with harts disappearing. If we had a config structure I'd just say stick some bits in there that describe possible behavior, although that still doesn't handle the case where the DM itself doesn't even know whether the system it is in can honor the request.

Seeing how we're freezing on Monday, I'd like to merge this as-is, and you can open an issue for 1.1 to address discoverability. Thoughts?

@RobertChyla
Copy link
Copy Markdown

I was surprised that my innocent email paragraph end-up as spec change.
I did not wanted to overload with allowing discovery nor 'confirmation' that setting is effective as this can be really outside of DM. This may be better handled by Config Structure (for DM and/or Power Controller).

@pdonahue-ventana
Copy link
Copy Markdown
Collaborator

Seeing how we're freezing on Monday, I'd like to merge this as-is, and you can open an issue for 1.1 to address discoverability. Thoughts?

OK

@timsifive Tim Newsome (timsifive) merged commit 9fea4c5 into master Nov 13, 2020
@timsifive Tim Newsome (timsifive) deleted the keepalive branch November 13, 2020 20:54
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 this pull request may close these issues.

4 participants