| Message ID | 20220127162127.2391947-1-james.morse@arm.com |
|---|---|
| Headers | show |
| Series | arm64: insn: Generate 64 bit mask immediates correctly | expand |
On Thu, 27 Jan 2022 16:21:24 +0000, James Morse wrote: > aarch64_insn_gen_logical_immediate() is generating the wrong code if > it is handed a 64bit immediate which has a single span of 1s (i.e. a > mask), with bit 63 set, and 0s in the remaining upper 32 bits. > Clear as mud. An example always helps: 0x800000003fffffff would be wrongly > encoded, but 0x000000003fffffff is unaffected. > > It would appear eBPF is unable to hit these cases, as build_insn()'s > imm value is a s32, so when used with BPF_ALU64, the sign-extended > u64 immediate would always have all-1s or all-0s in the upper 32 bits. > > [...] Applied to arm64 (for-next/insn), thanks! [1/3] arm64: selftests: Generate all the possible logical immediates as a header (no commit info) [2/3] arm64: insn: Add tests for aarch64_insn_gen_logical_immediate() (no commit info) [3/3] arm64: insn: Generate 64 bit mask immediates correctly https://git.kernel.org/arm64/c/a6aab0188299 Cheers,
Hello, aarch64_insn_gen_logical_immediate() is generating the wrong code if it is handed a 64bit immediate which has a single span of 1s (i.e. a mask), with bit 63 set, and 0s in the remaining upper 32 bits. Clear as mud. An example always helps: 0x800000003fffffff would be wrongly encoded, but 0x000000003fffffff is unaffected. It would appear eBPF is unable to hit these cases, as build_insn()'s imm value is a s32, so when used with BPF_ALU64, the sign-extended u64 immediate would always have all-1s or all-0s in the upper 32 bits. KVM does not generate a va_mask with any of the top bits set as these VA wouldn't be usable with TTBR0_EL2. Patch 3 fixes it, and doesn't depend on the rest of the series. As the instruction encoder is a source of headaches, the first two patches add tests to help illustrate there is a problem, and that patch 3 fixes it. The tests generate a header file of the expected values so it can be compared against other sources of the same information. Objdump can be used to check the header file is generated correctly. Embedding the code in gen_logic_imm in test_insn.c would give less confidence that the encoder is doing the right thing. This series is based on v5.17-rc1, and can be retrieved from: https://git.gitlab.arm.com/linux-arm/linux-jm.git insn_encoder/fls_bug/v1 Thanks, James Morse (3): arm64: selftests: Generate all the possible logical immediates as a header arm64: insn: Add tests for aarch64_insn_gen_logical_immediate() arm64: insn: Generate 64 bit mask immediates correctly arch/arm64/Kconfig.debug | 3 + arch/arm64/Makefile | 3 + arch/arm64/lib/Makefile | 2 + arch/arm64/lib/insn.c | 5 +- arch/arm64/lib/test_insn.c | 90 ++++++++++ arch/arm64/tools/.gitignore | 2 + arch/arm64/tools/Makefile | 12 +- arch/arm64/tools/gen_logic_imm.c | 190 +++++++++++++++++++++ tools/testing/selftests/arm64/Makefile | 2 +- tools/testing/selftests/arm64/lib/Makefile | 6 + tools/testing/selftests/arm64/lib/config | 1 + tools/testing/selftests/arm64/lib/insn.sh | 5 + 12 files changed, 318 insertions(+), 3 deletions(-) create mode 100644 arch/arm64/lib/test_insn.c create mode 100644 arch/arm64/tools/.gitignore create mode 100644 arch/arm64/tools/gen_logic_imm.c create mode 100644 tools/testing/selftests/arm64/lib/Makefile create mode 100644 tools/testing/selftests/arm64/lib/config create mode 100755 tools/testing/selftests/arm64/lib/insn.sh