[vendor, hw, sw] Modify axi_to_mem to support CHERI and integrate it#390
[vendor, hw, sw] Modify axi_to_mem to support CHERI and integrate it#390thommythomaso wants to merge 4 commits intolowRISC:mainfrom
Conversation
raylau1
left a comment
There was a problem hiding this comment.
Thanks for the good work! I'm glad to see verilator and FPGA tests passing in CI and dvsim tests passing locally on my side. Note that I'll be on vacation and won't be able to give further reviews next week.
| if (DataWidth == 32'd64) begin : gen_DW64_w_cap_checks | ||
| assign is_w_cap_sized = (meta.size == 3'd3) & (meta.len == 8'd1); | ||
| assign is_w_cap_aligned = meta.last ? meta.addr[3:0] == 4'd8 : meta.addr[3:0] == 4'd0; | ||
| end else if (DataWidth == 32'd128) begin : gen_DW128_w_cap_checks | ||
| assign is_w_cap_sized = (meta.size == 3'd4) & (meta.len == 8'd0); | ||
| assign is_w_cap_aligned = meta.addr[3:0] == 4'd0; |
There was a problem hiding this comment.
This will work fine in the current cache-less system but will malfunction with accesses which contain more than 1 capabilities after we integrate caches. I'm not sure if that should be left as a future enhancement. @marnovandermaas what do you think?
There was a problem hiding this comment.
I am fully aware of this limitation. How to handle (longer) bursts (of capabilities or mixed data) will be a fundamental question. Currently the burden of detecting valid capability accesses lays with the SRAM controller (which I personally don't find a good idea); adding bursts will make this detection much more complex.
In the presence of a more sophisticated cache, which fetches more than 128 bit at a time, the cache has to ensure to handle sub-capability accesses properly and invalidating them. If this is the case, we can safely remove these checks.
Co-authored-by: Ray Lau <ray.lau@lowrisc.org>
6b1d9f1 to
d8bbfcd
Compare
Addresses #343. Updates #358.
Possible optimizations:
r.userandw.userfrom logic to a packed struct (see Review AXI crossbar and SRAM tagging #338)