enjoy-digital / litex

Build your hardware, easily!
Other
2.89k stars 555 forks source link

Separate address maps for CPU and SoC? #1154

Open sergachev opened 2 years ago

sergachev commented 2 years ago

Some hard CPU cores (Zynq-7000, EOS S3) do address translation on peripheral buses - say for the CPU addresses of a bus start at 0x40020000 and CSR_BASE C definition has to have that value, but the address output to that bus in the logic will have this base offset removed and start from 0x0 (so decoding using the offset done by LiteX by default breaks the access). I'm not sure what is the best way to deal with this - to manage a separate address space with all boundary checks etc or to simply redefine CSR_BASE in the C code or else. I solved it temporarily this way: https://github.com/sergachev/litex/commit/197a6353d597a4e06bab7d2af4e3e9dde31fabdd + https://github.com/sergachev/litex/commit/e93d543891599eb4c4b2b34ace4e158cfe19f437

trabucayre commented 2 years ago

for Zynq7000 I'm a bit asthonish, all my tests shows using same addr at CPU and logic side works perfectly. But It's true for EOS and ZynqMP FPGAs. I have doing a quite similar hack to you (before reading your commits) here The idea is to use in this specifics cases a dict to provides CPU addr (key) and logic addr (value). Result is working for both FPGAs. For all others devices using an int works without any modifications I don't know which, from both, is the best answer to this problem

sergachev commented 2 years ago

for Zynq7000 I'm a bit asthonish, all my tests shows using same addr at CPU and logic side works perfectly.

It depends on how do you assign CSR address in the SoC memory map: if you set it to the value valid for the CPU (smth like 0x43c0_0000) CSR_BASE in C will be correct but LiteX will generate a decoder which will prevent the bus from functioning normally. However if you set mem_map["csr"] to 0 (I think I've seen that somewhere in your code) you will get a correct bus connection without a decoder and if you then override CSR_BASE with the right value everything will work fine.

So I'm thinking that an easy temporary solution could be to just have an option to remove the bus decoder - given all current problematic situations require a zero base value on the logic side.

trabucayre commented 2 years ago

It's true, with master branch for both litex and litex-boards repositories, the communication is not working for redpitaya and arty_z7, But I have sucessfully used both boards, by the past. By moving litex to 14e0aeb92a80edab4665d802344ddcd1ec7ba86d and litex-boards to bb92bb00a829d783d62b80819621f0828d30c9f7 communication works.

By reading the verilog I see (in the flow order)

assign main_axi2wishbone_axi_lite2wishbone_r_addr = (main_axi2wishbone_ar_payload_addr - 31'd1073741824);
assign main_axi2wishbone_axi_lite2wishbone_w_addr = (main_axi2wishbone_aw_payload_addr - 31'd1073741824);
[...]
assign builder_slave_sel = (builder_shared_adr[29:14] == 15'd16384);

first two lines and last one are mutually exclusives.

I have added an hack similar to your in add_csr_bridge and in SoCRegion to have base_addr (PS side) and 0 in logic and now I'm able to communicate

So I think a simple bool at soc class level to fix decoding to 0 at SoCRegion level make sense.

Have to try with zynqmp and quicklogic

trabucayre commented 2 years ago

I have tested this PoC successfully for both zynq.

enjoy-digital commented 2 years ago

With https://github.com/enjoy-digital/litex/pull/1208 merged we can probably close this. Thanks @sergachev, @trabucayre.