diff mbox series

[RESEND,bpf-next,v3,4/6] riscv, bpf: Add necessary Zbb instructions

Message ID 20240115131235.2914289-5-pulehui@huaweicloud.com (mailing list archive)
State Accepted
Commit 647b93f65daa128d9a0e4aac744a5fcf5f58b2d2
Delegated to: BPF
Headers show
Series Zbb support and code simplification for RV64 JIT | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next
netdev/ynl success SINGLE THREAD; Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers success CCed 0 of 0 maintainers
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch warning WARNING: line length of 97 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-18 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-14 fail Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-13 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-15 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat

Commit Message

Pu Lehui Jan. 15, 2024, 1:12 p.m. UTC
From: Pu Lehui <pulehui@huawei.com>

Add necessary Zbb instructions introduced by [0] to reduce code size and
improve performance of RV64 JIT. Meanwhile, a runtime deteted helper is
added to check whether the CPU supports Zbb instructions.

Link: https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf [0]
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
 arch/riscv/net/bpf_jit.h | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Comments

Björn Töpel Jan. 27, 2024, 5:16 p.m. UTC | #1
Pu Lehui <pulehui@huaweicloud.com> writes:

> From: Pu Lehui <pulehui@huawei.com>
>
> Add necessary Zbb instructions introduced by [0] to reduce code size and
> improve performance of RV64 JIT. Meanwhile, a runtime deteted helper is
> added to check whether the CPU supports Zbb instructions.
>
> Link: https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf [0]
> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> ---
>  arch/riscv/net/bpf_jit.h | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
> index e30501b46f8f..51f6d214086f 100644
> --- a/arch/riscv/net/bpf_jit.h
> +++ b/arch/riscv/net/bpf_jit.h
> @@ -18,6 +18,11 @@ static inline bool rvc_enabled(void)
>  	return IS_ENABLED(CONFIG_RISCV_ISA_C);
>  }
>  
> +static inline bool rvzbb_enabled(void)
> +{
> +	return IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && riscv_has_extension_likely(RISCV_ISA_EXT_ZBB);

Hmm, I'm thinking about the IS_ENABLED(CONFIG_RISCV_ISA_ZBB) semantics
for a kernel JIT compiler.

IS_ENABLED(CONFIG_RISCV_ISA_ZBB) affects the kernel compiler flags.
Should it be enough to just have the run-time check? Should a kernel
built w/o Zbb be able to emit Zbb from the JIT?


Björn
Pu Lehui Jan. 29, 2024, 9:13 a.m. UTC | #2
On 2024/1/28 1:16, Björn Töpel wrote:
> Pu Lehui <pulehui@huaweicloud.com> writes:
> 
>> From: Pu Lehui <pulehui@huawei.com>
>>
>> Add necessary Zbb instructions introduced by [0] to reduce code size and
>> improve performance of RV64 JIT. Meanwhile, a runtime deteted helper is
>> added to check whether the CPU supports Zbb instructions.
>>
>> Link: https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf [0]
>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>> ---
>>   arch/riscv/net/bpf_jit.h | 32 ++++++++++++++++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>>
>> diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
>> index e30501b46f8f..51f6d214086f 100644
>> --- a/arch/riscv/net/bpf_jit.h
>> +++ b/arch/riscv/net/bpf_jit.h
>> @@ -18,6 +18,11 @@ static inline bool rvc_enabled(void)
>>   	return IS_ENABLED(CONFIG_RISCV_ISA_C);
>>   }
>>   
>> +static inline bool rvzbb_enabled(void)
>> +{
>> +	return IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && riscv_has_extension_likely(RISCV_ISA_EXT_ZBB);
> 
> Hmm, I'm thinking about the IS_ENABLED(CONFIG_RISCV_ISA_ZBB) semantics
> for a kernel JIT compiler.
> 
> IS_ENABLED(CONFIG_RISCV_ISA_ZBB) affects the kernel compiler flags.
> Should it be enough to just have the run-time check? Should a kernel
> built w/o Zbb be able to emit Zbb from the JIT?
> 

Not enough, because riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) is a 
platform capability check, and the other one is a kernel image 
capability check. We can pass the check 
riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) when 
CONFIG_RISCV_ISA_ZBB=n. And my local test prove it.

