Repository navigation
libhns, util: Fix the aarch64 mmio_memcpy_x64() length check - #1851
Merged
Merged
Conversation
0z5a
force-pushed
the
fix/hns-dwqe-mmio-memcpy
branch
from
November 9, 2026 09:22
9fdec78 to
65c07fb
Compare
hginjgerx
reviewed
Nov 9, 2026
| static void hns_roce_write512(uint64_t *dest, uint64_t *val) | ||
| { | ||
| mmio_memcpy_x64(dest, val, sizeof(struct hns_roce_rc_sq_wqe)); | ||
| mmio_memcpy_x64(dest, val, 64); |
There was a problem hiding this comment.
I would prefer 1 << HNS_ROCE_SQWQE_SHIFT
Contributor
Author
There was a problem hiding this comment.
Done, and added your Reviewed-by and Tested-by to both patches. Thanks for testing!
|
I tested this PR on an hns hardware and it works well. Feel free to add: |
hns_roce_write512() passes sizeof(struct hns_roce_rc_sq_wqe), which is 32, to mmio_memcpy_x64(). It writes a 64-byte direct WQE only because the aarch64 macro takes its 64-byte path for any constant length. On the generic implementation a length of 32 makes the size_t counter wrap, and the copy does not stop. Pass the size of a direct WQE, 1 << HNS_ROCE_SQWQE_SHIFT. On aarch64 the generated code does not change. Fixes: 159933c ("libhns: Add support for direct wqe") Reviewed-by: Junxian Huang <[email protected]> Tested-by: Junxian Huang <[email protected]> Assisted-by: Claude:claude-opus-5-5 <[email protected]> Signed-off-by: Dezhen Lu <[email protected]>
__builtin_constant_p((bytecount) == 64) is true when the comparison has a known result, whatever that result is, so the macro writes only 64 bytes for any length the compiler can reason about. A literal 128 is truncated, and so is a variable after "if (n <= 64) return;". Take the 64-byte path only when the length is known to be 64. libhns was the one in-tree caller that relied on the old behaviour, and the previous patch removes that. Fixes: 159933c ("libhns: Add support for direct wqe") Reviewed-by: Junxian Huang <[email protected]> Tested-by: Junxian Huang <[email protected]> Assisted-by: Claude:claude-opus-5-5 <[email protected]> Signed-off-by: Dezhen Lu <[email protected]>
0z5a
force-pushed
the
fix/hns-dwqe-mmio-memcpy
branch
from
November 9, 2026 12:00
65c07fb to
b9c0359
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The aarch64
mmio_memcpy_x64()macro writes only 64 bytes for any length the compiler can reason about, because__builtin_constant_p((bytecount) == 64)tests whether the comparison's result is known.hns_roce_write512()passes 32 and relies on this to write its 64-byte direct WQE, so the libhns caller is fixed first and the macro second. Each commit is safe on its own.Reported to the linux-rdma mailing list on 2026-11-07; Message-ID:
<20261107231102.Horde.VTLAyGut_WGch71PWzwoxG_@webmail.uni-tuebingen.de>. Junxian Huang replied on the list that the proposed fix looks good and offered to test it on hns hardware.Validation on aarch64 (Neoverse V2, gcc 13.3), against
a70151dfe:.textin any library..textis unchanged in libhns, libmlx4 and libmlx5, also for those translation units at-O0to-Os.ionic_post_cmb_common(). The copy loops are still there in both.On x86_64 (gcc 13.3,
-DENABLE_WERROR=1) all three trees build without warnings. The libhns commit changes.textonly in libhns, and the macro commit changes no library. Before the libhns commit, the genericmmio_memcpy_x64()compileshns_roce_write_dwqe()into a loop with no exit, because 32 minus 64 in asize_tnever reaches 0. After it, the function is eight 8-byte stores.No hns hardware was available, so this has not run against a device.
buildlib/cbuildwas not run, and clang was only used on macOS with stub headers.I used Claude, an AI coding assistant, to find this, write the tests and draft the commits; both commits carry an Assisted-by tag.