ibex icon indicating copy to clipboard operation
ibex copied to clipboard

Ease of adapting Ibex memory protocol to tilelink for non 32-bit access

Open GregAC opened this issue 4 years ago • 10 comments

Tilelink has a size field it its message payload, that the native Ibex protocol does not have.

When accessing 'standard' memory this is not a big problem. An Ibex -> Tilelink adapater could just always use 32-bit sizes and set all byte-enables for read (Get in Tilelink terms) and for writes take the byte-enables from the data_be_o from Ibex.

However this could give some difficulties around reads to devices where a read could trigger some action. Specifically if a device triggers some action on particular byte reads and the simple adapter described above is used a single lb instruction will turn into a 4-byte Get in Tilelink. The device might then trigger an action because certain bytes have been read but from a programmer's perspective these bytes haven't been touched.

In Tilelink for a Get message you cannot partially set the byte mask, so you must use the size field to communicate what bytes you actually intend to read where this matters. Ibex (from a quick look at the RTL anyway) does generate data_be_o that indicates the precise bytes you want so it would be possible to build some logic that can work out an appropriate size from the data_be_o.

You may also find that your TileLink devices only respond to PutFullData so in order to do a byte-write to such a device you'd need to generate PutFullData with a byte size. Again the logic mentioned above could be employed to ensure the correct size field is generated along with some logic to understand if it's a PullFullData or not.

This logic would be a little fiddly, add extra area and increase the depth of logic. In some sense it's simply undoing what ibex_load_store_unit has done internally (taking the size and address and generating a be). We could introduce a data_size_o to the Ibex top-level interface allowing simpler adaption to Tilelink and other similar protocols.

We probably also want a requirement that data_be_o matches tilelink requires for its mask field but I think this is already the case, but worth documenting.

Another potential addition would a data_full_o that indicates data_be_o is set for every byte that's covered by data_size_o. This would allows a simple way to choose PutPartialData vs PutFullData on the tilelink interface. However it may be the adapter would always emit PutPartialData anyway. I will open an OpenTitan issue to discuss this point.

GregAC avatar Apr 01 '20 10:04 GregAC

@davideschiavone @imphil @tomroberts-lowrisc @vogelpi @rswarbrick any thoughts?

GregAC avatar Apr 01 '20 10:04 GregAC

Possibly a silly question, but is it possible to have reads with "sparse" byte enables? (The top and bottom bytes, but not the ones in the middle). It seems like this would require two TileLink transactions, each of size 1. Is that correct?

rswarbrick avatar Apr 01 '20 10:04 rswarbrick

"Sparse" reads are not possible in TL-UL, and are currently not emitted by Ibex. I thought we had an assert for that in the adapter code somewhere but I can't find it any more, maybe it got removed or it was never there ...

imphil avatar Apr 01 '20 10:04 imphil

In which case, I guess another option would be to get rid of the byte enables entirely, changing Ibex's LSU to generate requests with size+address instead. Then it's not possible to encode the thing we don't support.

rswarbrick avatar Apr 01 '20 10:04 rswarbrick

If at all possible, it would be great to have a memory interface that matches the one used in other PULP-derived IP, e.g. CV32E40P (RISCY), Ariane, and the debug module.

@GregAC can we have a look at a potential AXI adapter as well to see if such an adapter has the same problem?

imphil avatar Apr 01 '20 10:04 imphil

FYI: For CV32E40P I wrote a document describing its bus interface in a bit more detail. Just to give it a name the protocol is now called OBI (Open Bus Interface). The pinout will stay identical to what it was on RI5CY, but certain combinatorial paths inside the core will (still need to) be fixed. The specification for OBI can now be found at https://github.com/openhwgroup/core-v-docs/blob/master/cores/cv32e40p/OBI-v1.0.pdf. In section 3.4.1 I tried to describe the legal byte enable values. I decided to forbid 'sparse reads' as it was currently not used and it eased the conversion to e.g. AHB.

Silabs-ArjanB avatar Apr 01 '20 10:04 Silabs-ArjanB

In which case, maybe Greg's original suggestion of adding data_size_o to the LSU makes the most sense. Then synthesis will throw away the generated byte enable registers if we're connecting it to a TileLink target.

rswarbrick avatar Apr 01 '20 10:04 rswarbrick

Opened OT related issue here https://github.com/lowRISC/opentitan/issues/1867

GregAC avatar Apr 01 '20 11:04 GregAC

Adding data_size_o is just a matter of wiring out the internal signal. Though probably re-encoding it too as it uses the reverse of tilelink (2'b00 == word where 2'b00 == byte in tilelink). Need to understand if there's an internal advantage in Ibex to that encoding, matches RISC-V instruction encoding maybe?

GregAC avatar Apr 01 '20 11:04 GregAC

I'm working on such processor based on AXI bus. I want to remove the AXI bus and replace it by an OBI interface for an ASIC implementation. My question is: is that possible to insert the OBI interface at top level and not into the load store unit and prefetch buffer for data and instruction? which means at the top, I'll have the OBI interface plus the ibex core. I'm doing that, but it is not possible to generate some inputs for this interface.

Shadi1987 avatar Sep 21 '21 08:09 Shadi1987