[PATCH 2/5] lib: sbi: Introduce hart protection abstraction
Samuel Holland
samuel.holland at sifive.com
Mon Dec 8 23:04:10 PST 2025
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.
>>> +}
>>> +
>>> +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?
>> 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
More information about the opensbi
mailing list