Add Axi bus behind the DMA engine #1878
Conversation
ArthurHeymans
requested review from
korran,
fdamato,
jlmahowa-amd,
mhatrevi,
jhand2,
swenson,
vsonims,
ajisaxena,
JohnTraverAmd and
rusty1968
as code owners
sw-emulator/lib/types/src/axi_bus.rs
Outdated
|
|
||
| /// Represents an abstract AXI memory bus. Used to read and write from RAM and | ||
| /// peripheral addresses. | ||
| pub trait AxiBus { |
There was a problem hiding this comment.
I absolutely hate this copy-paste. Why didn't we just increase the address size to 64-bit everywhere?
There was a problem hiding this comment.
(assuming the only reason for this change is the increased word size): my preference:
- Use 64-bit addresses and data in Bus trait everywhere; the 32-bit parts could ignore the high bits and error when size is 64-bit.
- If that's not workable for some reason we could add associated types to the existing bus trait:
trait Bus {
type Addr: num_traits::PrimInt + num_traits::Unsigned + From<u32>;
type Data: num_traits::PrimInt + num_traits::Unsigned + From<u32>;
fn read(&mut self, addr: Self::Addr) -> Result<Self::Data, ()>;
// ...
}There was a problem hiding this comment.
poll and reset() could be moved to their own separate trait if necessary.
There was a problem hiding this comment.
I agree I don't love the copy-pasta, especially for the derive macros and the register types.
I like the idea of 64-bit everywhere, maybe either with a type parameter or a function that specifies whether 64-bit is allowed.
Another idea that I think would be okay: keep the separate AxiBus trait for read64/write64, but don't support derive. Instead, use autogeneration to automatically create the AxiBus implementation. (This is the approach we are mostly using in the MCU, for example: /~https://github.com/chipsalliance/caliptra-mcu-sw/blob/main/registers/generated-emulator/src/root_bus.rs).
Or only support the dynamic bus.
Since the AXI bus doesn't need to be quite as flexible as the main bus, I think it might be okay to go the autogenerated route.
There was a problem hiding this comment.
@korran both these approaches require a lot of code changes all across the emulator code.
I think the simplest things is to implement the toplevel axi_root_bus by hand, which dispatches to Bus. This removes the possibility to do 64bit load/store, but I don't think those are needed.
ArthurHeymans
force-pushed
the
AxiBus
branch
3 times, most recently
from
25a9e87 to
ba108e4
Compare
| match addr { | ||
| Self::TEST_REG_OFFSET => return Register::read(&self.reg, size), | ||
| Self::RECOVERY_REGISTER_INTERFACE_OFFSET..=Self::RECOVERY_REGISTER_INTERFACE_END => { | ||
| let addr = (addr - Self::RECOVERY_REGISTER_INTERFACE_OFFSET) as RvAddr; |
There was a problem hiding this comment.
Why is the address subtracted from Self::RECOVERY_REGISTER_INTERFACE_OFFSET?
There was a problem hiding this comment.
That's how busses work in the sw-emulator: All peripherals gets load and stores values that are relative to their own base.
There was a problem hiding this comment.
Btw derive Bus macro results in very similar code.
ArthurHeymans
force-pushed
the
AxiBus
branch
2 times, most recently
from
1707e05 to
0879541
Compare
ArthurHeymans
force-pushed
the
AxiBus
branch
2 times, most recently
from
13b95e9 to
5ea6b54
Compare
There was confusion on how that the dma engine operated on a axi bus that replaced the apb bus. This is not true. AXI is a completely separate bus and the DMA widget is a bridge to it. Signed-off-by: Arthur Heymans <arthur.heymans@9elements.com>
DMA operates on a separate bus and not the root bus so remove it. Signed-off-by: Arthur Heymans <arthur.heymans@9elements.com>
This rewrites the dma widget to work with a separate axi_root_bus bus. Signed-off-by: Arthur Heymans <arthur.heymans@9elements.com>
This also adds the offset in the soc_ifc registers. Signed-off-by: Arthur Heymans <arthur.heymans@9elements.com>
Reading the production debug fuses needs this. Signed-off-by: Arthur Heymans <arthur.heymans@9elements.com>
Signed-off-by: Arthur Heymans <arthur.heymans@9elements.com>
Don't use memcpy as this results in unaligned reads on the DMA FIFO read/write registers. Signed-off-by: Arthur Heymans <arthur.heymans@9elements.com>
Done |
Addressed it all. I also added some driver and other fixes that should make your test branch get further. |
This rewrites the DMA engine to operate on the Axi bus which is a 64bit bus.
The recovery register interface is placed on that bus.
An Axi address offset is placed in soc_ifc.