Re: [PATCH v2 1/3] lib: sbi: Add RISC-V vector context save/restore support (eager switching)

Samuel Holland
2026-03-26T14:27:55Z

Hi Dave,

On 2026-03-26 6:55 AM, dave.patel@riscstar.com wrote:
> From: Dave Patel <dave.patel@riscstar.com>
>
> 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 vl, vtype, vcsr CSRs along with storage for all
> 32 vector registers. The register state is saved and restored using byte-wise
> vector load/store instructions (vse8.v/vle8.v), making the implementation
> independent of current SEW/LMUL configuration.
>
> 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 <dave.patel@riscstar.com>
> ---
> include/sbi/sbi_vector.h | 35 ++++++++++
> lib/sbi/objects.mk | 7 ++
> lib/sbi/sbi_vector.c | 146 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 188 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..ae151406
> --- /dev/null
> +++ b/include/sbi/sbi_vector.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (c) 2026 RISCstar Solutions.
> + *
> + * Authors:
> + * Dave Patel <dave.patel@riscstar.com>
> + */
> +
> +#ifndef __SBI_VECTOR_H__
> +#define __SBI_VECTOR_H__
> +
> +#include <sbi/sbi_types.h>
> +
> +#ifdef CONFIG_SBI_MAX_VLENB
> +#define SBI_MAX_VLENB CONFIG_SBI_MAX_VLENB
> +#else
> +#define SBI_MAX_VLENB 256
> +#endif

You should include the corresponding Kconfig change in this patch, and then
CONFIG_SBI_MAX_VLENB will always be defined, so you will not need a fallback.

Also, you need to check vector_vlenb() against CONFIG_SBI_MAX_VLENB somewhere
during the init sequence, and refuse to boot if it is too large.

> +
> +struct sbi_vector_context {
> + unsigned long vl;
> + unsigned long vtype;
> + unsigned long vcsr;
> + unsigned long vstart;
> +
> + /* size depends on VLEN */
> + uint8_t vregs[32 * SBI_MAX_VLENB];
> +};

OpenSBI follows the Linux kernel coding style. All of the indentation in your
patches is incorrect. Please use scripts/checkpatch.pl on your patches.

> +
> +struct sbi_vector_context *sbi_current_vector_context(void);

This function no longer exists.

> +void sbi_vector_save(struct sbi_vector_context *dst);
> +void sbi_vector_restore(const struct sbi_vector_context *src);
> +
> +#endif //__SBI_VECTOR_H__
> +
> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
> index ea816e92..9fd378d0 100644
> --- a/lib/sbi/objects.mk
> +++ b/lib/sbi/objects.mk
> @@ -106,3 +106,10 @@ 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
> +
> +RISCV_EXTS := $(patsubst rv32%,%,$(patsubst rv64%,%,$(PLATFORM_RISCV_ISA)))
> +$(info RESULT = $(findstring v,$(RISCV_EXTS)))

This check will never succeed, because it is not safe to include "v" in the
compiler's ISA string. See commit 5545602f778e ("Makefile: Don't enable
V-extension using -march option"). You need to use ".option arch" in the inline
assembly, like in lib/sbi/sbi_trap_v_ldst.c.

> +
> +ifneq ($(findstring v,$(RISCV_EXTS)),)
> +libsbi-objs-y += sbi_vector.o
> +endif
> diff --git a/lib/sbi/sbi_vector.c b/lib/sbi/sbi_vector.c
> new file mode 100644
> index 00000000..e14b658c
> --- /dev/null
> +++ b/lib/sbi/sbi_vector.c
> @@ -0,0 +1,146 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (c) 2026 RISCstar Solutions.
> + *
> + * Authors:
> + * Dave Patel <dave.patel@riscstar.com>
> + */
> +
> +#include <sbi/sbi_domain.h>
> +#include <sbi/riscv_encoding.h>
> +#include <sbi/riscv_asm.h>
> +#include <sbi/sbi_vector.h>
> +
> +static inline unsigned long vector_vlenb(void)
> +{
> + unsigned long vlenb;
> + asm volatile ("csrr %0, vlenb" : "=r"(vlenb));
> + return vlenb;
> +}
> +
> +void sbi_vector_save(struct sbi_vector_context *dst)
> +{
> + if (!dst)
> + return;
> +
> + uint8_t *base = dst->vregs;
> + unsigned long vlenb;
> +
> + /* Save CSRs */
> + asm volatile("csrr %0, vtype" : "=r"(dst->vtype));
> + asm volatile("csrr %0, vl" : "=r"(dst->vl));
> + asm volatile("csrr %0, vcsr" : "=r"(dst->vcsr));
> + asm volatile("csrr %0, vstart" : "=r"(dst->vstart));
> +
> + /*
> + * Set a known vector configuration before accessing registers.
> + * This ensures the hardware is in a consistent state for save.
> + */
> + {
> + unsigned long tmp;
> + asm volatile("vsetvl %0, %1, %2"
> + : "=r"(tmp)
> + : "r"(dst->vl), "r"(dst->vtype));
> + }
> +
> + vlenb = vector_vlenb();
> +
> +#define SAVE_VREG(i) \
> + asm volatile("vse8.v v" #i ", (%0)" :: "r"(base + (i) * vlenb) : "memory")
> +
> + SAVE_VREG(0);
> + SAVE_VREG(1);
> + SAVE_VREG(2);
> + SAVE_VREG(3);
> + SAVE_VREG(4);
> + SAVE_VREG(5);
> + SAVE_VREG(6);
> + SAVE_VREG(7);
> + SAVE_VREG(8);
> + SAVE_VREG(9);
> + SAVE_VREG(10);
> + SAVE_VREG(11);
> + SAVE_VREG(12);
> + SAVE_VREG(13);
> + SAVE_VREG(14);
> + SAVE_VREG(15);
> + SAVE_VREG(16);
> + SAVE_VREG(17);
> + SAVE_VREG(18);
> + SAVE_VREG(19);
> + SAVE_VREG(20);
> + SAVE_VREG(21);
> + SAVE_VREG(22);
> + SAVE_VREG(23);
> + SAVE_VREG(24);
> + SAVE_VREG(25);
> + SAVE_VREG(26);
> + SAVE_VREG(27);
> + SAVE_VREG(28);
> + SAVE_VREG(29);
> + SAVE_VREG(30);
> + SAVE_VREG(31);
> +
> +#undef SAVE_VREG
> +}
> +
> +void sbi_vector_restore(const struct sbi_vector_context *src)
> +{
> + if (!src)
> + return;
> +
> + const uint8_t *base = src->vregs;
> + unsigned long vlenb = vector_vlenb();
> +
> + /* Restore CSRs first */
> + asm volatile("csrw vtype, %0" :: "r"(src->vtype));
> + asm volatile("csrw vl, %0" :: "r"(src->vl));
> + asm volatile("csrw vcsr, %0" :: "r"(src->vcsr));
> + asm volatile("csrw vstart, %0" :: "r"(src->vstart));
> +
> + /* Re-establish vector state */
> + {
> + unsigned long tmp;
> + asm volatile("vsetvl %0, %1, %2"
> + : "=r"(tmp)
> + : "r"(src->vl), "r"(src->vtype));
> + }

This clobbers the CSRs you just restored. You have to restore the CSRs last.

Regards,
Samuel

> +
> +#define RESTORE_VREG(i) \
> + asm volatile("vle8.v v" #i ", (%0)" :: "r"(base + (i) * vlenb) : "memory")
> +
> + RESTORE_VREG(0);
> + RESTORE_VREG(1);
> + RESTORE_VREG(2);
> + RESTORE_VREG(3);
> + RESTORE_VREG(4);
> + RESTORE_VREG(5);
> + RESTORE_VREG(6);
> + RESTORE_VREG(7);
> + RESTORE_VREG(8);
> + RESTORE_VREG(9);
> + RESTORE_VREG(10);
> + RESTORE_VREG(11);
> + RESTORE_VREG(12);
> + RESTORE_VREG(13);
> + RESTORE_VREG(14);
> + RESTORE_VREG(15);
> + RESTORE_VREG(16);
> + RESTORE_VREG(17);
> + RESTORE_VREG(18);
> + RESTORE_VREG(19);
> + RESTORE_VREG(20);
> + RESTORE_VREG(21);
> + RESTORE_VREG(22);
> + RESTORE_VREG(23);
> + RESTORE_VREG(24);
> + RESTORE_VREG(25);
> + RESTORE_VREG(26);
> + RESTORE_VREG(27);
> + RESTORE_VREG(28);
> + RESTORE_VREG(29);
> + RESTORE_VREG(30);
> + RESTORE_VREG(31);
> +
> +#undef RESTORE_VREG
> +}
> --
> 2.43.0
>