> 
> Björn
Andrew Jones Jan. 29, 2024, 11:43 a.m. UTC | #3
On Sat, Jan 27, 2024 at 06:16:41PM +0100, Björn Töpel wrote:
> Pu Lehui <pulehui@huaweicloud.com> writes:
> 
> > From: Pu Lehui <pulehui@huawei.com>
> >
> > Add necessary Zbb instructions introduced by [0] to reduce code size and
> > improve performance of RV64 JIT. Meanwhile, a runtime deteted helper is
> > added to check whether the CPU supports Zbb instructions.
> >
> > Link: https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf [0]
> > Signed-off-by: Pu Lehui <pulehui@huawei.com>
> > ---
> >  arch/riscv/net/bpf_jit.h | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >
> > diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
> > index e30501b46f8f..51f6d214086f 100644
> > --- a/arch/riscv/net/bpf_jit.h
> > +++ b/arch/riscv/net/bpf_jit.h
> > @@ -18,6 +18,11 @@ static inline bool rvc_enabled(void)
> >  	return IS_ENABLED(CONFIG_RISCV_ISA_C);
> >  }
> >  
> > +static inline bool rvzbb_enabled(void)
> > +{
> > +	return IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && riscv_has_extension_likely(RISCV_ISA_EXT_ZBB);
> 
> Hmm, I'm thinking about the IS_ENABLED(CONFIG_RISCV_ISA_ZBB) semantics
> for a kernel JIT compiler.
> 
> IS_ENABLED(CONFIG_RISCV_ISA_ZBB) affects the kernel compiler flags.
> Should it be enough to just have the run-time check? Should a kernel
> built w/o Zbb be able to emit Zbb from the JIT?
>

My two cents (which might be worth less than two cents due to my lack of
BPF knowledge) is yes, the JIT should be allowed to emit Zbb instructions
even when the kernel is not built with a compiler which has done so. In
fact, we have insn-def.h for situations similar to this.

Thanks,
drew
Daniel Borkmann Jan. 29, 2024, 3:32 p.m. UTC | #4
On 1/29/24 10:13 AM, Pu Lehui wrote:
> On 2024/1/28 1:16, Björn Töpel wrote:
>> Pu Lehui <pulehui@huaweicloud.com> writes:
>>
>>> From: Pu Lehui <pulehui@huawei.com>
>>>
>>> Add necessary Zbb instructions introduced by [0] to reduce code size and
>>> improve performance of RV64 JIT. Meanwhile, a runtime deteted helper is
>>> added to check whether the CPU supports Zbb instructions.
>>>
>>> Link: https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf [0]
>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>>> ---
>>>   arch/riscv/net/bpf_jit.h | 32 ++++++++++++++++++++++++++++++++
>>>   1 file changed, 32 insertions(+)
>>>
>>> diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
>>> index e30501b46f8f..51f6d214086f 100644
>>> --- a/arch/riscv/net/bpf_jit.h
>>> +++ b/arch/riscv/net/bpf_jit.h
>>> @@ -18,6 +18,11 @@ static inline bool rvc_enabled(void)
>>>       return IS_ENABLED(CONFIG_RISCV_ISA_C);
>>>   }
>>> +static inline bool rvzbb_enabled(void)
>>> +{
>>> +    return IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && riscv_has_extension_likely(RISCV_ISA_EXT_ZBB);
>>
>> Hmm, I'm thinking about the IS_ENABLED(CONFIG_RISCV_ISA_ZBB) semantics
>> for a kernel JIT compiler.
>>
>> IS_ENABLED(CONFIG_RISCV_ISA_ZBB) affects the kernel compiler flags.
>> Should it be enough to just have the run-time check? Should a kernel
>> built w/o Zbb be able to emit Zbb from the JIT?
> 
> Not enough, because riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) is a platform capability check, and the other one is a kernel image capability check. We can pass the check riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) when CONFIG_RISCV_ISA_ZBB=n. And my local test prove it.

So if I understand you correctly, only relying on the riscv_has_extension_likely(RISCV_ISA_EXT_ZBB)
part would not work - iow, the IS_ENABLED(CONFIG_RISCV_ISA_ZBB) is mandatory here?

Thanks,
Daniel

P.s.: Given Bjorn's review and tests I took the series into bpf-next now. Thanks everyone!
Pu Lehui Jan. 30, 2024, 1 a.m. UTC | #5
On 2024/1/29 23:32, Daniel Borkmann wrote:
> On 1/29/24 10:13 AM, Pu Lehui wrote:
>> On 2024/1/28 1:16, Björn Töpel wrote:
>>> Pu Lehui <pulehui@huaweicloud.com> writes:
>>>
>>>> From: Pu Lehui <pulehui@huawei.com>
>>>>
>>>> Add necessary Zbb instructions introduced by [0] to reduce code size 
>>>> and
>>>> improve performance of RV64 JIT. Meanwhile, a runtime deteted helper is
>>>> added to check whether the CPU supports Zbb instructions.
>>>>
>>>> Link: 
>>>> https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf [0]
>>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>>>> ---
>>>>   arch/riscv/net/bpf_jit.h | 32 ++++++++++++++++++++++++++++++++
>>>>   1 file changed, 32 insertions(+)
>>>>
>>>> diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
>>>> index e30501b46f8f..51f6d214086f 100644
>>>> --- a/arch/riscv/net/bpf_jit.h
>>>> +++ b/arch/riscv/net/bpf_jit.h
>>>> @@ -18,6 +18,11 @@ static inline bool rvc_enabled(void)
>>>>       return IS_ENABLED(CONFIG_RISCV_ISA_C);
>>>>   }
>>>> +static inline bool rvzbb_enabled(void)
>>>> +{
>>>> +    return IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && 
>>>> riscv_has_extension_likely(RISCV_ISA_EXT_ZBB);
>>>
>>> Hmm, I'm thinking about the IS_ENABLED(CONFIG_RISCV_ISA_ZBB) semantics
>>> for a kernel JIT compiler.
>>>
>>> IS_ENABLED(CONFIG_RISCV_ISA_ZBB) affects the kernel compiler flags.
>>> Should it be enough to just have the run-time check? Should a kernel
>>> built w/o Zbb be able to emit Zbb from the JIT?
>>
>> Not enough, because riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) is a 
>> platform capability check, and the other one is a kernel image 
>> capability check. We can pass the check 
>> riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) when 
>> CONFIG_RISCV_ISA_ZBB=n. And my local test prove it.
> 
> So if I understand you correctly, only relying on the 
> riscv_has_extension_likely(RISCV_ISA_EXT_ZBB)
> part would not work - iow, the IS_ENABLED(CONFIG_RISCV_ISA_ZBB) is 
> mandatory here?
> 

Yes, it should be IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && 
riscv_has_extension_likely(RISCV_ISA_EXT_ZBB).

> Thanks,
> Daniel
> 
> P.s.: Given Bjorn's review and tests I took the series into bpf-next 
> now. Thanks everyone!

Thanks Daniel and Björn
Björn Töpel Jan. 30, 2024, 6:18 a.m. UTC | #6
Daniel Borkmann <daniel@iogearbox.net> writes:

