[PATCH 00/17] Introduce and use simple heap allocator

[PATCH 00/17] Introduce and use simple heap allocator

[Anup Patel]

Currently, we use global variables instead of explicitly allocating
memory in different parts of OpenSBI. This leads to a bloated BSS
section in the firmware binaries and this will continue to grow as
more drivers (and frameworks) with global variables are added.

To improve memory utilizaiton in OpenSBI:
1) We introduce a simple heap allocator which based on linked list.
   This is a reasonable heap implementation to start with because
   heap allocation and free-up won't be in hot path.
2) Use a combination of heap allocations and scratch space allocations
   to improve the reduce the BSS memory foot-print.

As a by-product, this series also redunces global arrays indexed by hartid
which is preparatory work for the upcoming full sparse hartid support.

These patches can also be found in simple_heap_v1 branch at:
https://github.com/avpatel/opensbi.git

Anup Patel (17):
  platform: Allow platforms to specify heap size
  lib: sbi: Introduce simple heap allocator
  lib: sbi: Print scratch size and usage at boot time
  lib: sbi_pmu: Use heap for per-HART PMU state
  lib: sbi: Use heap for root domain creation
  lib: sbi: Use scratch space to save per-HART domain pointer
  lib: utils/gpio: Use heap in SiFive and StartFive GPIO drivers
  lib: utils/i2c: Use heap in DesignWare and SiFive I2C drivers
  lib: utils/ipi: Use heap in ACLINT MSWI driver
  lib: utils/irqchip: Use heap in PLIC, APLIC and IMSIC drivers
  lib: utils/timer: Use heap in ACLINT MTIMER driver
  lib: utils/fdt: Use heap in FDT domain parsing
  lib: utils/ipi: Use scratch space to save per-HART MSWI pointer
  lib: utils/timer: Use scratch space to save per-HART MTIMER pointer
  lib: utils/irqchip: Use scratch space to save per-HART PLIC pointer
  lib: utils/irqchip: Don't check hartid in imsic_update_hartid_table()
  lib: utils/irqchip: Use scratch space to save per-HART IMSIC pointer

 firmware/fw_base.S                    |  15 ++
 include/sbi/sbi_domain.h              |   6 +-
 include/sbi/sbi_heap.h                |  44 +++++
 include/sbi/sbi_platform.h            |  18 ++-
 include/sbi/sbi_scratch.h             |  31 ++--
 lib/sbi/objects.mk                    |   1 +
 lib/sbi/sbi_domain.c                  |  83 ++++++++--
 lib/sbi/sbi_heap.c                    | 204 ++++++++++++++++++++++++
 lib/sbi/sbi_init.c                    |  20 +++
 lib/sbi/sbi_pmu.c                     | 221 ++++++++++++++++----------
 lib/sbi/sbi_scratch.c                 |  11 ++
 lib/utils/fdt/fdt_domain.c            | 111 ++++++++-----
 lib/utils/gpio/fdt_gpio_sifive.c      |  21 ++-
 lib/utils/gpio/fdt_gpio_starfive.c    |  20 +--
 lib/utils/i2c/fdt_i2c_dw.c            |  24 ++-
 lib/utils/i2c/fdt_i2c_sifive.c        |  23 ++-
 lib/utils/ipi/aclint_mswi.c           |  49 ++++--
 lib/utils/ipi/fdt_ipi_mswi.c          |  21 ++-
 lib/utils/irqchip/fdt_irqchip_aplic.c |  24 +--
 lib/utils/irqchip/fdt_irqchip_imsic.c |  37 +++--
 lib/utils/irqchip/fdt_irqchip_plic.c  | 142 ++++++++++++-----
 lib/utils/irqchip/imsic.c             |  87 ++++++++--
 lib/utils/timer/aclint_mtimer.c       |  82 ++++++++--
 lib/utils/timer/fdt_timer_mtimer.c    |  47 ++++--
 platform/fpga/ariane/platform.c       |   1 +
 platform/fpga/openpiton/platform.c    |   2 +
 platform/generic/platform.c           |   3 +-
 platform/kendryte/k210/platform.c     |   2 +
 platform/nuclei/ux600/platform.c      |   2 +
 platform/template/platform.c          |   1 +
 30 files changed, 1004 insertions(+), 349 deletions(-)
 create mode 100644 include/sbi/sbi_heap.h
 create mode 100644 lib/sbi/sbi_heap.c

-- 
2.34.1

[PATCH 01/17] platform: Allow platforms to specify heap size

[Anup Patel]

We extend struct sbi_platform and struct sbi_scratch to allow platforms
specify the heap size to the OpenSBI firmwares. The OpenSBI firmwares
will use this information to determine the location of heap and provide
heap base address in per-HART scratch space.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 firmware/fw_base.S                 | 15 +++++++++++++++
 include/sbi/sbi_platform.h         | 18 +++++++++++++++---
 include/sbi/sbi_scratch.h          | 28 ++++++++++++++++++----------
 platform/fpga/ariane/platform.c    |  1 +
 platform/fpga/openpiton/platform.c |  2 ++
 platform/generic/platform.c        |  3 ++-
 platform/kendryte/k210/platform.c  |  2 ++
 platform/nuclei/ux600/platform.c   |  2 ++
 platform/template/platform.c       |  1 +
 9 files changed, 58 insertions(+), 14 deletions(-)

diff --git a/firmware/fw_base.S b/firmware/fw_base.S
index e37df09..fff09e1 100644
--- a/firmware/fw_base.S
+++ b/firmware/fw_base.S
@@ -255,20 +255,28 @@ _bss_zero:
 	/* Preload HART details
 	 * s7 -> HART Count
 	 * s8 -> HART Stack Size
+	 * s9 -> Heap Size
+	 * s10 -> Heap Offset
 	 */
 	lla	a4, platform
 #if __riscv_xlen > 32
 	lwu	s7, SBI_PLATFORM_HART_COUNT_OFFSET(a4)
 	lwu	s8, SBI_PLATFORM_HART_STACK_SIZE_OFFSET(a4)
+	lwu	s9, SBI_PLATFORM_HEAP_SIZE_OFFSET(a4)
 #else
 	lw	s7, SBI_PLATFORM_HART_COUNT_OFFSET(a4)
 	lw	s8, SBI_PLATFORM_HART_STACK_SIZE_OFFSET(a4)
+	lw	s9, SBI_PLATFORM_HEAP_SIZE_OFFSET(a4)
 #endif
 
 	/* Setup scratch space for all the HARTs*/
 	lla	tp, _fw_end
 	mul	a5, s7, s8
 	add	tp, tp, a5
+	/* Setup heap base address */
+	lla	s10, _fw_start
+	sub	s10, tp, s10
+	add	tp, tp, s9
 	/* Keep a copy of tp */
 	add	t3, tp, zero
 	/* Counter */
@@ -283,8 +291,11 @@ _scratch_init:
 	 * t3 -> the firmware end address
 	 * s7 -> HART count
 	 * s8 -> HART stack size
+	 * s9 -> Heap Size
+	 * s10 -> Heap Offset
 	 */
 	add	tp, t3, zero
+	sub	tp, tp, s9
 	mul	a5, s8, t1
 	sub	tp, tp, a5
 	li	a5, SBI_SCRATCH_SIZE
@@ -302,6 +313,10 @@ _scratch_init:
 	REG_L	a5, 0(a4)
 	REG_S	a5, SBI_SCRATCH_FW_RW_OFFSET(tp)
 
+	/* Store fw_heap_offset and fw_heap_size in scratch space */
+	REG_S	s10, SBI_SCRATCH_FW_HEAP_OFFSET(tp)
+	REG_S	s9, SBI_SCRATCH_FW_HEAP_SIZE_OFFSET(tp)
+
 	/* Store next arg1 in scratch space */
 	MOV_3R	s0, a0, s1, a1, s2, a2
 	call	fw_next_arg1
diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
index 546c0a6..3e9616f 100644
--- a/include/sbi/sbi_platform.h
+++ b/include/sbi/sbi_platform.h
@@ -29,12 +29,16 @@
 #define SBI_PLATFORM_HART_COUNT_OFFSET (0x50)
 /** Offset of hart_stack_size in struct sbi_platform */
 #define SBI_PLATFORM_HART_STACK_SIZE_OFFSET (0x54)
+/** Offset of heap_size in struct sbi_platform */
+#define SBI_PLATFORM_HEAP_SIZE_OFFSET (0x58)
+/** Offset of reserved in struct sbi_platform */
+#define SBI_PLATFORM_RESERVED_OFFSET (0x5c)
 /** Offset of platform_ops_addr in struct sbi_platform */
-#define SBI_PLATFORM_OPS_OFFSET (0x58)
+#define SBI_PLATFORM_OPS_OFFSET (0x60)
 /** Offset of firmware_context in struct sbi_platform */
-#define SBI_PLATFORM_FIRMWARE_CONTEXT_OFFSET (0x58 + __SIZEOF_POINTER__)
+#define SBI_PLATFORM_FIRMWARE_CONTEXT_OFFSET (0x60 + __SIZEOF_POINTER__)
 /** Offset of hart_index2id in struct sbi_platform */
-#define SBI_PLATFORM_HART_INDEX2ID_OFFSET (0x58 + (__SIZEOF_POINTER__ * 2))
+#define SBI_PLATFORM_HART_INDEX2ID_OFFSET (0x60 + (__SIZEOF_POINTER__ * 2))
 
 #define SBI_PLATFORM_TLB_RANGE_FLUSH_LIMIT_DEFAULT		(1UL << 12)
 
@@ -138,6 +142,10 @@ struct sbi_platform_operations {
 /** Platform default per-HART stack size for exception/interrupt handling */
 #define SBI_PLATFORM_DEFAULT_HART_STACK_SIZE	8192
 
+/** Platform default heap size */
+#define SBI_PLATFORM_DEFAULT_HEAP_SIZE(__num_hart)	\
+					(0x8000 + 0x800 * (__num_hart))
+
 /** Representation of a platform */
 struct sbi_platform {
 	/**
@@ -160,6 +168,10 @@ struct sbi_platform {
 	u32 hart_count;
 	/** Per-HART stack size for exception/interrupt handling */
 	u32 hart_stack_size;
+	/** Size of heap shared by all HARTs */
+	u32 heap_size;
+	/** Reserved for future use */
+	u32 reserved;
 	/** Pointer to sbi platform operations */
 	unsigned long platform_ops_addr;
 	/** Pointer to system firmware specific context */
diff --git a/include/sbi/sbi_scratch.h b/include/sbi/sbi_scratch.h
index 2966188..2376995 100644
--- a/include/sbi/sbi_scratch.h
+++ b/include/sbi/sbi_scratch.h
@@ -20,26 +20,30 @@
 #define SBI_SCRATCH_FW_SIZE_OFFSET		(1 * __SIZEOF_POINTER__)
 /** Offset (in sbi_scratch) of the R/W Offset */
 #define SBI_SCRATCH_FW_RW_OFFSET		(2 * __SIZEOF_POINTER__)
+/** Offset of fw_heap_offset member in sbi_scratch */
+#define SBI_SCRATCH_FW_HEAP_OFFSET		(3 * __SIZEOF_POINTER__)
+/** Offset of fw_heap_size_offset member in sbi_scratch */
+#define SBI_SCRATCH_FW_HEAP_SIZE_OFFSET		(4 * __SIZEOF_POINTER__)
 /** Offset of next_arg1 member in sbi_scratch */
-#define SBI_SCRATCH_NEXT_ARG1_OFFSET		(3 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_NEXT_ARG1_OFFSET		(5 * __SIZEOF_POINTER__)
 /** Offset of next_addr member in sbi_scratch */
-#define SBI_SCRATCH_NEXT_ADDR_OFFSET		(4 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_NEXT_ADDR_OFFSET		(6 * __SIZEOF_POINTER__)
 /** Offset of next_mode member in sbi_scratch */
-#define SBI_SCRATCH_NEXT_MODE_OFFSET		(5 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_NEXT_MODE_OFFSET		(7 * __SIZEOF_POINTER__)
 /** Offset of warmboot_addr member in sbi_scratch */
-#define SBI_SCRATCH_WARMBOOT_ADDR_OFFSET	(6 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_WARMBOOT_ADDR_OFFSET	(8 * __SIZEOF_POINTER__)
 /** Offset of platform_addr member in sbi_scratch */
-#define SBI_SCRATCH_PLATFORM_ADDR_OFFSET	(7 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_PLATFORM_ADDR_OFFSET	(9 * __SIZEOF_POINTER__)
 /** Offset of hartid_to_scratch member in sbi_scratch */
-#define SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET	(8 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET	(10 * __SIZEOF_POINTER__)
 /** Offset of trap_exit member in sbi_scratch */
-#define SBI_SCRATCH_TRAP_EXIT_OFFSET		(9 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_TRAP_EXIT_OFFSET		(11 * __SIZEOF_POINTER__)
 /** Offset of tmp0 member in sbi_scratch */
-#define SBI_SCRATCH_TMP0_OFFSET			(10 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_TMP0_OFFSET			(12 * __SIZEOF_POINTER__)
 /** Offset of options member in sbi_scratch */
-#define SBI_SCRATCH_OPTIONS_OFFSET		(11 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_OPTIONS_OFFSET		(13 * __SIZEOF_POINTER__)
 /** Offset of extra space in sbi_scratch */
-#define SBI_SCRATCH_EXTRA_SPACE_OFFSET		(12 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_EXTRA_SPACE_OFFSET		(14 * __SIZEOF_POINTER__)
 /** Maximum size of sbi_scratch (4KB) */
 #define SBI_SCRATCH_SIZE			(0x1000)
 
@@ -57,6 +61,10 @@ struct sbi_scratch {
 	unsigned long fw_size;
 	/** Offset (in bytes) of the R/W section */
 	unsigned long fw_rw_offset;
+	/** Offset (in bytes) of the heap area */
+	unsigned long fw_heap_offset;
+	/** Size (in bytes) of the heap area */
+	unsigned long fw_heap_size;
 	/** Arg1 (or 'a1' register) of next booting stage for this HART */
 	unsigned long next_arg1;
 	/** Address of next booting stage for this HART */
diff --git a/platform/fpga/ariane/platform.c b/platform/fpga/ariane/platform.c
index 1e341c2..975528f 100644
--- a/platform/fpga/ariane/platform.c
+++ b/platform/fpga/ariane/platform.c
@@ -185,5 +185,6 @@ const struct sbi_platform platform = {
 	.features = SBI_PLATFORM_DEFAULT_FEATURES,
 	.hart_count = ARIANE_HART_COUNT,
 	.hart_stack_size = SBI_PLATFORM_DEFAULT_HART_STACK_SIZE,
+	.heap_size = SBI_PLATFORM_DEFAULT_HEAP_SIZE(ARIANE_HART_COUNT),
 	.platform_ops_addr = (unsigned long)&platform_ops
 };
diff --git a/platform/fpga/openpiton/platform.c b/platform/fpga/openpiton/platform.c
index 57ae698..e59dc99 100644
--- a/platform/fpga/openpiton/platform.c
+++ b/platform/fpga/openpiton/platform.c
@@ -220,5 +220,7 @@ const struct sbi_platform platform = {
 	.features = SBI_PLATFORM_DEFAULT_FEATURES,
 	.hart_count = OPENPITON_DEFAULT_HART_COUNT,
 	.hart_stack_size = SBI_PLATFORM_DEFAULT_HART_STACK_SIZE,
+	.heap_size =
+		SBI_PLATFORM_DEFAULT_HEAP_SIZE(OPENPITON_DEFAULT_HART_COUNT),
 	.platform_ops_addr = (unsigned long)&platform_ops
 };
diff --git a/platform/generic/platform.c b/platform/generic/platform.c
index eeefef4..0c9cd95 100644
--- a/platform/generic/platform.c
+++ b/platform/generic/platform.c
@@ -115,7 +115,7 @@ unsigned long fw_platform_init(unsigned long arg0, unsigned long arg1,
 	}
 
 	platform.hart_count = hart_count;
-
+	platform.heap_size = SBI_PLATFORM_DEFAULT_HEAP_SIZE(hart_count);
 	platform_has_mlevel_imsic = fdt_check_imsic_mlevel(fdt);
 
 	/* Return original FDT pointer */
@@ -315,5 +315,6 @@ struct sbi_platform platform = {
 	.hart_count		= SBI_HARTMASK_MAX_BITS,
 	.hart_index2id		= generic_hart_index2id,
 	.hart_stack_size	= SBI_PLATFORM_DEFAULT_HART_STACK_SIZE,
+	.heap_size		= SBI_PLATFORM_DEFAULT_HEAP_SIZE(0),
 	.platform_ops_addr	= (unsigned long)&platform_ops
 };
diff --git a/platform/kendryte/k210/platform.c b/platform/kendryte/k210/platform.c
index 7eb9015..637a217 100644
--- a/platform/kendryte/k210/platform.c
+++ b/platform/kendryte/k210/platform.c
@@ -196,5 +196,7 @@ const struct sbi_platform platform = {
 	.features		= 0,
 	.hart_count		= K210_HART_COUNT,
 	.hart_stack_size	= SBI_PLATFORM_DEFAULT_HART_STACK_SIZE,
+	.heap_size		=
+			SBI_PLATFORM_DEFAULT_HEAP_SIZE(K210_HART_COUNT),
 	.platform_ops_addr	= (unsigned long)&platform_ops
 };
diff --git a/platform/nuclei/ux600/platform.c b/platform/nuclei/ux600/platform.c
index 4eccff1..6fd6cd7 100644
--- a/platform/nuclei/ux600/platform.c
+++ b/platform/nuclei/ux600/platform.c
@@ -244,5 +244,7 @@ const struct sbi_platform platform = {
 	.features		= SBI_PLATFORM_DEFAULT_FEATURES,
 	.hart_count		= UX600_HART_COUNT,
 	.hart_stack_size	= SBI_PLATFORM_DEFAULT_HART_STACK_SIZE,
+	.heap_size		=
+			SBI_PLATFORM_DEFAULT_HEAP_SIZE(UX600_HART_COUNT),
 	.platform_ops_addr	= (unsigned long)&platform_ops
 };
diff --git a/platform/template/platform.c b/platform/template/platform.c
index 8adc431..86381ca 100644
--- a/platform/template/platform.c
+++ b/platform/template/platform.c
@@ -152,5 +152,6 @@ const struct sbi_platform platform = {
 	.features		= SBI_PLATFORM_DEFAULT_FEATURES,
 	.hart_count		= 1,
 	.hart_stack_size	= SBI_PLATFORM_DEFAULT_HART_STACK_SIZE,
+	.heap_size		= SBI_PLATFORM_DEFAULT_HEAP_SIZE(1),
 	.platform_ops_addr	= (unsigned long)&platform_ops
 };
-- 
2.34.1

[PATCH 02/17] lib: sbi: Introduce simple heap allocator

[Anup Patel]

We provide simple heap allocator to manage the heap space provided
by OpenSBI firmware and platform.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 include/sbi/sbi_heap.h |  44 +++++++++
 lib/sbi/objects.mk     |   1 +
 lib/sbi/sbi_heap.c     | 204 +++++++++++++++++++++++++++++++++++++++++
 lib/sbi/sbi_init.c     |  15 +++
 4 files changed, 264 insertions(+)
 create mode 100644 include/sbi/sbi_heap.h
 create mode 100644 lib/sbi/sbi_heap.c

diff --git a/include/sbi/sbi_heap.h b/include/sbi/sbi_heap.h
new file mode 100644
index 0000000..88d176e
--- /dev/null
+++ b/include/sbi/sbi_heap.h
@@ -0,0 +1,44 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2023 Ventana Micro Systems Inc.
+ *
+ * Authors:
+ *   Anup Patel<apatel@ventanamicro.com>
+ */
+
+#ifndef __SBI_HEAP_H__
+#define __SBI_HEAP_H__
+
+#include <sbi/sbi_types.h>
+
+struct sbi_scratch;
+
+/** Allocate from heap area */
+void *sbi_malloc(size_t size);
+
+/** Zero allocate from heap area */
+void *sbi_zalloc(size_t size);
+
+/** Allocate array from heap area */
+static inline void *sbi_calloc(size_t nitems, size_t size)
+{
+	return sbi_zalloc(nitems * size);
+}
+
+/** Free-up to heap area */
+void sbi_free(void *ptr);
+
+/** Amount (in bytes) of free space in the heap area */
+unsigned long sbi_heap_free_space(void);
+
+/** Amount (in bytes) of used space in the heap area */
+unsigned long sbi_heap_used_space(void);
+
+/** Amount (in bytes) of reserved space in the heap area */
+unsigned long sbi_heap_reserved_space(void);
+
+/** Initialize heap area */
+int sbi_heap_init(struct sbi_scratch *scratch);
+
+#endif
diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
index 7d691c6..c699187 100644
--- a/lib/sbi/objects.mk
+++ b/lib/sbi/objects.mk
@@ -59,6 +59,7 @@ libsbi-objs-y += sbi_domain.o
 libsbi-objs-y += sbi_emulate_csr.o
 libsbi-objs-y += sbi_fifo.o
 libsbi-objs-y += sbi_hart.o
+libsbi-objs-y += sbi_heap.o
 libsbi-objs-y += sbi_math.o
 libsbi-objs-y += sbi_hfence.o
 libsbi-objs-y += sbi_hsm.o
diff --git a/lib/sbi/sbi_heap.c b/lib/sbi/sbi_heap.c
new file mode 100644
index 0000000..82aa1ec
--- /dev/null
+++ b/lib/sbi/sbi_heap.c
@@ -0,0 +1,204 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2023 Ventana Micro Systems Inc.
+ *
+ * Authors:
+ *   Anup Patel<apatel@ventanamicro.com>
+ */
+
+#include <sbi/riscv_locks.h>
+#include <sbi/sbi_error.h>
+#include <sbi/sbi_heap.h>
+#include <sbi/sbi_list.h>
+#include <sbi/sbi_scratch.h>
+#include <sbi/sbi_string.h>
+
+#define HEAP_BASE_ALIGN			1024
+#define HEAP_ALLOC_ALIGN		64
+#define HEAP_HOUSEKEEPING_FACTOR	16
+
+struct heap_node {
+	struct sbi_dlist head;
+	unsigned long addr;
+	unsigned long size;
+};
+
+struct heap_control {
+	spinlock_t lock;
+	unsigned long base;
+	unsigned long size;
+	unsigned long hkbase;
+	unsigned long hksize;
+	struct sbi_dlist free_node_list;
+	struct sbi_dlist free_space_list;
+	struct sbi_dlist used_space_list;
+};
+
+static struct heap_control hpctrl;
+
+void *sbi_malloc(size_t size)
+{
+	void *ret = NULL;
+	struct heap_node *n, *np;
+
+	if (!size)
+		return NULL;
+
+	size += HEAP_ALLOC_ALIGN - 1;
+	size &= ~((unsigned long)HEAP_ALLOC_ALIGN - 1);
+
+	spin_lock(&hpctrl.lock);
+
+	np = NULL;
+	sbi_list_for_each_entry(n, &hpctrl.free_space_list, head) {
+		if (size <= n->size) {
+			np = n;
+			break;
+		}
+	}
+	if (np) {
+		if ((size < np->size) &&
+		    !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);
+			ret = (void *)n->addr;
+		} else if (size == np->size) {
+			sbi_list_del(&np->head);
+			sbi_list_add_tail(&np->head, &hpctrl.used_space_list);
+			ret = (void *)np->addr;
+		}
+	}
+
+	spin_unlock(&hpctrl.lock);
+
+	return ret;
+}
+
+void *sbi_zalloc(size_t size)
+{
+	void *ret = sbi_malloc(size);
+
+	if (ret)
+		sbi_memset(ret, 0, size);
+	return ret;
+}
+
+void sbi_free(void *ptr)
+{
+	struct heap_node *n, *np;
+
+	if (!ptr)
+		return;
+
+	spin_lock(&hpctrl.lock);
+
+	np = NULL;
+	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;
+			break;
+		}
+	}
+	if (!np) {
+		spin_unlock(&hpctrl.lock);
+		return;
+	}
+
+	sbi_list_del(&np->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);
+			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);
+			np = NULL;
+			break;
+		} else if ((n->addr + n->size) < np->addr) {
+			sbi_list_add(&np->head, &n->head);
+			np = NULL;
+			break;
+		}
+	}
+	if (np)
+		sbi_list_add_tail(&np->head, &hpctrl.free_space_list);
+
+	spin_unlock(&hpctrl.lock);
+}
+
+unsigned long sbi_heap_free_space(void)
+{
+	struct heap_node *n;
+	unsigned long ret = 0;
+
+	spin_lock(&hpctrl.lock);
+	sbi_list_for_each_entry(n, &hpctrl.free_space_list, head)
+		ret += n->size;
+	spin_unlock(&hpctrl.lock);
+
+	return ret;
+}
+
+unsigned long sbi_heap_used_space(void)
+{
+	return hpctrl.size - hpctrl.hksize - sbi_heap_free_space();
+}
+
+unsigned long sbi_heap_reserved_space(void)
+{
+	return hpctrl.hksize;
+}
+
+int sbi_heap_init(struct sbi_scratch *scratch)
+{
+	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);
+
+	/* Prepare free node list */
+	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);
+	}
+
+	/* Prepare free space 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);
+
+	return 0;
+}
diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
index 5db8e7f..f09a7ac 100644
--- a/lib/sbi/sbi_init.c
+++ b/lib/sbi/sbi_init.c
@@ -17,6 +17,7 @@
 #include <sbi/sbi_ecall.h>
 #include <sbi/sbi_hart.h>
 #include <sbi/sbi_hartmask.h>
+#include <sbi/sbi_heap.h>
 #include <sbi/sbi_hsm.h>
 #include <sbi/sbi_ipi.h>
 #include <sbi/sbi_irqchip.h>
@@ -118,6 +119,15 @@ static void sbi_boot_print_general(struct sbi_scratch *scratch)
 	sbi_printf("Firmware Size             : %d KB\n",
 		   (u32)(scratch->fw_size / 1024));
 	sbi_printf("Firmware RW Offset        : 0x%lx\n", scratch->fw_rw_offset);
+	sbi_printf("Firmware RW Size          : %d KB\n",
+		   (u32)((scratch->fw_size - scratch->fw_rw_offset) / 1024));
+	sbi_printf("Firmware Heap Offset      : 0x%lx\n", scratch->fw_heap_offset);
+	sbi_printf("Firmware Heap Size        : "
+		   "%d KB (total), %d KB (reserved), %d KB (used), %d KB (free)\n",
+		   (u32)(scratch->fw_heap_size / 1024),
+		   (u32)(sbi_heap_reserved_space() / 1024),
+		   (u32)(sbi_heap_used_space() / 1024),
+		   (u32)(sbi_heap_free_space() / 1024));
 
 	/* SBI details */
 	sbi_printf("Runtime SBI Version       : %d.%d\n",
@@ -258,6 +268,11 @@ static void __noreturn init_coldboot(struct sbi_scratch *scratch, u32 hartid)
 		sbi_hart_hang();
 
 	/* Note: This has to be second thing in coldboot init sequence */
+	rc = sbi_heap_init(scratch);
+	if (rc)
+		sbi_hart_hang();
+
+	/* Note: This has to be thing thing in coldboot init sequence */
 	rc = sbi_domain_init(scratch, hartid);
 	if (rc)
 		sbi_hart_hang();
-- 
2.34.1

[PATCH 03/17] lib: sbi: Print scratch size and usage at boot time

[Anup Patel]

The scratch space being a scarce resource so let us print it's
size and usage at boot time.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 include/sbi/sbi_scratch.h |  3 +++
 lib/sbi/sbi_init.c        |  5 +++++
 lib/sbi/sbi_scratch.c     | 11 +++++++++++
 3 files changed, 19 insertions(+)

diff --git a/include/sbi/sbi_scratch.h b/include/sbi/sbi_scratch.h
index 2376995..7c85ac7 100644
--- a/include/sbi/sbi_scratch.h
+++ b/include/sbi/sbi_scratch.h
@@ -175,6 +175,9 @@ unsigned long sbi_scratch_alloc_offset(unsigned long size);
 /** Free-up extra space in sbi_scratch */
 void sbi_scratch_free_offset(unsigned long offset);
 
+/** Amount (in bytes) of used space in in sbi_scratch */
+unsigned long sbi_scratch_used_space(void);
+
 /** Get pointer from offset in sbi_scratch */
 #define sbi_scratch_offset_ptr(scratch, offset)	(void *)((char *)(scratch) + (offset))
 
diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
index f09a7ac..c12fd33 100644
--- a/lib/sbi/sbi_init.c
+++ b/lib/sbi/sbi_init.c
@@ -128,6 +128,11 @@ static void sbi_boot_print_general(struct sbi_scratch *scratch)
 		   (u32)(sbi_heap_reserved_space() / 1024),
 		   (u32)(sbi_heap_used_space() / 1024),
 		   (u32)(sbi_heap_free_space() / 1024));
+	sbi_printf("Firmware Scratch Size     : "
+		   "%d B (total), %d B (used), %d B (free)\n",
+		   SBI_SCRATCH_SIZE,
+		   (u32)sbi_scratch_used_space(),
+		   (u32)(SBI_SCRATCH_SIZE - sbi_scratch_used_space()));
 
 	/* SBI details */
 	sbi_printf("Runtime SBI Version       : %d.%d\n",
diff --git a/lib/sbi/sbi_scratch.c b/lib/sbi/sbi_scratch.c
index 55ebdbb..87ef84c 100644
--- a/lib/sbi/sbi_scratch.c
+++ b/lib/sbi/sbi_scratch.c
@@ -97,3 +97,14 @@ void sbi_scratch_free_offset(unsigned long offset)
 	 * brain-dead allocator.
 	 */
 }
+
+unsigned long sbi_scratch_used_space(void)
+{
+	unsigned long ret = 0;
+
+	spin_lock(&extra_lock);
+	ret = extra_offset;
+	spin_unlock(&extra_lock);
+
+	return ret;
+}
-- 
2.34.1

[PATCH 04/17] lib: sbi_pmu: Use heap for per-HART PMU state

[Anup Patel]

Instead of using a global array for per-HART PMU state, we should
use heap to on-demand allocate per-HART PMU state when the HART
is initialized in cold boot or warm boot path.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/sbi/sbi_pmu.c | 221 ++++++++++++++++++++++++++++------------------
 1 file changed, 133 insertions(+), 88 deletions(-)

diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 939f29d..71562e4 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -12,7 +12,7 @@
 #include <sbi/sbi_console.h>
 #include <sbi/sbi_ecall_interface.h>
 #include <sbi/sbi_hart.h>
-#include <sbi/sbi_hartmask.h>
+#include <sbi/sbi_heap.h>
 #include <sbi/sbi_platform.h>
 #include <sbi/sbi_pmu.h>
 #include <sbi/sbi_scratch.h>
@@ -50,27 +50,49 @@ union sbi_pmu_ctr_info {
 	};
 };
 
-/* Platform specific PMU device */
-static const struct sbi_pmu_device *pmu_dev = NULL;
-
-/* Mapping between event range and possible counters  */
-static struct sbi_pmu_hw_event hw_event_map[SBI_PMU_HW_EVENT_MAX] = {0};
-
-/* counter to enabled event mapping */
-static uint32_t active_events[SBI_HARTMASK_MAX_BITS][SBI_PMU_HW_CTR_MAX + SBI_PMU_FW_CTR_MAX];
-
-/* Bitmap of firmware counters started on each HART */
 #if SBI_PMU_FW_CTR_MAX >= BITS_PER_LONG
 #error "Can't handle firmware counters beyond BITS_PER_LONG"
 #endif
-static unsigned long fw_counters_started[SBI_HARTMASK_MAX_BITS];
 
-/*
- * Counter values for SBI firmware events and event codes for platform
- * firmware events. Both are mutually exclusive and hence can optimally share
- * the same memory.
- */
-static uint64_t fw_counters_data[SBI_HARTMASK_MAX_BITS][SBI_PMU_FW_CTR_MAX] = {0};
+/** Per-HART state of the PMU counters */
+struct sbi_pmu_hart_state {
+	/* HART to which this state belongs */
+	uint32_t hartid;
+	/* Counter to enabled event mapping */
+	uint32_t active_events[SBI_PMU_HW_CTR_MAX + SBI_PMU_FW_CTR_MAX];
+	/* Bitmap of firmware counters started */
+	unsigned long fw_counters_started;
+	/*
+	 * Counter values for SBI firmware events and event codes
+	 * for platform firmware events. Both are mutually exclusive
+	 * and hence can optimally share the same memory.
+	 */
+	uint64_t fw_counters_data[SBI_PMU_FW_CTR_MAX];
+};
+
+/** Offset of pointer to PMU HART state in scratch space */
+static unsigned long phs_ptr_offset;
+
+#define pmu_get_hart_state_ptr(__scratch)				\
+({									\
+	(void *)(*((ulong *)sbi_scratch_offset_ptr((__scratch),		\
+						   phs_ptr_offset)));	\
+})
+
+#define pmu_thishart_state_ptr()					\
+	pmu_get_hart_state_ptr(sbi_scratch_thishart_ptr())
+
+#define pmu_set_hart_state_ptr(__scratch, __phs)			\
+do {									\
+	*((ulong *)sbi_scratch_offset_ptr((__scratch), phs_ptr_offset))	\
+					= (ulong)(__phs);		\
+} while (0)
+
+/* Platform specific PMU device */
+static const struct sbi_pmu_device *pmu_dev = NULL;
+
+/* Mapping between event range and possible counters  */
+static struct sbi_pmu_hw_event *hw_event_map;
 
 /* Maximum number of hardware events available */
 static uint32_t num_hw_events;
@@ -111,13 +133,13 @@ static bool pmu_event_select_overlap(struct sbi_pmu_hw_event *evt,
 	return false;
 }
 
