Re: [PATCH bpf-next 4/6] riscv, bpf: Add necessary Zbb instructions

[Pu Lehui <pulehui@huawei.com>]

On 2023/9/14 21:02, Conor Dooley wrote:
> On Wed, Sep 13, 2023 at 05:23:48PM +0100, Conor Dooley wrote:
>> On Wed, Sep 13, 2023 at 11:34:11PM +0800, Pu Lehui wrote:
>>> From: Pu Lehui <pulehui@huawei.com>
>>>
>>> Add necessary Zbb instructions introduced by [0] to reduce code size and
>>> improve performance of RV64 JIT. At the same time, a helper is added to
>>> check whether the CPU supports Zbb instructions.
>>>
>>> [0] https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf
>>>
>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>>> ---
>>>   arch/riscv/net/bpf_jit.h | 26 ++++++++++++++++++++++++++
>>>   1 file changed, 26 insertions(+)
>>>
>>> diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
>>> index 8e0ef4d08..7ee59d1f6 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);
>>> +}
>>
>> I dunno much about bpf, so passing question that may be a bit obvious:
>> Is this meant to be a test as to whether the kernel binary is built with
>> support for the extension, or whether the underlying platform is capable
>> of executing zbb instructions.
>>
>> Sorry if that would be obvious to a bpf aficionado, context I have here
>> is the later user and the above rvc_enabled() test, which functions
>> differently to Zbb and so doesn't really help me.
> 
> FTR, I got an off-list reply about this & it is meant to be a check as
> to whether the underlying platform supports the extension. The current
> test here is insufficient for that.
> 

Thanks Conor for explain me lot about the difference between Compressed 
instructions and Zbb instructions. As the compressed instructions are a 
build-time option, while the Zbb is runtime detected. We need to add 
additional runtime detection as show bellow:

riscv_has_extension_likely(RISCV_ISA_EXT_ZBB)

will patch this suggestion to the next version.

Thanks,
Lehui.

> Thanks,
> Conor.