> On 1/29/24 10:13 AM, Pu Lehui wrote:
>> On 2024/1/28 1:16, Björn Töpel wrote:
>>> Pu Lehui <pulehui@huaweicloud.com> writes:
>>>
>>>> From: Pu Lehui <pulehui@huawei.com>
>>>>
>>>> Add necessary Zbb instructions introduced by [0] to reduce code size and
>>>> improve performance of RV64 JIT. Meanwhile, a runtime deteted helper is
>>>> added to check whether the CPU supports Zbb instructions.
>>>>
>>>> Link: https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf [0]
>>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>>>> ---
>>>>   arch/riscv/net/bpf_jit.h | 32 ++++++++++++++++++++++++++++++++
>>>>   1 file changed, 32 insertions(+)
>>>>
>>>> diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
>>>> index e30501b46f8f..51f6d214086f 100644
>>>> --- a/arch/riscv/net/bpf_jit.h
>>>> +++ b/arch/riscv/net/bpf_jit.h
>>>> @@ -18,6 +18,11 @@ static inline bool rvc_enabled(void)
>>>>       return IS_ENABLED(CONFIG_RISCV_ISA_C);
>>>>   }
>>>> +static inline bool rvzbb_enabled(void)
>>>> +{
>>>> +    return IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && riscv_has_extension_likely(RISCV_ISA_EXT_ZBB);
>>>
>>> Hmm, I'm thinking about the IS_ENABLED(CONFIG_RISCV_ISA_ZBB) semantics
>>> for a kernel JIT compiler.
>>>
>>> IS_ENABLED(CONFIG_RISCV_ISA_ZBB) affects the kernel compiler flags.
>>> Should it be enough to just have the run-time check? Should a kernel
>>> built w/o Zbb be able to emit Zbb from the JIT?
>> 
>> Not enough, because riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) is
>> a platform capability check, and the other one is a kernel image
>> capability check. We can pass the check
>> riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) when
>> CONFIG_RISCV_ISA_ZBB=n. And my local test prove it.

What I'm trying to say (and drew as well in the other reply) is that
"riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) when
CONFIG_RISCV_ISA_ZBB=n" should also make the JIT emit Zbb insns. The
platform check should be sufficient.

> So if I understand you correctly, only relying on the
> riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) part would not work -
> iow, the IS_ENABLED(CONFIG_RISCV_ISA_ZBB) is mandatory here?
>
> Thanks,
> Daniel
>
> P.s.: Given Bjorn's review and tests I took the series into bpf-next
> now. Thanks everyone!

Thanks! Yes, this is mainly a semantic discussion, and it can be further
relaxed later with a follow up -- if applicable.


Björn
Pu Lehui Jan. 30, 2024, 8:20 a.m. UTC | #7
On 2024/1/30 14:18, Björn Töpel wrote:
> Daniel Borkmann <daniel@iogearbox.net> writes:
> 
>> On 1/29/24 10:13 AM, Pu Lehui wrote:
>>> On 2024/1/28 1:16, Björn Töpel wrote:
>>>> Pu Lehui <pulehui@huaweicloud.com> writes:
>>>>
>>>>> From: Pu Lehui <pulehui@huawei.com>
>>>>>
>>>>> Add necessary Zbb instructions introduced by [0] to reduce code size and
>>>>> improve performance of RV64 JIT. Meanwhile, a runtime deteted helper is
>>>>> added to check whether the CPU supports Zbb instructions.
>>>>>
>>>>> Link: https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf [0]
>>>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>>>>> ---
>>>>>    arch/riscv/net/bpf_jit.h | 32 ++++++++++++++++++++++++++++++++
>>>>>    1 file changed, 32 insertions(+)
>>>>>
>>>>> diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
>>>>> index e30501b46f8f..51f6d214086f 100644
>>>>> --- a/arch/riscv/net/bpf_jit.h
>>>>> +++ b/arch/riscv/net/bpf_jit.h
>>>>> @@ -18,6 +18,11 @@ static inline bool rvc_enabled(void)
>>>>>        return IS_ENABLED(CONFIG_RISCV_ISA_C);
>>>>>    }
>>>>> +static inline bool rvzbb_enabled(void)
>>>>> +{
>>>>> +    return IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && riscv_has_extension_likely(RISCV_ISA_EXT_ZBB);
>>>>
>>>> Hmm, I'm thinking about the IS_ENABLED(CONFIG_RISCV_ISA_ZBB) semantics
>>>> for a kernel JIT compiler.
>>>>
>>>> IS_ENABLED(CONFIG_RISCV_ISA_ZBB) affects the kernel compiler flags.
>>>> Should it be enough to just have the run-time check? Should a kernel
>>>> built w/o Zbb be able to emit Zbb from the JIT?
>>>
>>> Not enough, because riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) is
>>> a platform capability check, and the other one is a kernel image
>>> capability check. We can pass the check
>>> riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) when
>>> CONFIG_RISCV_ISA_ZBB=n. And my local test prove it.
> 
> What I'm trying to say (and drew as well in the other reply) is that
> "riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) when
> CONFIG_RISCV_ISA_ZBB=n" should also make the JIT emit Zbb insns. The
> platform check should be sufficient.

Ooh, this is really beyond my expectation. The test_progs can pass when 
with only platform check and it can recognize the zbb instructions. Now 
I know it. Sorry for misleading.
Björn Töpel Jan. 30, 2024, 5:34 p.m. UTC | #8
Pu Lehui <pulehui@huawei.com> writes:

> On 2024/1/30 14:18, Björn Töpel wrote:
>> Daniel Borkmann <daniel@iogearbox.net> writes:
>> 
>>> On 1/29/24 10:13 AM, Pu Lehui wrote:
>>>> On 2024/1/28 1:16, Björn Töpel wrote:
>>>>> Pu Lehui <pulehui@huaweicloud.com> writes:
>>>>>
>>>>>> From: Pu Lehui <pulehui@huawei.com>
>>>>>>
>>>>>> Add necessary Zbb instructions introduced by [0] to reduce code size and
>>>>>> improve performance of RV64 JIT. Meanwhile, a runtime deteted helper is
>>>>>> added to check whether the CPU supports Zbb instructions.
>>>>>>
>>>>>> Link: https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf [0]
>>>>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>>>>>> ---
>>>>>>    arch/riscv/net/bpf_jit.h | 32 ++++++++++++++++++++++++++++++++
>>>>>>    1 file changed, 32 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
>>>>>> index e30501b46f8f..51f6d214086f 100644
>>>>>> --- a/arch/riscv/net/bpf_jit.h
>>>>>> +++ b/arch/riscv/net/bpf_jit.h
>>>>>> @@ -18,6 +18,11 @@ static inline bool rvc_enabled(void)
>>>>>>        return IS_ENABLED(CONFIG_RISCV_ISA_C);
>>>>>>    }
>>>>>> +static inline bool rvzbb_enabled(void)
>>>>>> +{
>>>>>> +    return IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && riscv_has_extension_likely(RISCV_ISA_EXT_ZBB);
>>>>>
>>>>> Hmm, I'm thinking about the IS_ENABLED(CONFIG_RISCV_ISA_ZBB) semantics
>>>>> for a kernel JIT compiler.
>>>>>
>>>>> IS_ENABLED(CONFIG_RISCV_ISA_ZBB) affects the kernel compiler flags.
>>>>> Should it be enough to just have the run-time check? Should a kernel
>>>>> built w/o Zbb be able to emit Zbb from the JIT?
>>>>
>>>> Not enough, because riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) is
>>>> a platform capability check, and the other one is a kernel image
>>>> capability check. We can pass the check
>>>> riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) when
>>>> CONFIG_RISCV_ISA_ZBB=n. And my local test prove it.
>> 
>> What I'm trying to say (and drew as well in the other reply) is that
>> "riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) when
>> CONFIG_RISCV_ISA_ZBB=n" should also make the JIT emit Zbb insns. The
>> platform check should be sufficient.
>
> Ooh, this is really beyond my expectation. The test_progs can pass when 
> with only platform check and it can recognize the zbb instructions. Now 
> I know it. Sorry for misleading.
Pu Lehui Jan. 31, 2024, 9:22 a.m. UTC | #9
On 2024/1/31 1:34, Björn Töpel wrote:
> Pu Lehui <pulehui@huawei.com> writes:
> 
>> On 2024/1/30 14:18, Björn Töpel wrote:
>>> Daniel Borkmann <daniel@iogearbox.net> writes:
>>>
>>>> On 1/29/24 10:13 AM, Pu Lehui wrote:
>>>>> On 2024/1/28 1:16, Björn Töpel wrote:
>>>>>> Pu Lehui <pulehui@huaweicloud.com> writes:
>>>>>>
>>>>>>> From: Pu Lehui <pulehui@huawei.com>
>>>>>>>
>>>>>>> Add necessary Zbb instructions introduced by [0] to reduce code size and
>>>>>>> improve performance of RV64 JIT. Meanwhile, a runtime deteted helper is
>>>>>>> added to check whether the CPU supports Zbb instructions.
>>>>>>>
>>>>>>> Link: https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf [0]
>>>>>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>>>>>>> ---
>>>>>>>     arch/riscv/net/bpf_jit.h | 32 ++++++++++++++++++++++++++++++++
>>>>>>>     1 file changed, 32 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
>>>>>>> index e30501b46f8f..51f6d214086f 100644
>>>>>>> --- a/arch/riscv/net/bpf_jit.h
>>>>>>> +++ b/arch/riscv/net/bpf_jit.h
>>>>>>> @@ -18,6 +18,11 @@ static inline bool rvc_enabled(void)
>>>>>>>         return IS_ENABLED(CONFIG_RISCV_ISA_C);
>>>>>>>     }
>>>>>>> +static inline bool rvzbb_enabled(void)
>>>>>>> +{
>>>>>>> +    return IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && riscv_has_extension_likely(RISCV_ISA_EXT_ZBB);
>>>>>>
>>>>>> Hmm, I'm thinking about the IS_ENABLED(CONFIG_RISCV_ISA_ZBB) semantics
>>>>>> for a kernel JIT compiler.
>>>>>>
>>>>>> IS_ENABLED(CONFIG_RISCV_ISA_ZBB) affects the kernel compiler flags.
>>>>>> Should it be enough to just have the run-time check? Should a kernel
>>>>>> built w/o Zbb be able to emit Zbb from the JIT?
>>>>>
>>>>> Not enough, because riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) is
>>>>> a platform capability check, and the other one is a kernel image
>>>>> capability check. We can pass the check
>>>>> riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) when
>>>>> CONFIG_RISCV_ISA_ZBB=n. And my local test prove it.
>>>
>>> What I'm trying to say (and drew as well in the other reply) is that
>>> "riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) when
>>> CONFIG_RISCV_ISA_ZBB=n" should also make the JIT emit Zbb insns. The
>>> platform check should be sufficient.
>>
>> Ooh, this is really beyond my expectation. The test_progs can pass when
>> with only platform check and it can recognize the zbb instructions. Now
>> I know it. Sorry for misleading.
diff mbox series