-static int pmu_event_validate(unsigned long event_idx, uint64_t edata)
+static int pmu_event_validate(struct sbi_pmu_hart_state *phs,
+			      unsigned long event_idx, uint64_t edata)
 {
 	uint32_t event_idx_type = get_cidx_type(event_idx);
 	uint32_t event_idx_code = get_cidx_code(event_idx);
 	uint32_t event_idx_code_max = -1;
 	uint32_t cache_ops_result, cache_ops_id, cache_id;
-	u32 hartid = current_hartid();
 
 	switch(event_idx_type) {
 	case SBI_PMU_EVENT_TYPE_HW:
@@ -131,7 +153,7 @@ static int pmu_event_validate(unsigned long event_idx, uint64_t edata)
 
 		if (SBI_PMU_FW_PLATFORM == event_idx_code &&
 		    pmu_dev && pmu_dev->fw_event_validate_encoding)
-			return pmu_dev->fw_event_validate_encoding(hartid,
+			return pmu_dev->fw_event_validate_encoding(phs->hartid,
 							           edata);
 		else
 			event_idx_code_max = SBI_PMU_FW_MAX;
@@ -165,16 +187,16 @@ static int pmu_event_validate(unsigned long event_idx, uint64_t edata)
 	return SBI_EINVAL;
 }
 
-static int pmu_ctr_validate(uint32_t cidx, uint32_t *event_idx_code)
+static int pmu_ctr_validate(struct sbi_pmu_hart_state *phs,
+			    uint32_t cidx, uint32_t *event_idx_code)
 {
 	uint32_t event_idx_val;
 	uint32_t event_idx_type;
-	u32 hartid = current_hartid();
 
 	if (cidx >= total_ctrs)
 		return SBI_EINVAL;
 
-	event_idx_val = active_events[hartid][cidx];
+	event_idx_val = phs->active_events[cidx];
 	event_idx_type = get_cidx_type(event_idx_val);
 	if (event_idx_val == SBI_PMU_EVENT_IDX_INVALID ||
 	    event_idx_type >= SBI_PMU_EVENT_TYPE_MAX)
@@ -189,9 +211,9 @@ int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
 {
 	int event_idx_type;
 	uint32_t event_code;
-	u32 hartid = current_hartid();
+	struct sbi_pmu_hart_state *phs = pmu_thishart_state_ptr();
 
-	event_idx_type = pmu_ctr_validate(cidx, &event_code);
+	event_idx_type = pmu_ctr_validate(phs, cidx, &event_code);
 	if (event_idx_type != SBI_PMU_EVENT_TYPE_FW)
 		return SBI_EINVAL;
 
@@ -202,13 +224,13 @@ int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
 
 	if (SBI_PMU_FW_PLATFORM == event_code) {
 		if (pmu_dev && pmu_dev->fw_counter_read_value)
-			*cval = pmu_dev->fw_counter_read_value(hartid,
+			*cval = pmu_dev->fw_counter_read_value(phs->hartid,
 							       cidx -
 							       num_hw_ctrs);
 		else
 			*cval = 0;
 	} else
-		*cval = fw_counters_data[hartid][cidx - num_hw_ctrs];
+		*cval = phs->fw_counters_data[cidx - num_hw_ctrs];
 
 	return 0;
 }
@@ -376,12 +398,11 @@ int sbi_pmu_irq_bit(void)
 	return 0;
 }
 
-static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
+static int pmu_ctr_start_fw(struct sbi_pmu_hart_state *phs,
+			    uint32_t cidx, uint32_t event_code,
 			    uint64_t event_data, uint64_t ival,
 			    bool ival_update)
 {
-	u32 hartid = current_hartid();
-
 	if ((event_code >= SBI_PMU_FW_MAX &&
 	    event_code <= SBI_PMU_FW_RESERVED_MAX) ||
 	    event_code > SBI_PMU_FW_PLATFORM)
@@ -395,18 +416,19 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
 		    }
 
 		if (ival_update)
-			pmu_dev->fw_counter_write_value(hartid,
+			pmu_dev->fw_counter_write_value(phs->hartid,
 							cidx - num_hw_ctrs,
 							ival);
 
-		return pmu_dev->fw_counter_start(hartid, cidx - num_hw_ctrs,
+		return pmu_dev->fw_counter_start(phs->hartid,
+						 cidx - num_hw_ctrs,
 						 event_data);
 	} else {
 		if (ival_update)
-			fw_counters_data[hartid][cidx - num_hw_ctrs] = ival;
+			phs->fw_counters_data[cidx - num_hw_ctrs] = ival;
 	}
 
-	fw_counters_started[hartid] |= BIT(cidx - num_hw_ctrs);
+	phs->fw_counters_started |= BIT(cidx - num_hw_ctrs);
 
 	return 0;
 }
@@ -414,7 +436,7 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
 int sbi_pmu_ctr_start(unsigned long cbase, unsigned long cmask,
 		      unsigned long flags, uint64_t ival)
 {
-	u32 hartid = current_hartid();
+	struct sbi_pmu_hart_state *phs = pmu_thishart_state_ptr();
 	int event_idx_type;
 	uint32_t event_code;
 	int ret = SBI_EINVAL;
@@ -430,16 +452,16 @@ int sbi_pmu_ctr_start(unsigned long cbase, unsigned long cmask,
 
 	for_each_set_bit(i, &cmask, total_ctrs) {
 		cidx = i + cbase;
-		event_idx_type = pmu_ctr_validate(cidx, &event_code);
+		event_idx_type = pmu_ctr_validate(phs, cidx, &event_code);
 		if (event_idx_type < 0)
 			/* Continue the start operation for other counters */
 			continue;
 		else if (event_idx_type == SBI_PMU_EVENT_TYPE_FW) {
 			edata = (event_code == SBI_PMU_FW_PLATFORM) ?
-				 fw_counters_data[hartid][cidx - num_hw_ctrs]
+				 phs->fw_counters_data[cidx - num_hw_ctrs]
 				 : 0x0;
-			ret = pmu_ctr_start_fw(cidx, event_code, edata, ival,
-					       bUpdate);
+			ret = pmu_ctr_start_fw(phs, cidx, event_code, edata,
+					       ival, bUpdate);
 		}
 		else
 			ret = pmu_ctr_start_hw(cidx, ival, bUpdate);
@@ -470,9 +492,9 @@ static int pmu_ctr_stop_hw(uint32_t cidx)
 		return SBI_EALREADY_STOPPED;
 }
 
-static int pmu_ctr_stop_fw(uint32_t cidx, uint32_t event_code)
+static int pmu_ctr_stop_fw(struct sbi_pmu_hart_state *phs,
+			   uint32_t cidx, uint32_t event_code)
 {
-	u32 hartid = current_hartid();
 	int ret;
 
 	if ((event_code >= SBI_PMU_FW_MAX &&
@@ -482,12 +504,12 @@ static int pmu_ctr_stop_fw(uint32_t cidx, uint32_t event_code)
 
 	if (SBI_PMU_FW_PLATFORM == event_code &&
 	    pmu_dev && pmu_dev->fw_counter_stop) {
-		ret = pmu_dev->fw_counter_stop(hartid, cidx - num_hw_ctrs);
+		ret = pmu_dev->fw_counter_stop(phs->hartid, cidx - num_hw_ctrs);
 		if (ret)
 			return ret;
 	}
 
-	fw_counters_started[current_hartid()] &= ~BIT(cidx - num_hw_ctrs);
+	phs->fw_counters_started &= ~BIT(cidx - num_hw_ctrs);
 
 	return 0;
 }
@@ -511,7 +533,7 @@ static int pmu_reset_hw_mhpmevent(int ctr_idx)
 int sbi_pmu_ctr_stop(unsigned long cbase, unsigned long cmask,
 		     unsigned long flag)
 {
-	u32 hartid = current_hartid();
+	struct sbi_pmu_hart_state *phs = pmu_thishart_state_ptr();
 	int ret = SBI_EINVAL;
 	int event_idx_type;
 	uint32_t event_code;
@@ -522,18 +544,18 @@ int sbi_pmu_ctr_stop(unsigned long cbase, unsigned long cmask,
 
 	for_each_set_bit(i, &cmask, total_ctrs) {
 		cidx = i + cbase;
-		event_idx_type = pmu_ctr_validate(cidx, &event_code);
+		event_idx_type = pmu_ctr_validate(phs, cidx, &event_code);
 		if (event_idx_type < 0)
 			/* Continue the stop operation for other counters */
 			continue;
 
 		else if (event_idx_type == SBI_PMU_EVENT_TYPE_FW)
-			ret = pmu_ctr_stop_fw(cidx, event_code);
+			ret = pmu_ctr_stop_fw(phs, cidx, event_code);
 		else
 			ret = pmu_ctr_stop_hw(cidx);
 
 		if (cidx > (CSR_INSTRET - CSR_CYCLE) && flag & SBI_PMU_STOP_FLAG_RESET) {
-			active_events[hartid][cidx] = SBI_PMU_EVENT_IDX_INVALID;
+			phs->active_events[cidx] = SBI_PMU_EVENT_IDX_INVALID;
 			pmu_reset_hw_mhpmevent(cidx);
 		}
 	}
@@ -604,14 +626,15 @@ static int pmu_ctr_find_fixed_fw(unsigned long evt_idx_code)
 		return SBI_EINVAL;
 }
 
-static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned long flags,
+static int pmu_ctr_find_hw(struct sbi_pmu_hart_state *phs,
+			   unsigned long cbase, unsigned long cmask,
+			   unsigned long flags,
 			   unsigned long event_idx, uint64_t data)
 {
 	unsigned long ctr_mask;
 	int i, ret = 0, fixed_ctr, ctr_idx = SBI_ENOTSUPP;
 	struct sbi_pmu_hw_event *temp;
 	unsigned long mctr_inhbt = 0;
-	u32 hartid = current_hartid();
 	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
 
 	if (cbase >= num_hw_ctrs)
@@ -650,7 +673,7 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
 			 * Some of the platform may not support mcountinhibit.
 			 * Checking the active_events is enough for them
 			 */
-			if (active_events[hartid][cbase] != SBI_PMU_EVENT_IDX_INVALID)
+			if (phs->active_events[cbase] != SBI_PMU_EVENT_IDX_INVALID)
 				continue;
 			/* If mcountinhibit is supported, the bit must be enabled */
 			if ((sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11) &&
@@ -685,8 +708,9 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
  * Thus, select the first available fw counter after sanity
  * check.
  */
-static int pmu_ctr_find_fw(unsigned long cbase, unsigned long cmask,
-			   uint32_t event_code, u32 hartid, uint64_t edata)
+static int pmu_ctr_find_fw(struct sbi_pmu_hart_state *phs,
+			   unsigned long cbase, unsigned long cmask,
+			   uint32_t event_code, uint64_t edata)
 {
 	int i, cidx;
 
@@ -699,11 +723,11 @@ static int pmu_ctr_find_fw(unsigned long cbase, unsigned long cmask,
 		cidx = i + cbase;
 		if (cidx < num_hw_ctrs || total_ctrs <= cidx)
 			continue;
-		if (active_events[hartid][i] != SBI_PMU_EVENT_IDX_INVALID)
+		if (phs->active_events[i] != SBI_PMU_EVENT_IDX_INVALID)
 			continue;
 		if (SBI_PMU_FW_PLATFORM == event_code &&
 		    pmu_dev && pmu_dev->fw_counter_match_encoding) {
-			if (!pmu_dev->fw_counter_match_encoding(hartid,
+			if (!pmu_dev->fw_counter_match_encoding(phs->hartid,
 							    cidx - num_hw_ctrs,
 							    edata))
 				continue;
@@ -719,15 +743,15 @@ int sbi_pmu_ctr_cfg_match(unsigned long cidx_base, unsigned long cidx_mask,
 			  unsigned long flags, unsigned long event_idx,
 			  uint64_t event_data)
 {
-	int ret, ctr_idx = SBI_ENOTSUPP;
-	u32 event_code, hartid = current_hartid();
-	int event_type;
+	struct sbi_pmu_hart_state *phs = pmu_thishart_state_ptr();
+	int ret, event_type, ctr_idx = SBI_ENOTSUPP;
+	u32 event_code;
 
 	/* Do a basic sanity check of counter base & mask */
 	if ((cidx_base + sbi_fls(cidx_mask)) >= total_ctrs)
 		return SBI_EINVAL;
 
-	event_type = pmu_event_validate(event_idx, event_data);
+	event_type = pmu_event_validate(phs, event_idx, event_data);
 	if (event_type < 0)
 		return SBI_EINVAL;
 	event_code = get_cidx_code(event_idx);
@@ -742,7 +766,7 @@ int sbi_pmu_ctr_cfg_match(unsigned long cidx_base, unsigned long cidx_mask,
 		 */
 		unsigned long cidx_first = cidx_base + sbi_ffs(cidx_mask);
 
-		if (active_events[hartid][cidx_first] == SBI_PMU_EVENT_IDX_INVALID)
+		if (phs->active_events[cidx_first] == SBI_PMU_EVENT_IDX_INVALID)
 			return SBI_EINVAL;
 		ctr_idx = cidx_first;
 		goto skip_match;
@@ -750,20 +774,20 @@ int sbi_pmu_ctr_cfg_match(unsigned long cidx_base, unsigned long cidx_mask,
 
 	if (event_type == SBI_PMU_EVENT_TYPE_FW) {
 		/* Any firmware counter can be used track any firmware event */
-		ctr_idx = pmu_ctr_find_fw(cidx_base, cidx_mask, event_code,
-					  hartid, event_data);
+		ctr_idx = pmu_ctr_find_fw(phs, cidx_base, cidx_mask,
+					  event_code, event_data);
 		if (event_code == SBI_PMU_FW_PLATFORM)
-			fw_counters_data[hartid][ctr_idx - num_hw_ctrs] =
+			phs->fw_counters_data[ctr_idx - num_hw_ctrs] =
 								event_data;
 	} else {
-		ctr_idx = pmu_ctr_find_hw(cidx_base, cidx_mask, flags, event_idx,
-					  event_data);
+		ctr_idx = pmu_ctr_find_hw(phs, cidx_base, cidx_mask, flags,
+					  event_idx, event_data);
 	}
 
 	if (ctr_idx < 0)
 		return SBI_ENOTSUPP;
 
-	active_events[hartid][ctr_idx] = event_idx;
+	phs->active_events[ctr_idx] = event_idx;
 skip_match:
 	if (event_type == SBI_PMU_EVENT_TYPE_HW) {
 		if (flags & SBI_PMU_CFG_FLAG_CLEAR_VALUE)
@@ -772,16 +796,17 @@ skip_match:
 			pmu_ctr_start_hw(ctr_idx, 0, false);
 	} else if (event_type == SBI_PMU_EVENT_TYPE_FW) {
 		if (flags & SBI_PMU_CFG_FLAG_CLEAR_VALUE)
-			fw_counters_data[hartid][ctr_idx - num_hw_ctrs] = 0;
+			phs->fw_counters_data[ctr_idx - num_hw_ctrs] = 0;
 		if (flags & SBI_PMU_CFG_FLAG_AUTO_START) {
 			if (SBI_PMU_FW_PLATFORM == event_code &&
 			    pmu_dev && pmu_dev->fw_counter_start) {
-				ret = pmu_dev->fw_counter_start(hartid,
+				ret = pmu_dev->fw_counter_start(
+					phs->hartid,
 					ctr_idx - num_hw_ctrs, event_data);
 				if (ret)
 					return ret;
 			}
-			fw_counters_started[hartid] |= BIT(ctr_idx - num_hw_ctrs);
+			phs->fw_counters_started |= BIT(ctr_idx - num_hw_ctrs);
 		}
 	}
 
@@ -790,19 +815,20 @@ skip_match:
 
 int sbi_pmu_ctr_incr_fw(enum sbi_pmu_fw_event_code_id fw_id)
 {
-	u32 cidx, hartid = current_hartid();
+	u32 cidx;
 	uint64_t *fcounter = NULL;
+	struct sbi_pmu_hart_state *phs = pmu_thishart_state_ptr();
 
-	if (likely(!fw_counters_started[hartid]))
+	if (likely(!phs->fw_counters_started))
 		return 0;
 
 	if (unlikely(fw_id >= SBI_PMU_FW_MAX))
 		return SBI_EINVAL;
 
 	for (cidx = num_hw_ctrs; cidx < total_ctrs; cidx++) {
-		if (get_cidx_code(active_events[hartid][cidx]) == fw_id &&
-		    (fw_counters_started[hartid] & BIT(cidx - num_hw_ctrs))) {
-			fcounter = &fw_counters_data[hartid][cidx - num_hw_ctrs];
+		if (get_cidx_code(phs->active_events[cidx]) == fw_id &&
+		    (phs->fw_counters_started & BIT(cidx - num_hw_ctrs))) {
+			fcounter = &phs->fw_counters_data[cidx - num_hw_ctrs];
 			break;
 		}
 	}
@@ -854,16 +880,16 @@ int sbi_pmu_ctr_get_info(uint32_t cidx, unsigned long *ctr_info)
 	return 0;
 }
 
-static void pmu_reset_event_map(u32 hartid)
+static void pmu_reset_event_map(struct sbi_pmu_hart_state *phs)
 {
 	int j;
 
 	/* Initialize the counter to event mapping table */
 	for (j = 3; j < total_ctrs; j++)
-		active_events[hartid][j] = SBI_PMU_EVENT_IDX_INVALID;
+		phs->active_events[j] = SBI_PMU_EVENT_IDX_INVALID;
 	for (j = 0; j < SBI_PMU_FW_CTR_MAX; j++)
-		fw_counters_data[hartid][j] = 0;
-	fw_counters_started[hartid] = 0;
+		phs->fw_counters_data[j] = 0;
+	phs->fw_counters_started = 0;
 }
 
 const struct sbi_pmu_device *sbi_pmu_get_device(void)
@@ -881,22 +907,32 @@ void sbi_pmu_set_device(const struct sbi_pmu_device *dev)
 
 void sbi_pmu_exit(struct sbi_scratch *scratch)
 {
-	u32 hartid = current_hartid();
-
 	if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11)
 		csr_write(CSR_MCOUNTINHIBIT, 0xFFFFFFF8);
 
 	if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
 		csr_write(CSR_MCOUNTEREN, -1);
-	pmu_reset_event_map(hartid);
+
+	pmu_reset_event_map(pmu_get_hart_state_ptr(scratch));
 }
 
 int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot)
 {
+	struct sbi_pmu_hart_state *phs;
 	const struct sbi_platform *plat;
-	u32 hartid = current_hartid();
 
 	if (cold_boot) {
+		hw_event_map = sbi_calloc(sizeof(*hw_event_map),
+					  SBI_PMU_HW_EVENT_MAX);
+		if (!hw_event_map)
+			return SBI_ENOMEM;
+
+		phs_ptr_offset = sbi_scratch_alloc_offset(sizeof(ulong));
+		if (!phs_ptr_offset) {
+			sbi_free(hw_event_map);
+			return SBI_ENOMEM;
+		}
+
 		plat = sbi_platform_ptr(scratch);
 		/* Initialize hw pmu events */
 		sbi_platform_pmu_init(plat);
@@ -906,14 +942,23 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot)
 		total_ctrs = num_hw_ctrs + SBI_PMU_FW_CTR_MAX;
 	}
 
-	pmu_reset_event_map(hartid);
+	phs = pmu_get_hart_state_ptr(scratch);
+	if (!phs) {
+		phs = sbi_zalloc(sizeof(*phs));
+		if (!phs)
+			return SBI_ENOMEM;
+		phs->hartid = current_hartid();
+		pmu_set_hart_state_ptr(scratch, phs);
+	}
+
+	pmu_reset_event_map(phs);
 
 	/* First three counters are fixed by the priv spec and we enable it by default */
-	active_events[hartid][0] = SBI_PMU_EVENT_TYPE_HW << SBI_PMU_EVENT_IDX_TYPE_OFFSET |
-				   SBI_PMU_HW_CPU_CYCLES;
-	active_events[hartid][1] = SBI_PMU_EVENT_IDX_INVALID;
-	active_events[hartid][2] = SBI_PMU_EVENT_TYPE_HW << SBI_PMU_EVENT_IDX_TYPE_OFFSET |
-				   SBI_PMU_HW_INSTRUCTIONS;
+	phs->active_events[0] = (SBI_PMU_EVENT_TYPE_HW << SBI_PMU_EVENT_IDX_TYPE_OFFSET) |
+				SBI_PMU_HW_CPU_CYCLES;
+	phs->active_events[1] = SBI_PMU_EVENT_IDX_INVALID;
+	phs->active_events[2] = (SBI_PMU_EVENT_TYPE_HW << SBI_PMU_EVENT_IDX_TYPE_OFFSET) |
+				SBI_PMU_HW_INSTRUCTIONS;
 
 	return 0;
 }
-- 
2.34.1

[PATCH 05/17] lib: sbi: Use heap for root domain creation

[Anup Patel]

Let's use heap allocation in root domain creation instead of using
a fixed size global array.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/sbi/sbi_domain.c | 45 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
index 67b87f3..b07d310 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -11,6 +11,7 @@
 #include <sbi/sbi_console.h>
 #include <sbi/sbi_domain.h>
 #include <sbi/sbi_hartmask.h>
+#include <sbi/sbi_heap.h>
 #include <sbi/sbi_hsm.h>
 #include <sbi/sbi_math.h>
 #include <sbi/sbi_platform.h>
@@ -26,16 +27,13 @@ struct sbi_domain *hartid_to_domain_table[SBI_HARTMASK_MAX_BITS] = { 0 };
 static u32 domain_count = 0;
 static bool domain_finalized = false;
 
-static struct sbi_hartmask root_hmask = { 0 };
-
 #define ROOT_REGION_MAX	16
 static u32 root_memregs_count = 0;
-static struct sbi_domain_memregion root_memregs[ROOT_REGION_MAX + 1] = { 0 };
 
 struct sbi_domain root = {
 	.name = "root",
-	.possible_harts = &root_hmask,
-	.regions = root_memregs,
+	.possible_harts = NULL,
+	.regions = NULL,
 	.system_reset_allowed = true,
 	.system_suspend_allowed = true,
 	.fw_region_inited = false,
@@ -551,8 +549,7 @@ int sbi_domain_root_add_memregion(const struct sbi_domain_memregion *reg)
 	const struct sbi_platform *plat = sbi_platform_thishart_ptr();
 
 	/* Sanity checks */
-	if (!reg || domain_finalized ||
-	    (root.regions != root_memregs) ||
+	if (!reg || domain_finalized || !root.regions ||
 	    (ROOT_REGION_MAX <= root_memregs_count))
 		return SBI_EINVAL;
 
@@ -567,10 +564,10 @@ int sbi_domain_root_add_memregion(const struct sbi_domain_memregion *reg)
 	}
 
 	/* Append the memregion to root memregions */
-	nreg = &root_memregs[root_memregs_count];
+	nreg = &root.regions[root_memregs_count];
 	sbi_memcpy(nreg, reg, sizeof(*reg));
 	root_memregs_count++;
-	root_memregs[root_memregs_count].order = 0;
+	root.regions[root_memregs_count].order = 0;
 
 	/* Sort and optimize root regions */
 	do {
@@ -700,6 +697,9 @@ int sbi_domain_finalize(struct sbi_scratch *scratch, u32 cold_hartid)
 int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
 {
 	u32 i;
+	int rc;
+	struct sbi_hartmask *root_hmask;
+	struct sbi_domain_memregion *root_memregs;
 	const struct sbi_platform *plat = sbi_platform_ptr(scratch);
 
 	if (scratch->fw_rw_offset == 0 ||
@@ -715,6 +715,21 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
 		return SBI_EINVAL;
 	}
 
+	root_memregs = sbi_calloc(sizeof(*root_memregs), ROOT_REGION_MAX + 1);
+	if (!root_memregs) {
+		sbi_printf("%s: no memory for root regions\n", __func__);
+		return SBI_ENOMEM;
+	}
+	root.regions = root_memregs;
+
+	root_hmask = sbi_zalloc(sizeof(*root_hmask));
+	if (!root_hmask) {
+		sbi_printf("%s: no memory for root hartmask\n", __func__);
+		sbi_free(root_memregs);
+		return SBI_ENOMEM;
+	}
+	root.possible_harts = root_hmask;
+
 	/* Root domain firmware memory region */
 	sbi_domain_memregion_init(scratch->fw_start, scratch->fw_rw_offset,
 				  (SBI_DOMAIN_MEMREGION_M_READABLE |
@@ -751,8 +766,16 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
 	for (i = 0; i < SBI_HARTMASK_MAX_BITS; i++) {
 		if (sbi_platform_hart_invalid(plat, i))
 			continue;
-		sbi_hartmask_set_hart(i, &root_hmask);
+		sbi_hartmask_set_hart(i, root_hmask);
+	}
+
+	/* Finally register the root domain */
+	rc = sbi_domain_register(&root, root_hmask);
+	if (rc) {
+		sbi_free(root_hmask);
+		sbi_free(root_memregs);
+		return rc;
 	}
 
-	return sbi_domain_register(&root, &root_hmask);
+	return 0;
 }
-- 
2.34.1

[PATCH 06/17] lib: sbi: Use scratch space to save per-HART domain pointer

[Anup Patel]

Instead of using a global array indexed by hartid, we should use
scratch space to save per-HART domain pointer.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 include/sbi/sbi_domain.h |  6 +-----
 lib/sbi/sbi_domain.c     | 38 +++++++++++++++++++++++++++++++++++---
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h
index 124ea90..b05bcf4 100644
--- a/include/sbi/sbi_domain.h
+++ b/include/sbi/sbi_domain.h
@@ -129,12 +129,8 @@ struct sbi_domain {
 /** The root domain instance */
 extern struct sbi_domain root;
 
-/** HART id to domain table */
-extern struct sbi_domain *hartid_to_domain_table[];
-
 /** Get pointer to sbi_domain from HART id */
-#define sbi_hartid_to_domain(__hartid) \
-	hartid_to_domain_table[__hartid]
+struct sbi_domain *sbi_hartid_to_domain(u32 hartid);
 
 /** Get pointer to sbi_domain for current HART */
 #define sbi_domain_thishart_ptr() \
diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
index b07d310..531737c 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -23,7 +23,6 @@
  * the array to be null-terminated.
  */
 struct sbi_domain *domidx_to_domain_table[SBI_DOMAIN_MAX_INDEX + 1] = { 0 };
-struct sbi_domain *hartid_to_domain_table[SBI_HARTMASK_MAX_BITS] = { 0 };
 static u32 domain_count = 0;
 static bool domain_finalized = false;
 
@@ -39,6 +38,32 @@ struct sbi_domain root = {
 	.fw_region_inited = false,
 };
 
+static unsigned long domain_hart_ptr_offset;
+
+struct sbi_domain *sbi_hartid_to_domain(u32 hartid)
+{
+	struct sbi_scratch *scratch;
+
+	scratch = sbi_hartid_to_scratch(hartid);
+	if (!scratch || !domain_hart_ptr_offset)
+		return NULL;
+
+	return (void *)(*((ulong *)sbi_scratch_offset_ptr(scratch,
+						   domain_hart_ptr_offset)));
+}
+
+static void update_hartid_to_domain(u32 hartid, struct sbi_domain *dom)
+{
+	struct sbi_scratch *scratch;
+
+	scratch = sbi_hartid_to_scratch(hartid);
+	if (!scratch)
+		return;
+
+	*((ulong *)sbi_scratch_offset_ptr(scratch, domain_hart_ptr_offset))
+					= (ulong)dom;
+}
+
 bool sbi_domain_is_assigned_hart(const struct sbi_domain *dom, u32 hartid)
 {
 	if (dom)
@@ -519,11 +544,11 @@ int sbi_domain_register(struct sbi_domain *dom,
 		if (!sbi_hartmask_test_hart(i, dom->possible_harts))
 			continue;
 
-		tdom = hartid_to_domain_table[i];
+		tdom = sbi_hartid_to_domain(i);
 		if (tdom)
 			sbi_hartmask_clear_hart(i,
 					&tdom->assigned_harts);
-		hartid_to_domain_table[i] = dom;
+		update_hartid_to_domain(i, dom);
 		sbi_hartmask_set_hart(i, &dom->assigned_harts);
 
 		/*
@@ -715,9 +740,14 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
 		return SBI_EINVAL;
 	}
 
+	domain_hart_ptr_offset = sbi_scratch_alloc_offset(sizeof(ulong));
+	if (!domain_hart_ptr_offset)
+		return SBI_ENOMEM;
+
 	root_memregs = sbi_calloc(sizeof(*root_memregs), ROOT_REGION_MAX + 1);
 	if (!root_memregs) {
 		sbi_printf("%s: no memory for root regions\n", __func__);
+		sbi_scratch_free_offset(domain_hart_ptr_offset);
 		return SBI_ENOMEM;
 	}
 	root.regions = root_memregs;
@@ -726,6 +756,7 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
 	if (!root_hmask) {
 		sbi_printf("%s: no memory for root hartmask\n", __func__);
 		sbi_free(root_memregs);
+		sbi_scratch_free_offset(domain_hart_ptr_offset);
 		return SBI_ENOMEM;
 	}
 	root.possible_harts = root_hmask;
@@ -774,6 +805,7 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
 	if (rc) {
 		sbi_free(root_hmask);
 		sbi_free(root_memregs);
+		sbi_scratch_free_offset(domain_hart_ptr_offset);
 		return rc;
 	}
 
-- 
2.34.1

[PATCH 07/17] lib: utils/gpio: Use heap in SiFive and StartFive GPIO drivers

[Anup Patel]

Let's use heap allocation in SiFive and Starfive GPIO drivers
instead of using a fixed size global array.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/utils/gpio/fdt_gpio_sifive.c   | 21 ++++++++++-----------
 lib/utils/gpio/fdt_gpio_starfive.c | 20 ++++++++++----------
 2 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/lib/utils/gpio/fdt_gpio_sifive.c b/lib/utils/gpio/fdt_gpio_sifive.c
index 677f0fa..5e3f39d 100644
--- a/lib/utils/gpio/fdt_gpio_sifive.c
+++ b/lib/utils/gpio/fdt_gpio_sifive.c
@@ -9,11 +9,10 @@
 
 #include <sbi/riscv_io.h>
 #include <sbi/sbi_error.h>
+#include <sbi/sbi_heap.h>
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/gpio/fdt_gpio.h>
 
-#define SIFIVE_GPIO_CHIP_MAX	2
-
 #define SIFIVE_GPIO_PINS_MIN	1
 #define SIFIVE_GPIO_PINS_MAX	32
 #define SIFIVE_GPIO_PINS_DEF	16
@@ -27,9 +26,6 @@ struct sifive_gpio_chip {
 	struct gpio_chip chip;
 };
 
-static unsigned int sifive_gpio_chip_count;
-static struct sifive_gpio_chip sifive_gpio_chip_array[SIFIVE_GPIO_CHIP_MAX];
-
 static int sifive_gpio_direction_output(struct gpio_pin *gp, int value)
 {
 	unsigned int v;
@@ -73,13 +69,15 @@ static int sifive_gpio_init(void *fdt, int nodeoff, u32 phandle,
 	struct sifive_gpio_chip *chip;
 	uint64_t addr;
 
-	if (SIFIVE_GPIO_CHIP_MAX <= sifive_gpio_chip_count)
-		return SBI_ENOSPC;
-	chip = &sifive_gpio_chip_array[sifive_gpio_chip_count];
+	chip = sbi_zalloc(sizeof(*chip));
+	if (!chip)
+		return SBI_ENOMEM;
 
 	rc = fdt_get_node_addr_size(fdt, nodeoff, 0, &addr, NULL);
-	if (rc)
+	if (rc) {
+		sbi_free(chip);
 		return rc;
+	}
 
 	chip->addr = addr;
 	chip->chip.driver = &fdt_gpio_sifive;
@@ -88,10 +86,11 @@ static int sifive_gpio_init(void *fdt, int nodeoff, u32 phandle,
 	chip->chip.direction_output = sifive_gpio_direction_output;
 	chip->chip.set = sifive_gpio_set;
 	rc = gpio_chip_add(&chip->chip);
-	if (rc)
+	if (rc) {
+		sbi_free(chip);
 		return rc;
+	}
 
-	sifive_gpio_chip_count++;
 	return 0;
 }
 
diff --git a/lib/utils/gpio/fdt_gpio_starfive.c b/lib/utils/gpio/fdt_gpio_starfive.c
index 18cca72..f430b13 100644
--- a/lib/utils/gpio/fdt_gpio_starfive.c
+++ b/lib/utils/gpio/fdt_gpio_starfive.c
@@ -9,11 +9,11 @@
 
 #include <sbi/riscv_io.h>
 #include <sbi/sbi_error.h>
+#include <sbi/sbi_heap.h>
 #include <sbi/sbi_console.h>
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/gpio/fdt_gpio.h>
 
-#define STARFIVE_GPIO_CHIP_MAX		2
 #define STARFIVE_GPIO_PINS_DEF		64
 #define STARFIVE_GPIO_OUTVAL		0x40
 #define STARFIVE_GPIO_MASK		0xff
@@ -25,9 +25,6 @@ struct starfive_gpio_chip {
 	struct gpio_chip chip;
 };
 
-static unsigned int starfive_gpio_chip_count;
-static struct starfive_gpio_chip starfive_gpio_chip_array[STARFIVE_GPIO_CHIP_MAX];
-
 static int starfive_gpio_direction_output(struct gpio_pin *gp, int value)
 {
 	u32 val;
@@ -82,13 +79,15 @@ static int starfive_gpio_init(void *fdt, int nodeoff, u32 phandle,
 	struct starfive_gpio_chip *chip;
 	u64 addr;
 
-	if (starfive_gpio_chip_count >= STARFIVE_GPIO_CHIP_MAX)
-		return SBI_ENOSPC;
-	chip = &starfive_gpio_chip_array[starfive_gpio_chip_count];
+	chip = sbi_zalloc(sizeof(*chip));
+	if (!chip)
+		return SBI_ENOMEM;
 
 	rc = fdt_get_node_addr_size(fdt, nodeoff, 0, &addr, NULL);
-	if (rc)
+	if (rc) {
+		sbi_free(chip);
 		return rc;
+	}
 
 	chip->addr = addr;
 	chip->chip.driver = &fdt_gpio_starfive;
@@ -97,10 +96,11 @@ static int starfive_gpio_init(void *fdt, int nodeoff, u32 phandle,
 	chip->chip.direction_output = starfive_gpio_direction_output;
 	chip->chip.set = starfive_gpio_set;
 	rc = gpio_chip_add(&chip->chip);
-	if (rc)
+	if (rc) {
+		sbi_free(chip);
 		return rc;
+	}
 
-	starfive_gpio_chip_count++;
 	return 0;
 }
 
-- 
2.34.1

[PATCH 08/17] lib: utils/i2c: Use heap in DesignWare and SiFive I2C drivers

[Anup Patel]

Let's use heap allocation in DesignWare and SiFive I2C drivers
instead of using a fixed size global array.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/utils/i2c/fdt_i2c_dw.c     | 24 ++++++++++--------------
 lib/utils/i2c/fdt_i2c_sifive.c | 23 ++++++++++-------------
 2 files changed, 20 insertions(+), 27 deletions(-)

diff --git a/lib/utils/i2c/fdt_i2c_dw.c b/lib/utils/i2c/fdt_i2c_dw.c
index 71062f4..99b2ddb 100644
--- a/lib/utils/i2c/fdt_i2c_dw.c
+++ b/lib/utils/i2c/fdt_i2c_dw.c
@@ -9,17 +9,12 @@
 
 #include <sbi/riscv_io.h>
 #include <sbi/sbi_error.h>
+#include <sbi/sbi_heap.h>
 #include <sbi/sbi_string.h>
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/i2c/dw_i2c.h>
 #include <sbi_utils/i2c/fdt_i2c.h>
 
-#define FDT_DW_I2C_ADAPTER_MAX	7
-
-static unsigned int fdt_dw_i2c_adapter_count;
-static struct dw_i2c_adapter
-	fdt_dw_i2c_adapter_array[FDT_DW_I2C_ADAPTER_MAX];
-
 extern struct fdt_i2c_adapter fdt_i2c_adapter_dw;
 
 static int fdt_dw_i2c_init(void *fdt, int nodeoff,
@@ -29,23 +24,24 @@ static int fdt_dw_i2c_init(void *fdt, int nodeoff,
 	struct dw_i2c_adapter *adapter;
 	u64 addr;
 
-	if (fdt_dw_i2c_adapter_count >= FDT_DW_I2C_ADAPTER_MAX)
-		return SBI_ENOSPC;
-
-	adapter = &fdt_dw_i2c_adapter_array[fdt_dw_i2c_adapter_count];
+	adapter = sbi_zalloc(sizeof(*adapter));
+	if (!adapter)
+		return SBI_ENOMEM;
 
 	rc = fdt_get_node_addr_size(fdt, nodeoff, 0, &addr, NULL);
-	if (rc)
+	if (rc) {
+		sbi_free(adapter);
 		return rc;
+	}
 
 	adapter->addr = addr;
 	adapter->adapter.driver = &fdt_i2c_adapter_dw;
 
 	rc = dw_i2c_init(&adapter->adapter, nodeoff);
-	if (rc)
+	if (rc) {
+		sbi_free(adapter);
 		return rc;
-
-	fdt_dw_i2c_adapter_count++;
+	}
 
 	return 0;
 }
diff --git a/lib/utils/i2c/fdt_i2c_sifive.c b/lib/utils/i2c/fdt_i2c_sifive.c
index 195541c..b85b245 100644
--- a/lib/utils/i2c/fdt_i2c_sifive.c
+++ b/lib/utils/i2c/fdt_i2c_sifive.c
@@ -9,12 +9,11 @@
 
 #include <sbi/riscv_io.h>
 #include <sbi/sbi_error.h>
+#include <sbi/sbi_heap.h>
 #include <sbi/sbi_timer.h>
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/i2c/fdt_i2c.h>
 
-#define SIFIVE_I2C_ADAPTER_MAX	2
-
 #define SIFIVE_I2C_PRELO	0x00
 #define SIFIVE_I2C_PREHI	0x04
 #define SIFIVE_I2C_CTR		0x08
@@ -47,10 +46,6 @@ struct sifive_i2c_adapter {
 	struct i2c_adapter adapter;
 };
 
-static unsigned int sifive_i2c_adapter_count;
-static struct sifive_i2c_adapter
-	sifive_i2c_adapter_array[SIFIVE_I2C_ADAPTER_MAX];
-
 extern struct fdt_i2c_adapter fdt_i2c_adapter_sifive;
 
 static inline void sifive_i2c_setreg(struct sifive_i2c_adapter *adap,
@@ -244,14 +239,15 @@ static int sifive_i2c_init(void *fdt, int nodeoff,
 	struct sifive_i2c_adapter *adapter;
 	uint64_t addr;
 
-	if (sifive_i2c_adapter_count >= SIFIVE_I2C_ADAPTER_MAX)
-		return SBI_ENOSPC;
-
-	adapter = &sifive_i2c_adapter_array[sifive_i2c_adapter_count];
+	adapter = sbi_zalloc(sizeof(*adapter));
+	if (!adapter)
+		return SBI_ENOMEM;
 
 	rc = fdt_get_node_addr_size(fdt, nodeoff, 0, &addr, NULL);
-	if (rc)
+	if (rc) {
+		sbi_free(adapter);
 		return rc;
+	}
 
 	adapter->addr = addr;
 	adapter->adapter.driver = &fdt_i2c_adapter_sifive;
@@ -259,10 +255,11 @@ static int sifive_i2c_init(void *fdt, int nodeoff,
 	adapter->adapter.write = sifive_i2c_adapter_write;
 	adapter->adapter.read = sifive_i2c_adapter_read;
 	rc = i2c_adapter_add(&adapter->adapter);
-	if (rc)
+	if (rc) {
+		sbi_free(adapter);
 		return rc;
+	}
 
-	sifive_i2c_adapter_count++;
 	return 0;
 }
 
-- 
2.34.1

[PATCH 09/17] lib: utils/ipi: Use heap in ACLINT MSWI driver

[Anup Patel]

Let's use heap allocation in ACLINT MSWI driver instead of using
a fixed size global array.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/utils/ipi/fdt_ipi_mswi.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/lib/utils/ipi/fdt_ipi_mswi.c b/lib/utils/ipi/fdt_ipi_mswi.c
index 4dc91f2..e3a6378 100644
--- a/lib/utils/ipi/fdt_ipi_mswi.c
+++ b/lib/utils/ipi/fdt_ipi_mswi.c
@@ -8,15 +8,11 @@
  */
 
 #include <sbi/sbi_error.h>
+#include <sbi/sbi_heap.h>
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/ipi/fdt_ipi.h>
 #include <sbi_utils/ipi/aclint_mswi.h>
 
-#define MSWI_MAX_NR			16
-
-static unsigned long mswi_count = 0;
-static struct aclint_mswi_data mswi[MSWI_MAX_NR];
-
 static int ipi_mswi_cold_init(void *fdt, int nodeoff,
 			      const struct fdt_match *match)
 {
@@ -24,15 +20,17 @@ static int ipi_mswi_cold_init(void *fdt, int nodeoff,
 	unsigned long offset;
 	struct aclint_mswi_data *ms;
 
-	if (MSWI_MAX_NR <= mswi_count)
-		return SBI_ENOSPC;
-	ms = &mswi[mswi_count];
+	ms = sbi_zalloc(sizeof(*ms));
+	if (!ms)
+		return SBI_ENOMEM;
 
 	rc = fdt_parse_aclint_node(fdt, nodeoff, false,
 				   &ms->addr, &ms->size, NULL, NULL,
 				   &ms->first_hartid, &ms->hart_count);
-	if (rc)
+	if (rc) {
+		sbi_free(ms);
 		return rc;
+	}
 
 	if (match->data) {
 		/* Adjust MSWI address and size for CLINT device */
@@ -44,10 +42,11 @@ static int ipi_mswi_cold_init(void *fdt, int nodeoff,
 	}
 
 	rc = aclint_mswi_cold_init(ms);
-	if (rc)
+	if (rc) {
+		sbi_free(ms);
 		return rc;
+	}
 
-	mswi_count++;
 	return 0;
 }
 
-- 
2.34.1

[PATCH 10/17] lib: utils/irqchip: Use heap in PLIC, APLIC and IMSIC drivers

[Anup Patel]

Let's use heap allocation in PLIC, APLIC, and IMSIC irqchip drivers
instead of using a fixed size global array.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/utils/irqchip/fdt_irqchip_aplic.c | 24 ++++++++++--------
 lib/utils/irqchip/fdt_irqchip_imsic.c | 30 +++++++++++-----------
 lib/utils/irqchip/fdt_irqchip_plic.c  | 36 +++++++++++++--------------
 3 files changed, 47 insertions(+), 43 deletions(-)

diff --git a/lib/utils/irqchip/fdt_irqchip_aplic.c b/lib/utils/irqchip/fdt_irqchip_aplic.c
index 965f023..caa92a7 100644
--- a/lib/utils/irqchip/fdt_irqchip_aplic.c
+++ b/lib/utils/irqchip/fdt_irqchip_aplic.c
@@ -11,15 +11,11 @@
 #include <libfdt.h>
 #include <sbi/riscv_asm.h>
 #include <sbi/sbi_error.h>
+#include <sbi/sbi_heap.h>
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/irqchip/fdt_irqchip.h>
 #include <sbi_utils/irqchip/aplic.h>
 
-#define APLIC_MAX_NR			16
-
-static unsigned long aplic_count = 0;
-static struct aplic_data aplic[APLIC_MAX_NR];
-
 static int irqchip_aplic_warm_init(void)
 {
 	/* Nothing to do here. */
@@ -32,15 +28,23 @@ static int irqchip_aplic_cold_init(void *fdt, int nodeoff,
 	int rc;
 	struct aplic_data *pd;
 
-	if (APLIC_MAX_NR <= aplic_count)
-		return SBI_ENOSPC;
-	pd = &aplic[aplic_count++];
+	pd = sbi_zalloc(sizeof(*pd));
+	if (!pd)
+		return SBI_ENOMEM;
 
 	rc = fdt_parse_aplic_node(fdt, nodeoff, pd);
-	if (rc)
+	if (rc) {
+		sbi_free(pd);
 		return rc;
+	}
 
-	return aplic_cold_irqchip_init(pd);
+	rc = aplic_cold_irqchip_init(pd);
+	if (rc) {
+		sbi_free(pd);
+		return rc;
+	}
+
+	return 0;
 }
 
 static const struct fdt_match irqchip_aplic_match[] = {
diff --git a/lib/utils/irqchip/fdt_irqchip_imsic.c b/lib/utils/irqchip/fdt_irqchip_imsic.c
index 6020ac0..d890400 100644
--- a/lib/utils/irqchip/fdt_irqchip_imsic.c
+++ b/lib/utils/irqchip/fdt_irqchip_imsic.c
@@ -11,16 +11,12 @@
 #include <libfdt.h>
 #include <sbi/riscv_asm.h>
 #include <sbi/sbi_error.h>
+#include <sbi/sbi_heap.h>
 #include <sbi/sbi_hartmask.h>
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/irqchip/fdt_irqchip.h>
 #include <sbi_utils/irqchip/imsic.h>
 
-#define IMSIC_MAX_NR			16
-
-static unsigned long imsic_count = 0;
-static struct imsic_data imsic[IMSIC_MAX_NR];
-
 static int irqchip_imsic_update_hartid_table(void *fdt, int nodeoff,
 					     struct imsic_data *id)
 {
@@ -71,25 +67,31 @@ static int irqchip_imsic_cold_init(void *fdt, int nodeoff,
 	int rc;
 	struct imsic_data *id;
 
-	if (IMSIC_MAX_NR <= imsic_count)
-		return SBI_ENOSPC;
-	id = &imsic[imsic_count];
+	id = sbi_zalloc(sizeof(*id));
+	if (!id)
+		return SBI_ENOMEM;
 
 	rc = fdt_parse_imsic_node(fdt, nodeoff, id);
-	if (rc)
+	if (rc) {
+		sbi_free(id);
 		return rc;
-	if (!id->targets_mmode)
+	}
+	if (!id->targets_mmode) {
+		sbi_free(id);
 		return 0;
+	}
 
 	rc = irqchip_imsic_update_hartid_table(fdt, nodeoff, id);
-	if (rc)
+	if (rc) {
+		sbi_free(id);
 		return rc;
+	}
 
 	rc = imsic_cold_irqchip_init(id);
-	if (rc)
+	if (rc) {
+		sbi_free(id);
 		return rc;
-
-	imsic_count++;
+	}
 
 	return 0;
 }
diff --git a/lib/utils/irqchip/fdt_irqchip_plic.c b/lib/utils/irqchip/fdt_irqchip_plic.c
index 1aadf91..605f44a 100644
--- a/lib/utils/irqchip/fdt_irqchip_plic.c
+++ b/lib/utils/irqchip/fdt_irqchip_plic.c
@@ -11,18 +11,14 @@
 #include <sbi/riscv_asm.h>
 #include <sbi/riscv_io.h>
 #include <sbi/sbi_error.h>
+#include <sbi/sbi_heap.h>
 #include <sbi/sbi_hartmask.h>
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/irqchip/fdt_irqchip.h>
 #include <sbi_utils/irqchip/plic.h>
 
-#define PLIC_MAX_NR			16
-
-static unsigned long plic_count = 0;
-static struct plic_data plic[PLIC_MAX_NR];
-
 static struct plic_data *plic_hartid2data[SBI_HARTMASK_MAX_BITS];
-static int plic_hartid2context[SBI_HARTMASK_MAX_BITS][2];
+static int plic_hartid2context[SBI_HARTMASK_MAX_BITS][2] = { { -1 } };
 
 void fdt_plic_priority_save(u8 *priority, u32 num)
 {
@@ -114,16 +110,18 @@ static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
 static int irqchip_plic_cold_init(void *fdt, int nodeoff,
 				  const struct fdt_match *match)
 {
-	int i, rc;
+	int rc;
 	struct plic_data *pd;
 
-	if (PLIC_MAX_NR <= plic_count)
-		return SBI_ENOSPC;
-	pd = &plic[plic_count++];
+	pd = sbi_zalloc(sizeof(*pd));
+	if (!pd)
+		return SBI_ENOMEM;
 
 	rc = fdt_parse_plic_node(fdt, nodeoff, pd);
-	if (rc)
+	if (rc) {
+		sbi_free(pd);
 		return rc;
+	}
 
 	if (match->data) {
 		void (*plic_plat_init)(struct plic_data *) = match->data;
@@ -131,18 +129,18 @@ static int irqchip_plic_cold_init(void *fdt, int nodeoff,
 	}
 
 	rc = plic_cold_irqchip_init(pd);
-	if (rc)
+	if (rc) {
+		sbi_free(pd);
 		return rc;
+	}
 
-	if (plic_count == 1) {
-		for (i = 0; i < SBI_HARTMASK_MAX_BITS; i++) {
-			plic_hartid2data[i] = NULL;
-			plic_hartid2context[i][0] = -1;
-			plic_hartid2context[i][1] = -1;
-		}
+	rc = irqchip_plic_update_hartid_table(fdt, nodeoff, pd);
+	if (rc) {
+		sbi_free(pd);
+		return rc;
 	}
 
-	return irqchip_plic_update_hartid_table(fdt, nodeoff, pd);
+	return 0;
 }
 
 #define THEAD_PLIC_CTRL_REG 0x1ffffc
-- 
2.34.1

[PATCH 11/17] lib: utils/timer: Use heap in ACLINT MTIMER driver

[Anup Patel]

Let's use heap allocation in ACLINT MTIMER driver instead of using
a fixed size global array.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/utils/timer/fdt_timer_mtimer.c | 47 +++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/lib/utils/timer/fdt_timer_mtimer.c b/lib/utils/timer/fdt_timer_mtimer.c
index 5244f98..9eaa11d 100644
--- a/lib/utils/timer/fdt_timer_mtimer.c
+++ b/lib/utils/timer/fdt_timer_mtimer.c
@@ -9,44 +9,54 @@
 
 #include <libfdt.h>
 #include <sbi/sbi_error.h>
+#include <sbi/sbi_heap.h>
+#include <sbi/sbi_list.h>
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/timer/fdt_timer.h>
 #include <sbi_utils/timer/aclint_mtimer.h>
 
-#define MTIMER_MAX_NR			16
-
 struct timer_mtimer_quirks {
 	unsigned int	mtime_offset;
 	bool		has_64bit_mmio;
 	bool		without_mtime;
 };
 
-static unsigned long mtimer_count = 0;
-static struct aclint_mtimer_data mtimer[MTIMER_MAX_NR];
+struct timer_mtimer_node {
+	struct sbi_dlist head;
+	struct aclint_mtimer_data data;
+};
+static SBI_LIST_HEAD(mtn_list);
+
 static struct aclint_mtimer_data *mt_reference = NULL;
 
 static int timer_mtimer_cold_init(void *fdt, int nodeoff,
 				  const struct fdt_match *match)
 {
-	int i, rc;
+	int rc;
 	unsigned long addr[2], size[2];
+	struct timer_mtimer_node *mtn, *n;
 	struct aclint_mtimer_data *mt;
 
-	if (MTIMER_MAX_NR <= mtimer_count)
-		return SBI_ENOSPC;
-	mt = &mtimer[mtimer_count];
+	mtn = sbi_zalloc(sizeof(*mtn));
+	if (!mtn)
+		return SBI_ENOMEM;
+	mt = &mtn->data;
 
 	rc = fdt_parse_aclint_node(fdt, nodeoff, true,
 				   &addr[0], &size[0], &addr[1], &size[1],
 				   &mt->first_hartid, &mt->hart_count);
-	if (rc)
+	if (rc) {
+		sbi_free(mtn);
 		return rc;
+	}
 	mt->has_64bit_mmio = true;
 	mt->has_shared_mtime = false;
 
 	rc = fdt_parse_timebase_frequency(fdt, &mt->mtime_freq);
-	if (rc)
+	if (rc) {
+		sbi_free(mtn);
 		return rc;
+	}
 
 	if (match->data) { /* SiFive CLINT */
 		const struct timer_mtimer_quirks *quirks = match->data;
@@ -77,8 +87,8 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
 	/* Check if MTIMER device has shared MTIME address */
 	if (mt->mtime_size) {
 		mt->has_shared_mtime = false;
-		for (i = 0; i < mtimer_count; i++) {
-			if (mtimer[i].mtime_addr == mt->mtime_addr) {
+		sbi_list_for_each_entry(n, &mtn_list, head) {
+			if (n->data.mtime_addr == mt->mtime_addr) {
 				mt->has_shared_mtime = true;
 				break;
 			}
@@ -90,8 +100,10 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
 
 	/* Initialize the MTIMER device */
 	rc = aclint_mtimer_cold_init(mt, mt_reference);
-	if (rc)
+	if (rc) {
+		sbi_free(mtn);
 		return rc;
+	}
 
 	/*
 	 * Select first MTIMER device with no associated HARTs as our
@@ -106,16 +118,17 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
 		 * Set reference for already propbed MTIMER devices
 		 * with non-shared MTIME
 		 */
-		for (i = 0; i < mtimer_count; i++)
-			if (!mtimer[i].has_shared_mtime)
-				aclint_mtimer_set_reference(&mtimer[i], mt);
+		sbi_list_for_each_entry(n, &mtn_list, head) {
+			if (!n->data.has_shared_mtime)
+				aclint_mtimer_set_reference(&n->data, mt);
+		}
 	}
 
 	/* Explicitly sync-up MTIMER devices not associated with any HARTs */
 	if (!mt->hart_count)
 		aclint_mtimer_sync(mt);
 
-	mtimer_count++;
+	sbi_list_add_tail(&mtn->head, &mtn_list);
 	return 0;
 }
 
-- 
2.34.1

[PATCH 12/17] lib: utils/fdt: Use heap in FDT domain parsing

[Anup Patel]

Let's use heap allocation in FDT domain parsing instead of using
a fixed size global array.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/utils/fdt/fdt_domain.c | 111 +++++++++++++++++++++++--------------
 1 file changed, 68 insertions(+), 43 deletions(-)

diff --git a/lib/utils/fdt/fdt_domain.c b/lib/utils/fdt/fdt_domain.c
index bb6d17d..c4aed4d 100644
--- a/lib/utils/fdt/fdt_domain.c
+++ b/lib/utils/fdt/fdt_domain.c
@@ -13,6 +13,7 @@
 #include <sbi/sbi_domain.h>
 #include <sbi/sbi_error.h>
 #include <sbi/sbi_hartmask.h>
+#include <sbi/sbi_heap.h>
 #include <sbi/sbi_scratch.h>
 #include <sbi_utils/fdt/fdt_domain.h>
 #include <sbi_utils/fdt/fdt_helper.h>
@@ -219,14 +220,11 @@ skip_device_disable:
 	fdt_nop_node(fdt, poffset);
 }
 
-#define FDT_DOMAIN_MAX_COUNT		8
-#define FDT_DOMAIN_REGION_MAX_COUNT	16
-
-static u32 fdt_domains_count;
-static struct sbi_domain fdt_domains[FDT_DOMAIN_MAX_COUNT];
-static struct sbi_hartmask fdt_masks[FDT_DOMAIN_MAX_COUNT];
-static struct sbi_domain_memregion
-	fdt_regions[FDT_DOMAIN_MAX_COUNT][FDT_DOMAIN_REGION_MAX_COUNT + 1];
+struct parse_region_data {
+	struct sbi_domain *dom;
+	u32 region_count;
+	u32 max_regions;
+};
 
 static int __fdt_parse_region(void *fdt, int domain_offset,
 			      int region_offset, u32 region_access,
@@ -236,7 +234,7 @@ static int __fdt_parse_region(void *fdt, int domain_offset,
 	u32 val32;
 	u64 val64;
 	const u32 *val;
-	u32 *region_count = opaque;
+	struct parse_region_data *preg = opaque;
 	struct sbi_domain_memregion *region;
 
 	/*
@@ -252,9 +250,9 @@ static int __fdt_parse_region(void *fdt, int domain_offset,
 		return SBI_EINVAL;
 
 	/* Find next region of the domain */
-	if (FDT_DOMAIN_REGION_MAX_COUNT <= *region_count)
-		return SBI_EINVAL;
-	region = &fdt_regions[fdt_domains_count][*region_count];
+	if (preg->max_regions <= preg->region_count)
+		return SBI_ENOSPC;
+	region = &preg->dom->regions[preg->region_count];
 
 	/* Read "base" DT property */
 	val = fdt_getprop(fdt, region_offset, "base", &len);
@@ -278,7 +276,7 @@ static int __fdt_parse_region(void *fdt, int domain_offset,
 	if (fdt_get_property(fdt, region_offset, "mmio", NULL))
 		region->flags |= SBI_DOMAIN_MEMREGION_MMIO;
 
-	(*region_count)++;
+	preg->region_count++;
 
 	return 0;
 }
@@ -291,16 +289,30 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
 	struct sbi_domain *dom;
 	struct sbi_hartmask *mask;
 	struct sbi_hartmask assign_mask;
+	struct parse_region_data preg;
 	int *cold_domain_offset = opaque;
-	struct sbi_domain_memregion *reg, *regions;
-	int i, err, len, cpus_offset, cpu_offset, doffset;
+	struct sbi_domain_memregion *reg;
+	int i, err = 0, len, cpus_offset, cpu_offset, doffset;
 
-	/* Sanity check on maximum domains we can handle */
-	if (FDT_DOMAIN_MAX_COUNT <= fdt_domains_count)
-		return SBI_EINVAL;
-	dom = &fdt_domains[fdt_domains_count];
-	mask = &fdt_masks[fdt_domains_count];
-	regions = &fdt_regions[fdt_domains_count][0];
+	dom = sbi_zalloc(sizeof(*dom));
+	if (!dom)
+		return SBI_ENOMEM;
+
+	dom->regions = sbi_calloc(sizeof(*dom->regions), 16 + 1);
+	if (!dom->regions) {
+		sbi_free(dom);
+		return SBI_ENOMEM;
+	}
+	preg.dom = dom;
+	preg.region_count = 0;
+	preg.max_regions = 16;
+
+	mask = sbi_zalloc(sizeof(*mask));
+	if (!mask) {
+		sbi_free(dom->regions);
+		sbi_free(dom);
+		return SBI_ENOMEM;
+	}
 
 	/* Read DT node name */
 	strncpy(dom->name, fdt_get_name(fdt, domain_offset, NULL),
@@ -316,12 +328,14 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
 		for (i = 0; i < len; i++) {
 			cpu_offset = fdt_node_offset_by_phandle(fdt,
 							fdt32_to_cpu(val[i]));
-			if (cpu_offset < 0)
-				return cpu_offset;
+			if (cpu_offset < 0) {
+				err = cpu_offset;
+				goto fail_free_all;
+			}
 
 			err = fdt_parse_hart_id(fdt, cpu_offset, &val32);
 			if (err)
-				return err;
+				goto fail_free_all;
 
 			if (!fdt_node_is_enabled(fdt, cpu_offset))
 				continue;
@@ -331,14 +345,10 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
 	}
 
 	/* Setup memregions from DT */
-	val32 = 0;
-	memset(regions, 0,
-		   sizeof(*regions) * (FDT_DOMAIN_REGION_MAX_COUNT + 1));
-	dom->regions = regions;
-	err = fdt_iterate_each_memregion(fdt, domain_offset, &val32,
+	err = fdt_iterate_each_memregion(fdt, domain_offset, &preg,
 					 __fdt_parse_region);
 	if (err)
-		return err;
+		goto fail_free_all;
 
 	/*
 	 * Copy over root domain memregions which don't allow
@@ -354,9 +364,11 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
 		    (reg->flags & SBI_DOMAIN_MEMREGION_SU_WRITABLE) ||
 		    (reg->flags & SBI_DOMAIN_MEMREGION_SU_EXECUTABLE))
 			continue;
-		if (FDT_DOMAIN_REGION_MAX_COUNT <= val32)
-			return SBI_EINVAL;
-		memcpy(&regions[val32++], reg, sizeof(*reg));
+		if (preg.max_regions <= preg.region_count) {
+			err = SBI_EINVAL;
+			goto fail_free_all;
+		}
+		memcpy(&dom->regions[preg.region_count++], reg, sizeof(*reg));
 	}
 	dom->fw_region_inited = root.fw_region_inited;
 
@@ -427,8 +439,10 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
 
 	/* Find /cpus DT node */
 	cpus_offset = fdt_path_offset(fdt, "/cpus");
-	if (cpus_offset < 0)
-		return cpus_offset;
+	if (cpus_offset < 0) {
+		err = cpus_offset;
+		goto fail_free_all;
+	}
 
 	/* HART to domain assignment mask based on CPU DT nodes */
 	sbi_hartmask_clear_all(&assign_mask);
@@ -444,22 +458,33 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
 			continue;
 
 		val = fdt_getprop(fdt, cpu_offset, "opensbi-domain", &len);
-		if (!val || len < 4)
-			return SBI_EINVAL;
+		if (!val || len < 4) {
+			err = SBI_EINVAL;
+			goto fail_free_all;
+		}
 
 		doffset = fdt_node_offset_by_phandle(fdt, fdt32_to_cpu(*val));
-		if (doffset < 0)
-			return doffset;
+		if (doffset < 0) {
+			err = doffset;
+			goto fail_free_all;
+		}
 
 		if (doffset == domain_offset)
 			sbi_hartmask_set_hart(val32, &assign_mask);
 	}
 
-	/* Increment domains count */
-	fdt_domains_count++;
-
 	/* Register the domain */
-	return sbi_domain_register(dom, &assign_mask);
+	err = sbi_domain_register(dom, &assign_mask);
+	if (err)
+		goto fail_free_all;
+
+	return 0;
+
+fail_free_all:
+	sbi_free(dom->regions);
+	sbi_free(mask);
+	sbi_free(dom);
+	return err;
 }
 
 int fdt_domains_populate(void *fdt)
-- 
2.34.1

[PATCH 13/17] lib: utils/ipi: Use scratch space to save per-HART MSWI pointer

[Anup Patel]

Instead of using a global array indexed by hartid, we should use
scratch space to save per-HART MSWI pointer.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/utils/ipi/aclint_mswi.c | 49 +++++++++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/lib/utils/ipi/aclint_mswi.c b/lib/utils/ipi/aclint_mswi.c
index 6b004cb..a8a2872 100644
--- a/lib/utils/ipi/aclint_mswi.c
+++ b/lib/utils/ipi/aclint_mswi.c
@@ -12,21 +12,36 @@
 #include <sbi/riscv_io.h>
 #include <sbi/sbi_domain.h>
 #include <sbi/sbi_error.h>
-#include <sbi/sbi_hartmask.h>
 #include <sbi/sbi_ipi.h>
+#include <sbi/sbi_scratch.h>
 #include <sbi/sbi_timer.h>
 #include <sbi_utils/ipi/aclint_mswi.h>
 
-static struct aclint_mswi_data *mswi_hartid2data[SBI_HARTMASK_MAX_BITS];
+static unsigned long mswi_ptr_offset;
+
+#define mswi_get_hart_data_ptr(__scratch)				\
+({									\
+	(void *)(*((ulong *)sbi_scratch_offset_ptr((__scratch),		\
+						   mswi_ptr_offset)));	\
+})
+
+#define mswi_set_hart_data_ptr(__scratch, __mswi)			\
+do {									\
+	*((ulong *)sbi_scratch_offset_ptr((__scratch), mswi_ptr_offset))\
+					= (ulong)(__mswi);		\
+} while (0)
 
 static void mswi_ipi_send(u32 target_hart)
 {
 	u32 *msip;
+	struct sbi_scratch *scratch;
 	struct aclint_mswi_data *mswi;
 
-	if (SBI_HARTMASK_MAX_BITS <= target_hart)
+	scratch = sbi_hartid_to_scratch(target_hart);
+	if (!scratch)
 		return;
-	mswi = mswi_hartid2data[target_hart];
+
+	mswi = mswi_get_hart_data_ptr(scratch);
 	if (!mswi)
 		return;
 
@@ -38,11 +53,14 @@ static void mswi_ipi_send(u32 target_hart)
 static void mswi_ipi_clear(u32 target_hart)
 {
 	u32 *msip;
+	struct sbi_scratch *scratch;
 	struct aclint_mswi_data *mswi;
 
-	if (SBI_HARTMASK_MAX_BITS <= target_hart)
+	scratch = sbi_hartid_to_scratch(target_hart);
+	if (!scratch)
 		return;
-	mswi = mswi_hartid2data[target_hart];
+
+	mswi = mswi_get_hart_data_ptr(scratch);
 	if (!mswi)
 		return;
 
@@ -69,19 +87,30 @@ int aclint_mswi_cold_init(struct aclint_mswi_data *mswi)
 {
 	u32 i;
 	int rc;
+	struct sbi_scratch *scratch;
 	unsigned long pos, region_size;
 	struct sbi_domain_memregion reg;
 
 	/* Sanity checks */
 	if (!mswi || (mswi->addr & (ACLINT_MSWI_ALIGN - 1)) ||
 	    (mswi->size < (mswi->hart_count * sizeof(u32))) ||
-	    (mswi->first_hartid >= SBI_HARTMASK_MAX_BITS) ||
 	    (mswi->hart_count > ACLINT_MSWI_MAX_HARTS))
 		return SBI_EINVAL;
 
-	/* Update MSWI hartid table */
-	for (i = 0; i < mswi->hart_count; i++)
-		mswi_hartid2data[mswi->first_hartid + i] = mswi;
+	/* Allocate scratch space pointer */
+	if (!mswi_ptr_offset) {
+		mswi_ptr_offset = sbi_scratch_alloc_offset(sizeof(ulong));
+		if (!mswi_ptr_offset)
+			return SBI_ENOMEM;
+	}
+
+	/* Update MSWI pointer in scratch space */
+	for (i = 0; i < mswi->hart_count; i++) {
+		scratch = sbi_hartid_to_scratch(mswi->first_hartid + i);
+		if (!scratch)
+			return SBI_ENOENT;
+		mswi_set_hart_data_ptr(scratch, mswi);
+	}
 
 	/* Add MSWI regions to the root domain */
 	for (pos = 0; pos < mswi->size; pos += ACLINT_MSWI_ALIGN) {
-- 
2.34.1

[PATCH 14/17] lib: utils/timer: Use scratch space to save per-HART MTIMER pointer

[Anup Patel]

Instead of using a global array indexed by hartid, we should use
scratch space to save per-HART MTIMER pointer.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/utils/timer/aclint_mtimer.c | 82 +++++++++++++++++++++++++++------
 1 file changed, 67 insertions(+), 15 deletions(-)

diff --git a/lib/utils/timer/aclint_mtimer.c b/lib/utils/timer/aclint_mtimer.c
index 84ded4e..c75f790 100644
--- a/lib/utils/timer/aclint_mtimer.c
+++ b/lib/utils/timer/aclint_mtimer.c
@@ -13,12 +13,23 @@
 #include <sbi/sbi_bitops.h>
 #include <sbi/sbi_domain.h>
 #include <sbi/sbi_error.h>
-#include <sbi/sbi_hartmask.h>
-#include <sbi/sbi_ipi.h>
+#include <sbi/sbi_scratch.h>
 #include <sbi/sbi_timer.h>
 #include <sbi_utils/timer/aclint_mtimer.h>
 
-static struct aclint_mtimer_data *mtimer_hartid2data[SBI_HARTMASK_MAX_BITS];
+static unsigned long mtimer_ptr_offset;
+
+#define mtimer_get_hart_data_ptr(__scratch)				\
+({									\
+	(void *)(*((ulong *)sbi_scratch_offset_ptr((__scratch),		\
+						   mtimer_ptr_offset)));\
+})
+
+#define mtimer_set_hart_data_ptr(__scratch, __mtimer)			\
+do {									\
+	*((ulong *)sbi_scratch_offset_ptr((__scratch), mtimer_ptr_offset))\
+					= (ulong)(__mtimer);		\
+} while (0)
 
 #if __riscv_xlen != 32
 static u64 mtimer_time_rd64(volatile u64 *addr)
@@ -53,30 +64,54 @@ static void mtimer_time_wr32(bool timecmp, u64 value, volatile u64 *addr)
 
 static u64 mtimer_value(void)
 {
-	struct aclint_mtimer_data *mt = mtimer_hartid2data[current_hartid()];
-	u64 *time_val = (void *)mt->mtime_addr;
+	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
+	struct aclint_mtimer_data *mt;
+
+	mt = mtimer_get_hart_data_ptr(scratch);
+	if (!mt)
+		return 0;
 
 	/* Read MTIMER Time Value */
-	return mt->time_rd(time_val);
+	return mt->time_rd((void *)mt->mtime_addr);
 }
 
 static void mtimer_event_stop(void)
 {
 	u32 target_hart = current_hartid();
-	struct aclint_mtimer_data *mt = mtimer_hartid2data[target_hart];
-	u64 *time_cmp = (void *)mt->mtimecmp_addr;
+	struct sbi_scratch *scratch;
+	struct aclint_mtimer_data *mt;
+	u64 *time_cmp;
+
+	scratch = sbi_hartid_to_scratch(target_hart);
+	if (!scratch)
+		return;
+
+	mt = mtimer_get_hart_data_ptr(scratch);
+	if (!mt)
+		return;
 
 	/* Clear MTIMER Time Compare */
+	time_cmp = (void *)mt->mtimecmp_addr;
 	mt->time_wr(true, -1ULL, &time_cmp[target_hart - mt->first_hartid]);
 }
 
 static void mtimer_event_start(u64 next_event)
 {
 	u32 target_hart = current_hartid();
-	struct aclint_mtimer_data *mt = mtimer_hartid2data[target_hart];
-	u64 *time_cmp = (void *)mt->mtimecmp_addr;
+	struct sbi_scratch *scratch;
+	struct aclint_mtimer_data *mt;
+	u64 *time_cmp;
+
+	scratch = sbi_hartid_to_scratch(target_hart);
+	if (!scratch)
+		return;
+
+	mt = mtimer_get_hart_data_ptr(scratch);
+	if (!mt)
+		return;
 
 	/* Program MTIMER Time Compare */
+	time_cmp = (void *)mt->mtimecmp_addr;
 	mt->time_wr(true, next_event,
 		    &time_cmp[target_hart - mt->first_hartid]);
 }
@@ -126,8 +161,14 @@ int aclint_mtimer_warm_init(void)
 {
 	u64 *mt_time_cmp;
 	u32 target_hart = current_hartid();
-	struct aclint_mtimer_data *mt = mtimer_hartid2data[target_hart];
+	struct sbi_scratch *scratch;
+	struct aclint_mtimer_data *mt;
 
+	scratch = sbi_hartid_to_scratch(target_hart);
+	if (!scratch)
+		return SBI_ENOENT;
+
+	mt = mtimer_get_hart_data_ptr(scratch);
 	if (!mt)
 		return SBI_ENODEV;
 
@@ -147,6 +188,7 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
 {
 	u32 i;
 	int rc;
+	struct sbi_scratch *scratch;
 
 	/* Sanity checks */
 	if (!mt ||
@@ -155,12 +197,18 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
 	    (mt->mtime_size && (mt->mtime_size & (ACLINT_MTIMER_ALIGN - 1))) ||
 	    (mt->mtimecmp_addr & (ACLINT_MTIMER_ALIGN - 1)) ||
 	    (mt->mtimecmp_size & (ACLINT_MTIMER_ALIGN - 1)) ||
-	    (mt->first_hartid >= SBI_HARTMASK_MAX_BITS) ||
 	    (mt->hart_count > ACLINT_MTIMER_MAX_HARTS))
 		return SBI_EINVAL;
 	if (reference && mt->mtime_freq != reference->mtime_freq)
 		return SBI_EINVAL;
 
+	/* Allocate scratch space pointer */
+	if (!mtimer_ptr_offset) {
+		mtimer_ptr_offset = sbi_scratch_alloc_offset(sizeof(ulong));
+		if (!mtimer_ptr_offset)
+			return SBI_ENOMEM;
+	}
+
 	/* Initialize private data */
 	aclint_mtimer_set_reference(mt, reference);
 	mt->time_rd = mtimer_time_rd32;
@@ -174,9 +222,13 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
 	}
 #endif
 
-	/* Update MTIMER hartid table */
-	for (i = 0; i < mt->hart_count; i++)
-		mtimer_hartid2data[mt->first_hartid + i] = mt;
+	/* Update MTIMER pointer in scratch space */
+	for (i = 0; i < mt->hart_count; i++) {
+		scratch = sbi_hartid_to_scratch(mt->first_hartid + i);
+		if (!scratch)
+			return SBI_ENOENT;
+		mtimer_set_hart_data_ptr(scratch, mt);
+	}
 
 	if (!mt->mtime_size) {
 		/* Disable reading mtime when mtime is not available */
-- 
2.34.1

[PATCH 15/17] lib: utils/irqchip: Use scratch space to save per-HART PLIC pointer

[Anup Patel]

Instead of using a global array indexed by hartid, we should use
scratch space to save per-HART PLIC pointer and PLIC context numbers.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/utils/irqchip/fdt_irqchip_plic.c | 108 +++++++++++++++++++++------
 1 file changed, 86 insertions(+), 22 deletions(-)

diff --git a/lib/utils/irqchip/fdt_irqchip_plic.c b/lib/utils/irqchip/fdt_irqchip_plic.c
index 605f44a..eb1e412 100644
--- a/lib/utils/irqchip/fdt_irqchip_plic.c
+++ b/lib/utils/irqchip/fdt_irqchip_plic.c
@@ -12,54 +12,97 @@
 #include <sbi/riscv_io.h>
 #include <sbi/sbi_error.h>
 #include <sbi/sbi_heap.h>
-#include <sbi/sbi_hartmask.h>
+#include <sbi/sbi_scratch.h>
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/irqchip/fdt_irqchip.h>
 #include <sbi_utils/irqchip/plic.h>
 
-static struct plic_data *plic_hartid2data[SBI_HARTMASK_MAX_BITS];
-static int plic_hartid2context[SBI_HARTMASK_MAX_BITS][2] = { { -1 } };
+static unsigned long plic_ptr_offset;
+
+#define plic_get_hart_data_ptr(__scratch)				\
+({									\
+	(void *)(*((ulong *)sbi_scratch_offset_ptr((__scratch),		\
+						   plic_ptr_offset)));	\
+})
+
+#define plic_set_hart_data_ptr(__scratch, __imsic)			\
+do {									\
+	*((ulong *)sbi_scratch_offset_ptr((__scratch), plic_ptr_offset))\
+					= (ulong)(__imsic);		\
+} while (0)
+
+static unsigned long plic_mcontext_offset;
+
+#define plic_get_hart_mcontext(__scratch)				\
+({									\
+	long __ret =							\
+	*((long *)sbi_scratch_offset_ptr((__scratch), plic_mcontext_offset));\
+	(__ret - 1);							\
+})
+
+#define plic_set_hart_mcontext(__scratch, __mcontext)			\
+do {									\
+	*((long *)sbi_scratch_offset_ptr((__scratch), plic_mcontext_offset))\
+					= (long)(__mcontext + 1);	\
+} while (0)
+
+static unsigned long plic_scontext_offset;
+
+#define plic_get_hart_scontext(__scratch)				\
+({									\
+	long __ret =							\
+	*((long *)sbi_scratch_offset_ptr((__scratch), plic_scontext_offset));\
+	(__ret - 1);							\
+})
+
+#define plic_set_hart_scontext(__scratch, __scontext)			\
+do {									\
+	*((long *)sbi_scratch_offset_ptr((__scratch), plic_scontext_offset))\
+					= (long)(__scontext + 1);	\
+} while (0)
 
 void fdt_plic_priority_save(u8 *priority, u32 num)
 {
-	struct plic_data *plic = plic_hartid2data[current_hartid()];
+	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
 
-	plic_priority_save(plic, priority, num);
+	plic_priority_save(plic_get_hart_data_ptr(scratch), priority, num);
 }
 
 void fdt_plic_priority_restore(const u8 *priority, u32 num)
 {
-	struct plic_data *plic = plic_hartid2data[current_hartid()];
+	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
 
-	plic_priority_restore(plic, priority, num);
+	plic_priority_restore(plic_get_hart_data_ptr(scratch), priority, num);
 }
 
 void fdt_plic_context_save(bool smode, u32 *enable, u32 *threshold, u32 num)
 {
-	u32 hartid = current_hartid();
+	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
 
-	plic_context_save(plic_hartid2data[hartid],
-			  plic_hartid2context[hartid][smode],
+	plic_context_save(plic_get_hart_data_ptr(scratch),
+			  smode ? plic_get_hart_scontext(scratch) :
+				  plic_get_hart_mcontext(scratch),
 			  enable, threshold, num);
 }
 
 void fdt_plic_context_restore(bool smode, const u32 *enable, u32 threshold,
 			      u32 num)
 {
-	u32 hartid = current_hartid();
+	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
 
-	plic_context_restore(plic_hartid2data[hartid],
-			     plic_hartid2context[hartid][smode],
+	plic_context_restore(plic_get_hart_data_ptr(scratch),
+			     smode ? plic_get_hart_scontext(scratch) :
+				     plic_get_hart_mcontext(scratch),
 			     enable, threshold, num);
 }
 
 static int irqchip_plic_warm_init(void)
 {
-	u32 hartid = current_hartid();
+	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
 
-	return plic_warm_irqchip_init(plic_hartid2data[hartid],
-				      plic_hartid2context[hartid][0],
-				      plic_hartid2context[hartid][1]);
+	return plic_warm_irqchip_init(plic_get_hart_data_ptr(scratch),
+				      plic_get_hart_mcontext(scratch),
+				      plic_get_hart_scontext(scratch));
 }
 
 static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
@@ -67,6 +110,7 @@ static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
 {
 	const fdt32_t *val;
 	u32 phandle, hwirq, hartid;
+	struct sbi_scratch *scratch;
 	int i, err, count, cpu_offset, cpu_intc_offset;
 
 	val = fdt_getprop(fdt, nodeoff, "interrupts-extended", &count);
@@ -90,16 +134,17 @@ static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
 		if (err)
 			continue;
 
-		if (SBI_HARTMASK_MAX_BITS <= hartid)
+		scratch = sbi_hartid_to_scratch(hartid);
+		if (!scratch)
 			continue;
 
-		plic_hartid2data[hartid] = pd;
+		plic_set_hart_data_ptr(scratch, pd);
 		switch (hwirq) {
 		case IRQ_M_EXT:
-			plic_hartid2context[hartid][0] = i / 2;
+			plic_set_hart_mcontext(scratch, i / 2);
 			break;
 		case IRQ_S_EXT:
-			plic_hartid2context[hartid][1] = i / 2;
+			plic_set_hart_scontext(scratch, i / 2);
 			break;
 		}
 	}
@@ -113,6 +158,24 @@ static int irqchip_plic_cold_init(void *fdt, int nodeoff,
 	int rc;
 	struct plic_data *pd;
 
+	if (!plic_ptr_offset) {
+		plic_ptr_offset = sbi_scratch_alloc_offset(sizeof(ulong));
+		if (!plic_ptr_offset)
+			return SBI_ENOMEM;
+	}
+
+	if (!plic_mcontext_offset) {
+		plic_mcontext_offset = sbi_scratch_alloc_offset(sizeof(long));
+		if (!plic_mcontext_offset)
+			return SBI_ENOMEM;
+	}
+
+	if (!plic_scontext_offset) {
+		plic_scontext_offset = sbi_scratch_alloc_offset(sizeof(long));
+		if (!plic_scontext_offset)
+			return SBI_ENOMEM;
+	}
+
 	pd = sbi_zalloc(sizeof(*pd));
 	if (!pd)
 		return SBI_ENOMEM;
@@ -152,7 +215,8 @@ static void thead_plic_plat_init(struct plic_data *pd)
 
 void thead_plic_restore(void)
 {
-	struct plic_data *plic = plic_hartid2data[current_hartid()];
+	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
+	struct plic_data *plic = plic_get_hart_data_ptr(scratch);
 
 	thead_plic_plat_init(plic);
 }
-- 
2.34.1

[PATCH 16/17] lib: utils/irqchip: Don't check hartid in imsic_update_hartid_table()

[Anup Patel]

The imsic_map_hartid_to_data() already checks hartid before using
so we don't need to check in imsic_update_hartid_table().

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/utils/irqchip/fdt_irqchip_imsic.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lib/utils/irqchip/fdt_irqchip_imsic.c b/lib/utils/irqchip/fdt_irqchip_imsic.c
index d890400..574cadc 100644
--- a/lib/utils/irqchip/fdt_irqchip_imsic.c
+++ b/lib/utils/irqchip/fdt_irqchip_imsic.c
@@ -12,7 +12,6 @@
 #include <sbi/riscv_asm.h>
 #include <sbi/sbi_error.h>
 #include <sbi/sbi_heap.h>
-#include <sbi/sbi_hartmask.h>
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/irqchip/fdt_irqchip.h>
 #include <sbi_utils/irqchip/imsic.h>
@@ -44,8 +43,6 @@ static int irqchip_imsic_update_hartid_table(void *fdt, int nodeoff,
 		err = fdt_parse_hart_id(fdt, cpu_offset, &hartid);
 		if (err)
 			return SBI_EINVAL;
-		if (SBI_HARTMASK_MAX_BITS <= hartid)
-			return SBI_EINVAL;
 
 		switch (hwirq) {
 		case IRQ_M_EXT:
-- 
2.34.1

[PATCH 17/17] lib: utils/irqchip: Use scratch space to save per-HART IMSIC pointer

[Anup Patel]

Instead of using a global array indexed by hartid, we should use
scratch space to save per-HART IMSIC pointer and IMSIC file number.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/utils/irqchip/fdt_irqchip_imsic.c |  4 +-
 lib/utils/irqchip/imsic.c             | 87 ++++++++++++++++++++++-----
 2 files changed, 74 insertions(+), 17 deletions(-)

diff --git a/lib/utils/irqchip/fdt_irqchip_imsic.c b/lib/utils/irqchip/fdt_irqchip_imsic.c
index 574cadc..20ee0d8 100644
--- a/lib/utils/irqchip/fdt_irqchip_imsic.c
+++ b/lib/utils/irqchip/fdt_irqchip_imsic.c
@@ -78,13 +78,13 @@ static int irqchip_imsic_cold_init(void *fdt, int nodeoff,
 		return 0;
 	}
 
-	rc = irqchip_imsic_update_hartid_table(fdt, nodeoff, id);
+	rc = imsic_cold_irqchip_init(id);
 	if (rc) {
 		sbi_free(id);
 		return rc;
 	}
 
-	rc = imsic_cold_irqchip_init(id);
+	rc = irqchip_imsic_update_hartid_table(fdt, nodeoff, id);
 	if (rc) {
 		sbi_free(id);
 		return rc;
diff --git a/lib/utils/irqchip/imsic.c b/lib/utils/irqchip/imsic.c
index 11667cd..2b3e9da 100644
--- a/lib/utils/irqchip/imsic.c
+++ b/lib/utils/irqchip/imsic.c
@@ -13,10 +13,10 @@
 #include <sbi/riscv_encoding.h>
 #include <sbi/sbi_console.h>
 #include <sbi/sbi_domain.h>
-#include <sbi/sbi_hartmask.h>
 #include <sbi/sbi_ipi.h>
 #include <sbi/sbi_irqchip.h>
 #include <sbi/sbi_error.h>
+#include <sbi/sbi_scratch.h>
 #include <sbi_utils/irqchip/imsic.h>
 
 #define IMSIC_MMIO_PAGE_LE		0x00
@@ -79,33 +79,69 @@ do { \
 	csr_clear(CSR_MIREG, __v); \
 } while (0)
 
-static struct imsic_data *imsic_hartid2data[SBI_HARTMASK_MAX_BITS];
-static int imsic_hartid2file[SBI_HARTMASK_MAX_BITS];
+static unsigned long imsic_ptr_offset;
+
+#define imsic_get_hart_data_ptr(__scratch)				\
+({									\
+	(void *)(*((ulong *)sbi_scratch_offset_ptr((__scratch),		\
+						   imsic_ptr_offset)));\
+})
+
+#define imsic_set_hart_data_ptr(__scratch, __imsic)			\
+do {									\
+	*((ulong *)sbi_scratch_offset_ptr((__scratch), imsic_ptr_offset))\
+					= (ulong)(__imsic);		\
+} while (0)
+
+static unsigned long imsic_file_offset;
+
+#define imsic_get_hart_file(__scratch)					\
+({									\
+	*((long *)sbi_scratch_offset_ptr((__scratch), imsic_file_offset));\
+})
+
+#define imsic_set_hart_file(__scratch, __file)				\
+do {									\
+	*((long *)sbi_scratch_offset_ptr((__scratch), imsic_file_offset))\
+					= (long)(__file);		\
+} while (0)
 
 int imsic_map_hartid_to_data(u32 hartid, struct imsic_data *imsic, int file)
 {
-	if (!imsic || !imsic->targets_mmode ||
-	    (SBI_HARTMASK_MAX_BITS <= hartid))
+	struct sbi_scratch *scratch;
+
+	if (!imsic || !imsic->targets_mmode)
 		return SBI_EINVAL;
 
-	imsic_hartid2data[hartid] = imsic;
-	imsic_hartid2file[hartid] = file;
+	scratch = sbi_hartid_to_scratch(hartid);
+	if (!scratch)
+		return SBI_ENOENT;
+
+	imsic_set_hart_data_ptr(scratch, imsic);
+	imsic_set_hart_file(scratch, file);
 	return 0;
 }
 
 struct imsic_data *imsic_get_data(u32 hartid)
 {
-	if (SBI_HARTMASK_MAX_BITS <= hartid)
+	struct sbi_scratch *scratch;
+
+	scratch = sbi_hartid_to_scratch(hartid);
+	if (!scratch)
 		return NULL;
-	return imsic_hartid2data[hartid];
+
+	return imsic_get_hart_data_ptr(scratch);
 }
 
 int imsic_get_target_file(u32 hartid)
 {
-	if ((SBI_HARTMASK_MAX_BITS <= hartid) ||
-	    !imsic_hartid2data[hartid])
+	struct sbi_scratch *scratch;
+
+	scratch = sbi_hartid_to_scratch(hartid);
+	if (!scratch)
 		return SBI_ENOENT;
-	return imsic_hartid2file[hartid];
+
+	return imsic_get_hart_file(scratch);
 }
 
 static int imsic_external_irqfn(struct sbi_trap_regs *regs)
@@ -133,9 +169,16 @@ static void imsic_ipi_send(u32 target_hart)
 {
 	unsigned long reloff;
 	struct imsic_regs *regs;
-	struct imsic_data *data = imsic_hartid2data[target_hart];
-	int file = imsic_hartid2file[target_hart];
+	struct imsic_data *data;
+	struct sbi_scratch *scratch;
+	int file;
 
+	scratch = sbi_hartid_to_scratch(target_hart);
+	if (!scratch)
+		return;
+
+	data = imsic_get_hart_data_ptr(scratch);
+	file = imsic_get_hart_file(scratch);
 	if (!data || !data->targets_mmode)
 		return;
 
@@ -204,7 +247,7 @@ void imsic_local_irqchip_init(void)
 
 int imsic_warm_irqchip_init(void)
 {
-	struct imsic_data *imsic = imsic_hartid2data[current_hartid()];
+	struct imsic_data *imsic = imsic_get_data(current_hartid());
 
 	/* Sanity checks */
 	if (!imsic || !imsic->targets_mmode)
@@ -306,6 +349,20 @@ int imsic_cold_irqchip_init(struct imsic_data *imsic)
 	if (!imsic->targets_mmode)
 		return SBI_EINVAL;
 
+	/* Allocate scratch space pointer */
+	if (!imsic_ptr_offset) {
+		imsic_ptr_offset = sbi_scratch_alloc_offset(sizeof(ulong));
+		if (!imsic_ptr_offset)
+			return SBI_ENOMEM;
+	}
+
+	/* Allocate scratch space file */
+	if (!imsic_file_offset) {
+		imsic_file_offset = sbi_scratch_alloc_offset(sizeof(long));
+		if (!imsic_file_offset)
+			return SBI_ENOMEM;
+	}
+
 	/* Setup external interrupt function for IMSIC */
 	sbi_irqchip_set_irqfn(imsic_external_irqfn);
 
-- 
2.34.1

[PATCH 01/17] platform: Allow platforms to specify heap size

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:14PM +0530, Anup Patel wrote:
> We extend struct sbi_platform and struct sbi_scratch to allow platforms
> specify the heap size to the OpenSBI firmwares. The OpenSBI firmwares
> will use this information to determine the location of heap and provide
> heap base address in per-HART scratch space.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  firmware/fw_base.S                 | 15 +++++++++++++++
>  include/sbi/sbi_platform.h         | 18 +++++++++++++++---
>  include/sbi/sbi_scratch.h          | 28 ++++++++++++++++++----------
>  platform/fpga/ariane/platform.c    |  1 +
>  platform/fpga/openpiton/platform.c |  2 ++
>  platform/generic/platform.c        |  3 ++-
>  platform/kendryte/k210/platform.c  |  2 ++
>  platform/nuclei/ux600/platform.c   |  2 ++
>  platform/template/platform.c       |  1 +
>  9 files changed, 58 insertions(+), 14 deletions(-)
> 
> diff --git a/firmware/fw_base.S b/firmware/fw_base.S
> index e37df09..fff09e1 100644
> --- a/firmware/fw_base.S
> +++ b/firmware/fw_base.S
> @@ -255,20 +255,28 @@ _bss_zero:
>  	/* Preload HART details
>  	 * s7 -> HART Count
>  	 * s8 -> HART Stack Size
> +	 * s9 -> Heap Size
> +	 * s10 -> Heap Offset
>  	 */
>  	lla	a4, platform
>  #if __riscv_xlen > 32
>  	lwu	s7, SBI_PLATFORM_HART_COUNT_OFFSET(a4)
>  	lwu	s8, SBI_PLATFORM_HART_STACK_SIZE_OFFSET(a4)
> +	lwu	s9, SBI_PLATFORM_HEAP_SIZE_OFFSET(a4)
>  #else
>  	lw	s7, SBI_PLATFORM_HART_COUNT_OFFSET(a4)
>  	lw	s8, SBI_PLATFORM_HART_STACK_SIZE_OFFSET(a4)
> +	lw	s9, SBI_PLATFORM_HEAP_SIZE_OFFSET(a4)
>  #endif
>  
>  	/* Setup scratch space for all the HARTs*/
>  	lla	tp, _fw_end
>  	mul	a5, s7, s8
>  	add	tp, tp, a5
> +	/* Setup heap base address */
> +	lla	s10, _fw_start
> +	sub	s10, tp, s10
> +	add	tp, tp, s9
>  	/* Keep a copy of tp */
>  	add	t3, tp, zero
>  	/* Counter */
> @@ -283,8 +291,11 @@ _scratch_init:
>  	 * t3 -> the firmware end address
>  	 * s7 -> HART count
>  	 * s8 -> HART stack size
> +	 * s9 -> Heap Size
> +	 * s10 -> Heap Offset
>  	 */
>  	add	tp, t3, zero
> +	sub	tp, tp, s9
>  	mul	a5, s8, t1
>  	sub	tp, tp, a5
>  	li	a5, SBI_SCRATCH_SIZE
> @@ -302,6 +313,10 @@ _scratch_init:
>  	REG_L	a5, 0(a4)
>  	REG_S	a5, SBI_SCRATCH_FW_RW_OFFSET(tp)
>  
> +	/* Store fw_heap_offset and fw_heap_size in scratch space */
> +	REG_S	s10, SBI_SCRATCH_FW_HEAP_OFFSET(tp)
> +	REG_S	s9, SBI_SCRATCH_FW_HEAP_SIZE_OFFSET(tp)
> +
>  	/* Store next arg1 in scratch space */
>  	MOV_3R	s0, a0, s1, a1, s2, a2
>  	call	fw_next_arg1
> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> index 546c0a6..3e9616f 100644
> --- a/include/sbi/sbi_platform.h
> +++ b/include/sbi/sbi_platform.h
> @@ -29,12 +29,16 @@
>  #define SBI_PLATFORM_HART_COUNT_OFFSET (0x50)
>  /** Offset of hart_stack_size in struct sbi_platform */
>  #define SBI_PLATFORM_HART_STACK_SIZE_OFFSET (0x54)
> +/** Offset of heap_size in struct sbi_platform */
> +#define SBI_PLATFORM_HEAP_SIZE_OFFSET (0x58)
> +/** Offset of reserved in struct sbi_platform */
> +#define SBI_PLATFORM_RESERVED_OFFSET (0x5c)
>  /** Offset of platform_ops_addr in struct sbi_platform */
> -#define SBI_PLATFORM_OPS_OFFSET (0x58)
> +#define SBI_PLATFORM_OPS_OFFSET (0x60)
>  /** Offset of firmware_context in struct sbi_platform */
> -#define SBI_PLATFORM_FIRMWARE_CONTEXT_OFFSET (0x58 + __SIZEOF_POINTER__)
> +#define SBI_PLATFORM_FIRMWARE_CONTEXT_OFFSET (0x60 + __SIZEOF_POINTER__)
>  /** Offset of hart_index2id in struct sbi_platform */
> -#define SBI_PLATFORM_HART_INDEX2ID_OFFSET (0x58 + (__SIZEOF_POINTER__ * 2))
> +#define SBI_PLATFORM_HART_INDEX2ID_OFFSET (0x60 + (__SIZEOF_POINTER__ * 2))
>  
>  #define SBI_PLATFORM_TLB_RANGE_FLUSH_LIMIT_DEFAULT		(1UL << 12)
>  
> @@ -138,6 +142,10 @@ struct sbi_platform_operations {
>  /** Platform default per-HART stack size for exception/interrupt handling */
>  #define SBI_PLATFORM_DEFAULT_HART_STACK_SIZE	8192
>  
> +/** Platform default heap size */
> +#define SBI_PLATFORM_DEFAULT_HEAP_SIZE(__num_hart)	\
> +					(0x8000 + 0x800 * (__num_hart))
> +
>  /** Representation of a platform */
>  struct sbi_platform {
>  	/**
> @@ -160,6 +168,10 @@ struct sbi_platform {
>  	u32 hart_count;
>  	/** Per-HART stack size for exception/interrupt handling */
>  	u32 hart_stack_size;
> +	/** Size of heap shared by all HARTs */
> +	u32 heap_size;
> +	/** Reserved for future use */
> +	u32 reserved;
>  	/** Pointer to sbi platform operations */
>  	unsigned long platform_ops_addr;
>  	/** Pointer to system firmware specific context */
> diff --git a/include/sbi/sbi_scratch.h b/include/sbi/sbi_scratch.h
> index 2966188..2376995 100644
> --- a/include/sbi/sbi_scratch.h
> +++ b/include/sbi/sbi_scratch.h
> @@ -20,26 +20,30 @@
>  #define SBI_SCRATCH_FW_SIZE_OFFSET		(1 * __SIZEOF_POINTER__)
>  /** Offset (in sbi_scratch) of the R/W Offset */
>  #define SBI_SCRATCH_FW_RW_OFFSET		(2 * __SIZEOF_POINTER__)
> +/** Offset of fw_heap_offset member in sbi_scratch */
> +#define SBI_SCRATCH_FW_HEAP_OFFSET		(3 * __SIZEOF_POINTER__)
> +/** Offset of fw_heap_size_offset member in sbi_scratch */
> +#define SBI_SCRATCH_FW_HEAP_SIZE_OFFSET		(4 * __SIZEOF_POINTER__)
>  /** Offset of next_arg1 member in sbi_scratch */
> -#define SBI_SCRATCH_NEXT_ARG1_OFFSET		(3 * __SIZEOF_POINTER__)
> +#define SBI_SCRATCH_NEXT_ARG1_OFFSET		(5 * __SIZEOF_POINTER__)
>  /** Offset of next_addr member in sbi_scratch */
> -#define SBI_SCRATCH_NEXT_ADDR_OFFSET		(4 * __SIZEOF_POINTER__)
> +#define SBI_SCRATCH_NEXT_ADDR_OFFSET		(6 * __SIZEOF_POINTER__)
>  /** Offset of next_mode member in sbi_scratch */
> -#define SBI_SCRATCH_NEXT_MODE_OFFSET		(5 * __SIZEOF_POINTER__)
> +#define SBI_SCRATCH_NEXT_MODE_OFFSET		(7 * __SIZEOF_POINTER__)
>  /** Offset of warmboot_addr member in sbi_scratch */
> -#define SBI_SCRATCH_WARMBOOT_ADDR_OFFSET	(6 * __SIZEOF_POINTER__)
> +#define SBI_SCRATCH_WARMBOOT_ADDR_OFFSET	(8 * __SIZEOF_POINTER__)
>  /** Offset of platform_addr member in sbi_scratch */
> -#define SBI_SCRATCH_PLATFORM_ADDR_OFFSET	(7 * __SIZEOF_POINTER__)
> +#define SBI_SCRATCH_PLATFORM_ADDR_OFFSET	(9 * __SIZEOF_POINTER__)
>  /** Offset of hartid_to_scratch member in sbi_scratch */
> -#define SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET	(8 * __SIZEOF_POINTER__)
> +#define SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET	(10 * __SIZEOF_POINTER__)
>  /** Offset of trap_exit member in sbi_scratch */
> -#define SBI_SCRATCH_TRAP_EXIT_OFFSET		(9 * __SIZEOF_POINTER__)
> +#define SBI_SCRATCH_TRAP_EXIT_OFFSET		(11 * __SIZEOF_POINTER__)
>  /** Offset of tmp0 member in sbi_scratch */
> -#define SBI_SCRATCH_TMP0_OFFSET			(10 * __SIZEOF_POINTER__)
> +#define SBI_SCRATCH_TMP0_OFFSET			(12 * __SIZEOF_POINTER__)
>  /** Offset of options member in sbi_scratch */
> -#define SBI_SCRATCH_OPTIONS_OFFSET		(11 * __SIZEOF_POINTER__)
> +#define SBI_SCRATCH_OPTIONS_OFFSET		(13 * __SIZEOF_POINTER__)
>  /** Offset of extra space in sbi_scratch */
> -#define SBI_SCRATCH_EXTRA_SPACE_OFFSET		(12 * __SIZEOF_POINTER__)
> +#define SBI_SCRATCH_EXTRA_SPACE_OFFSET		(14 * __SIZEOF_POINTER__)
>  /** Maximum size of sbi_scratch (4KB) */
>  #define SBI_SCRATCH_SIZE			(0x1000)
>  
> @@ -57,6 +61,10 @@ struct sbi_scratch {
>  	unsigned long fw_size;
>  	/** Offset (in bytes) of the R/W section */
>  	unsigned long fw_rw_offset;
> +	/** Offset (in bytes) of the heap area */
> +	unsigned long fw_heap_offset;
> +	/** Size (in bytes) of the heap area */
> +	unsigned long fw_heap_size;
>  	/** Arg1 (or 'a1' register) of next booting stage for this HART */
>  	unsigned long next_arg1;
>  	/** Address of next booting stage for this HART */
> diff --git a/platform/fpga/ariane/platform.c b/platform/fpga/ariane/platform.c
> index 1e341c2..975528f 100644
> --- a/platform/fpga/ariane/platform.c
> +++ b/platform/fpga/ariane/platform.c
> @@ -185,5 +185,6 @@ const struct sbi_platform platform = {
>  	.features = SBI_PLATFORM_DEFAULT_FEATURES,
>  	.hart_count = ARIANE_HART_COUNT,
>  	.hart_stack_size = SBI_PLATFORM_DEFAULT_HART_STACK_SIZE,
> +	.heap_size = SBI_PLATFORM_DEFAULT_HEAP_SIZE(ARIANE_HART_COUNT),
>  	.platform_ops_addr = (unsigned long)&platform_ops
>  };
> diff --git a/platform/fpga/openpiton/platform.c b/platform/fpga/openpiton/platform.c
> index 57ae698..e59dc99 100644
> --- a/platform/fpga/openpiton/platform.c
> +++ b/platform/fpga/openpiton/platform.c
> @@ -220,5 +220,7 @@ const struct sbi_platform platform = {
>  	.features = SBI_PLATFORM_DEFAULT_FEATURES,
>  	.hart_count = OPENPITON_DEFAULT_HART_COUNT,
>  	.hart_stack_size = SBI_PLATFORM_DEFAULT_HART_STACK_SIZE,
> +	.heap_size =
> +		SBI_PLATFORM_DEFAULT_HEAP_SIZE(OPENPITON_DEFAULT_HART_COUNT),
>  	.platform_ops_addr = (unsigned long)&platform_ops
>  };
> diff --git a/platform/generic/platform.c b/platform/generic/platform.c
> index eeefef4..0c9cd95 100644
> --- a/platform/generic/platform.c
> +++ b/platform/generic/platform.c
> @@ -115,7 +115,7 @@ unsigned long fw_platform_init(unsigned long arg0, unsigned long arg1,
>  	}
>  
>  	platform.hart_count = hart_count;
> -
> +	platform.heap_size = SBI_PLATFORM_DEFAULT_HEAP_SIZE(hart_count);
>  	platform_has_mlevel_imsic = fdt_check_imsic_mlevel(fdt);
>  
>  	/* Return original FDT pointer */
> @@ -315,5 +315,6 @@ struct sbi_platform platform = {
>  	.hart_count		= SBI_HARTMASK_MAX_BITS,
>  	.hart_index2id		= generic_hart_index2id,
>  	.hart_stack_size	= SBI_PLATFORM_DEFAULT_HART_STACK_SIZE,
> +	.heap_size		= SBI_PLATFORM_DEFAULT_HEAP_SIZE(0),

Presumably I'll see later in the series how the generic platform expands
its heap for the number of harts.

>  	.platform_ops_addr	= (unsigned long)&platform_ops
>  };
> diff --git a/platform/kendryte/k210/platform.c b/platform/kendryte/k210/platform.c
> index 7eb9015..637a217 100644
> --- a/platform/kendryte/k210/platform.c
> +++ b/platform/kendryte/k210/platform.c
> @@ -196,5 +196,7 @@ const struct sbi_platform platform = {
>  	.features		= 0,
>  	.hart_count		= K210_HART_COUNT,
>  	.hart_stack_size	= SBI_PLATFORM_DEFAULT_HART_STACK_SIZE,
> +	.heap_size		=
> +			SBI_PLATFORM_DEFAULT_HEAP_SIZE(K210_HART_COUNT),
>  	.platform_ops_addr	= (unsigned long)&platform_ops
>  };
> diff --git a/platform/nuclei/ux600/platform.c b/platform/nuclei/ux600/platform.c
> index 4eccff1..6fd6cd7 100644
> --- a/platform/nuclei/ux600/platform.c
> +++ b/platform/nuclei/ux600/platform.c
> @@ -244,5 +244,7 @@ const struct sbi_platform platform = {
>  	.features		= SBI_PLATFORM_DEFAULT_FEATURES,
>  	.hart_count		= UX600_HART_COUNT,
>  	.hart_stack_size	= SBI_PLATFORM_DEFAULT_HART_STACK_SIZE,
> +	.heap_size		=
> +			SBI_PLATFORM_DEFAULT_HEAP_SIZE(UX600_HART_COUNT),
>  	.platform_ops_addr	= (unsigned long)&platform_ops
>  };
> diff --git a/platform/template/platform.c b/platform/template/platform.c
> index 8adc431..86381ca 100644
> --- a/platform/template/platform.c
> +++ b/platform/template/platform.c
> @@ -152,5 +152,6 @@ const struct sbi_platform platform = {
>  	.features		= SBI_PLATFORM_DEFAULT_FEATURES,
>  	.hart_count		= 1,
>  	.hart_stack_size	= SBI_PLATFORM_DEFAULT_HART_STACK_SIZE,
> +	.heap_size		= SBI_PLATFORM_DEFAULT_HEAP_SIZE(1),
>  	.platform_ops_addr	= (unsigned long)&platform_ops
>  };
> -- 
> 2.34.1
> 
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 02/17] lib: sbi: Introduce simple heap allocator

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:15PM +0530, Anup Patel wrote:
> We provide simple heap allocator to manage the heap space provided
> by OpenSBI firmware and platform.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  include/sbi/sbi_heap.h |  44 +++++++++
>  lib/sbi/objects.mk     |   1 +
>  lib/sbi/sbi_heap.c     | 204 +++++++++++++++++++++++++++++++++++++++++
>  lib/sbi/sbi_init.c     |  15 +++
>  4 files changed, 264 insertions(+)
>  create mode 100644 include/sbi/sbi_heap.h
>  create mode 100644 lib/sbi/sbi_heap.c
> 
> diff --git a/include/sbi/sbi_heap.h b/include/sbi/sbi_heap.h
> new file mode 100644
> index 0000000..88d176e
> --- /dev/null
> +++ b/include/sbi/sbi_heap.h
> @@ -0,0 +1,44 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (c) 2023 Ventana Micro Systems Inc.
> + *
> + * Authors:
> + *   Anup Patel<apatel@ventanamicro.com>
> + */
> +
> +#ifndef __SBI_HEAP_H__
> +#define __SBI_HEAP_H__
> +
> +#include <sbi/sbi_types.h>
> +
> +struct sbi_scratch;
> +
> +/** Allocate from heap area */
> +void *sbi_malloc(size_t size);
> +
> +/** Zero allocate from heap area */
> +void *sbi_zalloc(size_t size);
> +
> +/** Allocate array from heap area */
> +static inline void *sbi_calloc(size_t nitems, size_t size)
> +{
> +	return sbi_zalloc(nitems * size);
> +}
> +
> +/** Free-up to heap area */
> +void sbi_free(void *ptr);
> +
> +/** Amount (in bytes) of free space in the heap area */
> +unsigned long sbi_heap_free_space(void);
> +
> +/** Amount (in bytes) of used space in the heap area */
> +unsigned long sbi_heap_used_space(void);
> +
> +/** Amount (in bytes) of reserved space in the heap area */
> +unsigned long sbi_heap_reserved_space(void);
> +
> +/** Initialize heap area */
> +int sbi_heap_init(struct sbi_scratch *scratch);
> +
> +#endif
> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
> index 7d691c6..c699187 100644
> --- a/lib/sbi/objects.mk
> +++ b/lib/sbi/objects.mk
> @@ -59,6 +59,7 @@ libsbi-objs-y += sbi_domain.o
>  libsbi-objs-y += sbi_emulate_csr.o
>  libsbi-objs-y += sbi_fifo.o
>  libsbi-objs-y += sbi_hart.o
> +libsbi-objs-y += sbi_heap.o
>  libsbi-objs-y += sbi_math.o
>  libsbi-objs-y += sbi_hfence.o
>  libsbi-objs-y += sbi_hsm.o
> diff --git a/lib/sbi/sbi_heap.c b/lib/sbi/sbi_heap.c
> new file mode 100644
> index 0000000..82aa1ec
> --- /dev/null
> +++ b/lib/sbi/sbi_heap.c
> @@ -0,0 +1,204 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (c) 2023 Ventana Micro Systems Inc.
> + *
> + * Authors:
> + *   Anup Patel<apatel@ventanamicro.com>
> + */
> +
> +#include <sbi/riscv_locks.h>
> +#include <sbi/sbi_error.h>
> +#include <sbi/sbi_heap.h>
> +#include <sbi/sbi_list.h>
> +#include <sbi/sbi_scratch.h>
> +#include <sbi/sbi_string.h>
> +
> +#define HEAP_BASE_ALIGN			1024
> +#define HEAP_ALLOC_ALIGN		64

What's the rationale for these alignments. Maybe alloc-align is expected
cache size? But what about base-align? Can you add comment explaining
them?

> +#define HEAP_HOUSEKEEPING_FACTOR	16
> +
> +struct heap_node {
> +	struct sbi_dlist head;
> +	unsigned long addr;
> +	unsigned long size;
> +};
> +
> +struct heap_control {
> +	spinlock_t lock;
> +	unsigned long base;
> +	unsigned long size;
> +	unsigned long hkbase;
> +	unsigned long hksize;
> +	struct sbi_dlist free_node_list;
> +	struct sbi_dlist free_space_list;
> +	struct sbi_dlist used_space_list;
> +};
> +
> +static struct heap_control hpctrl;
> +
> +void *sbi_malloc(size_t size)
> +{
> +	void *ret = NULL;
> +	struct heap_node *n, *np;
> +
> +	if (!size)
> +		return NULL;
> +
> +	size += HEAP_ALLOC_ALIGN - 1;
> +	size &= ~((unsigned long)HEAP_ALLOC_ALIGN - 1);
> +
> +	spin_lock(&hpctrl.lock);
> +
> +	np = NULL;
> +	sbi_list_for_each_entry(n, &hpctrl.free_space_list, head) {
> +		if (size <= n->size) {
> +			np = n;
> +			break;
> +		}
> +	}
> +	if (np) {
> +		if ((size < np->size) &&
> +		    !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);
> +			ret = (void *)n->addr;
> +		} else if (size == np->size) {
> +			sbi_list_del(&np->head);
> +			sbi_list_add_tail(&np->head, &hpctrl.used_space_list);
> +			ret = (void *)np->addr;
> +		}
> +	}
> +
> +	spin_unlock(&hpctrl.lock);
> +
> +	return ret;
> +}
> +
> +void *sbi_zalloc(size_t size)
> +{
> +	void *ret = sbi_malloc(size);
> +
> +	if (ret)
> +		sbi_memset(ret, 0, size);
> +	return ret;
> +}
> +
> +void sbi_free(void *ptr)
> +{
> +	struct heap_node *n, *np;
> +
> +	if (!ptr)
> +		return;
> +
> +	spin_lock(&hpctrl.lock);
> +
> +	np = NULL;
> +	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;
> +			break;
> +		}
> +	}
> +	if (!np) {
> +		spin_unlock(&hpctrl.lock);
> +		return;
> +	}
> +
> +	sbi_list_del(&np->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);
> +			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);
> +			np = NULL;
> +			break;
> +		} else if ((n->addr + n->size) < np->addr) {
> +			sbi_list_add(&np->head, &n->head);
> +			np = NULL;
> +			break;
> +		}
> +	}
> +	if (np)
> +		sbi_list_add_tail(&np->head, &hpctrl.free_space_list);
> +
> +	spin_unlock(&hpctrl.lock);
> +}
> +
> +unsigned long sbi_heap_free_space(void)
> +{
> +	struct heap_node *n;
> +	unsigned long ret = 0;
> +
> +	spin_lock(&hpctrl.lock);
> +	sbi_list_for_each_entry(n, &hpctrl.free_space_list, head)
> +		ret += n->size;
> +	spin_unlock(&hpctrl.lock);
> +
> +	return ret;
> +}
> +
> +unsigned long sbi_heap_used_space(void)
> +{
> +	return hpctrl.size - hpctrl.hksize - sbi_heap_free_space();
> +}
> +
> +unsigned long sbi_heap_reserved_space(void)
> +{
> +	return hpctrl.hksize;
> +}
> +
> +int sbi_heap_init(struct sbi_scratch *scratch)
> +{
> +	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);
> +
> +	/* Prepare free node list */
> +	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);
> +	}
> +
> +	/* Prepare free space 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);
> +
> +	return 0;
> +}
> diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> index 5db8e7f..f09a7ac 100644
> --- a/lib/sbi/sbi_init.c
> +++ b/lib/sbi/sbi_init.c
> @@ -17,6 +17,7 @@
>  #include <sbi/sbi_ecall.h>
>  #include <sbi/sbi_hart.h>
>  #include <sbi/sbi_hartmask.h>
> +#include <sbi/sbi_heap.h>
>  #include <sbi/sbi_hsm.h>
>  #include <sbi/sbi_ipi.h>
>  #include <sbi/sbi_irqchip.h>
> @@ -118,6 +119,15 @@ static void sbi_boot_print_general(struct sbi_scratch *scratch)
>  	sbi_printf("Firmware Size             : %d KB\n",
>  		   (u32)(scratch->fw_size / 1024));
>  	sbi_printf("Firmware RW Offset        : 0x%lx\n", scratch->fw_rw_offset);
> +	sbi_printf("Firmware RW Size          : %d KB\n",
> +		   (u32)((scratch->fw_size - scratch->fw_rw_offset) / 1024));
> +	sbi_printf("Firmware Heap Offset      : 0x%lx\n", scratch->fw_heap_offset);
> +	sbi_printf("Firmware Heap Size        : "
> +		   "%d KB (total), %d KB (reserved), %d KB (used), %d KB (free)\n",
> +		   (u32)(scratch->fw_heap_size / 1024),
> +		   (u32)(sbi_heap_reserved_space() / 1024),
> +		   (u32)(sbi_heap_used_space() / 1024),
> +		   (u32)(sbi_heap_free_space() / 1024));
>  
>  	/* SBI details */
>  	sbi_printf("Runtime SBI Version       : %d.%d\n",
> @@ -258,6 +268,11 @@ static void __noreturn init_coldboot(struct sbi_scratch *scratch, u32 hartid)
>  		sbi_hart_hang();
>  
>  	/* Note: This has to be second thing in coldboot init sequence */
> +	rc = sbi_heap_init(scratch);
> +	if (rc)
> +		sbi_hart_hang();
> +
> +	/* Note: This has to be thing thing in coldboot init sequence */

...be the third thing...

>  	rc = sbi_domain_init(scratch, hartid);
>  	if (rc)
>  		sbi_hart_hang();
> -- 
> 2.34.1
>

Otherwise,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew

[PATCH 03/17] lib: sbi: Print scratch size and usage at boot time

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:16PM +0530, Anup Patel wrote:
> The scratch space being a scarce resource so let us print it's
> size and usage at boot time.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  include/sbi/sbi_scratch.h |  3 +++
>  lib/sbi/sbi_init.c        |  5 +++++
>  lib/sbi/sbi_scratch.c     | 11 +++++++++++
>  3 files changed, 19 insertions(+)
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 04/17] lib: sbi_pmu: Use heap for per-HART PMU state

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:17PM +0530, Anup Patel wrote:
> Instead of using a global array for per-HART PMU state, we should
> use heap to on-demand allocate per-HART PMU state when the HART
> is initialized in cold boot or warm boot path.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/sbi/sbi_pmu.c | 221 ++++++++++++++++++++++++++++------------------
>  1 file changed, 133 insertions(+), 88 deletions(-)
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 05/17] lib: sbi: Use heap for root domain creation

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:18PM +0530, Anup Patel wrote:
> Let's use heap allocation in root domain creation instead of using
> a fixed size global array.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/sbi/sbi_domain.c | 45 +++++++++++++++++++++++++++++++++-----------
>  1 file changed, 34 insertions(+), 11 deletions(-)
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 06/17] lib: sbi: Use scratch space to save per-HART domain pointer

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:19PM +0530, Anup Patel wrote:
> Instead of using a global array indexed by hartid, we should use
> scratch space to save per-HART domain pointer.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  include/sbi/sbi_domain.h |  6 +-----
>  lib/sbi/sbi_domain.c     | 38 +++++++++++++++++++++++++++++++++++---
>  2 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h
> index 124ea90..b05bcf4 100644
> --- a/include/sbi/sbi_domain.h
> +++ b/include/sbi/sbi_domain.h
> @@ -129,12 +129,8 @@ struct sbi_domain {
>  /** The root domain instance */
>  extern struct sbi_domain root;
>  
> -/** HART id to domain table */
> -extern struct sbi_domain *hartid_to_domain_table[];
> -
>  /** Get pointer to sbi_domain from HART id */
> -#define sbi_hartid_to_domain(__hartid) \
> -	hartid_to_domain_table[__hartid]
> +struct sbi_domain *sbi_hartid_to_domain(u32 hartid);
>  
>  /** Get pointer to sbi_domain for current HART */
>  #define sbi_domain_thishart_ptr() \
> diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
> index b07d310..531737c 100644
> --- a/lib/sbi/sbi_domain.c
> +++ b/lib/sbi/sbi_domain.c
> @@ -23,7 +23,6 @@
>   * the array to be null-terminated.
>   */
>  struct sbi_domain *domidx_to_domain_table[SBI_DOMAIN_MAX_INDEX + 1] = { 0 };
> -struct sbi_domain *hartid_to_domain_table[SBI_HARTMASK_MAX_BITS] = { 0 };
>  static u32 domain_count = 0;
>  static bool domain_finalized = false;
>  
> @@ -39,6 +38,32 @@ struct sbi_domain root = {
>  	.fw_region_inited = false,
>  };
>  
> +static unsigned long domain_hart_ptr_offset;
> +
> +struct sbi_domain *sbi_hartid_to_domain(u32 hartid)
> +{
> +	struct sbi_scratch *scratch;
> +
> +	scratch = sbi_hartid_to_scratch(hartid);
> +	if (!scratch || !domain_hart_ptr_offset)
> +		return NULL;
> +
> +	return (void *)(*((ulong *)sbi_scratch_offset_ptr(scratch,
> +						   domain_hart_ptr_offset)));
> +}
> +
> +static void update_hartid_to_domain(u32 hartid, struct sbi_domain *dom)
> +{
> +	struct sbi_scratch *scratch;
> +
> +	scratch = sbi_hartid_to_scratch(hartid);
> +	if (!scratch)
> +		return;
> +
> +	*((ulong *)sbi_scratch_offset_ptr(scratch, domain_hart_ptr_offset))
> +					= (ulong)dom;
> +}
> +
>  bool sbi_domain_is_assigned_hart(const struct sbi_domain *dom, u32 hartid)
>  {
>  	if (dom)
> @@ -519,11 +544,11 @@ int sbi_domain_register(struct sbi_domain *dom,
>  		if (!sbi_hartmask_test_hart(i, dom->possible_harts))
>  			continue;
>  
> -		tdom = hartid_to_domain_table[i];
> +		tdom = sbi_hartid_to_domain(i);
>  		if (tdom)
>  			sbi_hartmask_clear_hart(i,
>  					&tdom->assigned_harts);
> -		hartid_to_domain_table[i] = dom;
> +		update_hartid_to_domain(i, dom);
>  		sbi_hartmask_set_hart(i, &dom->assigned_harts);
>  
>  		/*
> @@ -715,9 +740,14 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
>  		return SBI_EINVAL;
>  	}
>  
> +	domain_hart_ptr_offset = sbi_scratch_alloc_offset(sizeof(ulong));
> +	if (!domain_hart_ptr_offset)
> +		return SBI_ENOMEM;
> +
>  	root_memregs = sbi_calloc(sizeof(*root_memregs), ROOT_REGION_MAX + 1);
>  	if (!root_memregs) {
>  		sbi_printf("%s: no memory for root regions\n", __func__);
> +		sbi_scratch_free_offset(domain_hart_ptr_offset);
>  		return SBI_ENOMEM;
>  	}
>  	root.regions = root_memregs;
> @@ -726,6 +756,7 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
>  	if (!root_hmask) {
>  		sbi_printf("%s: no memory for root hartmask\n", __func__);
>  		sbi_free(root_memregs);
> +		sbi_scratch_free_offset(domain_hart_ptr_offset);
>  		return SBI_ENOMEM;
>  	}
>  	root.possible_harts = root_hmask;
> @@ -774,6 +805,7 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
>  	if (rc) {
>  		sbi_free(root_hmask);
>  		sbi_free(root_memregs);
> +		sbi_scratch_free_offset(domain_hart_ptr_offset);
>  		return rc;
>  	}

Rather than duplicating all the free()'s we could use a sequence of error
labels and gotos.

Otherwise,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew

[PATCH 07/17] lib: utils/gpio: Use heap in SiFive and StartFive GPIO drivers

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:20PM +0530, Anup Patel wrote:
> Let's use heap allocation in SiFive and Starfive GPIO drivers
> instead of using a fixed size global array.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/utils/gpio/fdt_gpio_sifive.c   | 21 ++++++++++-----------
>  lib/utils/gpio/fdt_gpio_starfive.c | 20 ++++++++++----------
>  2 files changed, 20 insertions(+), 21 deletions(-)
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 08/17] lib: utils/i2c: Use heap in DesignWare and SiFive I2C drivers

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:21PM +0530, Anup Patel wrote:
> Let's use heap allocation in DesignWare and SiFive I2C drivers
> instead of using a fixed size global array.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/utils/i2c/fdt_i2c_dw.c     | 24 ++++++++++--------------
>  lib/utils/i2c/fdt_i2c_sifive.c | 23 ++++++++++-------------
>  2 files changed, 20 insertions(+), 27 deletions(-)
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 09/17] lib: utils/ipi: Use heap in ACLINT MSWI driver

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:22PM +0530, Anup Patel wrote:
> Let's use heap allocation in ACLINT MSWI driver instead of using
> a fixed size global array.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/utils/ipi/fdt_ipi_mswi.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 10/17] lib: utils/irqchip: Use heap in PLIC, APLIC and IMSIC drivers

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:23PM +0530, Anup Patel wrote:
> Let's use heap allocation in PLIC, APLIC, and IMSIC irqchip drivers
> instead of using a fixed size global array.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/utils/irqchip/fdt_irqchip_aplic.c | 24 ++++++++++--------
>  lib/utils/irqchip/fdt_irqchip_imsic.c | 30 +++++++++++-----------
>  lib/utils/irqchip/fdt_irqchip_plic.c  | 36 +++++++++++++--------------
>  3 files changed, 47 insertions(+), 43 deletions(-)
> 
> diff --git a/lib/utils/irqchip/fdt_irqchip_aplic.c b/lib/utils/irqchip/fdt_irqchip_aplic.c
> index 965f023..caa92a7 100644
> --- a/lib/utils/irqchip/fdt_irqchip_aplic.c
> +++ b/lib/utils/irqchip/fdt_irqchip_aplic.c
> @@ -11,15 +11,11 @@
>  #include <libfdt.h>
>  #include <sbi/riscv_asm.h>
>  #include <sbi/sbi_error.h>
> +#include <sbi/sbi_heap.h>
>  #include <sbi_utils/fdt/fdt_helper.h>
>  #include <sbi_utils/irqchip/fdt_irqchip.h>
>  #include <sbi_utils/irqchip/aplic.h>
>  
> -#define APLIC_MAX_NR			16
> -
> -static unsigned long aplic_count = 0;
> -static struct aplic_data aplic[APLIC_MAX_NR];
> -
>  static int irqchip_aplic_warm_init(void)
>  {
>  	/* Nothing to do here. */
> @@ -32,15 +28,23 @@ static int irqchip_aplic_cold_init(void *fdt, int nodeoff,
>  	int rc;
>  	struct aplic_data *pd;
>  
> -	if (APLIC_MAX_NR <= aplic_count)
> -		return SBI_ENOSPC;
> -	pd = &aplic[aplic_count++];
> +	pd = sbi_zalloc(sizeof(*pd));
> +	if (!pd)
> +		return SBI_ENOMEM;
>  
>  	rc = fdt_parse_aplic_node(fdt, nodeoff, pd);
> -	if (rc)
> +	if (rc) {
> +		sbi_free(pd);
>  		return rc;
> +	}
>  
> -	return aplic_cold_irqchip_init(pd);
> +	rc = aplic_cold_irqchip_init(pd);
> +	if (rc) {
> +		sbi_free(pd);
> +		return rc;
> +	}
> +
> +	return 0;
>  }
>  
>  static const struct fdt_match irqchip_aplic_match[] = {
> diff --git a/lib/utils/irqchip/fdt_irqchip_imsic.c b/lib/utils/irqchip/fdt_irqchip_imsic.c
> index 6020ac0..d890400 100644
> --- a/lib/utils/irqchip/fdt_irqchip_imsic.c
> +++ b/lib/utils/irqchip/fdt_irqchip_imsic.c
> @@ -11,16 +11,12 @@
>  #include <libfdt.h>
>  #include <sbi/riscv_asm.h>
>  #include <sbi/sbi_error.h>
> +#include <sbi/sbi_heap.h>
>  #include <sbi/sbi_hartmask.h>
>  #include <sbi_utils/fdt/fdt_helper.h>
>  #include <sbi_utils/irqchip/fdt_irqchip.h>
>  #include <sbi_utils/irqchip/imsic.h>
>  
> -#define IMSIC_MAX_NR			16
> -
> -static unsigned long imsic_count = 0;
> -static struct imsic_data imsic[IMSIC_MAX_NR];
> -
>  static int irqchip_imsic_update_hartid_table(void *fdt, int nodeoff,
>  					     struct imsic_data *id)
>  {
> @@ -71,25 +67,31 @@ static int irqchip_imsic_cold_init(void *fdt, int nodeoff,
>  	int rc;
>  	struct imsic_data *id;
>  
> -	if (IMSIC_MAX_NR <= imsic_count)
> -		return SBI_ENOSPC;
> -	id = &imsic[imsic_count];
> +	id = sbi_zalloc(sizeof(*id));
> +	if (!id)
> +		return SBI_ENOMEM;
>  
>  	rc = fdt_parse_imsic_node(fdt, nodeoff, id);
> -	if (rc)
> +	if (rc) {
> +		sbi_free(id);
>  		return rc;
> -	if (!id->targets_mmode)
> +	}
> +	if (!id->targets_mmode) {
> +		sbi_free(id);
>  		return 0;
> +	}
>  
>  	rc = irqchip_imsic_update_hartid_table(fdt, nodeoff, id);
> -	if (rc)
> +	if (rc) {
> +		sbi_free(id);
>  		return rc;
> +	}
>  
>  	rc = imsic_cold_irqchip_init(id);
> -	if (rc)
> +	if (rc) {
> +		sbi_free(id);
>  		return rc;
> -
> -	imsic_count++;
> +	}

Could have a label at the bottom

    return 0;
error:
    sbi_free(id);
    return rc;

Just need to set rc=0 before the id->targets_mmode test and everything can
goto error rather than needing to remember to free at each point.

>  
>  	return 0;
>  }
> diff --git a/lib/utils/irqchip/fdt_irqchip_plic.c b/lib/utils/irqchip/fdt_irqchip_plic.c
> index 1aadf91..605f44a 100644
> --- a/lib/utils/irqchip/fdt_irqchip_plic.c
> +++ b/lib/utils/irqchip/fdt_irqchip_plic.c
> @@ -11,18 +11,14 @@
>  #include <sbi/riscv_asm.h>
>  #include <sbi/riscv_io.h>
>  #include <sbi/sbi_error.h>
> +#include <sbi/sbi_heap.h>
>  #include <sbi/sbi_hartmask.h>
>  #include <sbi_utils/fdt/fdt_helper.h>
>  #include <sbi_utils/irqchip/fdt_irqchip.h>
>  #include <sbi_utils/irqchip/plic.h>
>  
> -#define PLIC_MAX_NR			16
> -
> -static unsigned long plic_count = 0;
> -static struct plic_data plic[PLIC_MAX_NR];
> -
>  static struct plic_data *plic_hartid2data[SBI_HARTMASK_MAX_BITS];
> -static int plic_hartid2context[SBI_HARTMASK_MAX_BITS][2];
> +static int plic_hartid2context[SBI_HARTMASK_MAX_BITS][2] = { { -1 } };
>  
>  void fdt_plic_priority_save(u8 *priority, u32 num)
>  {
> @@ -114,16 +110,18 @@ static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
>  static int irqchip_plic_cold_init(void *fdt, int nodeoff,
>  				  const struct fdt_match *match)
>  {
> -	int i, rc;
> +	int rc;
>  	struct plic_data *pd;
>  
> -	if (PLIC_MAX_NR <= plic_count)
> -		return SBI_ENOSPC;
> -	pd = &plic[plic_count++];
> +	pd = sbi_zalloc(sizeof(*pd));
> +	if (!pd)
> +		return SBI_ENOMEM;
>  
>  	rc = fdt_parse_plic_node(fdt, nodeoff, pd);
> -	if (rc)
> +	if (rc) {
> +		sbi_free(pd);
>  		return rc;
> +	}
>  
>  	if (match->data) {
>  		void (*plic_plat_init)(struct plic_data *) = match->data;
> @@ -131,18 +129,18 @@ static int irqchip_plic_cold_init(void *fdt, int nodeoff,
>  	}
>  
>  	rc = plic_cold_irqchip_init(pd);
> -	if (rc)
> +	if (rc) {
> +		sbi_free(pd);
>  		return rc;
> +	}
>  
> -	if (plic_count == 1) {
> -		for (i = 0; i < SBI_HARTMASK_MAX_BITS; i++) {
> -			plic_hartid2data[i] = NULL;
> -			plic_hartid2context[i][0] = -1;
> -			plic_hartid2context[i][1] = -1;
> -		}
> +	rc = irqchip_plic_update_hartid_table(fdt, nodeoff, pd);
> +	if (rc) {
> +		sbi_free(pd);
> +		return rc;
>  	}
>  
> -	return irqchip_plic_update_hartid_table(fdt, nodeoff, pd);
> +	return 0;

Same comment as above.

>  }
>  
>  #define THEAD_PLIC_CTRL_REG 0x1ffffc
> -- 
> 2.34.1
>

Otherwise,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew

[PATCH 11/17] lib: utils/timer: Use heap in ACLINT MTIMER driver

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:24PM +0530, Anup Patel wrote:
> Let's use heap allocation in ACLINT MTIMER driver instead of using
> a fixed size global array.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/utils/timer/fdt_timer_mtimer.c | 47 +++++++++++++++++++-----------
>  1 file changed, 30 insertions(+), 17 deletions(-)
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 12/17] lib: utils/fdt: Use heap in FDT domain parsing

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:25PM +0530, Anup Patel wrote:
> Let's use heap allocation in FDT domain parsing instead of using
> a fixed size global array.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/utils/fdt/fdt_domain.c | 111 +++++++++++++++++++++++--------------
>  1 file changed, 68 insertions(+), 43 deletions(-)
> 
> diff --git a/lib/utils/fdt/fdt_domain.c b/lib/utils/fdt/fdt_domain.c
> index bb6d17d..c4aed4d 100644
> --- a/lib/utils/fdt/fdt_domain.c
> +++ b/lib/utils/fdt/fdt_domain.c
> @@ -13,6 +13,7 @@
>  #include <sbi/sbi_domain.h>
>  #include <sbi/sbi_error.h>
>  #include <sbi/sbi_hartmask.h>
> +#include <sbi/sbi_heap.h>
>  #include <sbi/sbi_scratch.h>
>  #include <sbi_utils/fdt/fdt_domain.h>
>  #include <sbi_utils/fdt/fdt_helper.h>
> @@ -219,14 +220,11 @@ skip_device_disable:
>  	fdt_nop_node(fdt, poffset);
>  }
>  
> -#define FDT_DOMAIN_MAX_COUNT		8
> -#define FDT_DOMAIN_REGION_MAX_COUNT	16

Why not keep this define since we still use 16 below when allocating
dom->regions / max_regions?

> -
> -static u32 fdt_domains_count;
> -static struct sbi_domain fdt_domains[FDT_DOMAIN_MAX_COUNT];
> -static struct sbi_hartmask fdt_masks[FDT_DOMAIN_MAX_COUNT];
> -static struct sbi_domain_memregion
> -	fdt_regions[FDT_DOMAIN_MAX_COUNT][FDT_DOMAIN_REGION_MAX_COUNT + 1];
> +struct parse_region_data {
> +	struct sbi_domain *dom;
> +	u32 region_count;
> +	u32 max_regions;
> +};
>  
>  static int __fdt_parse_region(void *fdt, int domain_offset,
>  			      int region_offset, u32 region_access,
> @@ -236,7 +234,7 @@ static int __fdt_parse_region(void *fdt, int domain_offset,
>  	u32 val32;
>  	u64 val64;
>  	const u32 *val;
> -	u32 *region_count = opaque;
> +	struct parse_region_data *preg = opaque;
>  	struct sbi_domain_memregion *region;
>  
>  	/*
> @@ -252,9 +250,9 @@ static int __fdt_parse_region(void *fdt, int domain_offset,
>  		return SBI_EINVAL;
>  
>  	/* Find next region of the domain */
> -	if (FDT_DOMAIN_REGION_MAX_COUNT <= *region_count)
> -		return SBI_EINVAL;
> -	region = &fdt_regions[fdt_domains_count][*region_count];
> +	if (preg->max_regions <= preg->region_count)
> +		return SBI_ENOSPC;
> +	region = &preg->dom->regions[preg->region_count];
>  
>  	/* Read "base" DT property */
>  	val = fdt_getprop(fdt, region_offset, "base", &len);
> @@ -278,7 +276,7 @@ static int __fdt_parse_region(void *fdt, int domain_offset,
>  	if (fdt_get_property(fdt, region_offset, "mmio", NULL))
>  		region->flags |= SBI_DOMAIN_MEMREGION_MMIO;
>  
> -	(*region_count)++;
> +	preg->region_count++;
>  
>  	return 0;
>  }
> @@ -291,16 +289,30 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
>  	struct sbi_domain *dom;
>  	struct sbi_hartmask *mask;
>  	struct sbi_hartmask assign_mask;
> +	struct parse_region_data preg;
>  	int *cold_domain_offset = opaque;
> -	struct sbi_domain_memregion *reg, *regions;
> -	int i, err, len, cpus_offset, cpu_offset, doffset;
> +	struct sbi_domain_memregion *reg;
> +	int i, err = 0, len, cpus_offset, cpu_offset, doffset;
>  
> -	/* Sanity check on maximum domains we can handle */
> -	if (FDT_DOMAIN_MAX_COUNT <= fdt_domains_count)
> -		return SBI_EINVAL;
> -	dom = &fdt_domains[fdt_domains_count];
> -	mask = &fdt_masks[fdt_domains_count];
> -	regions = &fdt_regions[fdt_domains_count][0];
> +	dom = sbi_zalloc(sizeof(*dom));
> +	if (!dom)
> +		return SBI_ENOMEM;
> +
> +	dom->regions = sbi_calloc(sizeof(*dom->regions), 16 + 1);
> +	if (!dom->regions) {
> +		sbi_free(dom);
> +		return SBI_ENOMEM;
> +	}
> +	preg.dom = dom;
> +	preg.region_count = 0;
> +	preg.max_regions = 16;
> +
> +	mask = sbi_zalloc(sizeof(*mask));
> +	if (!mask) {
> +		sbi_free(dom->regions);
> +		sbi_free(dom);
> +		return SBI_ENOMEM;
> +	}

Can add two more labels above fail_free_all to use in above two failure
cases as well.

>  
>  	/* Read DT node name */
>  	strncpy(dom->name, fdt_get_name(fdt, domain_offset, NULL),
> @@ -316,12 +328,14 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
>  		for (i = 0; i < len; i++) {
>  			cpu_offset = fdt_node_offset_by_phandle(fdt,
>  							fdt32_to_cpu(val[i]));
> -			if (cpu_offset < 0)
> -				return cpu_offset;
> +			if (cpu_offset < 0) {
> +				err = cpu_offset;
> +				goto fail_free_all;
> +			}
>  
>  			err = fdt_parse_hart_id(fdt, cpu_offset, &val32);
>  			if (err)
> -				return err;
> +				goto fail_free_all;
>  
>  			if (!fdt_node_is_enabled(fdt, cpu_offset))
>  				continue;
> @@ -331,14 +345,10 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
>  	}
>  
>  	/* Setup memregions from DT */
> -	val32 = 0;
> -	memset(regions, 0,
> -		   sizeof(*regions) * (FDT_DOMAIN_REGION_MAX_COUNT + 1));
> -	dom->regions = regions;
> -	err = fdt_iterate_each_memregion(fdt, domain_offset, &val32,
> +	err = fdt_iterate_each_memregion(fdt, domain_offset, &preg,
>  					 __fdt_parse_region);
>  	if (err)
> -		return err;
> +		goto fail_free_all;
>  
>  	/*
>  	 * Copy over root domain memregions which don't allow
> @@ -354,9 +364,11 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
>  		    (reg->flags & SBI_DOMAIN_MEMREGION_SU_WRITABLE) ||
>  		    (reg->flags & SBI_DOMAIN_MEMREGION_SU_EXECUTABLE))
>  			continue;
> -		if (FDT_DOMAIN_REGION_MAX_COUNT <= val32)
> -			return SBI_EINVAL;
> -		memcpy(&regions[val32++], reg, sizeof(*reg));
> +		if (preg.max_regions <= preg.region_count) {
> +			err = SBI_EINVAL;
> +			goto fail_free_all;
> +		}
> +		memcpy(&dom->regions[preg.region_count++], reg, sizeof(*reg));
>  	}
>  	dom->fw_region_inited = root.fw_region_inited;
>  
> @@ -427,8 +439,10 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
>  
>  	/* Find /cpus DT node */
>  	cpus_offset = fdt_path_offset(fdt, "/cpus");
> -	if (cpus_offset < 0)
> -		return cpus_offset;
> +	if (cpus_offset < 0) {
> +		err = cpus_offset;
> +		goto fail_free_all;
> +	}
>  
>  	/* HART to domain assignment mask based on CPU DT nodes */
>  	sbi_hartmask_clear_all(&assign_mask);
> @@ -444,22 +458,33 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
>  			continue;
>  
>  		val = fdt_getprop(fdt, cpu_offset, "opensbi-domain", &len);
> -		if (!val || len < 4)
> -			return SBI_EINVAL;
> +		if (!val || len < 4) {
> +			err = SBI_EINVAL;
> +			goto fail_free_all;
> +		}
>  
>  		doffset = fdt_node_offset_by_phandle(fdt, fdt32_to_cpu(*val));
> -		if (doffset < 0)
> -			return doffset;
> +		if (doffset < 0) {
> +			err = doffset;
> +			goto fail_free_all;
> +		}
>  
>  		if (doffset == domain_offset)
>  			sbi_hartmask_set_hart(val32, &assign_mask);
>  	}
>  
> -	/* Increment domains count */
> -	fdt_domains_count++;
> -
>  	/* Register the domain */
> -	return sbi_domain_register(dom, &assign_mask);
> +	err = sbi_domain_register(dom, &assign_mask);
> +	if (err)
> +		goto fail_free_all;
> +
> +	return 0;
> +
> +fail_free_all:
> +	sbi_free(dom->regions);
> +	sbi_free(mask);
> +	sbi_free(dom);
> +	return err;
>  }
>  
>  int fdt_domains_populate(void *fdt)
> -- 
> 2.34.1
>

Otherwise,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 13/17] lib: utils/ipi: Use scratch space to save per-HART MSWI pointer

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:26PM +0530, Anup Patel wrote:
> Instead of using a global array indexed by hartid, we should use
> scratch space to save per-HART MSWI pointer.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/utils/ipi/aclint_mswi.c | 49 +++++++++++++++++++++++++++++--------
>  1 file changed, 39 insertions(+), 10 deletions(-)
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 14/17] lib: utils/timer: Use scratch space to save per-HART MTIMER pointer

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:27PM +0530, Anup Patel wrote:
> Instead of using a global array indexed by hartid, we should use
> scratch space to save per-HART MTIMER pointer.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/utils/timer/aclint_mtimer.c | 82 +++++++++++++++++++++++++++------
>  1 file changed, 67 insertions(+), 15 deletions(-)
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 15/17] lib: utils/irqchip: Use scratch space to save per-HART PLIC pointer

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:28PM +0530, Anup Patel wrote:
> Instead of using a global array indexed by hartid, we should use
> scratch space to save per-HART PLIC pointer and PLIC context numbers.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/utils/irqchip/fdt_irqchip_plic.c | 108 +++++++++++++++++++++------
>  1 file changed, 86 insertions(+), 22 deletions(-)
> 
> diff --git a/lib/utils/irqchip/fdt_irqchip_plic.c b/lib/utils/irqchip/fdt_irqchip_plic.c
> index 605f44a..eb1e412 100644
> --- a/lib/utils/irqchip/fdt_irqchip_plic.c
> +++ b/lib/utils/irqchip/fdt_irqchip_plic.c
> @@ -12,54 +12,97 @@
>  #include <sbi/riscv_io.h>
>  #include <sbi/sbi_error.h>
>  #include <sbi/sbi_heap.h>
> -#include <sbi/sbi_hartmask.h>
> +#include <sbi/sbi_scratch.h>
>  #include <sbi_utils/fdt/fdt_helper.h>
>  #include <sbi_utils/irqchip/fdt_irqchip.h>
>  #include <sbi_utils/irqchip/plic.h>
>  
> -static struct plic_data *plic_hartid2data[SBI_HARTMASK_MAX_BITS];
> -static int plic_hartid2context[SBI_HARTMASK_MAX_BITS][2] = { { -1 } };
> +static unsigned long plic_ptr_offset;
> +
> +#define plic_get_hart_data_ptr(__scratch)				\
> +({									\
> +	(void *)(*((ulong *)sbi_scratch_offset_ptr((__scratch),		\
> +						   plic_ptr_offset)));	\
> +})
> +
> +#define plic_set_hart_data_ptr(__scratch, __imsic)			\
> +do {									\
> +	*((ulong *)sbi_scratch_offset_ptr((__scratch), plic_ptr_offset))\
> +					= (ulong)(__imsic);		\
> +} while (0)
> +
> +static unsigned long plic_mcontext_offset;
> +
> +#define plic_get_hart_mcontext(__scratch)				\
> +({									\
> +	long __ret =							\
> +	*((long *)sbi_scratch_offset_ptr((__scratch), plic_mcontext_offset));\
> +	(__ret - 1);							\
> +})
> +
> +#define plic_set_hart_mcontext(__scratch, __mcontext)			\
> +do {									\
> +	*((long *)sbi_scratch_offset_ptr((__scratch), plic_mcontext_offset))\
> +					= (long)(__mcontext + 1);	\
> +} while (0)
> +
> +static unsigned long plic_scontext_offset;
> +
> +#define plic_get_hart_scontext(__scratch)				\
> +({									\
> +	long __ret =							\
> +	*((long *)sbi_scratch_offset_ptr((__scratch), plic_scontext_offset));\
> +	(__ret - 1);							\
> +})
> +
> +#define plic_set_hart_scontext(__scratch, __scontext)			\
> +do {									\
> +	*((long *)sbi_scratch_offset_ptr((__scratch), plic_scontext_offset))\
> +					= (long)(__scontext + 1);	\
> +} while (0)

I guess the +/- 1 stuff is to ensure that the m/scontext offsets can be
tested with zero to see if they need to be allocated. It'd be nice to
add a comment explaining it.

>  
>  void fdt_plic_priority_save(u8 *priority, u32 num)
>  {
> -	struct plic_data *plic = plic_hartid2data[current_hartid()];
> +	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
>  
> -	plic_priority_save(plic, priority, num);
> +	plic_priority_save(plic_get_hart_data_ptr(scratch), priority, num);
>  }
>  
>  void fdt_plic_priority_restore(const u8 *priority, u32 num)
>  {
> -	struct plic_data *plic = plic_hartid2data[current_hartid()];
> +	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
>  
> -	plic_priority_restore(plic, priority, num);
> +	plic_priority_restore(plic_get_hart_data_ptr(scratch), priority, num);
>  }
>  
>  void fdt_plic_context_save(bool smode, u32 *enable, u32 *threshold, u32 num)
>  {
> -	u32 hartid = current_hartid();
> +	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
>  
> -	plic_context_save(plic_hartid2data[hartid],
> -			  plic_hartid2context[hartid][smode],
> +	plic_context_save(plic_get_hart_data_ptr(scratch),
> +			  smode ? plic_get_hart_scontext(scratch) :
> +				  plic_get_hart_mcontext(scratch),
>  			  enable, threshold, num);
>  }
>  
>  void fdt_plic_context_restore(bool smode, const u32 *enable, u32 threshold,
>  			      u32 num)
>  {
> -	u32 hartid = current_hartid();
> +	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
>  
> -	plic_context_restore(plic_hartid2data[hartid],
> -			     plic_hartid2context[hartid][smode],
> +	plic_context_restore(plic_get_hart_data_ptr(scratch),
> +			     smode ? plic_get_hart_scontext(scratch) :
> +				     plic_get_hart_mcontext(scratch),
>  			     enable, threshold, num);
>  }
>  
>  static int irqchip_plic_warm_init(void)
>  {
> -	u32 hartid = current_hartid();
> +	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
>  
> -	return plic_warm_irqchip_init(plic_hartid2data[hartid],
> -				      plic_hartid2context[hartid][0],
> -				      plic_hartid2context[hartid][1]);
> +	return plic_warm_irqchip_init(plic_get_hart_data_ptr(scratch),
> +				      plic_get_hart_mcontext(scratch),
> +				      plic_get_hart_scontext(scratch));
>  }
>  
>  static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
> @@ -67,6 +110,7 @@ static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
>  {
>  	const fdt32_t *val;
>  	u32 phandle, hwirq, hartid;
> +	struct sbi_scratch *scratch;
>  	int i, err, count, cpu_offset, cpu_intc_offset;
>  
>  	val = fdt_getprop(fdt, nodeoff, "interrupts-extended", &count);
> @@ -90,16 +134,17 @@ static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
>  		if (err)
>  			continue;
>  
> -		if (SBI_HARTMASK_MAX_BITS <= hartid)
> +		scratch = sbi_hartid_to_scratch(hartid);
> +		if (!scratch)
>  			continue;
>  
> -		plic_hartid2data[hartid] = pd;
> +		plic_set_hart_data_ptr(scratch, pd);
>  		switch (hwirq) {
>  		case IRQ_M_EXT:
> -			plic_hartid2context[hartid][0] = i / 2;
> +			plic_set_hart_mcontext(scratch, i / 2);
>  			break;
>  		case IRQ_S_EXT:
> -			plic_hartid2context[hartid][1] = i / 2;
> +			plic_set_hart_scontext(scratch, i / 2);
>  			break;
>  		}
>  	}
> @@ -113,6 +158,24 @@ static int irqchip_plic_cold_init(void *fdt, int nodeoff,
>  	int rc;
>  	struct plic_data *pd;
>  
> +	if (!plic_ptr_offset) {
> +		plic_ptr_offset = sbi_scratch_alloc_offset(sizeof(ulong));
> +		if (!plic_ptr_offset)
> +			return SBI_ENOMEM;
> +	}
> +
> +	if (!plic_mcontext_offset) {
> +		plic_mcontext_offset = sbi_scratch_alloc_offset(sizeof(long));
> +		if (!plic_mcontext_offset)
> +			return SBI_ENOMEM;
> +	}
> +
> +	if (!plic_scontext_offset) {
> +		plic_scontext_offset = sbi_scratch_alloc_offset(sizeof(long));
> +		if (!plic_scontext_offset)
> +			return SBI_ENOMEM;
> +	}
> +
>  	pd = sbi_zalloc(sizeof(*pd));
>  	if (!pd)
>  		return SBI_ENOMEM;
> @@ -152,7 +215,8 @@ static void thead_plic_plat_init(struct plic_data *pd)
>  
>  void thead_plic_restore(void)
>  {
> -	struct plic_data *plic = plic_hartid2data[current_hartid()];
> +	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> +	struct plic_data *plic = plic_get_hart_data_ptr(scratch);
>  
>  	thead_plic_plat_init(plic);
>  }
> -- 
> 2.34.1
>

Otherwise,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 16/17] lib: utils/irqchip: Don't check hartid in imsic_update_hartid_table()

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:29PM +0530, Anup Patel wrote:
> The imsic_map_hartid_to_data() already checks hartid before using
> so we don't need to check in imsic_update_hartid_table().
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/utils/irqchip/fdt_irqchip_imsic.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/lib/utils/irqchip/fdt_irqchip_imsic.c b/lib/utils/irqchip/fdt_irqchip_imsic.c
> index d890400..574cadc 100644
> --- a/lib/utils/irqchip/fdt_irqchip_imsic.c
> +++ b/lib/utils/irqchip/fdt_irqchip_imsic.c
> @@ -12,7 +12,6 @@
>  #include <sbi/riscv_asm.h>
>  #include <sbi/sbi_error.h>
>  #include <sbi/sbi_heap.h>
> -#include <sbi/sbi_hartmask.h>
>  #include <sbi_utils/fdt/fdt_helper.h>
>  #include <sbi_utils/irqchip/fdt_irqchip.h>
>  #include <sbi_utils/irqchip/imsic.h>
> @@ -44,8 +43,6 @@ static int irqchip_imsic_update_hartid_table(void *fdt, int nodeoff,
>  		err = fdt_parse_hart_id(fdt, cpu_offset, &hartid);
>  		if (err)
>  			return SBI_EINVAL;
> -		if (SBI_HARTMASK_MAX_BITS <= hartid)
> -			return SBI_EINVAL;
>  
>  		switch (hwirq) {
>  		case IRQ_M_EXT:
> -- 
> 2.34.1
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 17/17] lib: utils/irqchip: Use scratch space to save per-HART IMSIC pointer

[Andrew Jones]

On Tue, Apr 25, 2023 at 06:02:30PM +0530, Anup Patel wrote:
> Instead of using a global array indexed by hartid, we should use
> scratch space to save per-HART IMSIC pointer and IMSIC file number.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/utils/irqchip/fdt_irqchip_imsic.c |  4 +-
>  lib/utils/irqchip/imsic.c             | 87 ++++++++++++++++++++++-----
>  2 files changed, 74 insertions(+), 17 deletions(-)
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 02/17] lib: sbi: Introduce simple heap allocator

[Anup Patel]

On Wed, May 31, 2023 at 5:41?PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Tue, Apr 25, 2023 at 06:02:15PM +0530, Anup Patel wrote:
> > We provide simple heap allocator to manage the heap space provided
> > by OpenSBI firmware and platform.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  include/sbi/sbi_heap.h |  44 +++++++++
> >  lib/sbi/objects.mk     |   1 +
> >  lib/sbi/sbi_heap.c     | 204 +++++++++++++++++++++++++++++++++++++++++
> >  lib/sbi/sbi_init.c     |  15 +++
> >  4 files changed, 264 insertions(+)
> >  create mode 100644 include/sbi/sbi_heap.h
> >  create mode 100644 lib/sbi/sbi_heap.c
> >
> > diff --git a/include/sbi/sbi_heap.h b/include/sbi/sbi_heap.h
> > new file mode 100644
> > index 0000000..88d176e
> > --- /dev/null
> > +++ b/include/sbi/sbi_heap.h
> > @@ -0,0 +1,44 @@
> > +/*
> > + * SPDX-License-Identifier: BSD-2-Clause
> > + *
> > + * Copyright (c) 2023 Ventana Micro Systems Inc.
> > + *
> > + * Authors:
> > + *   Anup Patel<apatel@ventanamicro.com>
> > + */
> > +
> > +#ifndef __SBI_HEAP_H__
> > +#define __SBI_HEAP_H__
> > +
> > +#include <sbi/sbi_types.h>
> > +
> > +struct sbi_scratch;
> > +
> > +/** Allocate from heap area */
> > +void *sbi_malloc(size_t size);
> > +
> > +/** Zero allocate from heap area */
> > +void *sbi_zalloc(size_t size);
> > +
> > +/** Allocate array from heap area */
> > +static inline void *sbi_calloc(size_t nitems, size_t size)
> > +{
> > +     return sbi_zalloc(nitems * size);
> > +}
> > +
> > +/** Free-up to heap area */
> > +void sbi_free(void *ptr);
> > +
> > +/** Amount (in bytes) of free space in the heap area */
> > +unsigned long sbi_heap_free_space(void);
> > +
> > +/** Amount (in bytes) of used space in the heap area */
> > +unsigned long sbi_heap_used_space(void);
> > +
> > +/** Amount (in bytes) of reserved space in the heap area */
> > +unsigned long sbi_heap_reserved_space(void);
> > +
> > +/** Initialize heap area */
> > +int sbi_heap_init(struct sbi_scratch *scratch);
> > +
> > +#endif
> > diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
> > index 7d691c6..c699187 100644
> > --- a/lib/sbi/objects.mk
> > +++ b/lib/sbi/objects.mk
> > @@ -59,6 +59,7 @@ libsbi-objs-y += sbi_domain.o
> >  libsbi-objs-y += sbi_emulate_csr.o
> >  libsbi-objs-y += sbi_fifo.o
> >  libsbi-objs-y += sbi_hart.o
> > +libsbi-objs-y += sbi_heap.o
> >  libsbi-objs-y += sbi_math.o
> >  libsbi-objs-y += sbi_hfence.o
> >  libsbi-objs-y += sbi_hsm.o
> > diff --git a/lib/sbi/sbi_heap.c b/lib/sbi/sbi_heap.c
> > new file mode 100644
> > index 0000000..82aa1ec
> > --- /dev/null
> > +++ b/lib/sbi/sbi_heap.c
> > @@ -0,0 +1,204 @@
> > +/*
> > + * SPDX-License-Identifier: BSD-2-Clause
> > + *
> > + * Copyright (c) 2023 Ventana Micro Systems Inc.
> > + *
> > + * Authors:
> > + *   Anup Patel<apatel@ventanamicro.com>
> > + */
> > +
> > +#include <sbi/riscv_locks.h>
> > +#include <sbi/sbi_error.h>
> > +#include <sbi/sbi_heap.h>
> > +#include <sbi/sbi_list.h>
> > +#include <sbi/sbi_scratch.h>
> > +#include <sbi/sbi_string.h>
> > +
> > +#define HEAP_BASE_ALIGN                      1024
> > +#define HEAP_ALLOC_ALIGN             64
>
> What's the rationale for these alignments. Maybe alloc-align is expected
> cache size? But what about base-align? Can you add comment explaining
> them?

The HEAP_BASE_ALIGN is used to ensure that the base address
and size of the heap is 1KB aligned.

Also, the HEAP_ALLOC_ALIGN should be roughly the same as
cache-line size. For now this is fixed, but in the future, platform
might be able to provide cache line size.

Okay, I will add comments.

>
> > +#define HEAP_HOUSEKEEPING_FACTOR     16
> > +
> > +struct heap_node {
> > +     struct sbi_dlist head;
> > +     unsigned long addr;
> > +     unsigned long size;
> > +};
> > +
> > +struct heap_control {
> > +     spinlock_t lock;
> > +     unsigned long base;
> > +     unsigned long size;
> > +     unsigned long hkbase;
> > +     unsigned long hksize;
> > +     struct sbi_dlist free_node_list;
> > +     struct sbi_dlist free_space_list;
> > +     struct sbi_dlist used_space_list;
> > +};
> > +
> > +static struct heap_control hpctrl;
> > +
> > +void *sbi_malloc(size_t size)
> > +{
> > +     void *ret = NULL;
> > +     struct heap_node *n, *np;
> > +
> > +     if (!size)
> > +             return NULL;
> > +
> > +     size += HEAP_ALLOC_ALIGN - 1;
> > +     size &= ~((unsigned long)HEAP_ALLOC_ALIGN - 1);
> > +
> > +     spin_lock(&hpctrl.lock);
> > +
> > +     np = NULL;
> > +     sbi_list_for_each_entry(n, &hpctrl.free_space_list, head) {
> > +             if (size <= n->size) {
> > +                     np = n;
> > +                     break;
> > +             }
> > +     }
> > +     if (np) {
> > +             if ((size < np->size) &&
> > +                 !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);
> > +                     ret = (void *)n->addr;
> > +             } else if (size == np->size) {
> > +                     sbi_list_del(&np->head);
> > +                     sbi_list_add_tail(&np->head, &hpctrl.used_space_list);
> > +                     ret = (void *)np->addr;
> > +             }
> > +     }
> > +
> > +     spin_unlock(&hpctrl.lock);
> > +
> > +     return ret;
> > +}
> > +
> > +void *sbi_zalloc(size_t size)
> > +{
> > +     void *ret = sbi_malloc(size);
> > +
> > +     if (ret)
> > +             sbi_memset(ret, 0, size);
> > +     return ret;
> > +}
> > +
> > +void sbi_free(void *ptr)
> > +{
> > +     struct heap_node *n, *np;
> > +
> > +     if (!ptr)
> > +             return;
> > +
> > +     spin_lock(&hpctrl.lock);
> > +
> > +     np = NULL;
> > +     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;
> > +                     break;
> > +             }
> > +     }
> > +     if (!np) {
> > +             spin_unlock(&hpctrl.lock);
> > +             return;
> > +     }
> > +
> > +     sbi_list_del(&np->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);
> > +                     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);
> > +                     np = NULL;
> > +                     break;
> > +             } else if ((n->addr + n->size) < np->addr) {
> > +                     sbi_list_add(&np->head, &n->head);
> > +                     np = NULL;
> > +                     break;
> > +             }
> > +     }
> > +     if (np)
> > +             sbi_list_add_tail(&np->head, &hpctrl.free_space_list);
> > +
> > +     spin_unlock(&hpctrl.lock);
> > +}
> > +
> > +unsigned long sbi_heap_free_space(void)
> > +{
> > +     struct heap_node *n;
> > +     unsigned long ret = 0;
> > +
> > +     spin_lock(&hpctrl.lock);
> > +     sbi_list_for_each_entry(n, &hpctrl.free_space_list, head)
> > +             ret += n->size;
> > +     spin_unlock(&hpctrl.lock);
> > +
> > +     return ret;
> > +}
> > +
> > +unsigned long sbi_heap_used_space(void)
> > +{
> > +     return hpctrl.size - hpctrl.hksize - sbi_heap_free_space();
> > +}
> > +
> > +unsigned long sbi_heap_reserved_space(void)
> > +{
> > +     return hpctrl.hksize;
> > +}
> > +
> > +int sbi_heap_init(struct sbi_scratch *scratch)
> > +{
> > +     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);
> > +
> > +     /* Prepare free node list */
> > +     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);
> > +     }
> > +
> > +     /* Prepare free space 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);
> > +
> > +     return 0;
> > +}
> > diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> > index 5db8e7f..f09a7ac 100644
> > --- a/lib/sbi/sbi_init.c
> > +++ b/lib/sbi/sbi_init.c
> > @@ -17,6 +17,7 @@
> >  #include <sbi/sbi_ecall.h>
> >  #include <sbi/sbi_hart.h>
> >  #include <sbi/sbi_hartmask.h>
> > +#include <sbi/sbi_heap.h>
> >  #include <sbi/sbi_hsm.h>
> >  #include <sbi/sbi_ipi.h>
> >  #include <sbi/sbi_irqchip.h>
> > @@ -118,6 +119,15 @@ static void sbi_boot_print_general(struct sbi_scratch *scratch)
> >       sbi_printf("Firmware Size             : %d KB\n",
> >                  (u32)(scratch->fw_size / 1024));
> >       sbi_printf("Firmware RW Offset        : 0x%lx\n", scratch->fw_rw_offset);
> > +     sbi_printf("Firmware RW Size          : %d KB\n",
> > +                (u32)((scratch->fw_size - scratch->fw_rw_offset) / 1024));
> > +     sbi_printf("Firmware Heap Offset      : 0x%lx\n", scratch->fw_heap_offset);
> > +     sbi_printf("Firmware Heap Size        : "
> > +                "%d KB (total), %d KB (reserved), %d KB (used), %d KB (free)\n",
> > +                (u32)(scratch->fw_heap_size / 1024),
> > +                (u32)(sbi_heap_reserved_space() / 1024),
> > +                (u32)(sbi_heap_used_space() / 1024),
> > +                (u32)(sbi_heap_free_space() / 1024));
> >
> >       /* SBI details */
> >       sbi_printf("Runtime SBI Version       : %d.%d\n",
> > @@ -258,6 +268,11 @@ static void __noreturn init_coldboot(struct sbi_scratch *scratch, u32 hartid)
> >               sbi_hart_hang();
> >
> >       /* Note: This has to be second thing in coldboot init sequence */
> > +     rc = sbi_heap_init(scratch);
> > +     if (rc)
> > +             sbi_hart_hang();
> > +
> > +     /* Note: This has to be thing thing in coldboot init sequence */
>
> ...be the third thing...

Okay, I will update.

>
> >       rc = sbi_domain_init(scratch, hartid);
> >       if (rc)
> >               sbi_hart_hang();
> > --
> > 2.34.1
> >
>
> Otherwise,
>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>
> Thanks,
> drew

Regards,
Anup

[PATCH 06/17] lib: sbi: Use scratch space to save per-HART domain pointer

[Anup Patel]

On Wed, May 31, 2023 at 6:09?PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Tue, Apr 25, 2023 at 06:02:19PM +0530, Anup Patel wrote:
> > Instead of using a global array indexed by hartid, we should use
> > scratch space to save per-HART domain pointer.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  include/sbi/sbi_domain.h |  6 +-----
> >  lib/sbi/sbi_domain.c     | 38 +++++++++++++++++++++++++++++++++++---
> >  2 files changed, 36 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h
> > index 124ea90..b05bcf4 100644
> > --- a/include/sbi/sbi_domain.h
> > +++ b/include/sbi/sbi_domain.h
> > @@ -129,12 +129,8 @@ struct sbi_domain {
> >  /** The root domain instance */
> >  extern struct sbi_domain root;
> >
> > -/** HART id to domain table */
> > -extern struct sbi_domain *hartid_to_domain_table[];
> > -
> >  /** Get pointer to sbi_domain from HART id */
> > -#define sbi_hartid_to_domain(__hartid) \
> > -     hartid_to_domain_table[__hartid]
> > +struct sbi_domain *sbi_hartid_to_domain(u32 hartid);
> >
> >  /** Get pointer to sbi_domain for current HART */
> >  #define sbi_domain_thishart_ptr() \
> > diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
> > index b07d310..531737c 100644
> > --- a/lib/sbi/sbi_domain.c
> > +++ b/lib/sbi/sbi_domain.c
> > @@ -23,7 +23,6 @@
> >   * the array to be null-terminated.
> >   */
> >  struct sbi_domain *domidx_to_domain_table[SBI_DOMAIN_MAX_INDEX + 1] = { 0 };
> > -struct sbi_domain *hartid_to_domain_table[SBI_HARTMASK_MAX_BITS] = { 0 };
> >  static u32 domain_count = 0;
> >  static bool domain_finalized = false;
> >
> > @@ -39,6 +38,32 @@ struct sbi_domain root = {
> >       .fw_region_inited = false,
> >  };
> >
> > +static unsigned long domain_hart_ptr_offset;
> > +
> > +struct sbi_domain *sbi_hartid_to_domain(u32 hartid)
> > +{
> > +     struct sbi_scratch *scratch;
> > +
> > +     scratch = sbi_hartid_to_scratch(hartid);
> > +     if (!scratch || !domain_hart_ptr_offset)
> > +             return NULL;
> > +
> > +     return (void *)(*((ulong *)sbi_scratch_offset_ptr(scratch,
> > +                                                domain_hart_ptr_offset)));
> > +}
> > +
> > +static void update_hartid_to_domain(u32 hartid, struct sbi_domain *dom)
> > +{
> > +     struct sbi_scratch *scratch;
> > +
> > +     scratch = sbi_hartid_to_scratch(hartid);
> > +     if (!scratch)
> > +             return;
> > +
> > +     *((ulong *)sbi_scratch_offset_ptr(scratch, domain_hart_ptr_offset))
> > +                                     = (ulong)dom;
> > +}
> > +
> >  bool sbi_domain_is_assigned_hart(const struct sbi_domain *dom, u32 hartid)
> >  {
> >       if (dom)
> > @@ -519,11 +544,11 @@ int sbi_domain_register(struct sbi_domain *dom,
> >               if (!sbi_hartmask_test_hart(i, dom->possible_harts))
> >                       continue;
> >
> > -             tdom = hartid_to_domain_table[i];
> > +             tdom = sbi_hartid_to_domain(i);
> >               if (tdom)
> >                       sbi_hartmask_clear_hart(i,
> >                                       &tdom->assigned_harts);
> > -             hartid_to_domain_table[i] = dom;
> > +             update_hartid_to_domain(i, dom);
> >               sbi_hartmask_set_hart(i, &dom->assigned_harts);
> >
> >               /*
> > @@ -715,9 +740,14 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
> >               return SBI_EINVAL;
> >       }
> >
> > +     domain_hart_ptr_offset = sbi_scratch_alloc_offset(sizeof(ulong));
> > +     if (!domain_hart_ptr_offset)
> > +             return SBI_ENOMEM;
> > +
> >       root_memregs = sbi_calloc(sizeof(*root_memregs), ROOT_REGION_MAX + 1);
> >       if (!root_memregs) {
> >               sbi_printf("%s: no memory for root regions\n", __func__);
> > +             sbi_scratch_free_offset(domain_hart_ptr_offset);
> >               return SBI_ENOMEM;
> >       }
> >       root.regions = root_memregs;
> > @@ -726,6 +756,7 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
> >       if (!root_hmask) {
> >               sbi_printf("%s: no memory for root hartmask\n", __func__);
> >               sbi_free(root_memregs);
> > +             sbi_scratch_free_offset(domain_hart_ptr_offset);
> >               return SBI_ENOMEM;
> >       }
> >       root.possible_harts = root_hmask;
> > @@ -774,6 +805,7 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
> >       if (rc) {
> >               sbi_free(root_hmask);
> >               sbi_free(root_memregs);
> > +             sbi_scratch_free_offset(domain_hart_ptr_offset);
> >               return rc;
> >       }
>
> Rather than duplicating all the free()'s we could use a sequence of error
> labels and gotos.

Okay, I will update.

>
> Otherwise,
>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>
> Thanks,
> drew

Regards,
Anup

[PATCH 10/17] lib: utils/irqchip: Use heap in PLIC, APLIC and IMSIC drivers

[Anup Patel]

On Wed, May 31, 2023 at 6:16?PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Tue, Apr 25, 2023 at 06:02:23PM +0530, Anup Patel wrote:
> > Let's use heap allocation in PLIC, APLIC, and IMSIC irqchip drivers
> > instead of using a fixed size global array.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  lib/utils/irqchip/fdt_irqchip_aplic.c | 24 ++++++++++--------
> >  lib/utils/irqchip/fdt_irqchip_imsic.c | 30 +++++++++++-----------
> >  lib/utils/irqchip/fdt_irqchip_plic.c  | 36 +++++++++++++--------------
> >  3 files changed, 47 insertions(+), 43 deletions(-)
> >
> > diff --git a/lib/utils/irqchip/fdt_irqchip_aplic.c b/lib/utils/irqchip/fdt_irqchip_aplic.c
> > index 965f023..caa92a7 100644
> > --- a/lib/utils/irqchip/fdt_irqchip_aplic.c
> > +++ b/lib/utils/irqchip/fdt_irqchip_aplic.c
> > @@ -11,15 +11,11 @@
> >  #include <libfdt.h>
> >  #include <sbi/riscv_asm.h>
> >  #include <sbi/sbi_error.h>
> > +#include <sbi/sbi_heap.h>
> >  #include <sbi_utils/fdt/fdt_helper.h>
> >  #include <sbi_utils/irqchip/fdt_irqchip.h>
> >  #include <sbi_utils/irqchip/aplic.h>
> >
> > -#define APLIC_MAX_NR                 16
> > -
> > -static unsigned long aplic_count = 0;
> > -static struct aplic_data aplic[APLIC_MAX_NR];
> > -
> >  static int irqchip_aplic_warm_init(void)
> >  {
> >       /* Nothing to do here. */
> > @@ -32,15 +28,23 @@ static int irqchip_aplic_cold_init(void *fdt, int nodeoff,
> >       int rc;
> >       struct aplic_data *pd;
> >
> > -     if (APLIC_MAX_NR <= aplic_count)
> > -             return SBI_ENOSPC;
> > -     pd = &aplic[aplic_count++];
> > +     pd = sbi_zalloc(sizeof(*pd));
> > +     if (!pd)
> > +             return SBI_ENOMEM;
> >
> >       rc = fdt_parse_aplic_node(fdt, nodeoff, pd);
> > -     if (rc)
> > +     if (rc) {
> > +             sbi_free(pd);
> >               return rc;
> > +     }
> >
> > -     return aplic_cold_irqchip_init(pd);
> > +     rc = aplic_cold_irqchip_init(pd);
> > +     if (rc) {
> > +             sbi_free(pd);
> > +             return rc;
> > +     }
> > +
> > +     return 0;
> >  }
> >
> >  static const struct fdt_match irqchip_aplic_match[] = {
> > diff --git a/lib/utils/irqchip/fdt_irqchip_imsic.c b/lib/utils/irqchip/fdt_irqchip_imsic.c
> > index 6020ac0..d890400 100644
> > --- a/lib/utils/irqchip/fdt_irqchip_imsic.c
> > +++ b/lib/utils/irqchip/fdt_irqchip_imsic.c
> > @@ -11,16 +11,12 @@
> >  #include <libfdt.h>
> >  #include <sbi/riscv_asm.h>
> >  #include <sbi/sbi_error.h>
> > +#include <sbi/sbi_heap.h>
> >  #include <sbi/sbi_hartmask.h>
> >  #include <sbi_utils/fdt/fdt_helper.h>
> >  #include <sbi_utils/irqchip/fdt_irqchip.h>
> >  #include <sbi_utils/irqchip/imsic.h>
> >
> > -#define IMSIC_MAX_NR                 16
> > -
> > -static unsigned long imsic_count = 0;
> > -static struct imsic_data imsic[IMSIC_MAX_NR];
> > -
> >  static int irqchip_imsic_update_hartid_table(void *fdt, int nodeoff,
> >                                            struct imsic_data *id)
> >  {
> > @@ -71,25 +67,31 @@ static int irqchip_imsic_cold_init(void *fdt, int nodeoff,
> >       int rc;
> >       struct imsic_data *id;
> >
> > -     if (IMSIC_MAX_NR <= imsic_count)
> > -             return SBI_ENOSPC;
> > -     id = &imsic[imsic_count];
> > +     id = sbi_zalloc(sizeof(*id));
> > +     if (!id)
> > +             return SBI_ENOMEM;
> >
> >       rc = fdt_parse_imsic_node(fdt, nodeoff, id);
> > -     if (rc)
> > +     if (rc) {
> > +             sbi_free(id);
> >               return rc;
> > -     if (!id->targets_mmode)
> > +     }
> > +     if (!id->targets_mmode) {
> > +             sbi_free(id);
> >               return 0;
> > +     }
> >
> >       rc = irqchip_imsic_update_hartid_table(fdt, nodeoff, id);
> > -     if (rc)
> > +     if (rc) {
> > +             sbi_free(id);
> >               return rc;
> > +     }
> >
> >       rc = imsic_cold_irqchip_init(id);
> > -     if (rc)
> > +     if (rc) {
> > +             sbi_free(id);
> >               return rc;
> > -
> > -     imsic_count++;
> > +     }
>
> Could have a label at the bottom
>
>     return 0;
> error:
>     sbi_free(id);
>     return rc;
>
> Just need to set rc=0 before the id->targets_mmode test and everything can
> goto error rather than needing to remember to free at each point.

Okay, I will update.

>
> >
> >       return 0;
> >  }
> > diff --git a/lib/utils/irqchip/fdt_irqchip_plic.c b/lib/utils/irqchip/fdt_irqchip_plic.c
> > index 1aadf91..605f44a 100644
> > --- a/lib/utils/irqchip/fdt_irqchip_plic.c
> > +++ b/lib/utils/irqchip/fdt_irqchip_plic.c
> > @@ -11,18 +11,14 @@
> >  #include <sbi/riscv_asm.h>
> >  #include <sbi/riscv_io.h>
> >  #include <sbi/sbi_error.h>
> > +#include <sbi/sbi_heap.h>
> >  #include <sbi/sbi_hartmask.h>
> >  #include <sbi_utils/fdt/fdt_helper.h>
> >  #include <sbi_utils/irqchip/fdt_irqchip.h>
> >  #include <sbi_utils/irqchip/plic.h>
> >
> > -#define PLIC_MAX_NR                  16
> > -
> > -static unsigned long plic_count = 0;
> > -static struct plic_data plic[PLIC_MAX_NR];
> > -
> >  static struct plic_data *plic_hartid2data[SBI_HARTMASK_MAX_BITS];
> > -static int plic_hartid2context[SBI_HARTMASK_MAX_BITS][2];
> > +static int plic_hartid2context[SBI_HARTMASK_MAX_BITS][2] = { { -1 } };
> >
> >  void fdt_plic_priority_save(u8 *priority, u32 num)
> >  {
> > @@ -114,16 +110,18 @@ static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
> >  static int irqchip_plic_cold_init(void *fdt, int nodeoff,
> >                                 const struct fdt_match *match)
> >  {
> > -     int i, rc;
> > +     int rc;
> >       struct plic_data *pd;
> >
> > -     if (PLIC_MAX_NR <= plic_count)
> > -             return SBI_ENOSPC;
> > -     pd = &plic[plic_count++];
> > +     pd = sbi_zalloc(sizeof(*pd));
> > +     if (!pd)
> > +             return SBI_ENOMEM;
> >
> >       rc = fdt_parse_plic_node(fdt, nodeoff, pd);
> > -     if (rc)
> > +     if (rc) {
> > +             sbi_free(pd);
> >               return rc;
> > +     }
> >
> >       if (match->data) {
> >               void (*plic_plat_init)(struct plic_data *) = match->data;
> > @@ -131,18 +129,18 @@ static int irqchip_plic_cold_init(void *fdt, int nodeoff,
> >       }
> >
> >       rc = plic_cold_irqchip_init(pd);
> > -     if (rc)
> > +     if (rc) {
> > +             sbi_free(pd);
> >               return rc;
> > +     }
> >
> > -     if (plic_count == 1) {
> > -             for (i = 0; i < SBI_HARTMASK_MAX_BITS; i++) {
> > -                     plic_hartid2data[i] = NULL;
> > -                     plic_hartid2context[i][0] = -1;
> > -                     plic_hartid2context[i][1] = -1;
> > -             }
> > +     rc = irqchip_plic_update_hartid_table(fdt, nodeoff, pd);
> > +     if (rc) {
> > +             sbi_free(pd);
> > +             return rc;
> >       }
> >
> > -     return irqchip_plic_update_hartid_table(fdt, nodeoff, pd);
> > +     return 0;
>
> Same comment as above.

Okay, I will update.

>
> >  }
> >
> >  #define THEAD_PLIC_CTRL_REG 0x1ffffc
> > --
> > 2.34.1
> >
>
> Otherwise,
>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>
> Thanks,
> drew

Regards,
Anup

[PATCH 12/17] lib: utils/fdt: Use heap in FDT domain parsing

[Anup Patel]

On Wed, May 31, 2023 at 6:32?PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Tue, Apr 25, 2023 at 06:02:25PM +0530, Anup Patel wrote:
> > Let's use heap allocation in FDT domain parsing instead of using
> > a fixed size global array.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  lib/utils/fdt/fdt_domain.c | 111 +++++++++++++++++++++++--------------
> >  1 file changed, 68 insertions(+), 43 deletions(-)
> >
> > diff --git a/lib/utils/fdt/fdt_domain.c b/lib/utils/fdt/fdt_domain.c
> > index bb6d17d..c4aed4d 100644
> > --- a/lib/utils/fdt/fdt_domain.c
> > +++ b/lib/utils/fdt/fdt_domain.c
> > @@ -13,6 +13,7 @@
> >  #include <sbi/sbi_domain.h>
> >  #include <sbi/sbi_error.h>
> >  #include <sbi/sbi_hartmask.h>
> > +#include <sbi/sbi_heap.h>
> >  #include <sbi/sbi_scratch.h>
> >  #include <sbi_utils/fdt/fdt_domain.h>
> >  #include <sbi_utils/fdt/fdt_helper.h>
> > @@ -219,14 +220,11 @@ skip_device_disable:
> >       fdt_nop_node(fdt, poffset);
> >  }
> >
> > -#define FDT_DOMAIN_MAX_COUNT         8
> > -#define FDT_DOMAIN_REGION_MAX_COUNT  16
>
> Why not keep this define since we still use 16 below when allocating
> dom->regions / max_regions?

Okay, I will keep FDT_DOMAIN_REGION_MAX_COUNT

>
> > -
> > -static u32 fdt_domains_count;
> > -static struct sbi_domain fdt_domains[FDT_DOMAIN_MAX_COUNT];
> > -static struct sbi_hartmask fdt_masks[FDT_DOMAIN_MAX_COUNT];
> > -static struct sbi_domain_memregion
> > -     fdt_regions[FDT_DOMAIN_MAX_COUNT][FDT_DOMAIN_REGION_MAX_COUNT + 1];
> > +struct parse_region_data {
> > +     struct sbi_domain *dom;
> > +     u32 region_count;
> > +     u32 max_regions;
> > +};
> >
> >  static int __fdt_parse_region(void *fdt, int domain_offset,
> >                             int region_offset, u32 region_access,
> > @@ -236,7 +234,7 @@ static int __fdt_parse_region(void *fdt, int domain_offset,
> >       u32 val32;
> >       u64 val64;
> >       const u32 *val;
> > -     u32 *region_count = opaque;
> > +     struct parse_region_data *preg = opaque;
> >       struct sbi_domain_memregion *region;
> >
> >       /*
> > @@ -252,9 +250,9 @@ static int __fdt_parse_region(void *fdt, int domain_offset,
> >               return SBI_EINVAL;
> >
> >       /* Find next region of the domain */
> > -     if (FDT_DOMAIN_REGION_MAX_COUNT <= *region_count)
> > -             return SBI_EINVAL;
> > -     region = &fdt_regions[fdt_domains_count][*region_count];
> > +     if (preg->max_regions <= preg->region_count)
> > +             return SBI_ENOSPC;
> > +     region = &preg->dom->regions[preg->region_count];
> >
> >       /* Read "base" DT property */
> >       val = fdt_getprop(fdt, region_offset, "base", &len);
> > @@ -278,7 +276,7 @@ static int __fdt_parse_region(void *fdt, int domain_offset,
> >       if (fdt_get_property(fdt, region_offset, "mmio", NULL))
> >               region->flags |= SBI_DOMAIN_MEMREGION_MMIO;
> >
> > -     (*region_count)++;
> > +     preg->region_count++;
> >
> >       return 0;
> >  }
> > @@ -291,16 +289,30 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
> >       struct sbi_domain *dom;
> >       struct sbi_hartmask *mask;
> >       struct sbi_hartmask assign_mask;
> > +     struct parse_region_data preg;
> >       int *cold_domain_offset = opaque;
> > -     struct sbi_domain_memregion *reg, *regions;
> > -     int i, err, len, cpus_offset, cpu_offset, doffset;
> > +     struct sbi_domain_memregion *reg;
> > +     int i, err = 0, len, cpus_offset, cpu_offset, doffset;
> >
> > -     /* Sanity check on maximum domains we can handle */
> > -     if (FDT_DOMAIN_MAX_COUNT <= fdt_domains_count)
> > -             return SBI_EINVAL;
> > -     dom = &fdt_domains[fdt_domains_count];
> > -     mask = &fdt_masks[fdt_domains_count];
> > -     regions = &fdt_regions[fdt_domains_count][0];
> > +     dom = sbi_zalloc(sizeof(*dom));
> > +     if (!dom)
> > +             return SBI_ENOMEM;
> > +
> > +     dom->regions = sbi_calloc(sizeof(*dom->regions), 16 + 1);
> > +     if (!dom->regions) {
> > +             sbi_free(dom);
> > +             return SBI_ENOMEM;
> > +     }
> > +     preg.dom = dom;
> > +     preg.region_count = 0;
> > +     preg.max_regions = 16;
> > +
> > +     mask = sbi_zalloc(sizeof(*mask));
> > +     if (!mask) {
> > +             sbi_free(dom->regions);
> > +             sbi_free(dom);
> > +             return SBI_ENOMEM;
> > +     }
>
> Can add two more labels above fail_free_all to use in above two failure
> cases as well.

Sure, will do.

>
> >
> >       /* Read DT node name */
> >       strncpy(dom->name, fdt_get_name(fdt, domain_offset, NULL),
> > @@ -316,12 +328,14 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
> >               for (i = 0; i < len; i++) {
> >                       cpu_offset = fdt_node_offset_by_phandle(fdt,
> >                                                       fdt32_to_cpu(val[i]));
> > -                     if (cpu_offset < 0)
> > -                             return cpu_offset;
> > +                     if (cpu_offset < 0) {
> > +                             err = cpu_offset;
> > +                             goto fail_free_all;
> > +                     }
> >
> >                       err = fdt_parse_hart_id(fdt, cpu_offset, &val32);
> >                       if (err)
> > -                             return err;
> > +                             goto fail_free_all;
> >
> >                       if (!fdt_node_is_enabled(fdt, cpu_offset))
> >                               continue;
> > @@ -331,14 +345,10 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
> >       }
> >
> >       /* Setup memregions from DT */
> > -     val32 = 0;
> > -     memset(regions, 0,
> > -                sizeof(*regions) * (FDT_DOMAIN_REGION_MAX_COUNT + 1));
> > -     dom->regions = regions;
> > -     err = fdt_iterate_each_memregion(fdt, domain_offset, &val32,
> > +     err = fdt_iterate_each_memregion(fdt, domain_offset, &preg,
> >                                        __fdt_parse_region);
> >       if (err)
> > -             return err;
> > +             goto fail_free_all;
> >
> >       /*
> >        * Copy over root domain memregions which don't allow
> > @@ -354,9 +364,11 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
> >                   (reg->flags & SBI_DOMAIN_MEMREGION_SU_WRITABLE) ||
> >                   (reg->flags & SBI_DOMAIN_MEMREGION_SU_EXECUTABLE))
> >                       continue;
> > -             if (FDT_DOMAIN_REGION_MAX_COUNT <= val32)
> > -                     return SBI_EINVAL;
> > -             memcpy(&regions[val32++], reg, sizeof(*reg));
> > +             if (preg.max_regions <= preg.region_count) {
> > +                     err = SBI_EINVAL;
> > +                     goto fail_free_all;
> > +             }
> > +             memcpy(&dom->regions[preg.region_count++], reg, sizeof(*reg));
> >       }
> >       dom->fw_region_inited = root.fw_region_inited;
> >
> > @@ -427,8 +439,10 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
> >
> >       /* Find /cpus DT node */
> >       cpus_offset = fdt_path_offset(fdt, "/cpus");
> > -     if (cpus_offset < 0)
> > -             return cpus_offset;
> > +     if (cpus_offset < 0) {
> > +             err = cpus_offset;
> > +             goto fail_free_all;
> > +     }
> >
> >       /* HART to domain assignment mask based on CPU DT nodes */
> >       sbi_hartmask_clear_all(&assign_mask);
> > @@ -444,22 +458,33 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
> >                       continue;
> >
> >               val = fdt_getprop(fdt, cpu_offset, "opensbi-domain", &len);
> > -             if (!val || len < 4)
> > -                     return SBI_EINVAL;
> > +             if (!val || len < 4) {
> > +                     err = SBI_EINVAL;
> > +                     goto fail_free_all;
> > +             }
> >
> >               doffset = fdt_node_offset_by_phandle(fdt, fdt32_to_cpu(*val));
> > -             if (doffset < 0)
> > -                     return doffset;
> > +             if (doffset < 0) {
> > +                     err = doffset;
> > +                     goto fail_free_all;
> > +             }
> >
> >               if (doffset == domain_offset)
> >                       sbi_hartmask_set_hart(val32, &assign_mask);
> >       }
> >
> > -     /* Increment domains count */
> > -     fdt_domains_count++;
> > -
> >       /* Register the domain */
> > -     return sbi_domain_register(dom, &assign_mask);
> > +     err = sbi_domain_register(dom, &assign_mask);
> > +     if (err)
> > +             goto fail_free_all;
> > +
> > +     return 0;
> > +
> > +fail_free_all:
> > +     sbi_free(dom->regions);
> > +     sbi_free(mask);
> > +     sbi_free(dom);
> > +     return err;
> >  }
> >
> >  int fdt_domains_populate(void *fdt)
> > --
> > 2.34.1
> >
>
> Otherwise,
>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Regards,
Anup

[PATCH 15/17] lib: utils/irqchip: Use scratch space to save per-HART PLIC pointer

[Anup Patel]

On Wed, May 31, 2023 at 6:42?PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Tue, Apr 25, 2023 at 06:02:28PM +0530, Anup Patel wrote:
> > Instead of using a global array indexed by hartid, we should use
> > scratch space to save per-HART PLIC pointer and PLIC context numbers.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  lib/utils/irqchip/fdt_irqchip_plic.c | 108 +++++++++++++++++++++------
> >  1 file changed, 86 insertions(+), 22 deletions(-)
> >
> > diff --git a/lib/utils/irqchip/fdt_irqchip_plic.c b/lib/utils/irqchip/fdt_irqchip_plic.c
> > index 605f44a..eb1e412 100644
> > --- a/lib/utils/irqchip/fdt_irqchip_plic.c
> > +++ b/lib/utils/irqchip/fdt_irqchip_plic.c
> > @@ -12,54 +12,97 @@
> >  #include <sbi/riscv_io.h>
> >  #include <sbi/sbi_error.h>
> >  #include <sbi/sbi_heap.h>
> > -#include <sbi/sbi_hartmask.h>
> > +#include <sbi/sbi_scratch.h>
> >  #include <sbi_utils/fdt/fdt_helper.h>
> >  #include <sbi_utils/irqchip/fdt_irqchip.h>
> >  #include <sbi_utils/irqchip/plic.h>
> >
> > -static struct plic_data *plic_hartid2data[SBI_HARTMASK_MAX_BITS];
> > -static int plic_hartid2context[SBI_HARTMASK_MAX_BITS][2] = { { -1 } };
> > +static unsigned long plic_ptr_offset;
> > +
> > +#define plic_get_hart_data_ptr(__scratch)                            \
> > +({                                                                   \
> > +     (void *)(*((ulong *)sbi_scratch_offset_ptr((__scratch),         \
> > +                                                plic_ptr_offset)));  \
> > +})
> > +
> > +#define plic_set_hart_data_ptr(__scratch, __imsic)                   \
> > +do {                                                                 \
> > +     *((ulong *)sbi_scratch_offset_ptr((__scratch), plic_ptr_offset))\
> > +                                     = (ulong)(__imsic);             \
> > +} while (0)
> > +
> > +static unsigned long plic_mcontext_offset;
> > +
> > +#define plic_get_hart_mcontext(__scratch)                            \
> > +({                                                                   \
> > +     long __ret =                                                    \
> > +     *((long *)sbi_scratch_offset_ptr((__scratch), plic_mcontext_offset));\
> > +     (__ret - 1);                                                    \
> > +})
> > +
> > +#define plic_set_hart_mcontext(__scratch, __mcontext)                        \
> > +do {                                                                 \
> > +     *((long *)sbi_scratch_offset_ptr((__scratch), plic_mcontext_offset))\
> > +                                     = (long)(__mcontext + 1);       \
> > +} while (0)
> > +
> > +static unsigned long plic_scontext_offset;
> > +
> > +#define plic_get_hart_scontext(__scratch)                            \
> > +({                                                                   \
> > +     long __ret =                                                    \
> > +     *((long *)sbi_scratch_offset_ptr((__scratch), plic_scontext_offset));\
> > +     (__ret - 1);                                                    \
> > +})
> > +
> > +#define plic_set_hart_scontext(__scratch, __scontext)                        \
> > +do {                                                                 \
> > +     *((long *)sbi_scratch_offset_ptr((__scratch), plic_scontext_offset))\
> > +                                     = (long)(__scontext + 1);       \
> > +} while (0)
>
> I guess the +/- 1 stuff is to ensure that the m/scontext offsets can be
> tested with zero to see if they need to be allocated. It'd be nice to
> add a comment explaining it.

This +/-1 stuff is to ensure that if [ms]context is not set then
plic_get_hart_[ms]context() will return -1. This approach
exploits the fact that per-HART scratch space is initialized
to zero.

>
> >
> >  void fdt_plic_priority_save(u8 *priority, u32 num)
> >  {
> > -     struct plic_data *plic = plic_hartid2data[current_hartid()];
> > +     struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> >
> > -     plic_priority_save(plic, priority, num);
> > +     plic_priority_save(plic_get_hart_data_ptr(scratch), priority, num);
> >  }
> >
> >  void fdt_plic_priority_restore(const u8 *priority, u32 num)
> >  {
> > -     struct plic_data *plic = plic_hartid2data[current_hartid()];
> > +     struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> >
> > -     plic_priority_restore(plic, priority, num);
> > +     plic_priority_restore(plic_get_hart_data_ptr(scratch), priority, num);
> >  }
> >
> >  void fdt_plic_context_save(bool smode, u32 *enable, u32 *threshold, u32 num)
> >  {
> > -     u32 hartid = current_hartid();
> > +     struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> >
> > -     plic_context_save(plic_hartid2data[hartid],
> > -                       plic_hartid2context[hartid][smode],
> > +     plic_context_save(plic_get_hart_data_ptr(scratch),
> > +                       smode ? plic_get_hart_scontext(scratch) :
> > +                               plic_get_hart_mcontext(scratch),
> >                         enable, threshold, num);
> >  }
> >
> >  void fdt_plic_context_restore(bool smode, const u32 *enable, u32 threshold,
> >                             u32 num)
> >  {
> > -     u32 hartid = current_hartid();
> > +     struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> >
> > -     plic_context_restore(plic_hartid2data[hartid],
> > -                          plic_hartid2context[hartid][smode],
> > +     plic_context_restore(plic_get_hart_data_ptr(scratch),
> > +                          smode ? plic_get_hart_scontext(scratch) :
> > +                                  plic_get_hart_mcontext(scratch),
> >                            enable, threshold, num);
> >  }
> >
> >  static int irqchip_plic_warm_init(void)
> >  {
> > -     u32 hartid = current_hartid();
> > +     struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> >
> > -     return plic_warm_irqchip_init(plic_hartid2data[hartid],
> > -                                   plic_hartid2context[hartid][0],
> > -                                   plic_hartid2context[hartid][1]);
> > +     return plic_warm_irqchip_init(plic_get_hart_data_ptr(scratch),
> > +                                   plic_get_hart_mcontext(scratch),
> > +                                   plic_get_hart_scontext(scratch));
> >  }
> >
> >  static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
> > @@ -67,6 +110,7 @@ static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
> >  {
> >       const fdt32_t *val;
> >       u32 phandle, hwirq, hartid;
> > +     struct sbi_scratch *scratch;
> >       int i, err, count, cpu_offset, cpu_intc_offset;
> >
> >       val = fdt_getprop(fdt, nodeoff, "interrupts-extended", &count);
> > @@ -90,16 +134,17 @@ static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
> >               if (err)
> >                       continue;
> >
> > -             if (SBI_HARTMASK_MAX_BITS <= hartid)
> > +             scratch = sbi_hartid_to_scratch(hartid);
> > +             if (!scratch)
> >                       continue;
> >
> > -             plic_hartid2data[hartid] = pd;
> > +             plic_set_hart_data_ptr(scratch, pd);
> >               switch (hwirq) {
> >               case IRQ_M_EXT:
> > -                     plic_hartid2context[hartid][0] = i / 2;
> > +                     plic_set_hart_mcontext(scratch, i / 2);
> >                       break;
> >               case IRQ_S_EXT:
> > -                     plic_hartid2context[hartid][1] = i / 2;
> > +                     plic_set_hart_scontext(scratch, i / 2);
> >                       break;
> >               }
> >       }
> > @@ -113,6 +158,24 @@ static int irqchip_plic_cold_init(void *fdt, int nodeoff,
> >       int rc;
> >       struct plic_data *pd;
> >
> > +     if (!plic_ptr_offset) {
> > +             plic_ptr_offset = sbi_scratch_alloc_offset(sizeof(ulong));
> > +             if (!plic_ptr_offset)
> > +                     return SBI_ENOMEM;
> > +     }
> > +
> > +     if (!plic_mcontext_offset) {
> > +             plic_mcontext_offset = sbi_scratch_alloc_offset(sizeof(long));
> > +             if (!plic_mcontext_offset)
> > +                     return SBI_ENOMEM;
> > +     }
> > +
> > +     if (!plic_scontext_offset) {
> > +             plic_scontext_offset = sbi_scratch_alloc_offset(sizeof(long));
> > +             if (!plic_scontext_offset)
> > +                     return SBI_ENOMEM;
> > +     }
> > +
> >       pd = sbi_zalloc(sizeof(*pd));
> >       if (!pd)
> >               return SBI_ENOMEM;
> > @@ -152,7 +215,8 @@ static void thead_plic_plat_init(struct plic_data *pd)
> >
> >  void thead_plic_restore(void)
> >  {
> > -     struct plic_data *plic = plic_hartid2data[current_hartid()];
> > +     struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> > +     struct plic_data *plic = plic_get_hart_data_ptr(scratch);
> >
> >       thead_plic_plat_init(plic);
> >  }
> > --
> > 2.34.1
> >
>
> Otherwise,
>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>

Regards,
Anup

[PATCH 15/17] lib: utils/irqchip: Use scratch space to save per-HART PLIC pointer

[Jessica Clarke]

On 25 Apr 2023, at 13:32, Anup Patel <apatel@ventanamicro.com> wrote:
> 
> Instead of using a global array indexed by hartid, we should use
> scratch space to save per-HART PLIC pointer and PLIC context numbers.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> lib/utils/irqchip/fdt_irqchip_plic.c | 108 +++++++++++++++++++++------
> 1 file changed, 86 insertions(+), 22 deletions(-)
> 
> diff --git a/lib/utils/irqchip/fdt_irqchip_plic.c b/lib/utils/irqchip/fdt_irqchip_plic.c
> index 605f44a..eb1e412 100644
> --- a/lib/utils/irqchip/fdt_irqchip_plic.c
> +++ b/lib/utils/irqchip/fdt_irqchip_plic.c
> @@ -12,54 +12,97 @@
> #include <sbi/riscv_io.h>
> #include <sbi/sbi_error.h>
> #include <sbi/sbi_heap.h>
> -#include <sbi/sbi_hartmask.h>
> +#include <sbi/sbi_scratch.h>
> #include <sbi_utils/fdt/fdt_helper.h>
> #include <sbi_utils/irqchip/fdt_irqchip.h>
> #include <sbi_utils/irqchip/plic.h>
> 
> -static struct plic_data *plic_hartid2data[SBI_HARTMASK_MAX_BITS];
> -static int plic_hartid2context[SBI_HARTMASK_MAX_BITS][2] = { { -1 } };
> +static unsigned long plic_ptr_offset;
> +
> +#define plic_get_hart_data_ptr(__scratch) \
> +({ \
> + (void *)(*((ulong *)sbi_scratch_offset_ptr((__scratch), \
> +   plic_ptr_offset))); \

Why all the casting rather than just storing the pointer?

Jess

> +})
> +
> +#define plic_set_hart_data_ptr(__scratch, __imsic) \
> +do { \
> + *((ulong *)sbi_scratch_offset_ptr((__scratch), plic_ptr_offset))\
> + = (ulong)(__imsic); \
> +} while (0)
> +
> +static unsigned long plic_mcontext_offset;
> +
> +#define plic_get_hart_mcontext(__scratch) \
> +({ \
> + long __ret = \
> + *((long *)sbi_scratch_offset_ptr((__scratch), plic_mcontext_offset));\
> + (__ret - 1); \
> +})
> +
> +#define plic_set_hart_mcontext(__scratch, __mcontext) \
> +do { \
> + *((long *)sbi_scratch_offset_ptr((__scratch), plic_mcontext_offset))\
> + = (long)(__mcontext + 1); \
> +} while (0)
> +
> +static unsigned long plic_scontext_offset;
> +
> +#define plic_get_hart_scontext(__scratch) \
> +({ \
> + long __ret = \
> + *((long *)sbi_scratch_offset_ptr((__scratch), plic_scontext_offset));\
> + (__ret - 1); \
> +})
> +
> +#define plic_set_hart_scontext(__scratch, __scontext) \
> +do { \
> + *((long *)sbi_scratch_offset_ptr((__scratch), plic_scontext_offset))\
> + = (long)(__scontext + 1); \
> +} while (0)
> 
> void fdt_plic_priority_save(u8 *priority, u32 num)
> {
> - struct plic_data *plic = plic_hartid2data[current_hartid()];
> + struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> 
> - plic_priority_save(plic, priority, num);
> + plic_priority_save(plic_get_hart_data_ptr(scratch), priority, num);
> }
> 
> void fdt_plic_priority_restore(const u8 *priority, u32 num)
> {
> - struct plic_data *plic = plic_hartid2data[current_hartid()];
> + struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> 
> - plic_priority_restore(plic, priority, num);
> + plic_priority_restore(plic_get_hart_data_ptr(scratch), priority, num);
> }
> 
> void fdt_plic_context_save(bool smode, u32 *enable, u32 *threshold, u32 num)
> {
> - u32 hartid = current_hartid();
> + struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> 
> - plic_context_save(plic_hartid2data[hartid],
> -  plic_hartid2context[hartid][smode],
> + plic_context_save(plic_get_hart_data_ptr(scratch),
> +  smode ? plic_get_hart_scontext(scratch) :
> +  plic_get_hart_mcontext(scratch),
>  enable, threshold, num);
> }
> 
> void fdt_plic_context_restore(bool smode, const u32 *enable, u32 threshold,
>      u32 num)
> {
> - u32 hartid = current_hartid();
> + struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> 
> - plic_context_restore(plic_hartid2data[hartid],
> -     plic_hartid2context[hartid][smode],
> + plic_context_restore(plic_get_hart_data_ptr(scratch),
> +     smode ? plic_get_hart_scontext(scratch) :
> +     plic_get_hart_mcontext(scratch),
>     enable, threshold, num);
> }
> 
> static int irqchip_plic_warm_init(void)
> {
> - u32 hartid = current_hartid();
> + struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> 
> - return plic_warm_irqchip_init(plic_hartid2data[hartid],
> -      plic_hartid2context[hartid][0],
> -      plic_hartid2context[hartid][1]);
> + return plic_warm_irqchip_init(plic_get_hart_data_ptr(scratch),
> +      plic_get_hart_mcontext(scratch),
> +      plic_get_hart_scontext(scratch));
> }
> 
> static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
> @@ -67,6 +110,7 @@ static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
> {
> const fdt32_t *val;
> u32 phandle, hwirq, hartid;
> + struct sbi_scratch *scratch;
> int i, err, count, cpu_offset, cpu_intc_offset;
> 
> val = fdt_getprop(fdt, nodeoff, "interrupts-extended", &count);
> @@ -90,16 +134,17 @@ static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
> if (err)
> continue;
> 
> - if (SBI_HARTMASK_MAX_BITS <= hartid)
> + scratch = sbi_hartid_to_scratch(hartid);
> + if (!scratch)
> continue;
> 
> - plic_hartid2data[hartid] = pd;
> + plic_set_hart_data_ptr(scratch, pd);
> switch (hwirq) {
> case IRQ_M_EXT:
> - plic_hartid2context[hartid][0] = i / 2;
> + plic_set_hart_mcontext(scratch, i / 2);
> break;
> case IRQ_S_EXT:
> - plic_hartid2context[hartid][1] = i / 2;
> + plic_set_hart_scontext(scratch, i / 2);
> break;
> }
> }
> @@ -113,6 +158,24 @@ static int irqchip_plic_cold_init(void *fdt, int nodeoff,
> int rc;
> struct plic_data *pd;
> 
> + if (!plic_ptr_offset) {
> + plic_ptr_offset = sbi_scratch_alloc_offset(sizeof(ulong));
> + if (!plic_ptr_offset)
> + return SBI_ENOMEM;
> + }
> +
> + if (!plic_mcontext_offset) {
> + plic_mcontext_offset = sbi_scratch_alloc_offset(sizeof(long));
> + if (!plic_mcontext_offset)
> + return SBI_ENOMEM;
> + }
> +
> + if (!plic_scontext_offset) {
> + plic_scontext_offset = sbi_scratch_alloc_offset(sizeof(long));
> + if (!plic_scontext_offset)
> + return SBI_ENOMEM;
> + }
> +
> pd = sbi_zalloc(sizeof(*pd));
> if (!pd)
> return SBI_ENOMEM;
> @@ -152,7 +215,8 @@ static void thead_plic_plat_init(struct plic_data *pd)
> 
> void thead_plic_restore(void)
> {
> - struct plic_data *plic = plic_hartid2data[current_hartid()];
> + struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> + struct plic_data *plic = plic_get_hart_data_ptr(scratch);
> 
> thead_plic_plat_init(plic);
> }
> -- 
> 2.34.1
> 
> 
> -- 
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH 15/17] lib: utils/irqchip: Use scratch space to save per-HART PLIC pointer

[Anup Patel]

On Mon, Jun 5, 2023 at 11:13?AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 25 Apr 2023, at 13:32, Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > Instead of using a global array indexed by hartid, we should use
> > scratch space to save per-HART PLIC pointer and PLIC context numbers.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > lib/utils/irqchip/fdt_irqchip_plic.c | 108 +++++++++++++++++++++------
> > 1 file changed, 86 insertions(+), 22 deletions(-)
> >
> > diff --git a/lib/utils/irqchip/fdt_irqchip_plic.c b/lib/utils/irqchip/fdt_irqchip_plic.c
> > index 605f44a..eb1e412 100644
> > --- a/lib/utils/irqchip/fdt_irqchip_plic.c
> > +++ b/lib/utils/irqchip/fdt_irqchip_plic.c
> > @@ -12,54 +12,97 @@
> > #include <sbi/riscv_io.h>
> > #include <sbi/sbi_error.h>
> > #include <sbi/sbi_heap.h>
> > -#include <sbi/sbi_hartmask.h>
> > +#include <sbi/sbi_scratch.h>
> > #include <sbi_utils/fdt/fdt_helper.h>
> > #include <sbi_utils/irqchip/fdt_irqchip.h>
> > #include <sbi_utils/irqchip/plic.h>
> >
> > -static struct plic_data *plic_hartid2data[SBI_HARTMASK_MAX_BITS];
> > -static int plic_hartid2context[SBI_HARTMASK_MAX_BITS][2] = { { -1 } };
> > +static unsigned long plic_ptr_offset;
> > +
> > +#define plic_get_hart_data_ptr(__scratch) \
> > +({ \
> > + (void *)(*((ulong *)sbi_scratch_offset_ptr((__scratch), \
> > +   plic_ptr_offset))); \
>
> Why all the casting rather than just storing the pointer?

We are storing pointer value in scratch space as ulong but
sbi_scratch_offset_ptr() returns a void pointer to a location in
scratch space so we have to do the above casting for getting
the pointer stored in scratch space.

Thinking about this more, it is better to have generic helper
macros in sbi_scartch.h so that we don't have to define it
everywhere.

Regards,
Anup

>
> Jess
>
> > +})
> > +
> > +#define plic_set_hart_data_ptr(__scratch, __imsic) \
> > +do { \
> > + *((ulong *)sbi_scratch_offset_ptr((__scratch), plic_ptr_offset))\
> > + = (ulong)(__imsic); \
> > +} while (0)
> > +
> > +static unsigned long plic_mcontext_offset;
> > +
> > +#define plic_get_hart_mcontext(__scratch) \
> > +({ \
> > + long __ret = \
> > + *((long *)sbi_scratch_offset_ptr((__scratch), plic_mcontext_offset));\
> > + (__ret - 1); \
> > +})
> > +
> > +#define plic_set_hart_mcontext(__scratch, __mcontext) \
> > +do { \
> > + *((long *)sbi_scratch_offset_ptr((__scratch), plic_mcontext_offset))\
> > + = (long)(__mcontext + 1); \
> > +} while (0)
> > +
> > +static unsigned long plic_scontext_offset;
> > +
> > +#define plic_get_hart_scontext(__scratch) \
> > +({ \
> > + long __ret = \
> > + *((long *)sbi_scratch_offset_ptr((__scratch), plic_scontext_offset));\
> > + (__ret - 1); \
> > +})
> > +
> > +#define plic_set_hart_scontext(__scratch, __scontext) \
> > +do { \
> > + *((long *)sbi_scratch_offset_ptr((__scratch), plic_scontext_offset))\
> > + = (long)(__scontext + 1); \
> > +} while (0)
> >
> > void fdt_plic_priority_save(u8 *priority, u32 num)
> > {
> > - struct plic_data *plic = plic_hartid2data[current_hartid()];
> > + struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> >
> > - plic_priority_save(plic, priority, num);
> > + plic_priority_save(plic_get_hart_data_ptr(scratch), priority, num);
> > }
> >
> > void fdt_plic_priority_restore(const u8 *priority, u32 num)
> > {
> > - struct plic_data *plic = plic_hartid2data[current_hartid()];
> > + struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> >
> > - plic_priority_restore(plic, priority, num);
> > + plic_priority_restore(plic_get_hart_data_ptr(scratch), priority, num);
> > }
> >
> > void fdt_plic_context_save(bool smode, u32 *enable, u32 *threshold, u32 num)
> > {
> > - u32 hartid = current_hartid();
> > + struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> >
> > - plic_context_save(plic_hartid2data[hartid],
> > -  plic_hartid2context[hartid][smode],
> > + plic_context_save(plic_get_hart_data_ptr(scratch),
> > +  smode ? plic_get_hart_scontext(scratch) :
> > +  plic_get_hart_mcontext(scratch),
> >  enable, threshold, num);
> > }
> >
> > void fdt_plic_context_restore(bool smode, const u32 *enable, u32 threshold,
> >      u32 num)
> > {
> > - u32 hartid = current_hartid();
> > + struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> >
> > - plic_context_restore(plic_hartid2data[hartid],
> > -     plic_hartid2context[hartid][smode],
> > + plic_context_restore(plic_get_hart_data_ptr(scratch),
> > +     smode ? plic_get_hart_scontext(scratch) :
> > +     plic_get_hart_mcontext(scratch),
> >     enable, threshold, num);
> > }
> >
> > static int irqchip_plic_warm_init(void)
> > {
> > - u32 hartid = current_hartid();
> > + struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> >
> > - return plic_warm_irqchip_init(plic_hartid2data[hartid],
> > -      plic_hartid2context[hartid][0],
> > -      plic_hartid2context[hartid][1]);
> > + return plic_warm_irqchip_init(plic_get_hart_data_ptr(scratch),
> > +      plic_get_hart_mcontext(scratch),
> > +      plic_get_hart_scontext(scratch));
> > }
> >
> > static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
> > @@ -67,6 +110,7 @@ static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
> > {
> > const fdt32_t *val;
> > u32 phandle, hwirq, hartid;
> > + struct sbi_scratch *scratch;
> > int i, err, count, cpu_offset, cpu_intc_offset;
> >
> > val = fdt_getprop(fdt, nodeoff, "interrupts-extended", &count);
> > @@ -90,16 +134,17 @@ static int irqchip_plic_update_hartid_table(void *fdt, int nodeoff,
> > if (err)
> > continue;
> >
> > - if (SBI_HARTMASK_MAX_BITS <= hartid)
> > + scratch = sbi_hartid_to_scratch(hartid);
> > + if (!scratch)
> > continue;
> >
> > - plic_hartid2data[hartid] = pd;
> > + plic_set_hart_data_ptr(scratch, pd);
> > switch (hwirq) {
> > case IRQ_M_EXT:
> > - plic_hartid2context[hartid][0] = i / 2;
> > + plic_set_hart_mcontext(scratch, i / 2);
> > break;
> > case IRQ_S_EXT:
> > - plic_hartid2context[hartid][1] = i / 2;
> > + plic_set_hart_scontext(scratch, i / 2);
> > break;
> > }
> > }
> > @@ -113,6 +158,24 @@ static int irqchip_plic_cold_init(void *fdt, int nodeoff,
> > int rc;
> > struct plic_data *pd;
> >
> > + if (!plic_ptr_offset) {
> > + plic_ptr_offset = sbi_scratch_alloc_offset(sizeof(ulong));
> > + if (!plic_ptr_offset)
> > + return SBI_ENOMEM;
> > + }
> > +
> > + if (!plic_mcontext_offset) {
> > + plic_mcontext_offset = sbi_scratch_alloc_offset(sizeof(long));
> > + if (!plic_mcontext_offset)
> > + return SBI_ENOMEM;
> > + }
> > +
> > + if (!plic_scontext_offset) {
> > + plic_scontext_offset = sbi_scratch_alloc_offset(sizeof(long));
> > + if (!plic_scontext_offset)
> > + return SBI_ENOMEM;
> > + }
> > +
> > pd = sbi_zalloc(sizeof(*pd));
> > if (!pd)
> > return SBI_ENOMEM;
> > @@ -152,7 +215,8 @@ static void thead_plic_plat_init(struct plic_data *pd)
> >
> > void thead_plic_restore(void)
> > {
> > - struct plic_data *plic = plic_hartid2data[current_hartid()];
> > + struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> > + struct plic_data *plic = plic_get_hart_data_ptr(scratch);
> >
> > thead_plic_plat_init(plic);
> > }
> > --
> > 2.34.1
> >
> >
> > --
> > opensbi mailing list
> > opensbi at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/opensbi
>

[PATCH 15/17] lib: utils/irqchip: Use scratch space to save per-HART PLIC pointer

[Jessica Clarke]

On 5 Jun 2023, at 07:51, Anup Patel <anup@brainfault.org> wrote:
> 
> On Mon, Jun 5, 2023 at 11:13?AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>> 
>> On 25 Apr 2023, at 13:32, Anup Patel <apatel@ventanamicro.com> wrote:
>>> 
>>> Instead of using a global array indexed by hartid, we should use
>>> scratch space to save per-HART PLIC pointer and PLIC context numbers.
>>> 
>>> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>>> ---
>>> lib/utils/irqchip/fdt_irqchip_plic.c | 108 +++++++++++++++++++++------
>>> 1 file changed, 86 insertions(+), 22 deletions(-)
>>> 
>>> diff --git a/lib/utils/irqchip/fdt_irqchip_plic.c b/lib/utils/irqchip/fdt_irqchip_plic.c
>>> index 605f44a..eb1e412 100644
>>> --- a/lib/utils/irqchip/fdt_irqchip_plic.c
>>> +++ b/lib/utils/irqchip/fdt_irqchip_plic.c
>>> @@ -12,54 +12,97 @@
>>> #include <sbi/riscv_io.h>
>>> #include <sbi/sbi_error.h>
>>> #include <sbi/sbi_heap.h>
>>> -#include <sbi/sbi_hartmask.h>
>>> +#include <sbi/sbi_scratch.h>
>>> #include <sbi_utils/fdt/fdt_helper.h>
>>> #include <sbi_utils/irqchip/fdt_irqchip.h>
>>> #include <sbi_utils/irqchip/plic.h>
>>> 
>>> -static struct plic_data *plic_hartid2data[SBI_HARTMASK_MAX_BITS];
>>> -static int plic_hartid2context[SBI_HARTMASK_MAX_BITS][2] = { { -1 } };
>>> +static unsigned long plic_ptr_offset;
>>> +
>>> +#define plic_get_hart_data_ptr(__scratch) \
>>> +({ \
>>> + (void *)(*((ulong *)sbi_scratch_offset_ptr((__scratch), \
>>> +   plic_ptr_offset))); \
>> 
>> Why all the casting rather than just storing the pointer?
> 
> We are storing pointer value in scratch space as ulong but
> sbi_scratch_offset_ptr() returns a void pointer to a location in
> scratch space so we have to do the above casting for getting
> the pointer stored in scratch space.
> 
> Thinking about this more, it is better to have generic helper
> macros in sbi_scartch.h so that we don't have to define it
> everywhere.

Pointers are not longs. This isn?t the Linux kernel, you don?t
need to be allergic to using the correct types for things.

Jess

[PATCH 15/17] lib: utils/irqchip: Use scratch space to save per-HART PLIC pointer

[Anup Patel]

On Mon, Jun 5, 2023 at 12:27?PM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 5 Jun 2023, at 07:51, Anup Patel <anup@brainfault.org> wrote:
> >
> > On Mon, Jun 5, 2023 at 11:13?AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
> >>
> >> On 25 Apr 2023, at 13:32, Anup Patel <apatel@ventanamicro.com> wrote:
> >>>
> >>> Instead of using a global array indexed by hartid, we should use
> >>> scratch space to save per-HART PLIC pointer and PLIC context numbers.
> >>>
> >>> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> >>> ---
> >>> lib/utils/irqchip/fdt_irqchip_plic.c | 108 +++++++++++++++++++++------
> >>> 1 file changed, 86 insertions(+), 22 deletions(-)
> >>>
> >>> diff --git a/lib/utils/irqchip/fdt_irqchip_plic.c b/lib/utils/irqchip/fdt_irqchip_plic.c
> >>> index 605f44a..eb1e412 100644
> >>> --- a/lib/utils/irqchip/fdt_irqchip_plic.c
> >>> +++ b/lib/utils/irqchip/fdt_irqchip_plic.c
> >>> @@ -12,54 +12,97 @@
> >>> #include <sbi/riscv_io.h>
> >>> #include <sbi/sbi_error.h>
> >>> #include <sbi/sbi_heap.h>
> >>> -#include <sbi/sbi_hartmask.h>
> >>> +#include <sbi/sbi_scratch.h>
> >>> #include <sbi_utils/fdt/fdt_helper.h>
> >>> #include <sbi_utils/irqchip/fdt_irqchip.h>
> >>> #include <sbi_utils/irqchip/plic.h>
> >>>
> >>> -static struct plic_data *plic_hartid2data[SBI_HARTMASK_MAX_BITS];
> >>> -static int plic_hartid2context[SBI_HARTMASK_MAX_BITS][2] = { { -1 } };
> >>> +static unsigned long plic_ptr_offset;
> >>> +
> >>> +#define plic_get_hart_data_ptr(__scratch) \
> >>> +({ \
> >>> + (void *)(*((ulong *)sbi_scratch_offset_ptr((__scratch), \
> >>> +   plic_ptr_offset))); \
> >>
> >> Why all the casting rather than just storing the pointer?
> >
> > We are storing pointer value in scratch space as ulong but
> > sbi_scratch_offset_ptr() returns a void pointer to a location in
> > scratch space so we have to do the above casting for getting
> > the pointer stored in scratch space.
> >
> > Thinking about this more, it is better to have generic helper
> > macros in sbi_scartch.h so that we don't have to define it
> > everywhere.
>
> Pointers are not longs. This isn?t the Linux kernel, you don?t
> need to be allergic to using the correct types for things.
>

Okay, I will improve this in v2.

Regards,
Anup