Re: [PATCH 5/5] powerpc64/bpf: fix handling of BPF stack in exception callback

[Hari Bathini <hbathini@linux.ibm.com>]

On 16/02/26 1:10 pm, bot+bpf-ci@kernel.org wrote:
>> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
>> --- a/arch/powerpc/net/bpf_jit_comp64.c
>> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> 
> [ ... ]
> 
>> @@ -287,6 +285,22 @@
>>   		 * program(main prog) as third arg
>>   		 */
>>   		EMIT(PPC_RAW_MR(_R1, _R5));
>> +		/*
>> +		 * Exception callback reuses the stack frame of exception boundary.
>> +		 * But BPF stack depth of exception callback and exception boundary
>> +		 * don't have to be same. If BPF stack depth is different, adjust the
>> +		 * stack frame size considering BPF stack depth of exception callback.
>> +		 * The non-volatile register save area remains unchanged. These non-
>> +		 * volatile registers are restored in exception callback's epilogue.
>> +		 */
>> +		EMIT(PPC_RAW_LD(bpf_to_ppc(TMP_REG_1), _R5, 0));
>> +		EMIT(PPC_RAW_SUB(bpf_to_ppc(TMP_REG_2), _R1, bpf_to_ppc(TMP_REG_1)));
> 
> Are the operands to PPC_RAW_SUB reversed here? At this point R1 holds
> boundary_sp (set by the MR above) and TMP_REG_1 holds prev_sp (loaded
> from [R5+0]). Since the stack grows downward, boundary_sp < prev_sp.
> 

> PPC_RAW_SUB(d, a, b) computes d = a - b, confirmed by how BPF_SUB is
> emitted in bpf_jit_build_body():
> 
>      case BPF_ALU64 | BPF_SUB | BPF_X: /* dst -= src */
>          EMIT(PPC_RAW_SUB(dst_reg, dst_reg, src_reg));

That was a good catch.
"subf Rx,Ry,Rz" vs "sub Rx,Rz,Ry" distinction on how the operation
is interpreted. Will respin.

- Hari