Re: [PATCH v5 1/4] lib: sbi: Add RISC-V vector context save/restore support

On 5/15/26 08:41, Anup Patel wrote:
> On Fri, May 15, 2026 at 11:12 AM <[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.
>>
>> Signed-off-by: Dave Patel <[email protected]>
>> ---
>>  include/sbi/sbi_vector.h |  28 ++++++++++
>>  lib/sbi/objects.mk       |   1 +
>>  lib/sbi/sbi_vector.c     | 109 +++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 138 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..bbd857c3
>> --- /dev/null
>> +++ b/include/sbi/sbi_vector.h
>> @@ -0,0 +1,28 @@
>> +/*
>> + * 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>
>> +
>> +struct sbi_vector_context {
>> +       unsigned long vcsr;
>> +       unsigned long vstart;
>> +
>> +       /* size depends on VLEN */
>> +       uint8_t vregs[];
>> +};
>> +
>> +void sbi_vector_save(struct sbi_vector_context *dst);
>> +void sbi_vector_restore(const struct sbi_vector_context *src);
>> +unsigned long vector_vlenb(void);
>> +
>> +#endif //__SBI_VECTOR_H__
>> +
>> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
>> index 97cc4521..ddb2e7ac 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..1d2ac944
>> --- /dev/null
>> +++ b/lib/sbi/sbi_vector.c
>> @@ -0,0 +1,109 @@
>> +/*
>> + * 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
>> +
>> +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;
>> +
>> +       /* Step 1: Save CSRs */
>> +       dst->vcsr = csr_read(vcsr);
>> +       dst->vstart = csr_read(vstart);
>> +
>> +       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);
> 
> Same issue as FP save/restore, this will trap and crash
> if Vector is disabled in mstatus.VS.
> 
> For correct implementation, refer __riscv_v_vstate_save()/restore()
> from <linux_source>/arch/riscv/include/asm/vector.h
> 
>> +
>> +#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 */
>> +       /* Restore CSRs first */
>> +       csr_write(vcsr,   src->vcsr);
>> +       csr_write(vstart, src->vstart);
>> +}
>> +
>> +#else
>> +
>> +void sbi_vector_save(struct sbi_vector_context *dst)
>> +{
>> +}
>> +
>> +void sbi_vector_restore(const struct sbi_vector_context *src)
>> +{
>> +}
>> +
>> +#endif /* OPENSBI_CC_SUPPORT_VECTOR */
>> --
>> 2.43.0
>>
>>
>> --
>> opensbi mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/opensbi
> 
> Regards,
> Anup

Hi Anup,
        Firstly thanks for all your review comments, I have endeavor to
cover all of them, so please confirm.

Rgarding this comment of mstatus.VS and FS (I think which includes 3
comments)

I have already have code inplace in patch 3, please see below line from
patch 3

+       /* Make sure FS and VS is on before context switch */

+       csr_set(CSR_MSTATUS, MSTATUS_FS | MSTATUS_VS);

Since the context of Floating and Vector is more granular in Opensbi I
thought of handle in context switch.

So there is no issue of trap and crash, however there is still a
question regarding if you are fine with this or you want it more fine
grained inside its individual functions?

Please let me know.

Thanks
Dave


-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi