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(-)
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. 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(-) -- 2.49.0
On Thu, 19 Jun 2025 12:03:12 PDT (-0700), rkrcmar@ventanamicro.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(-)
On Mon, 23 Jun 2025 15:53:58 -0700 (PDT) Palmer Dabbelt <palmer@dabbelt.com> wrote: > On Thu, 19 Jun 2025 12:03:12 PDT (-0700), rkrcmar@ventanamicro.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? Or does it mean they aren't guaranteed to be preserved? David
2025-06-24T09:09:23+01:00, David Laight <david.laight.linux@gmail.com>: > On Mon, 23 Jun 2025 15:53:58 -0700 (PDT) > Palmer Dabbelt <palmer@dabbelt.com> wrote: >> On Thu, 19 Jun 2025 12:03:12 PDT (-0700), rkrcmar@ventanamicro.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. At the beginning, SBI says that only a0-a5 can contain ecall arguments, and then each function indirectly says which registers actually contain arguments. a0-a5 that don't contain arguments are not reserved, just like all the other unspecified registers. >> 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? SBI explicitly reserves registers if they are intended to be used in the future. The result of the ecall must be exactly the same regardless of the value in unspecified (not reserved) registers. We can't really tell what an SBI implementation will do with unspecified registers, but the most sane thing would be to ignore them. > Or does it mean they aren't guaranteed to be preserved? No, they are preserved: "All registers except a0 & a1 must be preserved across an SBI call by the callee."
© 2016 - 2025 Red Hat, Inc.