[PATCH 2/5] lib: sbi: Introduce hart protection abstraction
Anup Patel
apatel at ventanamicro.com
Tue Dec 9 00:30:37 PST 2025
On Tue, Dec 9, 2025 at 12:34 PM Samuel Holland
<samuel.holland at sifive.com> wrote:
>
> Hi Anup,
>
> On 2025-12-08 12:58 AM, Anup Patel wrote:
> > On Sun, Dec 7, 2025 at 4:40 PM Samuel Holland <samuel.holland at sifive.com> wrote:
> >> On 2025-11-26 8:18 AM, Anup Patel wrote:
> >>> Currently, PMP and ePMP are the only hart protection mechanisms
> >>> available in OpenSBI but new protection mechanisms (such as Smmpt)
> >>> will be added in the near future.
> >>>
> >>> To allow multiple hart protection mechanisms, introduce hart
> >>> protection abstraction and related APIs.
> >>>
> >>> Signed-off-by: Anup Patel <apatel at ventanamicro.com>
> >>> ---
> >>> include/sbi/sbi_hart_protection.h | 88 ++++++++++++++++++++++++
> >>> lib/sbi/objects.mk | 1 +
> >>> lib/sbi/sbi_hart_protection.c | 110 ++++++++++++++++++++++++++++++
> >>> lib/sbi/sbi_init.c | 5 ++
> >>> 4 files changed, 204 insertions(+)
> >>> create mode 100644 include/sbi/sbi_hart_protection.h
> >>> create mode 100644 lib/sbi/sbi_hart_protection.c
> >>>
> >>> diff --git a/include/sbi/sbi_hart_protection.h b/include/sbi/sbi_hart_protection.h
> >>> new file mode 100644
> >>> index 00000000..c9e452db
> >>> --- /dev/null
> >>> +++ b/include/sbi/sbi_hart_protection.h
> >>> @@ -0,0 +1,88 @@
> >>> +/*
> >>> + * SPDX-License-Identifier: BSD-2-Clause
> >>> + *
> >>> + * Copyright (c) 2025 Ventana Micro Systems Inc.
> >>> + */
> >>> +
> >>> +#ifndef __SBI_HART_PROTECTION_H__
> >>> +#define __SBI_HART_PROTECTION_H__
> >>> +
> >>> +#include <sbi/sbi_types.h>
> >>> +#include <sbi/sbi_list.h>
> >>> +
> >>> +struct sbi_scratch;
> >>> +
> >>> +/** Representation of hart protection mechanism */
> >>> +struct sbi_hart_protection {
> >>> + /** List head */
> >>> + struct sbi_dlist head;
> >>> +
> >>> + /** Name of the hart protection mechanism */
> >>> + char name[32];
> >>> +
> >>> + /** Ratings of the hart protection mechanism (higher is better) */
> >>> + unsigned long rating;
> >>> +
> >>> + /** Configure protection for current HART (Mandatory) */
> >>> + int (*configure)(struct sbi_scratch *scratch);
> >>> +
> >>> + /** Unconfigure protection for current HART (Mandatory) */
> >>> + void (*unconfigure)(struct sbi_scratch *scratch);
> >>> +
> >>> + /** Create temporary mapping to access address range on current HART (Optional) */
> >>> + int (*map_range)(struct sbi_scratch *scratch,
> >>> + unsigned long base, unsigned long size);
> >>> +
> >>> + /** Destroy temporary mapping on current HART (Optional) */
> >>> + int (*unmap_range)(struct sbi_scratch *scratch,
> >>> + unsigned long base, unsigned long size);
> >>> +};
> >>> +
> >>> +/**
> >>> + * Get the best hart protection mechanism
> >>> + *
> >>> + * @return pointer to best hart protection mechanism
> >>> + */
> >>> +struct sbi_hart_protection *sbi_hart_protection_best(void);
> >>> +
> >>> +/**
> >>> + * Register a hart protection mechanism
> >>> + *
> >>> + * @return 0 on success and negative error code on failure
> >>> + */
> >>> +int sbi_hart_protection_register(struct sbi_hart_protection *hprot);
> >>> +
> >>> +/**
> >>> + * Unregister a hart protection mechanism
> >>> + *
> >>> + * @return 0 on success and negative error code on failure
> >>> + */
> >>> +int sbi_hart_protection_unregister(struct sbi_hart_protection *hprot);
> >>> +
> >>> +/**
> >>> + * Configure protection for current HART
> >>> + *
> >>> + * @return 0 on success and negative error code on failure
> >>> + */
> >>> +int sbi_hart_protection_configure(struct sbi_scratch *scratch);
> >>> +
> >>> +/**
> >>> + * Unconfigure protection for current HART
> >>> + */
> >>> +void sbi_hart_protection_unconfigure(struct sbi_scratch *scratch);
> >>> +
> >>> +/**
> >>> + * Create temporary mapping to access address range on current HART
> >>> + *
> >>> + * @return 0 on success and negative error code on failure
> >>> + */
> >>> +int sbi_hart_protection_map_range(unsigned long base, unsigned long size);
> >>> +
> >>> +/**
> >>> + * Destroy temporary mapping to access address range on current HART
> >>> + *
> >>> + * @return 0 on success and negative error code on failure
> >>> + */
> >>> +int sbi_hart_protection_unmap_range(unsigned long base, unsigned long size);
> >>
> >> These functions should document (or take a parameter for) the permissions of the
> >> temporary mapping. "saddr" sort of documents that for the existing functions.
> >
> > Sure, I will add parameter documentation.
> >
> >>
> >>> +
> >>> +#endif /* __SBI_HART_PROTECTION_H__ */
> >>> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
> >>> index 8abe1e8e..51588cd1 100644
> >>> --- a/lib/sbi/objects.mk
> >>> +++ b/lib/sbi/objects.mk
> >>> @@ -75,6 +75,7 @@ libsbi-objs-y += sbi_emulate_csr.o
> >>> libsbi-objs-y += sbi_fifo.o
> >>> libsbi-objs-y += sbi_fwft.o
> >>> libsbi-objs-y += sbi_hart.o
> >>> +libsbi-objs-y += sbi_hart_protection.o
> >>> libsbi-objs-y += sbi_heap.o
> >>> libsbi-objs-y += sbi_math.o
> >>> libsbi-objs-y += sbi_hfence.o
> >>> diff --git a/lib/sbi/sbi_hart_protection.c b/lib/sbi/sbi_hart_protection.c
> >>> new file mode 100644
> >>> index 00000000..831072ad
> >>> --- /dev/null
> >>> +++ b/lib/sbi/sbi_hart_protection.c
> >>> @@ -0,0 +1,110 @@
> >>> +/*
> >>> + * SPDX-License-Identifier: BSD-2-Clause
> >>> + *
> >>> + * Copyright (c) 2025 Ventana Micro Systems Inc.
> >>> + */
> >>> +
> >>> +#include <sbi/sbi_error.h>
> >>> +#include <sbi/sbi_hart_protection.h>
> >>> +#include <sbi/sbi_scratch.h>
> >>> +
> >>> +static SBI_LIST_HEAD(hart_protection_list);
> >>> +
> >>> +struct sbi_hart_protection *sbi_hart_protection_best(void)
> >>> +{
> >>> + if (sbi_list_empty(&hart_protection_list))
> >>> + return NULL;
> >>> +
> >>> + return sbi_list_first_entry(&hart_protection_list, struct sbi_hart_protection, head);
> >>> +}
> >>> +
> >>> +int sbi_hart_protection_register(struct sbi_hart_protection *hprot)
> >>> +{
> >>> + struct sbi_hart_protection *pos = NULL;
> >>> + bool found_pos = false;
> >>> +
> >>> + if (!hprot)
> >>> + return SBI_EINVAL;
> >>> +
> >>> + sbi_list_for_each_entry(pos, &hart_protection_list, head) {
> >>> + if (hprot->rating > pos->rating) {
> >>> + found_pos = true;
> >>> + break;
> >>> + }
> >>> + }
> >>> +
> >>> + if (found_pos)
> >>> + sbi_list_add_tail(&hprot->head, &pos->head);
> >>> + else
> >>> + sbi_list_add_tail(&hprot->head, &hart_protection_list);
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +int sbi_hart_protection_unregister(struct sbi_hart_protection *hprot)
> >>> +{
> >>> + struct sbi_hart_protection *pos = NULL;
> >>> + bool found_pos = false;
> >>> +
> >>> + if (!hprot)
> >>> + return SBI_EINVAL;
> >>> +
> >>> + sbi_list_for_each_entry(pos, &hart_protection_list, head) {
> >>> + if (hprot == pos) {
> >>> + found_pos = true;
> >>> + break;
> >>> + }
> >>> + }
> >>> +
> >>> + if (!found_pos)
> >>> + return SBI_ENOENT;
> >>> +
> >>> + sbi_list_del(&hprot->head);
> >>> + return 0;
> >>
> >> I don't think this function is needed. ISA extensions aren't going to disappear
> >> at runtime. Also, what would you expect to happen if the active hart protection
> >> mechanism is unregistered?
> >
> > I have added this so that if there is any failure in the function registering
> > hart protection mechanism then the function can unregister the hart
> > protection.
>
> OK, that makes sense. Then if we are keeping this function, I would suggest
> following the pattern of making the cleanup function idempotent/infallible since
> the return value would be ignored anyway in favor of the original error.
Okay, I will drop the return value from the unregister() function.
>
> >>> +}
> >>> +
> >>> +int sbi_hart_protection_configure(struct sbi_scratch *scratch)
> >>> +{
> >>> + struct sbi_hart_protection *hprot = sbi_hart_protection_best();
> >>> +
> >>> + if (!hprot)
> >>> + return SBI_EINVAL;
> >>> + if (!hprot->configure)
> >>> + return SBI_ENOSYS;
> >>> +
> >>> + return hprot->configure(scratch);
> >>> +}
> >>
> >> If you only ever use the "best" backend, there is no need to maintain a list,
> >> just a single pointer.
> >
> > I agree that since we are only using the "best" backend, using list seems
> > redundant. I have kept list to know all the available hart protection
> > mechanism plus there is very little overhead since the list is sorted
> > based on rating.
> >
> >>
> >> But then that seems too simplistic. We'll probably want to use (e)PMP and Smmpt
> >> together, to avoid splitting huge MPT pages to make M-mode carveouts. So we
> >> would want to loop through all registered protection schemes.
> >
> > Thinking about this more. It is better to register a composite hart protection
> > for (e)PMP+Smmpt instead of hart protection abstraction calling multiple
> > configure/unconfigure and map/unmap_range because this allows Smmpt
> > code to decide when to use (e)PMP for a domain memregion and when
> > to use Smmpt for a domain memregion. The (e)PMP+Smmpt and platform
> > specific hart protection can always share the PMP programing functions
> > with (e)PMP hart protection.
>
> In that case, the composite backend would have to know which set of oldpmp vs
> smepmp functions to call, which would look suspiciously similar to the code in
> sbi_hart.c removed by this patch series. Maybe the composite backend could just
> assume ePMP, since hardware with Smmpt is likely to support that?
The main use of Smepmp in the composite Smepmp+Smmpt hart protection
will be to enforce access checks for M-mode only regions and M-mode+S-mode
shared regions. The oldpmp is not effective in enforce M-mode access checks
so I don't see a composite oldpmp+Smmpt hart protection to be useful.
It certainly makes sense to register Smepmp+Smmpt hart protection only
when both Smepmp and Smmpt are present.
>
> >> I'm guessing you plan to use an entirely separate set of APIs for system-level
> >> protection schemes like WorldGuard and IOPMP?
> >>
> >
> > Yes, that's correct we will have separate system protection abstraction for
> > WorldGuard and IOPMP. The OpenSBI domain framework will use system
> > protection abstraction differently compared to how it uses hart protection
> > abstraction.
>
> OK, makes sense.
>
> Regards,
> Samuel
>
Regards,
Anup
More information about the opensbi
mailing list