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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2024/3/29 6:07, Conor Dooley wrote:
On Thu, Mar 28, 2024 at 03:34:31PM -0400, Stefan O'Rear wrote:
On Thu, Mar 28, 2024, at 8:49 AM, Pu Lehui wrote:
From: Pu Lehui <pulehui@xxxxxxxxxx>

This patch relaxes the restrictions on the Zbb instructions. The hardware
is capable of recognizing the Zbb instructions independently, eliminating
the need for reliance on kernel compile configurations.

This doesn't make sense to me.

It doesn't make sense to me either. Of course the hardware's capability
to understand an instruction is independent of whether or not a
toolchain is capable of actually emitting the instruction.

RISCV_ISA_ZBB is defined as:

            Adds support to dynamically detect the presence of the ZBB
            extension (basic bit manipulation) and enable its usage.

In other words, RISCV_ISA_ZBB=n should disable everything that attempts
to detect Zbb at runtime. It is mostly relevant for code size reduction,
which is relevant for BPF since if RISCV_ISA_ZBB=n all rvzbb_enabled()
checks can be constant-folded.

Thanks for review. My initial thought was the same as yours, but after discussions [0] and test verifications, the hardware can indeed recognize the zbb instruction even if the kernel has not enabled CONFIG_RISCV_ISA_ZBB. As Conor mentioned, we are just acting as a JIT to emit zbb instruction here. Maybe is_hw_zbb_capable() will be better?

Link: https://lore.kernel.org/bpf/20240129-d06c79a17a5091b3403fc5b6@orel/ [0]


If BPF needs to become an exception (why?), this should be mentioned in
Kconfig.

And in the commit message. On one hand I think this could be a reasonable
thing to do in bpf as it is acting as a jit here, and doesn't actually
need the alternatives that we are using elsewhere to enable the
optimisations nor the compiler support. On the other the intention of
that kconfig option is to control optimisations like rvzbb_enabled()
gates, so this is gonna need a proper justification as to

As I said on IRC to you earlier, I think the Kconfig options here are in
need of a bit of a spring cleaning - they should be modified to explain
their individual purposes, be that enabling optimisations in the kernel
or being required for userspace. I'll try to send a patch for that if
I remember tomorrow.

Thanks,
Conor.

Signed-off-by: Pu Lehui <pulehui@xxxxxxxxxx>
---
  arch/riscv/net/bpf_jit.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
index 5fc374ed98ea..bcf109b88df5 100644
--- a/arch/riscv/net/bpf_jit.h
+++ b/arch/riscv/net/bpf_jit.h
@@ -20,7 +20,7 @@ static inline bool rvc_enabled(void)

  static inline bool rvzbb_enabled(void)
  {
-	return IS_ENABLED(CONFIG_RISCV_ISA_ZBB) &&
riscv_has_extension_likely(RISCV_ISA_EXT_ZBB);
+	return riscv_has_extension_likely(RISCV_ISA_EXT_ZBB);
  }

  enum {
--
2.34.1


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

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





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux