[PATCH v2 1/2] lib: sbi: Support multiple heaps

Anup Patel anup at brainfault.org
Tue Aug 6 23:46:10 PDT 2024


On Thu, Jul 11, 2024 at 2:39 AM Gregor Haas <gregorhaas1997 at gmail.com> wrote:
>
> The upcoming SMMTT implementation will require some larger contiguous memory
> regions for the memory tracking tables. We plan to specify the memory region
> for these tables as a reserved-memory node in the device tree, and then
> dynamically allocate individual tables out of this region. These changes to the
> SBI heap allocator will allow us to explicitly create and allocate from a
> dedicated heap tied to the table memory region.

Need a "Signed-off-by" here.

> ---
>  include/sbi/sbi_heap.h |  18 +++++
>  lib/sbi/sbi_heap.c     | 146 +++++++++++++++++++++++++++--------------
>  2 files changed, 113 insertions(+), 51 deletions(-)
>
> diff --git a/include/sbi/sbi_heap.h b/include/sbi/sbi_heap.h
> index 16755ec..a3f5a0c 100644
> --- a/include/sbi/sbi_heap.h
> +++ b/include/sbi/sbi_heap.h
> @@ -12,6 +12,9 @@
>
>  #include <sbi/sbi_types.h>
>
> +/* Opaque declaration of heap control struct */
> +struct heap_control;
> +

Since we are exposing "heap_control" as part of the heap API,
I suggest renaming it sbi_heap_control.

>  /* Alignment of heap base address and size */
>  #define HEAP_BASE_ALIGN                        1024
>
> @@ -19,9 +22,11 @@ struct sbi_scratch;
>
>  /** Allocate from heap area */
>  void *sbi_malloc(size_t size);
> +void *sbi_malloc_from(struct heap_control *hpctrl, size_t size);
>
>  /** Zero allocate from heap area */
>  void *sbi_zalloc(size_t size);
> +void *sbi_zalloc_from(struct heap_control *hpctrl, size_t size);
>
>  /** Allocate array from heap area */
>  static inline void *sbi_calloc(size_t nitems, size_t size)
> @@ -29,19 +34,32 @@ static inline void *sbi_calloc(size_t nitems, size_t size)
>         return sbi_zalloc(nitems * size);
>  }
>
> +static inline void *sbi_calloc_from(struct heap_control *hpctrl,
> +                                   size_t nitems, size_t size)
> +{
> +       return sbi_zalloc_from(hpctrl, nitems * size);
> +}
> +
>  /** Free-up to heap area */
>  void sbi_free(void *ptr);
> +void sbi_free_from(struct heap_control *hpctrl, void *ptr);
>
>  /** Amount (in bytes) of free space in the heap area */
>  unsigned long sbi_heap_free_space(void);
> +unsigned long sbi_heap_free_space_from(struct heap_control *hpctrl);
>
>  /** Amount (in bytes) of used space in the heap area */
>  unsigned long sbi_heap_used_space(void);
> +unsigned long sbi_heap_used_space_from(struct heap_control *hpctrl);
>
>  /** Amount (in bytes) of reserved space in the heap area */
>  unsigned long sbi_heap_reserved_space(void);
> +unsigned long sbi_heap_reserved_space_from(struct heap_control *hpctrl);
>
>  /** Initialize heap area */
>  int sbi_heap_init(struct sbi_scratch *scratch);
> +int sbi_heap_init_new(struct heap_control *hpctrl, unsigned long base,
> +                      unsigned long size);
> +int sbi_heap_alloc_new(struct heap_control **hpctrl);
>
>  #endif
> diff --git a/lib/sbi/sbi_heap.c b/lib/sbi/sbi_heap.c
> index bcd404b..3be56f3 100644
> --- a/lib/sbi/sbi_heap.c
> +++ b/lib/sbi/sbi_heap.c
> @@ -35,9 +35,9 @@ struct heap_control {
>         struct sbi_dlist used_space_list;
>  };
>
> -static struct heap_control hpctrl;
> +static struct heap_control global_hpctrl;

You can add "extern struct sbi_heap_control global_hpctrl" in
sbi_heap.h. This will allow you to make sbi_malloc(), sbi_zalloc(),
sbi_free(), sbi_heap_free_space(), sbi_heap_used_space(), and
sbi_heap_reserved_space() as static inline functions in sbi_heap.h

>
> -void *sbi_malloc(size_t size)
> +void *sbi_malloc_from(struct heap_control *hpctrl, size_t size)
>  {
>         void *ret = NULL;
>         struct heap_node *n, *np;
> @@ -48,10 +48,10 @@ void *sbi_malloc(size_t size)
>         size += HEAP_ALLOC_ALIGN - 1;
>         size &= ~((unsigned long)HEAP_ALLOC_ALIGN - 1);
>
> -       spin_lock(&hpctrl.lock);
> +       spin_lock(&hpctrl->lock);
>
>         np = NULL;
> -       sbi_list_for_each_entry(n, &hpctrl.free_space_list, head) {
> +       sbi_list_for_each_entry(n, &hpctrl->free_space_list, head) {
>                 if (size <= n->size) {
>                         np = n;
>                         break;
> @@ -59,47 +59,57 @@ void *sbi_malloc(size_t size)
>         }
>         if (np) {
>                 if ((size < np->size) &&
> -                   !sbi_list_empty(&hpctrl.free_node_list)) {
> -                       n = sbi_list_first_entry(&hpctrl.free_node_list,
> +                   !sbi_list_empty(&hpctrl->free_node_list)) {
> +                       n = sbi_list_first_entry(&hpctrl->free_node_list,
>                                                  struct heap_node, head);
>                         sbi_list_del(&n->head);
>                         n->addr = np->addr + np->size - size;
>                         n->size = size;
>                         np->size -= size;
> -                       sbi_list_add_tail(&n->head, &hpctrl.used_space_list);
> +                       sbi_list_add_tail(&n->head, &hpctrl->used_space_list);
>                         ret = (void *)n->addr;
>                 } else if (size == np->size) {
>                         sbi_list_del(&np->head);
> -                       sbi_list_add_tail(&np->head, &hpctrl.used_space_list);
> +                       sbi_list_add_tail(&np->head, &hpctrl->used_space_list);
>                         ret = (void *)np->addr;
>                 }
>         }
>
> -       spin_unlock(&hpctrl.lock);
> +       spin_unlock(&hpctrl->lock);
>
>         return ret;
>  }
>
> -void *sbi_zalloc(size_t size)
> +void *sbi_malloc(size_t size)
>  {
> -       void *ret = sbi_malloc(size);
> +       return sbi_malloc_from(&global_hpctrl, size);
> +}
> +
> +void *sbi_zalloc_from(struct heap_control *hpctrl, size_t size)
> +{
> +       void *ret = sbi_malloc_from(hpctrl, size);
>
>         if (ret)
>                 sbi_memset(ret, 0, size);
>         return ret;
>  }
>
> -void sbi_free(void *ptr)
> +void *sbi_zalloc(size_t size)
> +{
> +       return sbi_malloc_from(&global_hpctrl, size);
> +}
> +
> +void sbi_free_from(struct heap_control *hpctrl, void *ptr)
>  {
>         struct heap_node *n, *np;
>
>         if (!ptr)
>                 return;
>
> -       spin_lock(&hpctrl.lock);
> +       spin_lock(&hpctrl->lock);
>
>         np = NULL;
> -       sbi_list_for_each_entry(n, &hpctrl.used_space_list, head) {
> +       sbi_list_for_each_entry(n, &hpctrl->used_space_list, head) {
>                 if ((n->addr <= (unsigned long)ptr) &&
>                     ((unsigned long)ptr < (n->addr + n->size))) {
>                         np = n;
> @@ -107,22 +117,22 @@ void sbi_free(void *ptr)
>                 }
>         }
>         if (!np) {
> -               spin_unlock(&hpctrl.lock);
> +               spin_unlock(&hpctrl->lock);
>                 return;
>         }
>
>         sbi_list_del(&np->head);
>
> -       sbi_list_for_each_entry(n, &hpctrl.free_space_list, head) {
> +       sbi_list_for_each_entry(n, &hpctrl->free_space_list, head) {
>                 if ((np->addr + np->size) == n->addr) {
>                         n->addr = np->addr;
>                         n->size += np->size;
> -                       sbi_list_add_tail(&np->head, &hpctrl.free_node_list);
> +                       sbi_list_add_tail(&np->head, &hpctrl->free_node_list);
>                         np = NULL;
>                         break;
>                 } else if (np->addr == (n->addr + n->size)) {
>                         n->size += np->size;
> -                       sbi_list_add_tail(&np->head, &hpctrl.free_node_list);
> +                       sbi_list_add_tail(&np->head, &hpctrl->free_node_list);
>                         np = NULL;
>                         break;
>                 } else if ((n->addr + n->size) < np->addr) {
> @@ -132,73 +142,107 @@ void sbi_free(void *ptr)
>                 }
>         }
>         if (np)
> -               sbi_list_add_tail(&np->head, &hpctrl.free_space_list);
> +               sbi_list_add_tail(&np->head, &hpctrl->free_space_list);
>
> -       spin_unlock(&hpctrl.lock);
> +       spin_unlock(&hpctrl->lock);
>  }
>
> -unsigned long sbi_heap_free_space(void)
> +void sbi_free(void *ptr)
> +{
> +       return sbi_free_from(&global_hpctrl, ptr);
> +}
> +
> +unsigned long sbi_heap_free_space_from(struct heap_control *hpctrl)
>  {
>         struct heap_node *n;
>         unsigned long ret = 0;
>
> -       spin_lock(&hpctrl.lock);
> -       sbi_list_for_each_entry(n, &hpctrl.free_space_list, head)
> +       spin_lock(&hpctrl->lock);
> +       sbi_list_for_each_entry(n, &hpctrl->free_space_list, head)
>                 ret += n->size;
> -       spin_unlock(&hpctrl.lock);
> +       spin_unlock(&hpctrl->lock);
>
>         return ret;
>  }
>
> +unsigned long sbi_heap_free_space(void)
> +{
> +       return sbi_heap_free_space_from(&global_hpctrl);
> +}
> +
> +unsigned long sbi_heap_used_space_from(struct heap_control *hpctrl)
> +{
> +       return hpctrl->size - hpctrl->hksize - sbi_heap_free_space();
> +}
> +
>  unsigned long sbi_heap_used_space(void)
>  {
> -       return hpctrl.size - hpctrl.hksize - sbi_heap_free_space();
> +       return sbi_heap_free_space_from(&global_hpctrl);
> +}
> +
> +unsigned long sbi_heap_reserved_space_from(struct heap_control *hpctrl)
> +{
> +       return hpctrl->hksize;
>  }
>
>  unsigned long sbi_heap_reserved_space(void)
>  {
> -       return hpctrl.hksize;
> +       return sbi_heap_free_space_from(&global_hpctrl);
>  }
>
> -int sbi_heap_init(struct sbi_scratch *scratch)
> +int sbi_heap_init_new(struct heap_control *hpctrl, unsigned long base,
> +                      unsigned long size)
>  {
>         unsigned long i;
>         struct heap_node *n;
>
> -       /* Sanity checks on heap offset and size */
> -       if (!scratch->fw_heap_size ||
> -           (scratch->fw_heap_size & (HEAP_BASE_ALIGN - 1)) ||
> -           (scratch->fw_heap_offset < scratch->fw_rw_offset) ||
> -           (scratch->fw_size < (scratch->fw_heap_offset + scratch->fw_heap_size)) ||
> -           (scratch->fw_heap_offset & (HEAP_BASE_ALIGN - 1)))
> -               return SBI_EINVAL;
> -
>         /* Initialize heap control */
> -       SPIN_LOCK_INIT(hpctrl.lock);
> -       hpctrl.base = scratch->fw_start + scratch->fw_heap_offset;
> -       hpctrl.size = scratch->fw_heap_size;
> -       hpctrl.hkbase = hpctrl.base;
> -       hpctrl.hksize = hpctrl.size / HEAP_HOUSEKEEPING_FACTOR;
> -       hpctrl.hksize &= ~((unsigned long)HEAP_BASE_ALIGN - 1);
> -       SBI_INIT_LIST_HEAD(&hpctrl.free_node_list);
> -       SBI_INIT_LIST_HEAD(&hpctrl.free_space_list);
> -       SBI_INIT_LIST_HEAD(&hpctrl.used_space_list);
> +       SPIN_LOCK_INIT(hpctrl->lock);
> +       hpctrl->base = base;
> +       hpctrl->size = size;
> +       hpctrl->hkbase = hpctrl->base;
> +       hpctrl->hksize = hpctrl->size / HEAP_HOUSEKEEPING_FACTOR;
> +       hpctrl->hksize &= ~((unsigned long)HEAP_BASE_ALIGN - 1);
> +       SBI_INIT_LIST_HEAD(&hpctrl->free_node_list);
> +       SBI_INIT_LIST_HEAD(&hpctrl->free_space_list);
> +       SBI_INIT_LIST_HEAD(&hpctrl->used_space_list);
>
>         /* Prepare free node list */
> -       for (i = 0; i < (hpctrl.hksize / sizeof(*n)); i++) {
> -               n = (struct heap_node *)(hpctrl.hkbase + (sizeof(*n) * i));
> +       for (i = 0; i < (hpctrl->hksize / sizeof(*n)); i++) {
> +               n = (struct heap_node *)(hpctrl->hkbase + (sizeof(*n) * i));
>                 SBI_INIT_LIST_HEAD(&n->head);
>                 n->addr = n->size = 0;
> -               sbi_list_add_tail(&n->head, &hpctrl.free_node_list);
> +               sbi_list_add_tail(&n->head, &hpctrl->free_node_list);
>         }
>
>         /* Prepare free space list */
> -       n = sbi_list_first_entry(&hpctrl.free_node_list,
> +       n = sbi_list_first_entry(&hpctrl->free_node_list,
>                                  struct heap_node, head);
>         sbi_list_del(&n->head);
> -       n->addr = hpctrl.hkbase + hpctrl.hksize;
> -       n->size = hpctrl.size - hpctrl.hksize;
> -       sbi_list_add_tail(&n->head, &hpctrl.free_space_list);
> +       n->addr = hpctrl->hkbase + hpctrl->hksize;
> +       n->size = hpctrl->size - hpctrl->hksize;
> +       sbi_list_add_tail(&n->head, &hpctrl->free_space_list);
>
>         return 0;
>  }
> +
> +int sbi_heap_init(struct sbi_scratch *scratch)
> +{
> +       /* Sanity checks on heap offset and size */
> +       if (!scratch->fw_heap_size ||
> +           (scratch->fw_heap_size & (HEAP_BASE_ALIGN - 1)) ||
> +           (scratch->fw_heap_offset < scratch->fw_rw_offset) ||
> +           (scratch->fw_size < (scratch->fw_heap_offset + scratch->fw_heap_size)) ||
> +           (scratch->fw_heap_offset & (HEAP_BASE_ALIGN - 1)))
> +               return SBI_EINVAL;
> +
> +       return sbi_heap_init_new(&global_hpctrl,
> +                                 scratch->fw_start + scratch->fw_heap_offset,
> +                                 scratch->fw_heap_size);
> +}
> +
> +int sbi_heap_alloc_new(struct heap_control **hpctrl)
> +{
> +       *hpctrl = sbi_calloc(1, sizeof(struct heap_control));
> +       return 0;
> +}
> \ No newline at end of file

Remove this "\ No newline at end of file".

> --
> 2.45.2
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

Regards,
Anup



More information about the opensbi mailing list