Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix WDATA and WSTRB to work with unaligned addresses #1109

Closed
Tracked by #1072
nathanielnrn opened this issue Jul 18, 2022 · 0 comments · Fixed by #1110
Closed
Tracked by #1072

Fix WDATA and WSTRB to work with unaligned addresses #1109

nathanielnrn opened this issue Jul 18, 2022 · 0 comments · Fixed by #1110

Comments

@nathanielnrn
Copy link
Contributor

I just took a look at this and it seems like the issue is that unaligned writes work differently than I expected. https://community.arm.com/support-forums/f/architectures-and-processors-forum/3518/does-an-axi4-master-have-to-assert-the-correct-wstrb-for-unaligned-transfers

The important note here is that axi slaves are allowed to ignore unaligned access information, and therefore wstrb and wdata have to be consistent with the case when the address is aligned to the nearest 512 bit location (same as dropping the bottom bits of the address). So in the current implementation, the valid wdata data is always located at WDATA[31:0], causing the data to get overwritten every time.

This seems like a very weird way for unaligned transfers to work (in particular, it is different from how unaligned reads work), but it is stated clearly in the AXI4 documentation. I think it works this way to simplify bursting, but I'm not quite sure.

Fortunately, there is an easy fix here. I just changed the way WDATA and WSTRB are set as follows:

    assign WDATA = {{15{32'b0}}, bram_read_data} << (send_addr_offset * data_element_width);
    assign WSTRB = {{15{4'h0}}, 4'hF} << (send_addr_offset * (data_element_width / 8));

so for example in this case where the data_element_width is 32 bits I set WDATA and WSTRB as follows:

    assign WDATA = {{15{32'b0}}, bram_read_data} << (send_addr_offset * 32);
    assign WSTRB = {{15{4'h0}}, 4'hF} << (send_addr_offset * 4);

Hopefully that all makes sense. I only tested this in xsim, so Adrian's point about the kernel.xml might also be correct, I haven't tried it to see if it actually makes a difference.

Originally posted by @andrewb1999 in #1022 (reply in thread)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant