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 Handled Elsewhere
Headers show
Series Zbb support and code simplification for RV64 JIT | expand

Checks

Context Check Description
conchuod/vmtest-for-next-PR success PR summary
conchuod/patch-4-test-1 success .github/scripts/patches/tests/build_rv32_defconfig.sh
conchuod/patch-4-test-2 success .github/scripts/patches/tests/build_rv64_clang_allmodconfig.sh
conchuod/patch-4-test-3 success .github/scripts/patches/tests/build_rv64_gcc_allmodconfig.sh
conchuod/patch-4-test-4 success .github/scripts/patches/tests/build_rv64_nommu_k210_defconfig.sh
conchuod/patch-4-test-5 success .github/scripts/patches/tests/build_rv64_nommu_virt_defconfig.sh
conchuod/patch-4-test-6 success .github/scripts/patches/tests/checkpatch.sh
conchuod/patch-4-test-7 success .github/scripts/patches/tests/dtb_warn_rv64.sh
conchuod/patch-4-test-8 success .github/scripts/patches/tests/header_inline.sh
conchuod/patch-4-test-9 success .github/scripts/patches/tests/kdoc.sh
conchuod/patch-4-test-10 success .github/scripts/patches/tests/module_param.sh
conchuod/patch-4-test-11 success .github/scripts/patches/tests/verify_fixes.sh
conchuod/patch-4-test-12 success .github/scripts/patches/tests/verify_signedoff.sh

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.
  *