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

[Dave Patel <dave.patel@riscstar.com>]

On 3/23/26 04:37, Samuel Holland wrote:
> Hi Dave,
> 
> On 2026-03-20 9:30 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.
>>
>> A per-hart pointer is used to track the current vector context owner.
> 
> Storing only one context per hart means that you must save/restore the context
> every time the hart traps to M-mode. This is unnecessary and inefficient,
> because OpenSBI itself does not use the vector registers (except where it
> explicitly modifies the lower-privilege vector state when emulating
> instructions). It is only necessary to save/restore the context when switching
> between domains, not when trapping to M-mode and returning to the same domain.
> So a better place for the context is in `struct hart_context` in
> lib/sbi/sbi_domain_context.c.

This was the comment of the email and RFC is for discussion as OpenSBI
does not use the vector registers, the implementation was to give the
context with the RFC  (overhead on every trap due to full state save and
restore). Is it possible to provide usecase and more information to
cover this as Opensbi won't be using the vector registers?
Also do we have any consideration of how this context switch work when
there a trap occurs in the middle of this context?
Also the domain context should be covered by S-mode for example Linux
kernel, how is it different to domain context in Opensbi?


> 
>> Notes:
>> - The maximum supported VLEN is capped via SBI_MAX_VLENB.
>> - The implementation assumes the vector unit is enabled when invoked.
> 
> This is not something you can assume. The code must not crash when running on
> hardware without vector ISA support (Zve32x).
Yes I was going to provide Complie switch, however left it open for
discussion. So should this only be compiled switch for
PLATFORM_RISCV_ISA=rv64gcv? and that we don't want this to be included
for Zve32x?

> 
>> - vstart CSR is not currently saved/restored and is expected to be zero
>> across context switch boundaries.
> 
> This is not something you can assume. A trap can happen anywhere (e.g. due to an
> interrupt), so vstart may be nonzero and must not leak across domains.
Yes correct, I deliberately left it open to have a discussion and view
point, so I will save vstart CSR for context aswell.

> 
>>
>> Signed-off-by: Dave Patel <dave.patel@riscstar.com>
>> ---
>>  include/sbi/sbi_vector.h |  30 +++++++++
>>  lib/sbi/objects.mk       |   1 +
>>  lib/sbi/sbi_vector.c     | 136 +++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 167 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..6be477c0
>> --- /dev/null
>> +++ b/include/sbi/sbi_vector.h
>> @@ -0,0 +1,30 @@
>> +/* 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>
>> +
>> +#define SBI_MAX_VLENB 256
>> +
>> +struct sbi_vector_context {
>> +    unsigned long vl;
>> +    unsigned long vtype;
>> +    unsigned long vcsr;
>> +
>> +    /* size depends on VLEN */
>> +    uint8_t vregs[32 * SBI_MAX_VLENB];
>> +};
>> +
>> +struct sbi_vector_context *sbi_current_vector_context(void);
>> +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..5c0caf39 100644
>> --- a/lib/sbi/objects.mk
>> +++ b/lib/sbi/objects.mk
>> @@ -106,3 +106,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..37239ce2
>> --- /dev/null
>> +++ b/lib/sbi/sbi_vector.c
>> @@ -0,0 +1,136 @@
>> +/* 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>
>> +
>> +/* Per-hart vector owner */
>> +static inline struct sbi_vector_context **vec_owner_ptr(void)
>> +{
>> +    struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
>> +    return &scratch->vec_ctx;
> 
> The code must compile and run after every patch in the series, but the vec_ctx
> member isn't added until patch 3, so this doesn't compile. However, as mentioned
> above, the context should be stored per-domain anyway.
ok, thanks.
> 
> Regards,
> Samuel
> 


Hi Samuel,
          Thanks for taking out time and providing comments: very much
appreciated.

I have some question.

Thanks
Dave

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi