| Message ID | 20240325155434.65589-3-puranjay12@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | bpf,riscv: Add support for BPF Arena | expand |
On Mon, Mar 25, 2024 at 03:54:34PM +0000, Puranjay Mohan wrote: > LLVM generates bpf_addr_space_cast instruction while translating > pointers between native (zero) address space and > __attribute__((address_space(N))). The addr_space=0 is reserved as > bpf_arena address space. > > rY = addr_space_cast(rX, 0, 1) is processed by the verifier and > converted to normal 32-bit move: wX = wY > > rY = addr_space_cast(rX, 1, 0) has to be converted by JIT. > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> Doesn't compile for allmodconfig: ../arch/riscv/net/bpf_jit_comp64.c:1086:7: error: call to undeclared function 'insn_is_cast_user'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] Cheers, Conor.
On Mon, Mar 25, 2024 at 8:10 PM Conor Dooley <conor@kernel.org> wrote: > > On Mon, Mar 25, 2024 at 03:54:34PM +0000, Puranjay Mohan wrote: > > LLVM generates bpf_addr_space_cast instruction while translating > > pointers between native (zero) address space and > > __attribute__((address_space(N))). The addr_space=0 is reserved as > > bpf_arena address space. > > > > rY = addr_space_cast(rX, 0, 1) is processed by the verifier and > > converted to normal 32-bit move: wX = wY > > > > rY = addr_space_cast(rX, 1, 0) has to be converted by JIT. > > > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> > > Doesn't compile for allmodconfig: > ../arch/riscv/net/bpf_jit_comp64.c:1086:7: error: call to undeclared function 'insn_is_cast_user'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] > > Cheers, > Conor. Yes, I mentioned in the cover letter that a patch is required. It just got merged in bpf-next/master: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=770546ae9f4c1ae1ebcaf0874f0dd9631d77ec97 So, rebasing on the latest bpf-next/master should fix the issue. Thanks, Puranjay
On Mon, Mar 25, 2024 at 08:13:10PM +0100, Puranjay Mohan wrote: > On Mon, Mar 25, 2024 at 8:10 PM Conor Dooley <conor@kernel.org> wrote: > > > > On Mon, Mar 25, 2024 at 03:54:34PM +0000, Puranjay Mohan wrote: > > > LLVM generates bpf_addr_space_cast instruction while translating > > > pointers between native (zero) address space and > > > __attribute__((address_space(N))). The addr_space=0 is reserved as > > > bpf_arena address space. > > > > > > rY = addr_space_cast(rX, 0, 1) is processed by the verifier and > > > converted to normal 32-bit move: wX = wY > > > > > > rY = addr_space_cast(rX, 1, 0) has to be converted by JIT. > > > > > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> > > > > Doesn't compile for allmodconfig: > > ../arch/riscv/net/bpf_jit_comp64.c:1086:7: error: call to undeclared function 'insn_is_cast_user'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] > > > > Cheers, > > Conor. > > Yes, > I mentioned in the cover letter that a patch is required. > It just got merged in bpf-next/master: > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=770546ae9f4c1ae1ebcaf0874f0dd9631d77ec97 > > So, rebasing on the latest bpf-next/master should fix the issue. Ah, I see now that there was a mention in the cover letter that I did not see first time around. Bjorn, do you think there's anything we can do about these kinda misleading CI failures for bpf stuff? Some stuff that touches bpf definitely is worth us building, but should we try and build it on top of the bpf tree instead?
On Mon, Mar 25, 2024 at 8:19 PM Conor Dooley <conor@kernel.org> wrote: > > On Mon, Mar 25, 2024 at 08:13:10PM +0100, Puranjay Mohan wrote: > > On Mon, Mar 25, 2024 at 8:10 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > On Mon, Mar 25, 2024 at 03:54:34PM +0000, Puranjay Mohan wrote: > > > > LLVM generates bpf_addr_space_cast instruction while translating > > > > pointers between native (zero) address space and > > > > __attribute__((address_space(N))). The addr_space=0 is reserved as > > > > bpf_arena address space. > > > > > > > > rY = addr_space_cast(rX, 0, 1) is processed by the verifier and > > > > converted to normal 32-bit move: wX = wY > > > > > > > > rY = addr_space_cast(rX, 1, 0) has to be converted by JIT. > > > > > > > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> > > > > > > Doesn't compile for allmodconfig: > > > ../arch/riscv/net/bpf_jit_comp64.c:1086:7: error: call to undeclared function 'insn_is_cast_user'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] > > > > > > Cheers, > > > Conor. > > > > Yes, > > I mentioned in the cover letter that a patch is required. > > It just got merged in bpf-next/master: > > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=770546ae9f4c1ae1ebcaf0874f0dd9631d77ec97 > > > > So, rebasing on the latest bpf-next/master should fix the issue. > > Ah, I see now that there was a mention in the cover letter that I did > not see first time around. > > Bjorn, do you think there's anything we can do about these kinda > misleading CI failures for bpf stuff? Some stuff that touches bpf > definitely is worth us building, but should we try and build it on top > of the bpf tree instead? Is there a separate CI for RISCV related stuff? is it public? I would be interested in adding RISC-V support in https://github.com/kernel-patches/bpf Is someone already working on this?
On Mon, Mar 25, 2024 at 08:23:18PM +0100, Puranjay Mohan wrote: > On Mon, Mar 25, 2024 at 8:19 PM Conor Dooley <conor@kernel.org> wrote: > > > > On Mon, Mar 25, 2024 at 08:13:10PM +0100, Puranjay Mohan wrote: > > > On Mon, Mar 25, 2024 at 8:10 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > On Mon, Mar 25, 2024 at 03:54:34PM +0000, Puranjay Mohan wrote: > > > > > LLVM generates bpf_addr_space_cast instruction while translating > > > > > pointers between native (zero) address space and > > > > > __attribute__((address_space(N))). The addr_space=0 is reserved as > > > > > bpf_arena address space. > > > > > > > > > > rY = addr_space_cast(rX, 0, 1) is processed by the verifier and > > > > > converted to normal 32-bit move: wX = wY > > > > > > > > > > rY = addr_space_cast(rX, 1, 0) has to be converted by JIT. > > > > > > > > > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> > > > > > > > > Doesn't compile for allmodconfig: > > > > ../arch/riscv/net/bpf_jit_comp64.c:1086:7: error: call to undeclared function 'insn_is_cast_user'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] > > > > > > > > Cheers, > > > > Conor. > > > > > > Yes, > > > I mentioned in the cover letter that a patch is required. > > > It just got merged in bpf-next/master: > > > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=770546ae9f4c1ae1ebcaf0874f0dd9631d77ec97 > > > > > > So, rebasing on the latest bpf-next/master should fix the issue. > > > > Ah, I see now that there was a mention in the cover letter that I did > > not see first time around. > > > > Bjorn, do you think there's anything we can do about these kinda > > misleading CI failures for bpf stuff? Some stuff that touches bpf > > definitely is worth us building, but should we try and build it on top > > of the bpf tree instead? > > Is there a separate CI for RISCV related stuff? is it public? It's based outta patchwork, just like the netdev/bpf stuff: https://patchwork.kernel.org/project/linux-riscv/list/ > I would be interested in adding RISC-V support in > https://github.com/kernel-patches/bpf > Is someone already working on this? If anyone is, it is probably something Bjorn knows about!
Conor Dooley <conor@kernel.org> writes: > Bjorn, do you think there's anything we can do about these kinda > misleading CI failures for bpf stuff? Some stuff that touches bpf > definitely is worth us building, but should we try and build it on top > of the bpf tree instead? IMO: The way to go is enabling RV support in the BPF CI (I'll expand on this in Puranjay's later mail), and ignore BPF series for the RV patchwork CI. I think having multiple trees in the RV CI is not worth the pain... Sort of related is that I think it could be worthwhile only building series that had some human interaction (a pair of eyes -- "yes, this does make sense to build"). Right now we're just building everything, and we have to pay (money *and* time) for it. ...and then the BPF series would e.g. not be built at the RV PW CI. (But mostly me thinking out loud! ;-)) Björn
Conor Dooley <conor@kernel.org> writes: >> Is there a separate CI for RISCV related stuff? is it public? > > It's based outta patchwork, just like the netdev/bpf stuff: > https://patchwork.kernel.org/project/linux-riscv/list/ *salesman mode on* ...and more information on the CI (and source!) can be found here: https://github.com/linux-riscv/github-ci/wiki https://wiki.riseproject.dev/display/HOME/PoC+Github+Runners+on+GKE Improvements are very much welcome. It's pretty much only Conor and me doing best effort hacking on the CI. Björn
Puranjay Mohan <puranjay12@gmail.com> writes: > Is there a separate CI for RISCV related stuff? is it public? > > I would be interested in adding RISC-V support in > https://github.com/kernel-patches/bpf > Is someone already working on this? +Cc Manu/Mykola/Nico who's doing all of the awesome BPF CI work at Meta, and can keep me honest. ;-) I did some early hacks for to add RISC-V support for the BPF CI, but haven't had time to work on it recently. :-( [1] https://github.com/libbpf/ci/pull/87 [2] https://github.com/kernel-patches/vmtest/pull/194 I've been talking recently to Lehui about it as well. Two major things are missing: 1. Cross-compilation support (expand on [1]) 2. Align the rootfs with what the other arch are using, to run the tests on Qemu/TCG (and proper HW at some point!). RISC-V does not have Debian Stable support, and would probably need Ubuntu or Debian Sid snapshop. Manu outlines some issues here: https://github.com/libbpf/ci/pull/83 Having to manually run BPF tests ("non-official RISC-V BPF CI") is a mess! Björn
On 2024/3/26 4:31, Björn Töpel wrote: > Puranjay Mohan <puranjay12@gmail.com> writes: > >> Is there a separate CI for RISCV related stuff? is it public? >> >> I would be interested in adding RISC-V support in >> https://github.com/kernel-patches/bpf >> Is someone already working on this? > > +Cc Manu/Mykola/Nico who's doing all of the awesome BPF CI work at Meta, > and can keep me honest. ;-) > > I did some early hacks for to add RISC-V support for the BPF CI, but > haven't had time to work on it recently. :-( > > [1] https://github.com/libbpf/ci/pull/87 > [2] https://github.com/kernel-patches/vmtest/pull/194 > > I've been talking recently to Lehui about it as well. > > Two major things are missing: > > 1. Cross-compilation support (expand on [1]) > 2. Align the rootfs with what the other arch are using, to run the tests > on Qemu/TCG (and proper HW at some point!). RISC-V does not have > Debian Stable support, and would probably need Ubuntu or Debian Sid > snapshop. Manu outlines some issues here: > https://github.com/libbpf/ci/pull/83 yeah, the current issue below is fixed and I think we can move forward. https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=443574b033876c85a35de4c65c14f7fe092222b2 > > Having to manually run BPF tests ("non-official RISC-V BPF CI") is a > mess! > > > Björn
diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h index 8a47da08dd9c..5fc374ed98ea 100644 --- a/arch/riscv/net/bpf_jit.h +++ b/arch/riscv/net/bpf_jit.h @@ -82,6 +82,7 @@ struct rv_jit_context { unsigned long flags; int stack_size; u64 arena_vm_start; + u64 user_vm_start; }; /* Convert from ninsns to bytes. */ diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c index 0c0588e327af..a5f049e72da2 100644 --- a/arch/riscv/net/bpf_jit_comp64.c +++ b/arch/riscv/net/bpf_jit_comp64.c @@ -1083,6 +1083,15 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx, /* dst = src */ case BPF_ALU | BPF_MOV | BPF_X: case BPF_ALU64 | BPF_MOV | BPF_X: + if (insn_is_cast_user(insn)) { + emit_mv(RV_REG_T1, rs, ctx); + emit_zextw(RV_REG_T1, RV_REG_T1, ctx); + emit_imm(rd, (ctx->user_vm_start >> 32) << 32, ctx); + emit(rv_beq(RV_REG_T1, RV_REG_ZERO, 4), ctx); + emit_or(RV_REG_T1, rd, RV_REG_T1, ctx); + emit_mv(rd, RV_REG_T1, ctx); + break; + } if (imm == 1) { /* Special mov32 for zext */ emit_zextw(rd, rd, ctx); @@ -2026,3 +2035,8 @@ bool bpf_jit_supports_ptr_xchg(void) { return true; } + +bool bpf_jit_supports_arena(void) +{ + return true; +} diff --git a/arch/riscv/net/bpf_jit_core.c b/arch/riscv/net/bpf_jit_core.c index 9ab739b9f9a2..8a69d6d81e32 100644 --- a/arch/riscv/net/bpf_jit_core.c +++ b/arch/riscv/net/bpf_jit_core.c @@ -81,6 +81,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog) } ctx->arena_vm_start = bpf_arena_get_kern_vm_start(prog->aux->arena); + ctx->user_vm_start = bpf_arena_get_user_vm_start(prog->aux->arena); ctx->prog = prog; ctx->offset = kcalloc(prog->len, sizeof(int), GFP_KERNEL); if (!ctx->offset) {
LLVM generates bpf_addr_space_cast instruction while translating pointers between native (zero) address space and __attribute__((address_space(N))). The addr_space=0 is reserved as bpf_arena address space. rY = addr_space_cast(rX, 0, 1) is processed by the verifier and converted to normal 32-bit move: wX = wY rY = addr_space_cast(rX, 1, 0) has to be converted by JIT. Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> --- arch/riscv/net/bpf_jit.h | 1 + arch/riscv/net/bpf_jit_comp64.c | 14 ++++++++++++++ arch/riscv/net/bpf_jit_core.c | 1 + 3 files changed, 16 insertions(+)