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

[rv_dm] Debug module does not support sub-word accesses over SBA #11095

Closed
sriyerg opened this issue Feb 25, 2022 · 9 comments
Closed

[rv_dm] Debug module does not support sub-word accesses over SBA #11095

sriyerg opened this issue Feb 25, 2022 · 9 comments

Comments

@sriyerg
Copy link
Contributor

sriyerg commented Feb 25, 2022

The debug spec requires the DMI CSR SBCS[SBACCESS] to be implemented as RW, but the design implements it as RO instead.
See: https://github.com/riscv/riscv-debug-spec/raw/4e0bb0fc2d843473db2356623792c6b7603b94d4/riscv-debug-release.pdf
Page 43.

The following logic 'overwrites' any changes made to sbaccess:

sbcs_d.sbaccess = (BusWidth == 32'd64) ? 3'd3 : 3'd2;

On line 431, it captures the data written to this field.

The dm_sba appears to support sub-word accesses, as it sets the byte masks correctly.

The end result is JTAG can only ever initiate 32-bit accesses over SBA.

@sriyerg sriyerg changed the title [rv_dm] Debug module does not seem to have implemented sbcs register correctly [rv_dm] Debug module does not support sub-word accesses over SBA Feb 25, 2022
@tjaychen
Copy link

@GregAC
@msfschaffner

@msfschaffner
Copy link
Contributor

There is a PR pending upstream that I think should fix this (pulp-platform/riscv-dbg#106), but since it has not been merged yet we may have to pull that over to our lowRISC fork in the mean time.

We did discuss this on this issue already #4975 and labeled this as a future enhancement. If that is needed for debug to work correctly, though, we may have to reclassify it as P1.

@tjaychen
Copy link

is this needed for debug to work correctly? The SBA is just a side band debug bus right?

@msfschaffner
Copy link
Contributor

Yeah I think that is the reason why we had labelled this as P3 in the first place.
Interactive debug should still be possible without it, afaik.

@msfschaffner
Copy link
Contributor

Support for this has been merged upstream: pulp-platform/riscv-dbg#106.

However, there is a fix for a CDC bug outstanding here pulp-platform/riscv-dbg#123 and I would recommend to wait for that one to land before we vendor in anything.

@msfschaffner
Copy link
Contributor

CC @jrrk2

@bluewww
Copy link

bluewww commented Mar 8, 2022

I reverted the issue that introduced the CDC trst bug here. The feature that introduced this bug will be re-introduced once it's clear whether this fix suffices.

@msfschaffner
Copy link
Contributor

Ah great, thanks for letting us know.
In that case it should be ok to proceed and vendor this in again.

@sriyerg I'll do that later today and will let you know once it is done.

@msfschaffner
Copy link
Contributor

This is now supported - closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment