linux-kernel - Re: [PATCH v2 0/2] RISC-V: turn sbi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <mhng-C1133FA3-71C3-4ECC-B3BF-13DC7640464D@palmerdabbelt-mac>
Date: Mon, 23 Jun 2025 15:53:58 -0700 (PDT)
From: Palmer Dabbelt <palmer@...belt.com>
To: rkrcmar@...tanamicro.com
CC: linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
  Paul Walmsley <paul.walmsley@...ive.com>, aou@...s.berkeley.edu, Alexandre Ghiti <alex@...ti.fr>,
  Atish Patra <atishp@...osinc.com>, ajones@...tanamicro.com, cleger@...osinc.com, apatel@...tanamicro.com,
  thomas.weissschuh@...utronix.de, david.laight.linux@...il.com
Subject:     Re: [PATCH v2 0/2] RISC-V: turn sbi_ecall into a variadic macro

On Thu, 19 Jun 2025 12:03:12 PDT (-0700), rkrcmar@...tanamicro.com wrote:
> v2 has a completely rewritten [1/2], and fixes some missed trailing
> zeroes in [2/2].  The fixes in [2/2] are important for v2, because
> sbi_ecall doesn't fill the registers with zeroes anymore.

The SBI spec says "Registers that are not defined in the SBI function 
call are not reserved." and I'm not really sure what to make of that.  
Specifically: does that mean implementations are allowed to ascribe 
custom meaning to those parameters and might start doing stuff if 
they're not set to zero?

> In the future, I think it would be nice to have a wrapper function for
> each sbi_ecall, to make the code less error-prone.
>
> GCC isn't doing a good job with sbi_ecall.  v2 is a bit better than v1,
> because some ecall registers are not used in the assembly, but nowhere
> near good enough...
> The compiler doesn't consider static key'd tracepoint branches to be
> special, and prepares for trace function calls outside of the unlikely
> path.  Instead of a single "nop" for a tracepoint, the non-trace path
> also does a lot of pointless register save/restore.
> I'm looking for help with this issue in [3/2].
>
> v2:
>  * use linux/args.h [Thomas] [1/2]
>  * completely rewrite [1/2]
>  * remove __sbi_ecall [1/2]
>  * add some missed trailing 0 in pmu [David] [2/2]
>  * adapt to the new sbi_ecall that doesn't allow a single argument [2/2]
>
> v1: https://lore.kernel.org/linux-riscv/20250612145754.2126147-2-rkrcmar@ventanamicro.com/T/#m1d441ab3de3e6d6b3b8d120b923f2e2081918a98
>
> Radim Krčmář (3):
>   RISC-V: sbi: turn sbi_ecall into variadic macro
>   RISC-V: make use of variadic sbi_ecall
>   RISC-V: sbi: remove sbi_ecall tracepoints
>
>  arch/riscv/include/asm/kvm_nacl.h |  4 +--
>  arch/riscv/include/asm/sbi.h      | 55 +++++++++++++++++++++++++----
>  arch/riscv/include/asm/trace.h    | 36 -------------------
>  arch/riscv/kernel/cpu_ops_sbi.c   |  6 ++--
>  arch/riscv/kernel/paravirt.c      |  2 +-
>  arch/riscv/kernel/sbi.c           | 57 ++++++++++++++-----------------
>  arch/riscv/kernel/sbi_ecall.c     | 34 +-----------------
>  arch/riscv/kernel/suspend.c       |  4 +--
>  arch/riscv/kvm/nacl.c             |  7 ++--
>  drivers/acpi/riscv/cppc.c         |  4 +--
>  drivers/perf/riscv_pmu_sbi.c      | 49 +++++++++++++-------------
>  11 files changed, 115 insertions(+), 143 deletions(-)