amaranth-lang
AXI Interface Signature
There should be a universal AXI Signature that can be used across the Amaranth ecosystem.
I'm planning on writing an AXI Manager in Amaranth (which I might make open-source) and it would be nice if there is an agreed upon signature that I could reuse.
I saw this discussion about more general "advanced" streams: https://github.com/amaranth-lang/amaranth/discussions/1588 Maybe that discussion should precede this one?
A few issues/notes:
- AXI has a lot of different versions (AXI3,4,5 and AXI-Lite). How do we support those?
- AXI-Lite is a subset of the corresponding AXI Version
- I'm not sure if the different AXI versions are backward compatible?
- There are a lot of signals most of which are optional
- Do we "encode" optionality in the signature or do we just provide every signal? Some signals are only required by one side of the interface.
- Do we provide all possible signals or do we limit the interface to the most common ones? (e.g. there are signals for virtual memory over AXI, do we implement that or is that out of scope for this package?)
- The AXI interface is based on channels which work similarly to amaranth streams, so maybe they could be reused.
- The previously mentioned discussion also lists a bunch of cores that could be included in this package.
- I think before including those cores we should first agree on a signature that those cores should use. (That way they could still be used effectively even if they're not included in this package)
I would be willing to write an RFC for the signature. I am, however, not an "Amaranth expert" so the first iterations might be crude.
Prior work:
- https://github.com/apertus-open-source-cinema/naps/tree/f39373a808e6005dd2c154360b5eac045b015bb2/naps/cores/axi
- https://github.com/satellogic/nmigen-wb2axip/tree/master
- https://github.com/akukulanski/hdl-utils/tree/70e643a7fcd5ac1fa5b3ee98252ea53b5ec27e18/src/hdl_utils/amaranth_utils/interfaces
- SpinalHDL: https://spinalhdl.github.io/SpinalDoc-RTD/master/SpinalHDL/Libraries/Bus/amba4/axi4.html
The AXI Specification can be found here
Also tagging @anuejn and @ld-cd as they've shown some interest in this afaik. (I saw some discussion in the element channel, however that's probably not the best place to write down results/decisions/reasons)
- The AXI interface is based on channels which work similarly to amaranth streams, so maybe they could be reused.
I do believe that the Amaranth AXI implementation should be composed of the five streams.
Some additional prior work here and here, plus other bits throughout the codebase. I have not yet converted this codebase to Amaranth streams (though it uses Signatures) and will probably not have time soon.
Note that unfortunately this is a weird AXI3/4 hybrid cause that's what the relevant hardware used. But I do agree that the five streams should be separate and that's what I have done there.
- The AXI interface is based on channels which work similarly to amaranth streams, so maybe they could be reused.
I do believe that the Amaranth AXI implementation should be composed of the five streams.
Agreed that this would be ideal. For a top module exported as verilog you probably want the individual lines in the payload struct of each stream broken out as wires with the spec standard names so that other tools recognize it (I'm thinking specifically vivado as that's what I'm using my prototype interface with).
I couldn't figure out a clean way to have the payload struct field<->wire correspondence, but perhaps things have changed since then, that was 0.4 I was developing against I think
Another thing that's worth thinking about is if we want to support non standard groupings of features that make sense semantically but are nominally not allowed by the spec; I have a couple of these written down, I will check my notes when I'm back from this conference.
I think it probably makes sense to err on the side of being as strict as possible for a first implementation
https://gist.github.com/ld-cd/1b226ebeb5771f52e2e18ef823be57ab
Here's the draft signature I wrote which is explicitly AXI4 only and does not use streams, there's also an untested draft decoder thats probably not worth looking at
I nominally should have some time to work on this for work soon, but that's been true for a year at this point
I created a proof of concept for a version-independent signature: https://gist.github.com/jorolf/dd6a0bdd36b86126a1bf4ebecf75c990
It's similar to @ld-cd and @tpwrules implementation but (I think) I made it more expandable. Also, I based the naming on the AXI spec.
It's still missing a few things though:
- AXI3 and AXI5 specific signals are missing, but could be added easily
- I couldn't use the Amaranth stream signature as it's final and cannot be extended
- I don't think it's currently possible to connect 2 differing interfaces that should still be able to be connected together
- There are different requirements for managers and subordinates, e.g. the subordinate needs to support transaction IDs but the manager can choose to omit them
- The behavior if the signal is missing is specified afaik
- Also there are some more complex requirements that I didn't implement yet or forgot/missed
I updated the signature in the gist to use amaranth streams. The problem of payload struct field<->wire correspondence for the top module mentioned by ld-cd remains.
Zyp mentioned the possibility of a shim to split out the payload wire. However, I don't know if this is something that can be applied automatically or if it should even be supplied by the library?
@jorolf I suggest finding some time to discuss this proposal in a more synchronous way (text or voice), will you be available?
Note that formally speaking, the maintainer of Amaranth SoC is @jfng, but I haven't heard from him in a few months, so that's something else we'd have to resolve.
From a prelim glance I like @jorolf proposed interface better than mine, some notes follow below:
* AXI3 and AXI5 specific signals are missing, but could be added easily
AXI3 is different enough I think it should probably be a separate protocol if we ever want to support it other than via an adapter.
AXI5 is a strict superset of 4 IIRC and does not change the behavior of any of the 4 signals so should be straightforward to support.
* I couldn't use the Amaranth stream signature as it's final and cannot be extended
This is (iirc) intentional, but looks like this confusion has been resolved
* I don't think it's currently possible to connect 2 differing interfaces that should still be able to be connected together * There are different requirements for managers and subordinates, e.g. the subordinate needs to support transaction IDs but the manager can choose to omit them * The behavior if the signal is missing is specified afaik
You are correct that the behavior is fully specified when a subordinate is missing a signal, but I don't expect this situation to come up much internal to an amaranth module tree. I think that requiring the interface signatures to be the same for connections internal to the ecosystem is a reasonable choice, if a signal is truly unnecessary the logic optimizer should take care of it. For connections to external ✨IP✨ it looks like we'll have to provide a wrapping shim anyways and IMO taking care of this situation there would be an appropriate solution
* Also there are some more complex requirements that I didn't implement yet or forgot/missed
There are some configurations that are semantically sensible, and seem reasonably likely to exist in the wild, but are nominally banned by the spec. I would err on the side of explicitly banning them until we see a given one in the wild and decide to explicitly allow it if it makes sense, but this is probably an artistic decision that should be left to @whitequark and @jfng
I'm planning on taking the proposed signature for a spin and I'll give some feedback if I really get a chance to. I'm also happy to meet this week and am happy to host if need be. I'm US/Pacific and currently working the day shift
Ok I've taken the proposed interface for a spin (and forced an unsuspecting undergrad to make an AXI peripheral as well):
- Having the properties distinct from the signature feels right to me even though its different from how wishbone is handled. Given how many features AXI allows it might make sense to have this properties class constructed builder style. Most of my signature instantiantions end up looking like this.
- This brings up a second question of how Cat feels about the AXIProperty class used for verification which is a bit unlike the way diagnostics are handled in other parts of the language. I didn't mind it but I'm pretty familiar with the spec, and the student I was working with took a while to understand what was going on there and what was permitted/not permitted
- I think an alternative here could be a something like a builder pattern that checks as you construct, or just some explicit docs
- The main problem I felt while using this was integrating with external tools. In this case I'm building amaranth modules that plug into an existing block design in vivado (I wish I was not doing this but there's not a better alternative that I have time for atm). My (very much not ideal and not upstreamable in its current form) solution to the lack of a struct<->interface correspondence was a "standardized" signature for both the AXI bus and AXI-4 Streams which breaks out the signals using the standard naming conventions Vivado 2022.1 recognizes this properly but it introduces a trailing underscore in the interface name which vivado shits the bed on for axi buses (but works for streams). Because of this I end up patching the generated verilog with sed
- I have since realized that I can add attributes to signals which is probably a better way around this but would be vendor by vendor
- This is also a very specific use case and I don't think it necessarily needs to be resolved in a first impl
As an example of what peripherals look like using this interface, I have a (wip) AXILite -> CSR bridge and a basic stream DMA
I'm happy to open a PR either here or as an RFC depending on what the current process is for soc
Thanks for trying out the signature!
Given how many features AXI allows it might make sense to have this properties class constructed builder style.
I agree that there should be a more concise way to set several properties. There are also some properties which depend on others like USER_DATA_WIDTH which has a max width of DATA_WIDTH / 2 iirc. Maybe a builder could help here. Although I believe that implementing such a builder is going to lead to more verbose code.
verification which is a bit unlike the way diagnostics are handled in other parts of the language
I thought that the "dataclass way" would be the most concise way to add all property requirements. Combining the declaration with the value range and default value also makes it harder to forget something. What do you mean by "other parts of the language"?
the student I was working with took a while to understand what was going on there and what was permitted/not permitted
Adding some explicit documentation what each signal/property does probably would've helped. However I don't think it should be amaranth's responsibility to explain how AXI works.
I think an alternative here could be a something like a builder pattern that checks as you construct
Could you elaborate that a bit more? I don't think that just replacing the dataclass with a builder is going to meaningfully change the diagnostics?
In this case I'm building amaranth modules that plug into an existing block design in vivado
I implemented a shim because I have the same use case! I'm not sure how usable it is outside of my project but I uploaded it anyway:
https://gist.github.com/jorolf/c3c88972a98efdf59835582cc480f1b4
Although you need to either remove the AXI-Stream shim or adjust it to your own version. After that you can simply write Shim(component) to get a shimmed component.
Regarding a meeting or just questions, feel free to message me on matrix.
Hey @ld-cd, I've tried out your AXI to CSR bridge and found an issue: The write response valid signal is always set to valid. However, the AXI spec specifically says that the signal must only be valid after a completed write address and write data handshake. (It actually locked up my AXI bus which meant I had to restart the system.)
I did like your pipelining idea and implemented my own bridge which has a configurable amount of pipeline stages: https://gist.github.com/jorolf/636d31fec2f626eda5d9a90247e90734
Although, for Vivado, it didn't have much impact. Adding more register stages to a bridge connected to a large register bank didn't improve timings even though the critical path was still in the CSR mux.