From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f170.google.com (mail-pg1-f170.google.com [209.85.215.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id ACABF212558 for ; Wed, 13 Aug 2025 10:47:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.170 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1755082052; cv=none; b=o2t/yE9WwnKk51QkSJ7OONv1xktIqQ+8hJbj/DNduBccsHFqpEZTw2kSQbE5J4rabU0qG4zquaavlAdNagxAD1YrXpZ3OoU+J9pphj34RobTh0m2weyfqaJ6G/lXLja8XGDBSPU3pRmT3xm4GLpQHrD6N5fJxhBmmxf6uzAAg1s= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1755082052; c=relaxed/simple; bh=GR1g0PMsvgFD3/A02pJ8nDTzvgFn6n1fOS80TKBlxWk=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=Uc6cLVjiXFehUgu5e39vUNI/Cz1vBTFzeNJP8wOXIxAEOmg5mfWizdXYUucsmM+lLETrwLiTAABDmPYF0jaWJlfsrGFReMFQZ7NUbxrcXCSeQuVNYJJHPgzRGj6tlr0Tgn3hbO3ZNfAMS/8LK+8kUISwcpD9CmF6Y6QTw+6I3hc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=rivosinc.com; spf=pass smtp.mailfrom=rivosinc.com; dkim=pass (2048-bit key) header.d=rivosinc.com header.i=@rivosinc.com header.b=K3yyDAgy; arc=none smtp.client-ip=209.85.215.170 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=rivosinc.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=rivosinc.com header.i=@rivosinc.com header.b="K3yyDAgy" Received: by mail-pg1-f170.google.com with SMTP id 41be03b00d2f7-b2c4331c50eso4796860a12.3 for ; Wed, 13 Aug 2025 03:47:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc.com; s=google; t=1755082048; x=1755686848; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=UMwiCaqujhTNBHW/YCq25i3NwMv6QDUgUPwVzr5ASRA=; b=K3yyDAgy0aVkaVaM2bcle0saV6eexOER0RlPnyvl7RO5703NnfGL/6fPEYdVtfX/Nl cAQ1DSCAeWJBsweRDHvJFpC/VlqvQtJ4DNWcgpn2hg+mVsDOM8Rb8rktHKkRqkrJfgLX gGe+73VDL8oRWBkmftCByD4iGrxYF1iHKjZGxEpz3pGj4Ge6U6JLusQPYRt5A0DVsWOg 29q4/b+EVY4LmolQs3jXaYcZMa2oaKCT/c3JvVfGmD1e5kWIANL4h2FueioTDm7HcPJA SXYT+NZkC+VmqXID0KC+pfpGYIBQN1RAzRp3aPamVESnhTxNkM0XQEk09LYQUDvVHP7X KeNg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1755082048; x=1755686848; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=UMwiCaqujhTNBHW/YCq25i3NwMv6QDUgUPwVzr5ASRA=; b=pbrhCAGpNUAlxICgXczclpRKgPhMBj23Nalm2VGAeWO3m541VTKJiHgtM05QEE7V8p HTx8j11j6XyCy9R1w/SVGZIUvvsyw8PTjkTo+A4xcZN2hdsyXKmYCdTfghKw+4hcU87K HtgSStk4/BjbKcxjkgpFKl4YiXkMg/hNN70EQ9sywOmm/zskjWei2GIc9XybIrw1lrGg +iUgdQahQjgoEfDRtTKdvUfSmEbUEG0LDyYSKKWOvW7rEgo8RVhbx3f+J4glf2frE8fJ AQerURkLukn1cGJwGgPPM8+XHIwhqmSD6rJVJC7ddc45zGJWF91L8wBzrlG7RA7HE+DI Yl1A== X-Forwarded-Encrypted: i=1; AJvYcCXCIpyvkUbtt6hLn/kz5bL0K//BW49LunJa1lo+lksqqH6PYUTzSLJdcFU/neFBWwbhKjabb2qL1ReRBfw=@vger.kernel.org X-Gm-Message-State: AOJu0Yyy4X3ihnO/h/bobaFYDovnsbNuRQcjWzV2nxbEzTJGeP5y3sfb 55z/3udhuhxs+a5u1ME4VQp9o+iDUticJQ29k1lS5XdTSHBWkXq0W13Mf+/hdtyHCl8= X-Gm-Gg: ASbGnctwcyKFgUbl0pQG4J6S1vc/uwG9i2ZyiO+9n/Dsb1wEPOYa6+KBNYJsemoreH3 oPHhMSxPYrP0LskOoQgf0trmbEnhcfgLUF4T2CbgrUEab/J/WK2BebFcn5032abdKRSr5lFOo1R mxVF0/NF+qBUWMdjNNsRSyw5FLE1WzwXGl6rVf3T1jvhfl5CzqS8UgBAld7QH3WP23G032h3iF9 usC5ih1F+q1xdHz5Sk0ooEFu4pP9yxGagtnj+WS1YDpYqIx+p5O5F2nstFaU9o1OZ9n9XPkwI30 j3SJRojqeuUZspAQGrtf/OcmDq0gAtduiV3HRY8oXecx99tFsF3DHJZt5ogvBpzuYoYlej//wYD Yi6fkIVoW/hIEXvC1gn/Rg7+6uIBnD2ixX2XJsNP9eFDM6P4ojGRaSyDVSvrLT2brY08GCbT3hB W3zQ== X-Google-Smtp-Source: AGHT+IEPNPTpres4zLkyLh59KK3PQ9gdsHp/n4FuebSNI5LKGErbXlt+tMP9c0msagpqbITvN2a0bQ== X-Received: by 2002:a17:903:2f90:b0:240:1bba:fc7d with SMTP id d9443c01a7336-2430d1f7e5emr37474125ad.33.1755082047573; Wed, 13 Aug 2025 03:47:27 -0700 (PDT) Received: from ?IPV6:2a01:e0a:e17:9700:16d2:7456:6634:9626? ([2a01:e0a:e17:9700:16d2:7456:6634:9626]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-241e899a8cdsm322505145ad.121.2025.08.13.03.47.20 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 13 Aug 2025 03:47:26 -0700 (PDT) Message-ID: <80e73e9c-2b16-4a6a-91b9-0100d7708fa2@rivosinc.com> Date: Wed, 13 Aug 2025 12:47:13 +0200 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6 2/5] riscv: add support for SBI Supervisor Software Events extension To: Alexandre Ghiti , Paul Walmsley , Palmer Dabbelt , linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Cc: Himanshu Chauhan , Anup Patel , Xu Lu , Atish Patra , =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= , Yunhui Cui References: <20250808153901.2477005-1-cleger@rivosinc.com> <20250808153901.2477005-3-cleger@rivosinc.com> Content-Language: en-US From: =?UTF-8?B?Q2zDqW1lbnQgTMOpZ2Vy?= In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 13/08/2025 12:09, Alexandre Ghiti wrote: > On 8/8/25 17:38, Clément Léger wrote: >> The SBI SSE extension allows the supervisor software to be notified by > > > I would change "notified" with "interrupted" no? > > >> the SBI of specific events that are not maskable. The context switch is > > > Nit: s/the SBI/the SBI implementation > > >> handled partially by the firmware which will save registers a6 and a7. >> When entering kernel we can rely on these 2 registers to setup the stack >> and save all the registers. >> >> Since SSE events can be delivered at any time to the kernel (including >> during exception handling, we need a way to locate the current_task for > > > s/handling/handling) > > >> context tracking. On RISC-V, it is sotred in scratch when in user space > > s/sotred/stored > >> or tp when in kernel space (in which case SSCRATCH is zero). But at a > > > Remove "at a" > > >> at the beginning of exception handling, SSCRATCH is used to swap tp and >> check the origin of the exception. If interrupted at that point, then, >> there is no way to reliably know were is located the current > > > s/were/where > > >> task_struct. Even checking the interruption location won't work as SSE >> event can be nested on top of each other so the original interruption >> site might be lost at some point. In order to retrieve it reliably, >> store the current task in an additional __sse_entry_task per_cpu array. >> This array is then used to retrieve the current task based on the >> hart ID that is passed to the SSE event handler in a6. >> >> That being said, the way the current task struct is stored should >> probably be reworked to find a better reliable alternative. >> >> Since each events (and each CPU for local events) have their own > > > "Since each event has its own" > > >> context and can preempt each other, allocate a stack (and a shadow stack >> if needed for each of them (and for each cpu for local events). > > > s/needed/needed) > > >> >> When completing the event, if we were coming from kernel with interrupts >> disabled, simply return there. If coming from userspace or kernel with >> interrupts enabled, simulate an interrupt exception by setting IE_SIE in >> CSR_IP to allow delivery of signals to user task. For instance this can >> happen, when a RAS event has been generated by a user application and a >> SIGBUS has been sent to a task. >> >> Signed-off-by: Clément Léger >> --- >>   arch/riscv/include/asm/asm.h         |  14 ++- >>   arch/riscv/include/asm/scs.h         |   7 ++ >>   arch/riscv/include/asm/sse.h         |  47 +++++++ >>   arch/riscv/include/asm/switch_to.h   |  14 +++ >>   arch/riscv/include/asm/thread_info.h |   1 + >>   arch/riscv/kernel/Makefile           |   1 + >>   arch/riscv/kernel/asm-offsets.c      |  14 +++ >>   arch/riscv/kernel/sse.c              | 154 +++++++++++++++++++++++ >>   arch/riscv/kernel/sse_entry.S        | 180 +++++++++++++++++++++++++++ >>   9 files changed, 429 insertions(+), 3 deletions(-) >>   create mode 100644 arch/riscv/include/asm/sse.h >>   create mode 100644 arch/riscv/kernel/sse.c >>   create mode 100644 arch/riscv/kernel/sse_entry.S >> >> diff --git a/arch/riscv/include/asm/asm.h b/arch/riscv/include/asm/asm.h >> index a8a2af6dfe9d..982c4be9a9c3 100644 >> --- a/arch/riscv/include/asm/asm.h >> +++ b/arch/riscv/include/asm/asm.h >> @@ -90,16 +90,24 @@ >>   #define PER_CPU_OFFSET_SHIFT 3 >>   #endif >>   -.macro asm_per_cpu dst sym tmp >> -    REG_L \tmp, TASK_TI_CPU_NUM(tp) >> -    slli  \tmp, \tmp, PER_CPU_OFFSET_SHIFT >> +.macro asm_per_cpu_with_cpu dst sym tmp cpu >> +    slli  \tmp, \cpu, PER_CPU_OFFSET_SHIFT >>       la    \dst, __per_cpu_offset >>       add   \dst, \dst, \tmp >>       REG_L \tmp, 0(\dst) >>       la    \dst, \sym >>       add   \dst, \dst, \tmp >>   .endm >> + >> +.macro asm_per_cpu dst sym tmp >> +    REG_L \tmp, TASK_TI_CPU_NUM(tp) > > > cpu is not xlen-wide, see https://lore.kernel.org/linux- > riscv/20250725165410.2896641-3-rkrcmar@ventanamicro.com/. Can you rebase > your next version on top my fixes branch which contains this fix? Here > it is https://git.kernel.org/pub/scm/linux/kernel/git/alexghiti/ > linux.git/log/?h=alex-fixes Hi Alex, Acked, I'll fix that. > > >> +    asm_per_cpu_with_cpu \dst \sym \tmp \tmp >> +.endm >>   #else /* CONFIG_SMP */ >> +.macro asm_per_cpu_with_cpu dst sym tmp cpu >> +    la    \dst, \sym >> +.endm >> + >>   .macro asm_per_cpu dst sym tmp >>       la    \dst, \sym >>   .endm >> diff --git a/arch/riscv/include/asm/scs.h b/arch/riscv/include/asm/scs.h >> index 0e45db78b24b..62344daad73d 100644 >> --- a/arch/riscv/include/asm/scs.h >> +++ b/arch/riscv/include/asm/scs.h >> @@ -18,6 +18,11 @@ >>       load_per_cpu gp, irq_shadow_call_stack_ptr, \tmp >>   .endm >>   +/* Load the per-CPU IRQ shadow call stack to gp. */ >> +.macro scs_load_sse_stack reg_evt >> +    REG_L gp, SSE_REG_EVT_SHADOW_STACK(\reg_evt) >> +.endm >> + >>   /* Load task_scs_sp(current) to gp. */ >>   .macro scs_load_current >>       REG_L    gp, TASK_TI_SCS_SP(tp) >> @@ -41,6 +46,8 @@ >>   .endm >>   .macro scs_load_irq_stack tmp >>   .endm >> +.macro scs_load_sse_stack reg_evt >> +.endm >>   .macro scs_load_current >>   .endm >>   .macro scs_load_current_if_task_changed prev >> diff --git a/arch/riscv/include/asm/sse.h b/arch/riscv/include/asm/sse.h >> new file mode 100644 >> index 000000000000..8929a268462c >> --- /dev/null >> +++ b/arch/riscv/include/asm/sse.h >> @@ -0,0 +1,47 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Copyright (C) 2024 Rivos Inc. >> + */ >> +#ifndef __ASM_SSE_H >> +#define __ASM_SSE_H >> + >> +#include >> + >> +#ifdef CONFIG_RISCV_SSE >> + >> +struct sse_event_interrupted_state { >> +    unsigned long a6; >> +    unsigned long a7; >> +}; >> + >> +struct sse_event_arch_data { >> +    void *stack; >> +    void *shadow_stack; >> +    unsigned long tmp; >> +    struct sse_event_interrupted_state interrupted; >> +    unsigned long interrupted_phys; >> +    u32 evt_id; >> +    unsigned int hart_id; >> +    unsigned int cpu_id; >> +}; >> + >> +static inline bool sse_event_is_global(u32 evt) >> +{ >> +    return !!(evt & SBI_SSE_EVENT_GLOBAL); >> +} >> + >> +void arch_sse_event_update_cpu(struct sse_event_arch_data *arch_evt, >> int cpu); >> +int arch_sse_init_event(struct sse_event_arch_data *arch_evt, u32 >> evt_id, >> +            int cpu); >> +void arch_sse_free_event(struct sse_event_arch_data *arch_evt); >> +int arch_sse_register_event(struct sse_event_arch_data *arch_evt); >> + >> +void sse_handle_event(struct sse_event_arch_data *arch_evt, >> +              struct pt_regs *regs); >> +asmlinkage void handle_sse(void); >> +asmlinkage void do_sse(struct sse_event_arch_data *arch_evt, >> +               struct pt_regs *reg); >> + >> +#endif >> + >> +#endif >> diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/ >> asm/switch_to.h >> index 0e71eb82f920..cd1cead0c682 100644 >> --- a/arch/riscv/include/asm/switch_to.h >> +++ b/arch/riscv/include/asm/switch_to.h >> @@ -88,6 +88,19 @@ static inline void __switch_to_envcfg(struct >> task_struct *next) >>               :: "r" (next->thread.envcfg) : "memory"); >>   } >>   +#ifdef CONFIG_RISCV_SSE >> +DECLARE_PER_CPU(struct task_struct *, __sse_entry_task); >> + >> +static inline void __switch_sse_entry_task(struct task_struct *next) >> +{ >> +    __this_cpu_write(__sse_entry_task, next); >> +} >> +#else >> +static inline void __switch_sse_entry_task(struct task_struct *next) >> +{ >> +} >> +#endif >> + >>   extern struct task_struct *__switch_to(struct task_struct *, >>                          struct task_struct *); >>   @@ -122,6 +135,7 @@ do {                            \ >>       if (switch_to_should_flush_icache(__next))    \ >>           local_flush_icache_all();        \ >>       __switch_to_envcfg(__next);            \ >> +    __switch_sse_entry_task(__next);            \ >>       ((last) = __switch_to(__prev, __next));        \ >>   } while (0) >>   diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/ >> include/asm/thread_info.h >> index f5916a70879a..28e9805e61fc 100644 >> --- a/arch/riscv/include/asm/thread_info.h >> +++ b/arch/riscv/include/asm/thread_info.h >> @@ -36,6 +36,7 @@ >>   #define OVERFLOW_STACK_SIZE     SZ_4K >>     #define IRQ_STACK_SIZE        THREAD_SIZE >> +#define SSE_STACK_SIZE        THREAD_SIZE >>     #ifndef __ASSEMBLY__ >>   diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile >> index c7b542573407..62e4490b34ee 100644 >> --- a/arch/riscv/kernel/Makefile >> +++ b/arch/riscv/kernel/Makefile >> @@ -99,6 +99,7 @@ obj-$(CONFIG_DYNAMIC_FTRACE)    += mcount-dyn.o >>   obj-$(CONFIG_PERF_EVENTS)    += perf_callchain.o >>   obj-$(CONFIG_HAVE_PERF_REGS)    += perf_regs.o >>   obj-$(CONFIG_RISCV_SBI)        += sbi.o sbi_ecall.o >> +obj-$(CONFIG_RISCV_SSE)        += sse.o sse_entry.o >>   ifeq ($(CONFIG_RISCV_SBI), y) >>   obj-$(CONFIG_SMP)        += sbi-ipi.o >>   obj-$(CONFIG_SMP) += cpu_ops_sbi.o >> diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm- >> offsets.c >> index 6e8c0d6feae9..315547c3a2ef 100644 >> --- a/arch/riscv/kernel/asm-offsets.c >> +++ b/arch/riscv/kernel/asm-offsets.c >> @@ -14,6 +14,8 @@ >>   #include >>   #include >>   #include >> +#include >> +#include >>   #include >>     void asm_offsets(void); >> @@ -528,4 +530,16 @@ void asm_offsets(void) >>       DEFINE(FREGS_A6,        offsetof(struct __arch_ftrace_regs, a6)); >>       DEFINE(FREGS_A7,        offsetof(struct __arch_ftrace_regs, a7)); >>   #endif >> + >> +#ifdef CONFIG_RISCV_SSE >> +    OFFSET(SSE_REG_EVT_STACK, sse_event_arch_data, stack); >> +    OFFSET(SSE_REG_EVT_SHADOW_STACK, sse_event_arch_data, shadow_stack); >> +    OFFSET(SSE_REG_EVT_TMP, sse_event_arch_data, tmp); >> +    OFFSET(SSE_REG_HART_ID, sse_event_arch_data, hart_id); >> +    OFFSET(SSE_REG_CPU_ID, sse_event_arch_data, cpu_id); >> + >> +    DEFINE(SBI_EXT_SSE, SBI_EXT_SSE); >> +    DEFINE(SBI_SSE_EVENT_COMPLETE, SBI_SSE_EVENT_COMPLETE); >> +    DEFINE(ASM_NR_CPUS, NR_CPUS); >> +#endif >>   } >> diff --git a/arch/riscv/kernel/sse.c b/arch/riscv/kernel/sse.c >> new file mode 100644 >> index 000000000000..d2da7e23a74a >> --- /dev/null >> +++ b/arch/riscv/kernel/sse.c >> @@ -0,0 +1,154 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * Copyright (C) 2024 Rivos Inc. >> + */ >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +DEFINE_PER_CPU(struct task_struct *, __sse_entry_task); >> + >> +void __weak sse_handle_event(struct sse_event_arch_data *arch_evt, >> struct pt_regs *regs) >> +{ >> +} >> + >> +void do_sse(struct sse_event_arch_data *arch_evt, struct pt_regs *regs) >> +{ >> +    nmi_enter(); >> + >> +    /* Retrieve missing GPRs from SBI */ >> +    sbi_ecall(SBI_EXT_SSE, SBI_SSE_EVENT_ATTR_READ, arch_evt->evt_id, >> +          SBI_SSE_ATTR_INTERRUPTED_A6, >> +          (SBI_SSE_ATTR_INTERRUPTED_A7 - SBI_SSE_ATTR_INTERRUPTED_A6) >> + 1, >> +          arch_evt->interrupted_phys, 0, 0); >> + >> +    memcpy(®s->a6, &arch_evt->interrupted, sizeof(arch_evt- >> >interrupted)); >> + >> +    sse_handle_event(arch_evt, regs); >> + >> +    /* >> +     * The SSE delivery path does not uses the "standard" exception path >> +     * (see sse_entry.S) and does not process any pending signal/ >> softirqs >> +     * due to being similar to a NMI. >> +     * Some drivers (PMU, RAS) enqueue pending work that needs to be >> handled >> +     * as soon as possible by bottom halves. For that purpose, set >> the SIP >> +     * software interrupt pending bit which will force a software >> interrupt >> +     * to be serviced once interrupts are reenabled in the interrupted >> +     * context if they were masked or directly if unmasked. >> +     */ >> +    csr_set(CSR_IP, IE_SIE); >> + >> +    nmi_exit(); >> +} >> + >> +static void *alloc_to_stack_pointer(void *alloc) >> +{ >> +    return alloc ? alloc + SSE_STACK_SIZE : NULL; >> +} >> + >> +static void *stack_pointer_to_alloc(void *stack) >> +{ >> +    return stack - SSE_STACK_SIZE; >> +} >> + >> +#ifdef CONFIG_VMAP_STACK >> +static void *sse_stack_alloc(unsigned int cpu) >> +{ >> +    void *stack = arch_alloc_vmap_stack(SSE_STACK_SIZE, >> cpu_to_node(cpu)); > > > I'm wondering what could happen if, in case of Svvptc, we take a trap in > the sse path because the vmalloced stack page table entry is not yet > seen by the page table walker, would that be an issue? I'll take a look at that but if it trap at the beginning of the SSE handler, it's going to clobber anything that wasn't saved. So basically fatal. Should I 'touch' the stack itself after allocation to be sure it's mapped in the page table ? > > >> + >> +    return alloc_to_stack_pointer(stack); >> +} >> + >> +static void sse_stack_free(void *stack) >> +{ >> +    vfree(stack_pointer_to_alloc(stack)); >> +} >> +#else /* CONFIG_VMAP_STACK */ >> +static void *sse_stack_alloc(unsigned int cpu) >> +{ >> +    void *stack = kmalloc(SSE_STACK_SIZE, GFP_KERNEL); >> + >> +    return alloc_to_stack_pointer(stack); >> +} >> + >> +static void sse_stack_free(void *stack) >> +{ >> +    kfree(stack_pointer_to_alloc(stack)); >> +} >> +#endif /* CONFIG_VMAP_STACK */ >> + >> +static int sse_init_scs(int cpu, struct sse_event_arch_data *arch_evt) >> +{ >> +    void *stack; >> + >> +    if (!scs_is_enabled()) >> +        return 0; >> + >> +    stack = scs_alloc(cpu_to_node(cpu)); >> +    if (!stack) >> +        return -ENOMEM; >> + >> +    arch_evt->shadow_stack = stack; >> + >> +    return 0; >> +} >> + >> +void arch_sse_event_update_cpu(struct sse_event_arch_data *arch_evt, >> int cpu) >> +{ >> +    arch_evt->cpu_id = cpu; >> +    arch_evt->hart_id = cpuid_to_hartid_map(cpu); >> +} >> + >> +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); >> +    if (!stack) >> +        return -ENOMEM; >> + >> +    arch_evt->stack = stack; >> + >> +    if (sse_init_scs(cpu, arch_evt)) { >> +        sse_stack_free(arch_evt->stack); >> +        return -ENOMEM; >> +    } >> + >> +    if (sse_event_is_global(evt_id)) { >> +        arch_evt->interrupted_phys = >> +                    virt_to_phys(&arch_evt->interrupted); > > > I may be missing something but I don't see why interrupted could not be > a userspace address? I'm not sure to follow what you are saying but arch_evt is allocated via kzalloc() from the driver layer (see next commit) and is passed to arch_sse_init_event(). There is no way for this struct to be allocated/passed by userspace. > > >> +    } else { >> +        arch_evt->interrupted_phys = >> +                per_cpu_ptr_to_phys(&arch_evt->interrupted); >> +    } >> + >> +    arch_sse_event_update_cpu(arch_evt, cpu); >> + >> +    return 0; >> +} >> + >> +void arch_sse_free_event(struct sse_event_arch_data *arch_evt) >> +{ >> +    scs_free(arch_evt->shadow_stack); >> +    sse_stack_free(arch_evt->stack); >> +} >> + >> +int arch_sse_register_event(struct sse_event_arch_data *arch_evt) >> +{ >> +    struct sbiret sret; >> + >> +    sret = sbi_ecall(SBI_EXT_SSE, SBI_SSE_EVENT_REGISTER, arch_evt- >> >evt_id, >> +             (unsigned long)handle_sse, (unsigned long)arch_evt, 0, >> +             0, 0); >> + >> +    return sbi_err_map_linux_errno(sret.error); >> +} >> diff --git a/arch/riscv/kernel/sse_entry.S b/arch/riscv/kernel/ >> sse_entry.S >> new file mode 100644 >> index 000000000000..112bdb7d4369 >> --- /dev/null >> +++ b/arch/riscv/kernel/sse_entry.S >> @@ -0,0 +1,180 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Copyright (C) 2024 Rivos Inc. >> + */ >> + >> +#include >> +#include >> + >> +#include >> +#include >> +#include >> + >> +/* When entering handle_sse, the following registers are set: >> + * a6: contains the hartid >> + * a7: contains a sse_event_arch_data struct pointer >> + */ >> +SYM_CODE_START(handle_sse) >> +    /* Save stack temporarily */ >> +    REG_S sp, SSE_REG_EVT_TMP(a7) >> +    /* Set entry stack */ >> +    REG_L sp, SSE_REG_EVT_STACK(a7) >> + >> +    addi sp, sp, -(PT_SIZE_ON_STACK) >> +    REG_S ra, PT_RA(sp) >> +    REG_S s0, PT_S0(sp) >> +    REG_S s1, PT_S1(sp) >> +    REG_S s2, PT_S2(sp) >> +    REG_S s3, PT_S3(sp) >> +    REG_S s4, PT_S4(sp) >> +    REG_S s5, PT_S5(sp) >> +    REG_S s6, PT_S6(sp) >> +    REG_S s7, PT_S7(sp) >> +    REG_S s8, PT_S8(sp) >> +    REG_S s9, PT_S9(sp) >> +    REG_S s10, PT_S10(sp) >> +    REG_S s11, PT_S11(sp) >> +    REG_S tp, PT_TP(sp) >> +    REG_S t0, PT_T0(sp) >> +    REG_S t1, PT_T1(sp) >> +    REG_S t2, PT_T2(sp) >> +    REG_S t3, PT_T3(sp) >> +    REG_S t4, PT_T4(sp) >> +    REG_S t5, PT_T5(sp) >> +    REG_S t6, PT_T6(sp) >> +    REG_S gp, PT_GP(sp) >> +    REG_S a0, PT_A0(sp) >> +    REG_S a1, PT_A1(sp) >> +    REG_S a2, PT_A2(sp) >> +    REG_S a3, PT_A3(sp) >> +    REG_S a4, PT_A4(sp) >> +    REG_S a5, PT_A5(sp) >> + >> +    /* Retrieve entry sp */ >> +    REG_L a4, SSE_REG_EVT_TMP(a7) >> +    /* Save CSRs */ >> +    csrr a0, CSR_EPC >> +    csrr a1, CSR_SSTATUS >> +    csrr a2, CSR_STVAL >> +    csrr a3, CSR_SCAUSE >> + >> +    REG_S a0, PT_EPC(sp) >> +    REG_S a1, PT_STATUS(sp) >> +    REG_S a2, PT_BADADDR(sp) >> +    REG_S a3, PT_CAUSE(sp) >> +    REG_S a4, PT_SP(sp) >> + >> +    /* Disable user memory access and floating/vector computing */ >> +    li t0, SR_SUM | SR_FS_VS >> +    csrc CSR_STATUS, t0 >> + >> +    load_global_pointer >> +    scs_load_sse_stack a7 >> + >> +    /* Restore current task struct from __sse_entry_task */ >> +    li t1, ASM_NR_CPUS >> +    mv t3, zero > > > t1 is only used in find_hart_id_slowpath and shouldn't we use the real > number of present cpus instead of NR_CPUS? Indeed, that should be moved to the slow path which iterates over hart_id_to_cpu_map. This array is sized based on NR_CPUS which is why I use NR_CPUS rather than loading some other variable with the count opf onlines cpus. Moreover, the slowpath should never loop up to NR_CPUS since the SBI must passed a valid hartid index. > > t3 is clobbered by cpu_id below, is that correct? In case of !SMP it is kept to zero. But I'll move that in a else below. > > >> + >> +#ifdef CONFIG_SMP >> +    REG_L t4, SSE_REG_HART_ID(a7) >> +    REG_L t3, SSE_REG_CPU_ID(a7) > > > hart_id and cpu_id are "unsigned int", not xlen-wide fields so you must > use lw here, not REG_L (a lot of bugs lately on the usage of this macro). Ouch, thanks for catching that. > > >> + >> +    bne t4, a6, .Lfind_hart_id_slowpath >> + >> +.Lcpu_id_found: >> +#endif >> +    asm_per_cpu_with_cpu t2 __sse_entry_task t1 t3 >> +    REG_L tp, 0(t2) >> + >> +    mv a1, sp /* pt_regs on stack */ >> + >> +    /* >> +     * Save sscratch for restoration since we might have interrupted the >> +     * kernel in early exception path and thus, we don't know the >> content of >> +     * sscratch. >> +     */ >> +    csrr s4, CSR_SSCRATCH >> +    /* In-kernel scratch is 0 */ >> +    csrw CSR_SCRATCH, x0 > > > csrrw s4, CSR_SSCRATCH, x0 instead? Ah yeah. Thanks ! Clément > > >> + >> +    mv a0, a7 >> + >> +    call do_sse >> + >> +    csrw CSR_SSCRATCH, s4 >> + >> +    REG_L a0, PT_STATUS(sp) >> +    REG_L a1, PT_EPC(sp) >> +    REG_L a2, PT_BADADDR(sp) >> +    REG_L a3, PT_CAUSE(sp) >> +    csrw CSR_SSTATUS, a0 >> +    csrw CSR_EPC, a1 >> +    csrw CSR_STVAL, a2 >> +    csrw CSR_SCAUSE, a3 >> + >> +    REG_L ra, PT_RA(sp) >> +    REG_L s0, PT_S0(sp) >> +    REG_L s1, PT_S1(sp) >> +    REG_L s2, PT_S2(sp) >> +    REG_L s3, PT_S3(sp) >> +    REG_L s4, PT_S4(sp) >> +    REG_L s5, PT_S5(sp) >> +    REG_L s6, PT_S6(sp) >> +    REG_L s7, PT_S7(sp) >> +    REG_L s8, PT_S8(sp) >> +    REG_L s9, PT_S9(sp) >> +    REG_L s10, PT_S10(sp) >> +    REG_L s11, PT_S11(sp) >> +    REG_L tp, PT_TP(sp) >> +    REG_L t0, PT_T0(sp) >> +    REG_L t1, PT_T1(sp) >> +    REG_L t2, PT_T2(sp) >> +    REG_L t3, PT_T3(sp) >> +    REG_L t4, PT_T4(sp) >> +    REG_L t5, PT_T5(sp) >> +    REG_L t6, PT_T6(sp) >> +    REG_L gp, PT_GP(sp) >> +    REG_L a0, PT_A0(sp) >> +    REG_L a1, PT_A1(sp) >> +    REG_L a2, PT_A2(sp) >> +    REG_L a3, PT_A3(sp) >> +    REG_L a4, PT_A4(sp) >> +    REG_L a5, PT_A5(sp) >> + >> +    REG_L sp, PT_SP(sp) >> + >> +    li a7, SBI_EXT_SSE >> +    li a6, SBI_SSE_EVENT_COMPLETE >> +    ecall >> + >> +#ifdef CONFIG_SMP >> +.Lfind_hart_id_slowpath: >> + >> +/* Slowpath to find the CPU id associated to the hart id */ >> +la t0, __cpuid_to_hartid_map > > > ^ Missing tab > > >> + >> +.Lhart_id_loop: >> +    REG_L t2, 0(t0) >> +    beq t2, a6, .Lcpu_id_found >> + >> +    /* Increment pointer and CPU number */ >> +    addi t3, t3, 1 >> +    addi t0, t0, RISCV_SZPTR >> +    bltu t3, t1, .Lhart_id_loop >> + >> +    /* >> +     * This should never happen since we expect the hart_id to match one >> +     * of our CPU, but better be safe than sorry >> +     */ >> +    la tp, init_task >> +    la a0, sse_hart_id_panic_string >> +    la t0, panic >> +    jalr t0 >> + > > This newline is not needed ^ > >> +#endif >> + >> +SYM_CODE_END(handle_sse) >> + >> +SYM_DATA_START_LOCAL(sse_hart_id_panic_string) >> +    .ascii "Unable to match hart_id with cpu\0" >> +SYM_DATA_END(sse_hart_id_panic_string) > > > Thanks, > > Alex >