From:   =?utf-8?B?QmrDtnJuIFTDtnBlbA==?= <bjorn@kernel.org>
To:     Pu Lehui <pulehui@huaweicloud.com>, bpf@vger.kernel.org,
        linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Cc:     Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        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>,
        Pu Lehui <pulehui@huawei.com>
Subject: Re: [PATCH bpf-next v2] riscv, bpf: Add kfunc support for RV64
In-Reply-To: <80e69e73-b873-6717-fe45-a854dbdd5476@huaweicloud.com>
Date:   Tue, 21 Feb 2023 08:02:43 +0100
Message-ID: <87h6vffqlo.fsf@all.your.base.are.belong.to.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Pu Lehui <pulehui@huaweicloud.com> writes:

> On 2023/2/20 22:34, Bj=C3=B6rn T=C3=B6pel wrote:
>> Pu Lehui <pulehui@huaweicloud.com> writes:
>>=20
>>> From: Pu Lehui <pulehui@huawei.com>
>>>
>>> As another important missing piece of RV64 JIT, kfunc allow bpf programs
>>> call kernel functions. For now, RV64 is sufficient to enable it.
>>=20
>> Thanks Lehui!
>>=20
>> Maybe we can reword/massage the commit message a bit? What do you think
>> about something like:
>>=20
>> "Now that the BPF trampoline is supported by RISC-V, it is possible to
>> use BPF programs with kfunc calls.
>>=20
>
> kfunc and bpf trampoline are functionally independent. kfunc [1], like=20
> bpf helper functions, allows bpf programs to call exported kernel=20
> functions, while bpf trampoline provides a more efficient way than=20
> kprobe to act as a mediator between kernel functions and bpf programs,=20
> and between bpf programs.
>
> In fact, it was already supported before the bpf trampoline=20
> implementation, I just turned it on.

Good point. I guess my (incorrect) kfunc mental model was that
struct_ops and kfunc were tightly coupled. (Then again, w/o struct_ops
working kfunc is a bit half-working in my view.)

Fair enough. I'm still a bit confused about the commit message, but
happy with the patch.

Acked-by: Bj=C3=B6rn T=C3=B6pel <bjorn@rivosinc.com>