External Regfile not honoring wr_ack

[roowatt]

Based on my reading of the documentation here I believe I can make a regfile external and use the user interface to connect to my own implementation.

I have used the following snippet of RDL to define the external register to memory map an AXI Stream Master, the address is decoded and used to determine whether TLAST is to be high or low.

regfile master_axis_iface_t #(longint unsigned STREAM_WIDTH = 32) {
	name = "AXI Stream Master Interface";
	desc = "Register bank to AXIS interface
	!!! warning
		This is a blocking interface, the processor will be stalled until the connected AXI Stream Slave can accept the written data.

    default sw = w;
    default hw = r;	

    external regfile {
		reg {
			field {
				desc = "Any write to this register will send the data over the AXI Stream Master interface with TLAST deasserted

				**NOTE:** Individual bit enables on bus interface are ignored.";
			} TDATA[STREAM_WIDTH-1:0];
		} TDATA;

		reg {
			field {
				desc = "Any write to this register will send the data over the AXI Stream Master interface with TLAST asserted

				**NOTE:** Individual bit enables on bus interface are ignored.";
			} TLAST[STREAM_WIDTH-1:0];
		} TLAST;
	} AXIS_MASTER;
};

Perhaps I misread or misunderstood the documentation, but I was under the impression that the write would be stalled until the wr_ack was asserted high.

However, even if I tie the wr_ack signal peramently low (as the sim shows), the write transaction completes and does not seem to honor the wr_ack signal.

Below is a sim screenshot (from Vivado with register block hooked up to a microblaze).

I will investigate further.

$ pip list | grep peakrdl
peakrdl                0.8.0
peakrdl-html           2.10.1
peakrdl-ipxact         3.4.1
peakrdl-regblock       0.16.0
peakrdl-systemrdl      0.3.0
peakrdl-uvm            2.3.0

[roowatt]

Upon further investigation...

It appears that the external interface does not implement the access types from the register definitions.

• Thinking about it , this makes sense as individual registers in the addres space made external could have different access types. Hence the read interface being exported too.

As the documentation says, the req and rd_ack and wr_ack etc are strobes.

• Again, after reflection perfectly understandable as this provides the opportunity for buffered reads/writes transactions rather than just stalling a single transaction.

However, as I was implementing a write only interface to a FIFO like device, I intended on protecting the end user from erroneously reading from the address ranges in question and deadlocking due to unimplemented read. To that end I permanently drove rd_ack high, thinking that this would prevent deadlocks if rd_ack was instead tied low.

But it looks like the internal logic retires the external bus transaction on either wr_ack or rd_ack, irresepective of the bus transaction type (read or write). This can be seen in the previously attached screenshot where wr_ack is tied low and rd_ack tied high. New bus operations continue to be performed despite the prior write operations never being acknowledged.

In sim I have done the following, but I am concerned about a combinational loop (I need to investgate)
assign rd_ack = req && !req_is_wr; Obviously I can register the rd_ack if needed.

The bulk of my issues were "user error", but perhaps some additional clarification on the details of external regsiters might help others. I can try and put some words toghter if that helps.

[amykyta3]

Thanks for the detailed analysis!
Agree that some more clarity should be added in the documentation.

assign rd_ack = req && !req_is_wr;

This is totally acceptable to do, and will not create a combo loop. I will add this as a recommendation to the documentation, since this pattern is expected and encouraged for the more simple cases.

Good point about the temptation to tie this signal off to a constant 1. I can see how that would be an easy mistake to make. I can add an assertion to the generated output that would detect this situation (assert that all ack strobes are 0 if no external access is pending). Unfortunately allowing constant tieoffs in this situation ultimately would end up adding logic (I'd have to keep track of the pending request state for each individual external device), where the tieoff you used should optimize and get absorbed into the internal logic pretty cleanly with most synthesizers.

Since this is still a relatively new feature, I'm open to other suggestions to improve this and make it more intuitive.
Thanks for trying it out!

[roowatt]

Happy to tie off that way, and if the recommendation is added to the documentation that would be sufficent. The other approach I can think of would be to not export the read interface in this case and do the tie off in the generated RTL. But I think this shifts the burden to the exporter and not worth the effort.

An alternative might be a monitor that users could include when integrating their custom logic? But that might be step too far. :)

[amykyta3]

I think that could be a valid case though - if the register contains no readable fields, then do not generate the read ports for it at all. Similar for the converse on writable things.

I think this would be a common enough case that it is justifiable for the exporter to do the right thing.

[amykyta3]

Implemented the above change as part of #58

Closing this ticket since it sounds like we've come to a good conclusion. Feel free to re-open if there is more to discuss.

[amykyta3]

Published xsim fix in 0.19.0