Skip to content

master_be_o signal in dm_sba.sv is not set correctly during reads #57

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

Closed
cr8601 opened this issue May 11, 2020 · 6 comments · May be fixed by #85
Closed

master_be_o signal in dm_sba.sv is not set correctly during reads #57

cr8601 opened this issue May 11, 2020 · 6 comments · May be fixed by #85

Comments

@cr8601
Copy link

cr8601 commented May 11, 2020

Hi,
within lowrisc/opentitan project I came to a problem when accessing the system bus via JTAG. Your documentation (https://github.com/pulp-platform/riscv-dbg/blob/master/doc/debug-system.md) states the following regarding this master_be signal:

be 1 output Byte Enable. Is set for the bytes to write/read, sent together with req

Within dm_sba.sv this signal is always 0 during read transactions. In my opinion this behaviour does not reflect the documentation...

In case of opentitan this signal is used for the a_mask signal of the TLUL bus. I was suggested to raise this topic here in order to fix it at the root, if possible.

For my case simply inserting "be = '1;" into line 92 solved the problem.

Can someone please comment whether this could be changed here?

For reference here's the issue I raised within opentitan:
lowRISC/opentitan#2126

Best regards
cr8601

@bluewww
Copy link
Collaborator

bluewww commented May 11, 2020

It looks like the documentation is not entirely correct.
If you look here

riscv-dbg/src/dm_csrs.sv

Lines 527 to 532 in 3670267

sbcs_d.sbaccess128 = 1'b0;
sbcs_d.sbaccess64 = logic'(BusWidth == 32'd64);
sbcs_d.sbaccess32 = logic'(BusWidth == 32'd32);
sbcs_d.sbaccess16 = 1'b0;
sbcs_d.sbaccess8 = 1'b0;
sbcs_d.sbaccess = (BusWidth == 32'd64) ? 3'd3 : 3'd2;

You see that currently we only fully support 32-bit or 64-bit accesses, though adding other access types doesn't seem too hard.

If you just hardcoded be= '1 then you will break other things, namely openocd.

I see though that we have a real bug here: sberror is not signaled if someone tries to access with an access type that is not valid

@cr8601
Copy link
Author

cr8601 commented May 12, 2020

Hi,
sorry, but I cannot really follow your comments. I think my problem is not related to any access type. I am talking about the host interface to the system bus which is defined by the ports master_*. There is a master_be port which is always tied to zero for read transactions (dm_sba.sv). The lowrisc/opentitan implementation (TLUL interface) uses these to mask the write and read bytes and therefore all system bus read requests, coming from external JTAG Debugger are failing (returning only zeros).

Can you please comment on this master_be signal? Is it always zero for read transactions by intention?

To simply get my system to work, I just put it to all 1 during read. Maybe this is not valid in general, but for now seems to fix my problem.

@bluewww
Copy link
Collaborator

bluewww commented May 12, 2020

Sorry I misunderstood your problem description. Indeed you are correct, the be enable signal doesn't seem to be set correctly

@bluewww bluewww changed the title Issue regarding master_be_o signal in dm_sba.sv / Wrong value during read transaction? master_be_o signal in dm_sba.sv is not set correctly during reads May 12, 2020
@zarubaf
Copy link
Contributor

zarubaf commented May 12, 2020

The be has no meaning in our system when performing a read (most of the stuff is AXI-based which doesn't use the be at all during reads) and is ignored. But seems an easy addition if that is necessary for the TL adapter.

@cr8601
Copy link
Author

cr8601 commented May 13, 2020

It would be nice if this could be added!
Thanks
cr8601

@Silabs-ArjanB
Copy link
Contributor

Hi @cr8601 , @bluewww Hasn't this issue been solved now by the merge of #70 ? If so, can it be closed?

@cr8601 cr8601 closed this as completed Aug 14, 2020
@bluewww bluewww linked a pull request Oct 28, 2020 that will close this issue
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