mbox series

[0/3] arm64: insn: Generate 64 bit mask immediates correctly

Message ID 20220127162127.2391947-1-james.morse@arm.com (mailing list archive)
Headers show
Series arm64: insn: Generate 64 bit mask immediates correctly | expand

Message

James Morse Jan. 27, 2022, 4:21 p.m. UTC
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

Comments

Will Deacon Feb. 15, 2022, 11:18 p.m. UTC | #1
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,