Patch

diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
index e30501b46f8f..51f6d214086f 100644
--- a/arch/riscv/net/bpf_jit.h
+++ b/arch/riscv/net/bpf_jit.h
@@ -18,6 +18,11 @@  static inline bool rvc_enabled(void)
 	return IS_ENABLED(CONFIG_RISCV_ISA_C);
 }
 
+static inline bool rvzbb_enabled(void)
+{
+	return IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && riscv_has_extension_likely(RISCV_ISA_EXT_ZBB);
+}
+
 enum {
 	RV_REG_ZERO =	0,	/* The constant value 0 */
 	RV_REG_RA =	1,	/* Return address */
@@ -730,6 +735,33 @@  static inline u16 rvc_swsp(u32 imm8, u8 rs2)
 	return rv_css_insn(0x6, imm, rs2, 0x2);
 }
 
+/* RVZBB instrutions. */
+static inline u32 rvzbb_sextb(u8 rd, u8 rs1)
+{
+	return rv_i_insn(0x604, rs1, 1, rd, 0x13);
+}
+
+static inline u32 rvzbb_sexth(u8 rd, u8 rs1)
+{
+	return rv_i_insn(0x605, rs1, 1, rd, 0x13);
+}
+
+static inline u32 rvzbb_zexth(u8 rd, u8 rs)
+{
+	if (IS_ENABLED(CONFIG_64BIT))
+		return rv_i_insn(0x80, rs, 4, rd, 0x3b);
+
+	return rv_i_insn(0x80, rs, 4, rd, 0x33);
+}
+
+static inline u32 rvzbb_rev8(u8 rd, u8 rs)
+{
+	if (IS_ENABLED(CONFIG_64BIT))
+		return rv_i_insn(0x6b8, rs, 5, rd, 0x13);
+
+	return rv_i_insn(0x698, rs, 5, rd, 0x13);
+}
+
 /*
  * RV64-only instructions.
  *