Received-SPF: pass (google.com: domain of linux-kernel+bounces-186151-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <922473a0-7e74-45e8-9929-154d0590d124@linux.ibm.com>
Date: Wed, 22 May 2024 17:07:28 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/5] powerpc64/bpf: jit support for unconditional byte
 swap
To: Artem Savkov <asavkov@redhat.com>, Michael Ellerman <mpe@ellerman.id.au>,
        Nicholas Piggin <npiggin@gmail.com>,
        Christophe Leroy <christophe.leroy@csgroup.eu>,
        "Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
        linuxppc-dev@lists.ozlabs.org
Cc: Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>, bpf@vger.kernel.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20240517075650.248801-1-asavkov@redhat.com>
 <20240517075650.248801-3-asavkov@redhat.com>
Content-Language: en-US
From: Hari Bathini <hbathini@linux.ibm.com>
In-Reply-To: <20240517075650.248801-3-asavkov@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


On 17/05/24 1:26 pm, Artem Savkov wrote:
> Add jit support for unconditional byte swap. Tested using BSWAP tests
> from test_bpf module.
> 
> Signed-off-by: Artem Savkov <asavkov@redhat.com>
> ---
>   arch/powerpc/net/bpf_jit_comp64.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
> index 3071205782b15..97191cf091bbf 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> @@ -699,11 +699,12 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, u32 *fimage, struct code
>   		 */
>   		case BPF_ALU | BPF_END | BPF_FROM_LE:
>   		case BPF_ALU | BPF_END | BPF_FROM_BE:

> +		case BPF_ALU64 | BPF_END | BPF_FROM_LE:

A comment here indicating this case does unconditional swap
could improve readability.

Other than this minor nit, the patchset looks good to me.
Also, tested the changes with test_bpf module and selftests.
For the series..

Reviewed-by: Hari Bathini <hbathini@linux.ibm.com>

>   #ifdef __BIG_ENDIAN__
>   			if (BPF_SRC(code) == BPF_FROM_BE)
>   				goto emit_clear;
>   #else /* !__BIG_ENDIAN__ */
> -			if (BPF_SRC(code) == BPF_FROM_LE)
> +			if (BPF_CLASS(code) == BPF_ALU && BPF_SRC(code) == BPF_FROM_LE)
>   				goto emit_clear;
>   #endif
>   			switch (imm) {