Re: [PATCH v3 2/4] riscv: add support for SBI Supervisor Software Events extension
From: Clément Léger <hidden>
Date: 2025-03-20 08:16:10
Also in:
linux-riscv, lkml
On 19/03/2025 18:08, Andrew Jones wrote:
On Fri, Dec 06, 2024 at 05:30:58PM +0100, Clément Léger wrote: ...quoted
+int arch_sse_init_event(struct sse_event_arch_data *arch_evt, u32 evt_id, int cpu) +{ + void *stack; + + arch_evt->evt_id = evt_id; + stack = sse_stack_alloc(cpu, SSE_STACK_SIZE); + if (!stack) + return -ENOMEM; + + arch_evt->stack = stack + SSE_STACK_SIZE; + + if (sse_init_scs(cpu, arch_evt)) + goto free_stack; + + if (is_kernel_percpu_address((unsigned long)&arch_evt->interrupted)) { + arch_evt->interrupted_state_phys = + per_cpu_ptr_to_phys(&arch_evt->interrupted); + } else { + arch_evt->interrupted_state_phys = + virt_to_phys(&arch_evt->interrupted); + } + + return 0;Hi Clément, Testing SSE support with tools/testing/selftests/kvm/riscv/sbi_pmu_test led to an opensbi sbi_trap_error because the output_phys_lo address passed to sbi_sse_read_attrs() wasn't a physical address. The reason is that is_kernel_percpu_address() can only be used on static percpu addresses, but local sse events get their percpu addresses with alloc_percpu(), so is_kernel_percpu_address() was returning false even for local events. I made the following changes to get things working.
Hi Andrew, Did something changed recently ? Because I tested that when it was send (PMU + some kernel internal testsuite) and didn't saw that. Anyway, I'll respin it with your changes as well. Thanks ! Clément
quoted hunk ↗ jump to hunk
Thanks, drewdiff --git a/arch/riscv/kernel/sse.c b/arch/riscv/kernel/sse.c index b48ae69dad8d..f46893946086 100644 --- a/arch/riscv/kernel/sse.c +++ b/arch/riscv/kernel/sse.c@@ -100,12 +100,12 @@ int arch_sse_init_event(struct sse_event_arch_data *arch_evt, u32 evt_id, int cp if (sse_init_scs(cpu, arch_evt)) goto free_stack; - if (is_kernel_percpu_address((unsigned long)&arch_evt->interrupted)) { + if (sse_event_is_global(evt_id)) { arch_evt->interrupted_state_phys = - per_cpu_ptr_to_phys(&arch_evt->interrupted); + virt_to_phys(&arch_evt->interrupted); } else { arch_evt->interrupted_state_phys = - virt_to_phys(&arch_evt->interrupted); + per_cpu_ptr_to_phys(&arch_evt->interrupted); } return 0;diff --git a/drivers/firmware/riscv/riscv_sse.c b/drivers/firmware/riscv/riscv_sse.c index 511db9ad7a9e..fef375046f75 100644 --- a/drivers/firmware/riscv/riscv_sse.c +++ b/drivers/firmware/riscv/riscv_sse.c@@ -62,11 +62,6 @@ void sse_handle_event(struct sse_event_arch_data *arch_event, ret); } -static bool sse_event_is_global(u32 evt) -{ - return !!(evt & SBI_SSE_EVENT_GLOBAL); -} - static struct sse_event *sse_event_get(u32 evt) {diff --git a/include/linux/riscv_sse.h b/include/linux/riscv_sse.h index 16700677f1e8..06b757b036b0 100644 --- a/include/linux/riscv_sse.h +++ b/include/linux/riscv_sse.h@@ -8,6 +8,7 @@ #include <linux/types.h> #include <linux/linkage.h> +#include <asm/sbi.h> struct sse_event; struct pt_regs;@@ -16,6 +17,11 @@ struct ghes; typedef int (sse_event_handler)(u32 event_num, void *arg, struct pt_regs *regs); +static inline bool sse_event_is_global(u32 evt) +{ + return !!(evt & SBI_SSE_EVENT_GLOBAL); +} + #ifdef CONFIG_RISCV_SSE struct sse_event *sse_event_register(u32 event_num, u32 priority,