Re: [PATCH 1/3] lib: sbi: Add RISC-V vector context save/restore support
Dave Patel <[email protected]>
| Newsgroups | org.infradead.lists.opensbi |
|---|---|
| Message-ID | <[email protected]> |
On 5/9/26 09:33, Anup Patel wrote: > On Wed, Apr 8, 2026 at 12:52 PM <[email protected]> wrote: >> >> From: Dave Patel <[email protected]> >> >> Eager context switch: Add support for saving and restoring RISC-V vector >> extension state in OpenSBI. This introduces a per-hart vector context >> structure and helper routines to perform full context save and restore. >> >> The vector context includes vcsr CSRs along with storage for all 32 vector >> registers. The register state is saved and restored using byte-wise vector >> load/store instructions (vs8r/vl8r). >> >> The implementation follows an eager context switching model where the entire >> vector state is saved and restored on every context switch. This provides a >> simple and deterministic mechanism without requiring lazy trap-based >> management. >> >> Notes: >> - The SBI_MAX_VLENB is configured using CONFIG_SBI_MAX_VLENB. >> >> Signed-off-by: Dave Patel <[email protected]> >> --- >> include/sbi/sbi_vector.h | 30 ++++++++ >> lib/sbi/Kconfig | 4 + >> lib/sbi/objects.mk | 1 + >> lib/sbi/sbi_vector.c | 155 +++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 190 insertions(+) >> create mode 100644 include/sbi/sbi_vector.h >> create mode 100644 lib/sbi/sbi_vector.c >> >> diff --git a/include/sbi/sbi_vector.h b/include/sbi/sbi_vector.h >> new file mode 100644 >> index 00000000..3b63b02c >> --- /dev/null >> +++ b/include/sbi/sbi_vector.h >> @@ -0,0 +1,30 @@ >> +/* >> + * SPDX-License-Identifier: BSD-2-Clause >> + * >> + * Copyright (c) 2026 RISCstar Solutions. >> + * >> + * Authors: >> + * Dave Patel <[email protected]> >> + */ >> + >> +#ifndef __SBI_VECTOR_H__ >> +#define __SBI_VECTOR_H__ >> + >> +#include <sbi/sbi_types.h> >> + >> +#define SBI_MAX_VLENB CONFIG_SBI_MAX_VLENB >> + >> +struct sbi_vector_context { >> + unsigned long vcsr; >> + unsigned long vstart; >> + >> + /* size depends on VLEN */ >> + uint8_t vregs[32 * SBI_MAX_VLENB]; > > We have same OpenSBI firmware running of multiple platforms > so please discover max_vlenb as a hart feature in sbi_hart.c > > The vregs over here should be "uint8_t vregs[]" and > "struct sbi_vector_context" must be dynamically allocated. I will allocate this dynamically, however the original changes were setting it dynamically and moved to to static due to review comment. Am I ok in understanding that the struct sbi_vector_context vec_ctx; should be struct sbi_vector_context * > >> +}; >> + >> +void sbi_vector_save(struct sbi_vector_context *dst); >> +void sbi_vector_restore(const struct sbi_vector_context *src); >> +int sbi_vector_domain_init(void); >> + >> +#endif //__SBI_VECTOR_H__ >> + >> diff --git a/lib/sbi/Kconfig b/lib/sbi/Kconfig >> index 8479f861..b2432150 100644 >> --- a/lib/sbi/Kconfig >> +++ b/lib/sbi/Kconfig >> @@ -74,4 +74,8 @@ config SBI_ECALL_VIRQ >> bool "VIRQ extension" >> default y >> >> +config SBI_MAX_VLENB >> + int "Vector VLENB size" >> + default 256 >> + >> endmenu >> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk >> index 68fc2036..ecb2b54e 100644 >> --- a/lib/sbi/objects.mk >> +++ b/lib/sbi/objects.mk >> @@ -109,3 +109,4 @@ libsbi-objs-y += sbi_trap_v_ldst.o >> libsbi-objs-y += sbi_unpriv.o >> libsbi-objs-y += sbi_expected_trap.o >> libsbi-objs-y += sbi_cppc.o >> +libsbi-objs-y += sbi_vector.o >> diff --git a/lib/sbi/sbi_vector.c b/lib/sbi/sbi_vector.c >> new file mode 100644 >> index 00000000..29434e2e >> --- /dev/null >> +++ b/lib/sbi/sbi_vector.c >> @@ -0,0 +1,155 @@ >> +/* >> + * SPDX-License-Identifier: BSD-2-Clause >> + * >> + * Copyright (c) 2026 RISCstar Solutions. >> + * >> + * Authors: >> + * Dave Patel <[email protected]> >> + */ >> + >> +#include <sbi/sbi_domain.h> >> +#include <sbi/riscv_encoding.h> >> +#include <sbi/riscv_asm.h> >> +#include <sbi/sbi_vector.h> >> +#include <sbi/sbi_types.h> >> +#include <sbi/sbi_hart.h> >> +#include <sbi/sbi_error.h> >> +#include <sbi/sbi_console.h> >> + >> +#ifdef OPENSBI_CC_SUPPORT_VECTOR >> + >> +static inline unsigned long vector_vlenb(void) >> +{ >> + unsigned long vlenb = 0; >> + >> + asm volatile ( >> + ".option push\n\t" >> + ".option arch, +v\n\t" >> + "csrr %0, vlenb\n\t" >> + ".option pop\n\t" >> + : "=r"(vlenb) >> + : >> + : "memory"); >> + >> + return vlenb; >> +} >> + >> +void sbi_vector_save(struct sbi_vector_context *dst) >> +{ >> + if (!dst) >> + return; >> + >> +#define READ_CSR(dst, csr) \ >> + ({ \ >> + asm volatile ( \ >> + " .option push\n\t" \ >> + " .option arch, +v\n\t" \ >> + " csrr %0, " #csr "\n\t" \ >> + " .option pop\n\t" \ >> + : "=r"(dst) \ >> + : \ >> + : "memory"); \ >> + }) \ >> + >> + /* Step 1: Save CSRs */ >> + READ_CSR(dst->vcsr, vcsr); >> + READ_CSR(dst->vstart, vstart); > > use csr_read() instead of READ_CSR(). ok thanks, will do this. > >> + >> +#undef READ_CSR >> + >> + ulong vlenb = vector_vlenb(); >> + uint8_t *base = dst->vregs; >> + >> + /* Step 3: Save vector registers */ >> +#define SAVE_VREG(i) \ >> + ({ \ >> + asm volatile( \ >> + " .option push\n\t" \ >> + " .option arch, +v\n\t" \ >> + " vs8r.v v" #i ", (%0)\n\t" \ >> + " .option pop\n\t" \ >> + :: "r"(base + (i) * vlenb) : "memory"); \ >> + }) \ >> + >> + SAVE_VREG(0); >> + SAVE_VREG(8); >> + SAVE_VREG(16); >> + SAVE_VREG(24); >> + >> +#undef SAVE_VREG >> +} >> + >> +void sbi_vector_restore(const struct sbi_vector_context *src) >> +{ >> + if (!src) >> + return; >> + >> + const uint8_t *base = src->vregs; >> + ulong vlenb = vector_vlenb(); >> + >> + /* Step 2: Restore vector registers */ >> +#define RESTORE_VREG(i) \ >> + ({ \ >> + asm volatile( \ >> + " .option push\n\t" \ >> + " .option arch, +v\n\t" \ >> + " vl8r.v v" #i ", (%0)\n\t" \ >> + " .option pop\n\t" \ >> + :: "r"(base + (i) * vlenb) : "memory"); \ >> + }) \ >> + >> + RESTORE_VREG(0); >> + RESTORE_VREG(8); >> + RESTORE_VREG(16); >> + RESTORE_VREG(24); >> +#undef RESTORE_VREG >> + >> + /* Step 3: Restore CSR's last */ >> +#define WRITE_CSR(csr, val) \ >> + ({ \ >> + asm volatile( \ >> + " .option push\n\t" \ >> + " .option arch, +v\n\t" \ >> + " csrw " #csr ", %0\n\t" \ >> + " .option pop\n\t" \ >> + : \ >> + : "r"(val) \ >> + : "memory"); \ >> + }) \ >> + >> + /* Restore CSRs first */ >> + WRITE_CSR(vcsr, src->vcsr); >> + WRITE_CSR(vstart, src->vstart); >> +#undef WRITE_CSR > > Use csr_write() instead of WRITE_CSR(). > >> +} >> + >> +int sbi_vector_domain_init(void) >> +{ >> + csr_set(CSR_MSTATUS, MSTATUS_VS); >> + ulong vlenb = vector_vlenb(); >> + >> + if (vlenb > SBI_MAX_VLENB) { >> + sbi_printf("[Vector ERR:] vlenb range error\n"); >> + return SBI_ERR_BAD_RANGE; >> + } >> + >> + csr_clear(CSR_MSTATUS, MSTATUS_VS); > > Why this ? > > The mstatus_init() in sbi_hart.c sets MSTATUS_VS. This is done once during init at the start to check the vlenb, which can only be fetched if VS is enabled, hence the change. I am clearing it back to it original state. > > Regards, > Anup Hi Anup, Thanks for the comments. Please see inline comments above and clarify it so that I can push changes. Thanks Dave -- opensbi mailing list [email protected] http://lists.infradead.org/mailman/listinfo/opensbi