[PATCH] riscv: net: bpf: add bpf_jit_supports_private_stack()

[PATCH] riscv: net: bpf: add bpf_jit_supports_private_stack()

[JaeJoon Jung]

The bpf_jit_supports_private_stack() function is defined with the __weak
attribute in kernel/bpf/core.c as shown below, and returns false.

include/linux/filter.h:1191:bool bpf_jit_supports_private_stack(void);
kernel/bpf/core.c:3381:bool __weak bpf_jit_supports_private_stack(void)

However, if this function is not defined in the corresponding architecture to
return true, an error occurs when loading BPF as follows, and it is not loaded.

$ cd tools/sched_ext
$ make
$ sudo ./build/bin/scx_simple

libbpf: prog 'simple_dispatch': BPF program load failed: -EACCES
libbpf: prog 'simple_dispatch': -- BEGIN PROG LOAD LOG --
Private stack not supported by jit

arm64, powerpc, and x86 are defined as follows and return true.

arch/arm64/net/bpf_jit_comp.c:2318:bool bpf_jit_supports_private_stack(void)
arch/powerpc/net/bpf_jit_comp.c:530:bool bpf_jit_supports_private_stack(void)
arch/x86/net/bpf_jit_comp.c:4111:bool bpf_jit_supports_private_stack(void)

It seems that it should be defined in the same way in RISCV as well.

Signed-off-by: JaeJoon Jung <rgbi3307@gmail.com>
---
 arch/riscv/net/bpf_jit_comp64.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 8fe8969fb8a0..c3e8d91836f1 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -2171,3 +2171,8 @@ bool bpf_jit_supports_timed_may_goto(void)
 {
 	return true;
 }
+
+bool bpf_jit_supports_private_stack(void)
+{
+        return true;
+}
--
2.43.0


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

[PATCH] riscv: net: bpf: add bpf_jit_supports_private_stack()

[JaeJoon Jung]

The bpf_jit_supports_private_stack() function is defined with the __weak
attribute in kernel/bpf/core.c as shown below, and returns false.

include/linux/filter.h:1191:bool bpf_jit_supports_private_stack(void);
kernel/bpf/core.c:3381:bool __weak bpf_jit_supports_private_stack(void)

However, if this function is not defined in the corresponding architecture to
return true, an error occurs when loading BPF as follows, and it is not loaded.

$ cd tools/sched_ext
$ make
$ sudo ./build/bin/scx_simple

libbpf: prog 'simple_dispatch': BPF program load failed: -EACCES
libbpf: prog 'simple_dispatch': -- BEGIN PROG LOAD LOG --
Private stack not supported by jit

arm64, powerpc, and x86 are defined as follows and return true.

arch/arm64/net/bpf_jit_comp.c:2318:bool bpf_jit_supports_private_stack(void)
arch/powerpc/net/bpf_jit_comp.c:530:bool bpf_jit_supports_private_stack(void)
arch/x86/net/bpf_jit_comp.c:4111:bool bpf_jit_supports_private_stack(void)

It seems that it should be defined in the same way in RISCV as well.

Signed-off-by: JaeJoon Jung <rgbi3307@gmail.com>
---
 arch/riscv/net/bpf_jit_comp64.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 8fe8969fb8a0..c3e8d91836f1 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -2171,3 +2171,8 @@ bool bpf_jit_supports_timed_may_goto(void)
 {
 	return true;
 }
+
+bool bpf_jit_supports_private_stack(void)
+{
+        return true;
+}
--
2.43.0

Re: [PATCH] riscv: net: bpf: add bpf_jit_supports_private_stack()

[Chen Pei]

Hi JaeJoon,

Thanks for the report. I have been looking at what still blocks
sched_ext on riscv64, and the private stack is indeed an important part
of it.

> $ sudo ./build/bin/scx_simple
> libbpf: prog 'simple_dispatch': BPF program load failed: -EACCES
> libbpf: prog 'simple_dispatch': -- BEGIN PROG LOAD LOG --
> Private stack not supported by jit

I don't think we can take the hook on its own, though.

The other architectures paid for this with real JIT work rather than
with the hook alone: arm64 (6c17a882d380), x86 (7d1cd70d4b16) and
powerpc64 (156d985123b6) each added the per-CPU allocation with
overflow/underflow guard regions, the prologue work that puts the
program on that stack, and the guard check plus free on the program
free path.

> It seems that it should be defined in the same way in RISCV as well.

Defining it the same way means that JIT work, not just the hook. With
the hook alone the verifier would let these programs load while riscv64
still runs them on the per-task stack and has no guard region, turning
the clean -EACCES above into a silent stack overflow.

Going through those three commits, this is what I expect the riscv64
side to need, and I plan to post it as an RFC series:

  1) per-CPU private stack allocation with overflow/underflow guard
     regions, plus the guard check and free_percpu() on program free;
  2) pointing BPF_REG_FP (S5) at priv_sp + the program's stack size
     while leaving SP on the kernel stack, the way arm64 does;
  3) emitting the per-CPU lookup that produces that pointer after the
     tail-call entry point, since RV_TAILCALL_OFFSET is a constant
     added to the target program's bpf_func;
  4) bpf_jit_supports_private_stack() returning true only once the
     above is in place.

Aiming to send RFC v1 within the next couple of weeks. JaeJoon, I hope
we can go over the testing together once it is posted.

Thanks,
Pei

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

Re: [PATCH] riscv: net: bpf: add bpf_jit_supports_private_stack()

[Chen Pei]

Hi JaeJoon,

Thanks for the report. I have been looking at what still blocks
sched_ext on riscv64, and the private stack is indeed an important part
of it.

> $ sudo ./build/bin/scx_simple
> libbpf: prog 'simple_dispatch': BPF program load failed: -EACCES
> libbpf: prog 'simple_dispatch': -- BEGIN PROG LOAD LOG --
> Private stack not supported by jit

I don't think we can take the hook on its own, though.

The other architectures paid for this with real JIT work rather than
with the hook alone: arm64 (6c17a882d380), x86 (7d1cd70d4b16) and
powerpc64 (156d985123b6) each added the per-CPU allocation with
overflow/underflow guard regions, the prologue work that puts the
program on that stack, and the guard check plus free on the program
free path.

> It seems that it should be defined in the same way in RISCV as well.

Defining it the same way means that JIT work, not just the hook. With
the hook alone the verifier would let these programs load while riscv64
still runs them on the per-task stack and has no guard region, turning
the clean -EACCES above into a silent stack overflow.

Going through those three commits, this is what I expect the riscv64
side to need, and I plan to post it as an RFC series:

  1) per-CPU private stack allocation with overflow/underflow guard
     regions, plus the guard check and free_percpu() on program free;
  2) pointing BPF_REG_FP (S5) at priv_sp + the program's stack size
     while leaving SP on the kernel stack, the way arm64 does;
  3) emitting the per-CPU lookup that produces that pointer after the
     tail-call entry point, since RV_TAILCALL_OFFSET is a constant
     added to the target program's bpf_func;
  4) bpf_jit_supports_private_stack() returning true only once the
     above is in place.

Aiming to send RFC v1 within the next couple of weeks. JaeJoon, I hope
we can go over the testing together once it is posted.

Thanks,
Pei