Re: [PATCH bpf-next 2/5] riscv, bpf: Relax restrictions on Zbb instructions

From: Pu Lehui <pulehui@huaweicloud.com>
To: "Conor Dooley" <conor@kernel.org>, "Björn Töpel" <bjorn@kernel.org>
Cc: Stefan O'Rear <sorear@fastmail.com>,
	bpf@vger.kernel.org, linux-riscv@lists.infradead.org,
	netdev@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Eduard Zingerman <eddyz87@gmail.com>, Song Liu <song@kernel.org>,
	Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>, Mykola Lysenko <mykolal@fb.com>,
	Manu Bretelle <chantr4@gmail.com>, Pu Lehui <pulehui@huawei.com>
Subject: Re: [PATCH bpf-next 2/5] riscv, bpf: Relax restrictions on Zbb instructions
Date: Wed, 3 Apr 2024 18:05:38 +0800	[thread overview]
Message-ID: <d6eae62b-60fd-4f3c-92e4-7ea5f1c4fc68@huaweicloud.com> (raw)
In-Reply-To: <20240403-gander-parting-a47c56401716@spud>

On 2024/4/3 9:20, Conor Dooley wrote:
> On Tue, Apr 02, 2024 at 09:00:45PM +0200, Björn Töpel wrote:
> 
>>>> I still think Lehui's patch is correct; Building a kernel that can boot
>>>> on multiple platforms (w/ or w/o Zbb support) and not having Zbb insn in
>>>> the kernel proper, and iff Zbb is available at run-time the BPF JIT will
>>>> emit Zbb.
>>>
>>> This sentence is -ENOPARSE to me, did you accidentally omit some words?
>>> Additionally he config option has nothing to do with building kernels that
>>> boot on multiple platforms, it only controls whether optimisations for Zbb
>>> are built so that if Zbb is detected they can be used.
>>
>> Ugh, sorry about that! I'm probably confused myself.
> 
> Reading this back, I a bunch of words too, so no worries...
> 
>>>> For these kind of optimizations, (IMO) it's better to let the BPF JIT
>>>> decide at run-time.
>>>
>>> Why is bpf a different case to any other user in this regard?
>>> I think that the commit message is misleading and needs to be changed,
>>> because the point "the hardware is capable of recognising the Zbb
>>> instructions independently..." is completely unrelated to the purpose
>>> of the config option. Of course the hardware understanding the option
> 
> This should have been "understanding the instructions"...
> 
>>> has nothing to do with kernel configuration. The commit message needs to
>>> explain why bpf is a special case and is exempt from an
> 
> And this s/from an//...
> 
>>> I totally understand any point about bpf being different in terms of
>>> needing toolchain support, but IIRC it was I who pointed out up-thread.
> 
> And "pointed that out".
> 
> I always make a mess of these emails that I re-write several times :)
> 
>>> The part of the conversation that you're replying to here is about the
>>> semantics of the Kconfig option and the original patch never mentioned
>>> trying to avoid a dependency on toolchains at all, just kernel
>>> configurations. The toolchain requirements I don't think are even super
>>> hard to fulfill either - the last 3 versions of ld and lld all meet the
>>> criteria.
>>
>> Thanks for making it more clear, and I agree that the toolchain
>> requirements are not hard to fulfull.
>>
>> My view has been that "BPF is like userland", but I realize now that's
>> odd.
> 
> Yeah, I can understand that perspective, but it does seem rather odd to
> someone that isn't a bpf-ist.
> 
>> Let's make BPF similar to the rest of the RV kernel. If ZBB=n, then
>> the BPF JIT doesn't know about emitting Zbb.
> 

Hi Conor and Björn,

Thanks for your explanation. I totally agree with what you said, 
"CONFIG_RISCV_ISA_ZBB only controls whether optimizations for Zbb are 
built so that if Zbb is detected they can be used.".

Since the instructions emited by bpf jit are in kernel space, they 
should indeed be aligned in this regard.

PS: It's a bit difficult to understand this,😅 if I'm wrong please don't 
hesitate to tell me.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv