Re: [PATCH v10 0/8] powerpc/bpf: address missing verifier selftest coverage

On 8/6/26 11:04 PM, Hari Bathini wrote:
>
>
> On 05/08/26 11:36 am, Saket Kumar Bhaskar wrote:
>> From: Abhishek Dubey <[email protected]>
>>
>> The verifier selftest validates JITed instructions by matching expected
>> disassembly output. The first two patches fix issues in powerpc 
>> instruction
>> disassembly that were causing test flow failures. The fix is common for
>> 64-bit & 32-bit powerpc. Add support for the powerpc-specific 
>> "__powerpc64"
>> architecture tag in the third patch, enabling proper test filtering in
>> verifier test files. Introduce verifier testcases for tailcalls on 
>> powerpc64.
>>
>> The first patch in series is fix patch, correcting memory alignment with
>> 8-byte boundary for long branch address field. The subsequent patches
>> enables verifier selftests on powerpc. The fifth patch in the series 
>> fixes
>> incorrect comparator usage for comparing tailcall info with tailcall
>> threshold. The last two patches fix JIT buffer overflow for large BPF 
>> progs
>> and private stack memory leak (identified by bot during reviews).
>>
>> Issue Details:
>> --------------
>>
>>      The Long branch stub in the trampoline implementation[1] provides
>>      flexibility to handles short as well as long branch distance to
>>      actual trampoline. Whereas, the 8 bytes long dummy_tramp_addr field
>>      sitting before long branch stub leads to failure when enabling
>>      verifier based seltest for ppc64.
>>           The verifier selftests require disassembing the final jited 
>> image
>>      to get native instructions. Later the disassembled instruction
>>      sequence is matched against sequence of instructions provided in
>>      test-file under __jited() wrapper. The final jited image contains
>>      Out-of-line stub and Long branch stub as part of epilogue jitting
>>      for a bpf program. The 8 bytes space for dummy_tramp is sandwiched
>>      between both above mentioned stubs. These 8 bytes contain memory
>>      address of dummy trampoline during trampoline invocation which 
>> don't
>>      correspond to any powerpc instructions. So, disassembly fails
>>      resulting in failure of verifier selftests.
>>           The following code snippet shows the problem with current 
>> arrangement
>>      made for dummy_tramp_addr.
>>           /* Out-of-line stub */
>>      mflr    r0
>>      [b|bl]  tramp
>>      mtlr    r0 //only with OOL
>>      b       bpf_func + 4
>>      /* Long branch stub */
>>      .long   <dummy_tramp_addr>  <---Invalid bytes sequence, 
>> disassembly fails
>>      mflr    r11
>>      bcl     20,31,$+4
>>      mflr    r12
>>      ld      r12, -8-SZL(r12)
>>      mtctr   r12
>>      mtlr    r11 //retain ftrace ABI
>>      bctr
>>
>>      Consider test program binary of size 112 bytes:
>>      0:  00000060 10004de8 00002039 f8ff21f9 81ff21f8 7000e1fb 3000e13b
>>      28: 3000e13b 2a006038 f8ff7ff8 00000039 7000e1eb 80002138 7843037d
>>      56: 2000804e a602087c 00000060 a603087c bcffff4b c0341d00 000000c0
>>      84: a602687d 05009f42 a602887d f0ff8ce9 a603897d a603687d 2004804e
>>
>>      Disassembly output of above binary for ppc64le:
>>      pc:0     left:112    00 00 00 60  :  nop
>>      pc:4     left:108    10 00 4d e8  :  ld 2, 16(13)
>>      pc:8     left:104    00 00 20 39  :  li 9, 0
>>      pc:12    left:100    f8 ff 21 f9  :  std 9, -8(1)
>>      pc:16    left:96     81 ff 21 f8  :  stdu 1, -128(1)
>>      pc:20    left:92     70 00 e1 fb  :  std 31, 112(1)
>>      pc:24    left:88     30 00 e1 3b  :  addi 31, 1, 48
>>      pc:28    left:84     30 00 e1 3b  :  addi 31, 1, 48
>>      pc:32    left:80     2a 00 60 38  :  li 3, 42
>>      pc:36    left:76     f8 ff 7f f8  :  std 3, -8(31)
>>      pc:40    left:72     00 00 00 39  :  li 8, 0
>>      pc:44    left:68     70 00 e1 eb  :  ld 31, 112(1)
>>      pc:48    left:64     80 00 21 38  :  addi 1, 1, 128
>>      pc:52    left:60     78 43 03 7d  :  mr    3, 8
>>      pc:56    left:56     20 00 80 4e  :  blr
>>      pc:60    left:52     a6 02 08 7c  :  mflr 0
>>      pc:64    left:48     00 00 00 60  :  nop
>>      pc:68    left:44     a6 03 08 7c  :  mtlr 0
>>      pc:72    left:40     bc ff ff 4b  :  b .-68
>>      pc:76    left:36     c0 34 1d 00  :
>>      ...
>>
>>      Failure log:
>>      Can't disasm instruction at offset 76: c0 34 1d 00 00 00 00 c0 
>> a6 02 68 7d 05 00 9f 42
>>      --------------------------------------
>>
>>      Observation:
>>      Can't disasm instruction at offset 76 as this address has
>>      ".long <dummy_tramp_addr>" (0xc0341d00000000c0)
>>      But valid instructions follow at offset 84 onwards.
>>
>>      Move the long branch address space to the bottom of the long
>>      branch stub. This allows uninterrupted disassembly until the
>>      last 8 bytes. Exclude these last bytes from the overall
>>      program length to prevent failure in assembly generation.
>>
>>      Following is disassembler output for same test program with 
>> moved down
>>      dummy_tramp_addr field:
>>      .....
>>      .....
>>      pc:68    left:44     a6 03 08 7c  :  mtlr 0
>>      pc:72    left:40     bc ff ff 4b  :  b .-68
>>      pc:76    left:36     a6 02 68 7d  :  mflr 11
>>      pc:80    left:32     05 00 9f 42  :  bcl 20, 31, .+4
>>      pc:84    left:28     a6 02 88 7d  :  mflr 12
>>      pc:88    left:24     14 00 8c e9  :  ld 12, 20(12)
>>      pc:92    left:20     a6 03 89 7d  :  mtctr 12
>>      pc:96    left:16     a6 03 68 7d  :  mtlr 11
>>      pc:100   left:12     20 04 80 4e  :  bctr
>>      pc:104   left:8      c0 34 1d 00  :
>>
>>      Failure log:
>>      Can't disasm instruction at offset 104: c0 34 1d 00 00 00 00 c0
>>      ---------------------------------------
>>      Disassembly logic can truncate at 104, ignoring last 8 bytes.
>>
>>      Update the dummy_tramp_addr field offset calculation from the end
>>      of the program to reflect its new location, for 
>> bpf_arch_text_poke()
>>      to update the actual trampoline's address in this field.
>>
>>      [1] 
>> https://lore.kernel.org/all/[email protected]
>>
>> v9->v10:
>>    Modified expected JIT instruction in selftest for
>>    CONFIG_PPC_KERNEL_PCREL incase of PPC_LI64 instruction.
>>    Fix for stale entries in exception table
>>    Update ARCH_POWERPC64 enum value to 0x40 to resolve rebase
>>    conflict with ARCH_LOONGARCH (0x20).
>>
>> v8->v9:
>>    Dynamic pass handling until code keeps shrinking
>>    Fix private stack memory leak
>>
>> v7->v8:
>>    Fixed bot identified issues of alt_exit_addr and BPF_EXIT
>>    Fixed 32-bit ppc function signature mismatch
>>
>> v6->v7:
>>    Fixed JIT buffer overflow in case of large BPF progs
>>    Addressed remaining bot comments
>>
>> v5->v6:
>>    Changed alignment NOP emittion dependency on fimage layout
>>    Adjust tail truncate length for 32-bit ppc
>>    Addressed few minor bot comments
>>
>> v4->v5:
>>    Handled alignment NOP emit logic and corresponding stub offsets
>>    Handled image buffer overflow problem in last pass
>>    Above changes took care of other bot reviews
>>    Included LLVMDisposeMessage() for graceful freeing
>>    Adjusted parameters in bpf_jit_build_fentry_stubs for ppc32
>>    Adjusted expected JIT inst. in tailcall test for
>> CONFIG_PPC_KERNEL_PCREL config
>>    Added fix patch at last for inaccurate use of cmplwi inst.
>>
>> v3->v4:
>>    Changed logic for emitting alignment NOP
>>
>> v2->v3:
>>    Removed fixed NOP from bottom of long branch stub
>>    Rebased on top of bpf-next
>>
>> v1->v2:
>>    Added fix-patch to correct memory alignment in-place
>>    Moved the optional alignmnet NOP before OOL stub
>>
>> [v1]: 
>> https://lore.kernel.org/bpf/[email protected]
>> [v2]: 
>> https://lore.kernel.org/bpf/[email protected]
>> [v3]: 
>> https://lore.kernel.org/bpf/[email protected]
>> [v4]: 
>> https://lore.kernel.org/bpf/[email protected]
>> [v5]: 
>> https://lore.kernel.org/bpf/[email protected]
>> [v6]: 
>> https://lore.kernel.org/bpf/[email protected]
>> [v7]: 
>> https://lore.kernel.org/bpf/[email protected]
>> [v8]: 
>> https://lore.kernel.org/bpf/[email protected]
>> [v9]: 
>> https://lore.kernel.org/bpf/[email protected]/
>>
>> Abhishek Dubey (8):
>>    powerpc/bpf: fix alignment of long branch trampoline addres to t
>>    powerpc/bpf: Move out dummy_tramp_addr after Long branch stub
>>    selftest/bpf: Fixing powerpc JIT disassembly failure
>>    selftest/bpf: Enable verifier selftest for powerpc64
>>    powerpc64/bpf: fix compare instruction emitted for tailcall
>>    selftest/bpf: Add tailcall verifier selftest for powerpc64
>>    powerpc/bpf: fix buffer overflow in JIT for large BPF programs
>>    powerpc64/bpf: fix percpu private stack leak on JIT failure
>>
>>   arch/powerpc/net/bpf_jit.h                    |  20 +++-
>>   arch/powerpc/net/bpf_jit_comp.c               | 102 +++++++++++++-----
>>   arch/powerpc/net/bpf_jit_comp32.c             |   7 +-
>>   arch/powerpc/net/bpf_jit_comp64.c             |  15 +--
>>   .../selftests/bpf/jit_disasm_helpers.c        |  25 ++++-
>>   tools/testing/selftests/bpf/progs/bpf_misc.h  |   1 +
>>   .../bpf/progs/verifier_tailcall_jit.c         |  74 +++++++++++++
>>   tools/testing/selftests/bpf/test_loader.c     |   5 +
>>   8 files changed, 209 insertions(+), 40 deletions(-)
>>
>
> Alexei, Maddy,
>
> Given this has changes in common code too, should this go via bpf tree?
> If yes, @Saket can you post the next revision with bpf tag to make
> it explicit...

If I get an ack from Alexei, I can take this via ppc tree.

Maddy

>
> - Hari