[PATCH 0/9] OpenSBI domain data support

[PATCH 0/9] OpenSBI domain data support

[Anup Patel]

This series extends the OpenSBI domain support to allow per-domain
data from different parts of OpenSBI. Also extend OpenSBI domain
context as the first user of OpenSBI domain data support.

The first 7 patches of this series are miscellaneous improvements
which were pending for quite some time.

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

Anup Patel (9):
  lib: utils/ipi: Use sbi_domain_root_add_memrange() for ACLINT mswi
  lib: utils/irqchip: Use sbi_domain_root_add_memrange() for IMSIC
  lib: utils/irqchip: Use sbi_domain_root_add_memrange() for APLIC
  lib: sbi_domain: Make sbi_domain_root_add_memregion() as local
    function
  lib: utils/fdt: Use sbi_domain_memregion_init() when parsing domains
  include: sbi: Remove cyclic include in sbi_domain_context.h
  lib: sbi: Introduce domain data
  lib: sbi_domain: Use domain data support for per-domain hart context
  docs: Remove hartindex_to_context_table from domain_support.md

 docs/domain_support.md           |   2 -
 include/sbi/sbi_domain.h         |  15 +---
 include/sbi/sbi_domain_context.h |  58 +++----------
 include/sbi/sbi_domain_data.h    |  93 +++++++++++++++++++++
 lib/sbi/objects.mk               |   1 +
 lib/sbi/sbi_domain.c             |  24 +++++-
 lib/sbi/sbi_domain_context.c     | 106 +++++++++++++++++++++---
 lib/sbi/sbi_domain_data.c        | 138 +++++++++++++++++++++++++++++++
 lib/utils/fdt/fdt_domain.c       |  14 ++--
 lib/utils/ipi/aclint_mswi.c      |  20 ++---
 lib/utils/irqchip/aplic.c        |  11 +--
 lib/utils/irqchip/imsic.c        |  14 ++--
 12 files changed, 384 insertions(+), 112 deletions(-)
 create mode 100755 include/sbi/sbi_domain_data.h
 create mode 100755 lib/sbi/sbi_domain_data.c

-- 
2.43.0

[PATCH 1/9] lib: utils/ipi: Use sbi_domain_root_add_memrange() for ACLINT mswi

[Anup Patel]

The sbi_domain_root_add_memrange() should be preferred for creating
multiple memregions over a range. Update ACLINT mswi driver to use
sbi_domain_root_add_memrange() instead of explicity registering
memregions.

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

diff --git a/lib/utils/ipi/aclint_mswi.c b/lib/utils/ipi/aclint_mswi.c
index 4ae6bb1c..39edacba 100644
--- a/lib/utils/ipi/aclint_mswi.c
+++ b/lib/utils/ipi/aclint_mswi.c
@@ -84,8 +84,6 @@ 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)) ||
@@ -114,18 +112,12 @@ int aclint_mswi_cold_init(struct aclint_mswi_data *mswi)
 	}
 
 	/* Add MSWI regions to the root domain */
-	for (pos = 0; pos < mswi->size; pos += ACLINT_MSWI_ALIGN) {
-		region_size = ((mswi->size - pos) < ACLINT_MSWI_ALIGN) ?
-			      (mswi->size - pos) : ACLINT_MSWI_ALIGN;
-		sbi_domain_memregion_init(mswi->addr + pos, region_size,
-					  (SBI_DOMAIN_MEMREGION_MMIO |
-					   SBI_DOMAIN_MEMREGION_M_READABLE |
-					   SBI_DOMAIN_MEMREGION_M_WRITABLE),
-					  &reg);
-		rc = sbi_domain_root_add_memregion(&reg);
-		if (rc)
-			return rc;
-	}
+	rc = sbi_domain_root_add_memrange(mswi->addr, mswi->size, ACLINT_MSWI_ALIGN,
+					  SBI_DOMAIN_MEMREGION_MMIO |
+					  SBI_DOMAIN_MEMREGION_M_READABLE |
+					  SBI_DOMAIN_MEMREGION_M_WRITABLE);
+	if (rc)
+		return rc;
 
 	sbi_ipi_set_device(&aclint_mswi);
 
-- 
2.43.0

[PATCH 1/9] lib: utils/ipi: Use sbi_domain_root_add_memrange() for ACLINT mswi

[Samuel Holland]

On 2024-09-23 6:56 AM, Anup Patel wrote:
> The sbi_domain_root_add_memrange() should be preferred for creating
> multiple memregions over a range. Update ACLINT mswi driver to use
> sbi_domain_root_add_memrange() instead of explicity registering

typo: explicitly

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

Reviewed-by: Samuel Holland <samuel.holland@sifive.com>

[PATCH 1/9] lib: utils/ipi: Use sbi_domain_root_add_memrange() for ACLINT mswi

[Anup Patel]

On Thu, Oct 10, 2024 at 4:30?AM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> On 2024-09-23 6:56 AM, Anup Patel wrote:
> > The sbi_domain_root_add_memrange() should be preferred for creating
> > multiple memregions over a range. Update ACLINT mswi driver to use
> > sbi_domain_root_add_memrange() instead of explicity registering
>
> typo: explicitly

Okay, I will update.

>
> > memregions.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  lib/utils/ipi/aclint_mswi.c | 20 ++++++--------------
> >  1 file changed, 6 insertions(+), 14 deletions(-)
>
> Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
>

Thanks,
Anup

[PATCH 2/9] lib: utils/irqchip: Use sbi_domain_root_add_memrange() for IMSIC

[Anup Patel]

The sbi_domain_root_add_memrange() should be preferred for creating
multiple memregions over a range. Update IMSIC driver to use
sbi_domain_root_add_memrange() instead of explicity registering
memregions.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/utils/irqchip/imsic.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/lib/utils/irqchip/imsic.c b/lib/utils/irqchip/imsic.c
index bac72d21..ddb372fb 100644
--- a/lib/utils/irqchip/imsic.c
+++ b/lib/utils/irqchip/imsic.c
@@ -348,7 +348,6 @@ int imsic_data_check(struct imsic_data *imsic)
 int imsic_cold_irqchip_init(struct imsic_data *imsic)
 {
 	int i, rc;
-	struct sbi_domain_memregion reg;
 
 	/* Sanity checks */
 	rc = imsic_data_check(imsic);
@@ -378,13 +377,12 @@ int imsic_cold_irqchip_init(struct imsic_data *imsic)
 
 	/* Add IMSIC regions to the root domain */
 	for (i = 0; i < IMSIC_MAX_REGS && imsic->regs[i].size; i++) {
-		sbi_domain_memregion_init(imsic->regs[i].addr,
-					  imsic->regs[i].size,
-					  (SBI_DOMAIN_MEMREGION_MMIO |
-					   SBI_DOMAIN_MEMREGION_M_READABLE |
-					   SBI_DOMAIN_MEMREGION_M_WRITABLE),
-					  &reg);
-		rc = sbi_domain_root_add_memregion(&reg);
+		rc = sbi_domain_root_add_memrange(imsic->regs[i].addr,
+						  imsic->regs[i].size,
+						  IMSIC_MMIO_PAGE_SZ,
+						  SBI_DOMAIN_MEMREGION_MMIO |
+						  SBI_DOMAIN_MEMREGION_M_READABLE |
+						  SBI_DOMAIN_MEMREGION_M_WRITABLE);
 		if (rc)
 			return rc;
 	}
-- 
2.43.0

[PATCH 2/9] lib: utils/irqchip: Use sbi_domain_root_add_memrange() for IMSIC

[Samuel Holland]

On 2024-09-23 6:56 AM, Anup Patel wrote:
> The sbi_domain_root_add_memrange() should be preferred for creating
> multiple memregions over a range. Update IMSIC driver to use
> sbi_domain_root_add_memrange() instead of explicity registering

typo: explicitly

> memregions.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/utils/irqchip/imsic.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)

Reviewed-by: Samuel Holland <samuel.holland@sifive.com>

[PATCH 2/9] lib: utils/irqchip: Use sbi_domain_root_add_memrange() for IMSIC

[Anup Patel]

On Thu, Oct 10, 2024 at 4:34?AM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> On 2024-09-23 6:56 AM, Anup Patel wrote:
> > The sbi_domain_root_add_memrange() should be preferred for creating
> > multiple memregions over a range. Update IMSIC driver to use
> > sbi_domain_root_add_memrange() instead of explicity registering
>
> typo: explicitly

Okay, I will update.

>
> > memregions.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  lib/utils/irqchip/imsic.c | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
>
> Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
>

Thanks,
Anup

[PATCH 3/9] lib: utils/irqchip: Use sbi_domain_root_add_memrange() for APLIC

[Anup Patel]

The sbi_domain_root_add_memrange() should be preferred for creating
multiple memregions over a range. Update APLIC driver to use
sbi_domain_root_add_memrange() instead of explicity registering
memregions.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/utils/irqchip/aplic.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/lib/utils/irqchip/aplic.c b/lib/utils/irqchip/aplic.c
index 10236e7a..28f2f26d 100644
--- a/lib/utils/irqchip/aplic.c
+++ b/lib/utils/irqchip/aplic.c
@@ -169,7 +169,6 @@ int aplic_cold_irqchip_init(struct aplic_data *aplic)
 {
 	int rc;
 	u32 i, j, tmp;
-	struct sbi_domain_memregion reg;
 	struct aplic_delegate_data *deleg;
 	u32 first_deleg_irq, last_deleg_irq;
 
@@ -268,12 +267,10 @@ int aplic_cold_irqchip_init(struct aplic_data *aplic)
 	    ((first_deleg_irq < last_deleg_irq) &&
 	    (last_deleg_irq == aplic->num_source) &&
 	    (first_deleg_irq == 1))) {
-		sbi_domain_memregion_init(aplic->addr, aplic->size,
-					  (SBI_DOMAIN_MEMREGION_MMIO |
-					   SBI_DOMAIN_MEMREGION_M_READABLE |
-					   SBI_DOMAIN_MEMREGION_M_WRITABLE),
-					  &reg);
-		rc = sbi_domain_root_add_memregion(&reg);
+		rc = sbi_domain_root_add_memrange(aplic->addr, aplic->size, PAGE_SIZE,
+						  SBI_DOMAIN_MEMREGION_MMIO |
+						  SBI_DOMAIN_MEMREGION_M_READABLE |
+						  SBI_DOMAIN_MEMREGION_M_WRITABLE);
 		if (rc)
 			return rc;
 	}
-- 
2.43.0

[PATCH 3/9] lib: utils/irqchip: Use sbi_domain_root_add_memrange() for APLIC

[Samuel Holland]

On 2024-09-23 6:56 AM, Anup Patel wrote:
> The sbi_domain_root_add_memrange() should be preferred for creating
> multiple memregions over a range. Update APLIC driver to use
> sbi_domain_root_add_memrange() instead of explicity registering

typo: explicitly

> memregions.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/utils/irqchip/aplic.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)

Reviewed-by: Samuel Holland <samuel.holland@sifive.com>

[PATCH 3/9] lib: utils/irqchip: Use sbi_domain_root_add_memrange() for APLIC

[Anup Patel]

On Thu, Oct 10, 2024 at 4:35?AM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> On 2024-09-23 6:56 AM, Anup Patel wrote:
> > The sbi_domain_root_add_memrange() should be preferred for creating
> > multiple memregions over a range. Update APLIC driver to use
> > sbi_domain_root_add_memrange() instead of explicity registering
>
> typo: explicitly

Okay, I will update.

>
> > memregions.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  lib/utils/irqchip/aplic.c | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
>
> Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
>

Thanks,
Anup

[PATCH 4/9] lib: sbi_domain: Make sbi_domain_root_add_memregion() as local function

[Anup Patel]

The sbi_domain_root_add_memregion() is only used within sbi_domain
implementation so make rename and make it a local function.

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

diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h
index 313ae156..22aa8e50 100644
--- a/include/sbi/sbi_domain.h
+++ b/include/sbi/sbi_domain.h
@@ -293,16 +293,6 @@ void sbi_domain_dump_all(const char *suffix);
 int sbi_domain_register(struct sbi_domain *dom,
 			const struct sbi_hartmask *assign_mask);
 
-/**
- * Add a memory region to the root domain
- * @param reg pointer to the memory region to be added
- *
- * @return 0 on success
- * @return SBI_EALREADY if memory region conflicts with the existing one
- * @return SBI_EINVAL otherwise
- */
-int sbi_domain_root_add_memregion(const struct sbi_domain_memregion *reg);
-
 /**
  * Add a memory range with its flags to the root domain
  * @param addr start physical address of memory range
diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
index 7198e27e..04e70d3b 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -590,7 +590,7 @@ int sbi_domain_register(struct sbi_domain *dom,
 	return 0;
 }
 
-int sbi_domain_root_add_memregion(const struct sbi_domain_memregion *reg)
+static int root_add_memregion(const struct sbi_domain_memregion *reg)
 {
 	int rc;
 	bool reg_merged;
@@ -668,7 +668,7 @@ int sbi_domain_root_add_memrange(unsigned long addr, unsigned long size,
 				(end - pos) : align;
 
 		sbi_domain_memregion_init(pos, rsize, region_flags, &reg);
-		rc = sbi_domain_root_add_memregion(&reg);
+		rc = root_add_memregion(&reg);
 		if (rc)
 			return rc;
 		pos += rsize;
-- 
2.43.0

[PATCH 4/9] lib: sbi_domain: Make sbi_domain_root_add_memregion() as local function

[Samuel Holland]

On 2024-09-23 6:56 AM, Anup Patel wrote:
> The sbi_domain_root_add_memregion() is only used within sbi_domain
> implementation so make rename and make it a local function.

typo: extra "make"

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

Reviewed-by: Samuel Holland <samuel.holland@sifive.com>

[PATCH 4/9] lib: sbi_domain: Make sbi_domain_root_add_memregion() as local function

[Anup Patel]

On Thu, Oct 10, 2024 at 4:36?AM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> On 2024-09-23 6:56 AM, Anup Patel wrote:
> > The sbi_domain_root_add_memregion() is only used within sbi_domain
> > implementation so make rename and make it a local function.
>
> typo: extra "make"

Okay, I will update.

>
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  include/sbi/sbi_domain.h | 10 ----------
> >  lib/sbi/sbi_domain.c     |  4 ++--
> >  2 files changed, 2 insertions(+), 12 deletions(-)
>
> Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
>

Thanks,
Anup

[PATCH 5/9] lib: utils/fdt: Use sbi_domain_memregion_init() when parsing domains

[Anup Patel]

Use sbi_domain_memregion_init() at the time of parsing domains from
FDT so that sbi_domain_memregion_init() is always used for setting
up all memregions.

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

diff --git a/lib/utils/fdt/fdt_domain.c b/lib/utils/fdt/fdt_domain.c
index 4fde50f5..4bc7ed86 100644
--- a/lib/utils/fdt/fdt_domain.c
+++ b/lib/utils/fdt/fdt_domain.c
@@ -253,8 +253,8 @@ static int __fdt_parse_region(const void *fdt, int domain_offset,
 	u32 val32;
 	u64 val64;
 	const u32 *val;
+	unsigned long base, order, flags;
 	struct parse_region_data *preg = opaque;
-	struct sbi_domain_memregion *region;
 
 	/*
 	 * Non-root domains cannot add a region with only M-mode
@@ -271,7 +271,6 @@ static int __fdt_parse_region(const void *fdt, int domain_offset,
 	/* Find next region of the domain */
 	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);
@@ -279,7 +278,7 @@ static int __fdt_parse_region(const void *fdt, int domain_offset,
 		return SBI_EINVAL;
 	val64 = fdt32_to_cpu(val[0]);
 	val64 = (val64 << 32) | fdt32_to_cpu(val[1]);
-	region->base = val64;
+	base = val64;
 
 	/* Read "order" DT property */
 	val = fdt_getprop(fdt, region_offset, "order", &len);
@@ -288,12 +287,15 @@ static int __fdt_parse_region(const void *fdt, int domain_offset,
 	val32 = fdt32_to_cpu(*val);
 	if (val32 < 3 || __riscv_xlen < val32)
 		return SBI_EINVAL;
-	region->order = val32;
+	order = val32;
 
 	/* Read "mmio" DT property */
-	region->flags = region_access & SBI_DOMAIN_MEMREGION_ACCESS_MASK;
+	flags = region_access & SBI_DOMAIN_MEMREGION_ACCESS_MASK;
 	if (fdt_get_property(fdt, region_offset, "mmio", NULL))
-		region->flags |= SBI_DOMAIN_MEMREGION_MMIO;
+		flags |= SBI_DOMAIN_MEMREGION_MMIO;
+
+	sbi_domain_memregion_init(base, (order == __riscv_xlen) ? ~0UL : BIT(order),
+				  flags, &preg->dom->regions[preg->region_count]);
 
 	preg->region_count++;
 
-- 
2.43.0

[PATCH 5/9] lib: utils/fdt: Use sbi_domain_memregion_init() when parsing domains

[Samuel Holland]

On 2024-09-23 6:56 AM, Anup Patel wrote:
> Use sbi_domain_memregion_init() at the time of parsing domains from
> FDT so that sbi_domain_memregion_init() is always used for setting
> up all memregions.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/utils/fdt/fdt_domain.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)

Reviewed-by: Samuel Holland <samuel.holland@sifive.com>

[PATCH 6/9] include: sbi: Remove cyclic include in sbi_domain_context.h

[Anup Patel]

The sbi_domain_context.h includes sbi_domain.h and the sbi_domain.h
also includes sbi_domain_context.h. Remove this cyclic include in
sbi_domain_context.h.

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

diff --git a/include/sbi/sbi_domain_context.h b/include/sbi/sbi_domain_context.h
index 0d25884a..3f43b622 100755
--- a/include/sbi/sbi_domain_context.h
+++ b/include/sbi/sbi_domain_context.h
@@ -9,7 +9,8 @@
 
 #include <sbi/sbi_types.h>
 #include <sbi/sbi_trap.h>
-#include <sbi/sbi_domain.h>
+
+struct sbi_domain;
 
 /** Context representation for a hart within a domain */
 struct sbi_context {
diff --git a/lib/sbi/sbi_domain_context.c b/lib/sbi/sbi_domain_context.c
index 1432e3a8..29e2d280 100755
--- a/lib/sbi/sbi_domain_context.c
+++ b/lib/sbi/sbi_domain_context.c
@@ -13,7 +13,7 @@
 #include <sbi/sbi_heap.h>
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
-#include <sbi/sbi_domain_context.h>
+#include <sbi/sbi_domain.h>
 
 /**
  * Switches the HART context from the current domain to the target domain.
-- 
2.43.0

[PATCH 6/9] include: sbi: Remove cyclic include in sbi_domain_context.h

[Samuel Holland]

On 2024-09-23 6:56 AM, Anup Patel wrote:
> The sbi_domain_context.h includes sbi_domain.h and the sbi_domain.h
> also includes sbi_domain_context.h. Remove this cyclic include in
> sbi_domain_context.h.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  include/sbi/sbi_domain_context.h | 3 ++-
>  lib/sbi/sbi_domain_context.c     | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Samuel Holland <samuel.holland@sifive.com>

[PATCH 6/9] include: sbi: Remove cyclic include in sbi_domain_context.h

[Samuel Holland]

Hi Anup,

On 2024-09-23 6:56 AM, Anup Patel wrote:
> The sbi_domain_context.h includes sbi_domain.h and the sbi_domain.h
> also includes sbi_domain_context.h. Remove this cyclic include in
> sbi_domain_context.h.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  include/sbi/sbi_domain_context.h | 3 ++-
>  lib/sbi/sbi_domain_context.c     | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/sbi/sbi_domain_context.h b/include/sbi/sbi_domain_context.h
> index 0d25884a..3f43b622 100755
> --- a/include/sbi/sbi_domain_context.h
> +++ b/include/sbi/sbi_domain_context.h
> @@ -9,7 +9,8 @@
>  
>  #include <sbi/sbi_types.h>
>  #include <sbi/sbi_trap.h>
> -#include <sbi/sbi_domain.h>
> +
> +struct sbi_domain;
>  
>  /** Context representation for a hart within a domain */
>  struct sbi_context {
> diff --git a/lib/sbi/sbi_domain_context.c b/lib/sbi/sbi_domain_context.c
> index 1432e3a8..29e2d280 100755
> --- a/lib/sbi/sbi_domain_context.c
> +++ b/lib/sbi/sbi_domain_context.c
> @@ -13,7 +13,7 @@
>  #include <sbi/sbi_heap.h>
>  #include <sbi/sbi_scratch.h>
>  #include <sbi/sbi_string.h>
> -#include <sbi/sbi_domain_context.h>

We should keep this include, so the functions defined in this file are checked
against their prototypes.

Regards,
Samuel

> +#include <sbi/sbi_domain.h>
>  
>  /**
>   * Switches the HART context from the current domain to the target domain.

[PATCH 6/9] include: sbi: Remove cyclic include in sbi_domain_context.h

[Anup Patel]

On Thu, Oct 10, 2024 at 4:45?AM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> Hi Anup,
>
> On 2024-09-23 6:56 AM, Anup Patel wrote:
> > The sbi_domain_context.h includes sbi_domain.h and the sbi_domain.h
> > also includes sbi_domain_context.h. Remove this cyclic include in
> > sbi_domain_context.h.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  include/sbi/sbi_domain_context.h | 3 ++-
> >  lib/sbi/sbi_domain_context.c     | 2 +-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/sbi/sbi_domain_context.h b/include/sbi/sbi_domain_context.h
> > index 0d25884a..3f43b622 100755
> > --- a/include/sbi/sbi_domain_context.h
> > +++ b/include/sbi/sbi_domain_context.h
> > @@ -9,7 +9,8 @@
> >
> >  #include <sbi/sbi_types.h>
> >  #include <sbi/sbi_trap.h>
> > -#include <sbi/sbi_domain.h>
> > +
> > +struct sbi_domain;
> >
> >  /** Context representation for a hart within a domain */
> >  struct sbi_context {
> > diff --git a/lib/sbi/sbi_domain_context.c b/lib/sbi/sbi_domain_context.c
> > index 1432e3a8..29e2d280 100755
> > --- a/lib/sbi/sbi_domain_context.c
> > +++ b/lib/sbi/sbi_domain_context.c
> > @@ -13,7 +13,7 @@
> >  #include <sbi/sbi_heap.h>
> >  #include <sbi/sbi_scratch.h>
> >  #include <sbi/sbi_string.h>
> > -#include <sbi/sbi_domain_context.h>
>
> We should keep this include, so the functions defined in this file are checked
> against their prototypes.

I dropped this include because it was already included through sbi_domain.h
but I don't mind keeping it. I will update.

>
> Regards,
> Samuel
>
> > +#include <sbi/sbi_domain.h>
> >
> >  /**
> >   * Switches the HART context from the current domain to the target domain.
>

Thanks,
Anup

[PATCH 7/9] lib: sbi: Introduce domain data

[Anup Patel]

Different parts of OpenSBI require their own per-domain data so
introduce domain data (or sbi_domain_data) which can be registered
by any part of OpenSBI. Using the domain data, the domain framework
will create a data pointer for every domain which can be used to
maintain some per-domain state.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 include/sbi/sbi_domain.h      |   3 +
 include/sbi/sbi_domain_data.h |  93 +++++++++++++++++++++++
 lib/sbi/objects.mk            |   1 +
 lib/sbi/sbi_domain.c          |  11 +++
 lib/sbi/sbi_domain_data.c     | 138 ++++++++++++++++++++++++++++++++++
 5 files changed, 246 insertions(+)
 create mode 100755 include/sbi/sbi_domain_data.h
 create mode 100755 lib/sbi/sbi_domain_data.c

diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h
index 22aa8e50..60d7a776 100644
--- a/include/sbi/sbi_domain.h
+++ b/include/sbi/sbi_domain.h
@@ -15,6 +15,7 @@
 #include <sbi/sbi_types.h>
 #include <sbi/sbi_hartmask.h>
 #include <sbi/sbi_domain_context.h>
+#include <sbi/sbi_domain_data.h>
 
 struct sbi_scratch;
 
@@ -163,6 +164,8 @@ struct sbi_domain_memregion {
 struct sbi_domain {
 	/** Node in linked list of domains */
 	struct sbi_dlist node;
+	/** Internal state of per-domain data */
+	struct sbi_domain_data_priv data_priv;
 	/** Logical index of this domain */
 	u32 index;
 	/** HARTs assigned to this domain */
diff --git a/include/sbi/sbi_domain_data.h b/include/sbi/sbi_domain_data.h
new file mode 100755
index 00000000..7eeafdce
--- /dev/null
+++ b/include/sbi/sbi_domain_data.h
@@ -0,0 +1,93 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2024 Ventana Micro Systems Inc.
+ */
+
+#ifndef __SBI_DOMAIN_DATA_H__
+#define __SBI_DOMAIN_DATA_H__
+
+#include <sbi/sbi_types.h>
+#include <sbi/sbi_list.h>
+
+struct sbi_domain;
+
+/** Maximum domain data per-domain */
+#define SBI_DOMAIN_MAX_DATA_PTRS		32
+
+/** Representation of per-domain data */
+struct sbi_domain_data_priv {
+	/** Array of domain data pointers indexed by domain data identifier */
+	void *idx_to_data_ptr[SBI_DOMAIN_MAX_DATA_PTRS];
+};
+
+/** Representation of a domain data */
+struct sbi_domain_data {
+	/**
+	 * Head is used for maintaining data list
+	 *
+	 * Note: initialized by domain framework
+	 */
+	struct sbi_dlist head;
+	/**
+	 * Identifier which used to locate per-domain data
+	 *
+	 * Note: initialized by domain framework
+	 */
+	unsigned long data_idx;
+	/** Size of per-domain data */
+	unsigned long data_size;
+	/** Optional callback to setup domain data */
+	int (*data_setup)(struct sbi_domain *dom,
+			  struct sbi_domain_data *data, void *data_ptr);
+	/** Optional callback to cleanup domain data */
+	void (*data_cleanup)(struct sbi_domain *dom,
+			     struct sbi_domain_data *data, void *data_ptr);
+};
+
+/**
+ * Get per-domain data pointer for a given domain
+ * @param dom pointer to domain
+ * @param data pointer to domain data
+ *
+ * @return per-domain data pointer
+ */
+void *sbi_domain_data_ptr(struct sbi_domain *dom, struct sbi_domain_data *data);
+
+/**
+ * Setup all domain data for a domain
+ * @param dom pointer to domain
+ *
+ * @return 0 on success and negative error code on failure
+ *
+ * Note: This function is used internally within domain framework.
+ */
+int sbi_domain_setup_data(struct sbi_domain *dom);
+
+/**
+ * Cleanup all domain data for a domain
+ * @param dom pointer to domain
+ *
+ * Note: This function is used internally within domain framework.
+ */
+void sbi_domain_cleanup_data(struct sbi_domain *dom);
+
+/**
+ * Register a domain data
+ * @param hndl pointer to domain data
+ *
+ * @return 0 on success and negative error code on failure
+ *
+ * Note: This function must be used only in cold boot path.
+ */
+int sbi_domain_register_data(struct sbi_domain_data *data);
+
+/**
+ * Unregister a domain data
+ * @param hndl pointer to domain data
+ *
+ * Note: This function must be used only in cold boot path.
+ */
+void sbi_domain_unregister_data(struct sbi_domain_data *data);
+
+#endif
diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
index 535aa709..0b114bbd 100644
--- a/lib/sbi/objects.mk
+++ b/lib/sbi/objects.mk
@@ -65,6 +65,7 @@ libsbi-objs-y += sbi_bitmap.o
 libsbi-objs-y += sbi_bitops.o
 libsbi-objs-y += sbi_console.o
 libsbi-objs-y += sbi_domain_context.o
+libsbi-objs-y += sbi_domain_data.o
 libsbi-objs-y += sbi_domain.o
 libsbi-objs-y += sbi_emulate_csr.o
 libsbi-objs-y += sbi_fifo.o
diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
index 04e70d3b..22c5c752 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -587,6 +587,15 @@ int sbi_domain_register(struct sbi_domain *dom,
 		}
 	}
 
+	/* Setup data for the discovered domain */
+	rc = sbi_domain_setup_data(dom);
+	if (rc) {
+		sbi_printf("%s: domain data setup failed for %s (error %d)\n",
+			   __func__, dom->name, rc);
+		sbi_list_del(&dom->node);
+		return rc;
+	}
+
 	return 0;
 }
 
@@ -753,6 +762,8 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
 	struct sbi_domain_memregion *root_memregs;
 	const struct sbi_platform *plat = sbi_platform_ptr(scratch);
 
+	SBI_INIT_LIST_HEAD(&domain_list);
+
 	if (scratch->fw_rw_offset == 0 ||
 	    (scratch->fw_rw_offset & (scratch->fw_rw_offset - 1)) != 0) {
 		sbi_printf("%s: fw_rw_offset is not a power of 2 (0x%lx)\n",
diff --git a/lib/sbi/sbi_domain_data.c b/lib/sbi/sbi_domain_data.c
new file mode 100755
index 00000000..04f0edf9
--- /dev/null
+++ b/lib/sbi/sbi_domain_data.c
@@ -0,0 +1,138 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2024 Ventana Micro Systems Inc.
+ */
+
+#include <sbi/sbi_bitmap.h>
+#include <sbi/sbi_domain.h>
+#include <sbi/sbi_error.h>
+#include <sbi/sbi_heap.h>
+
+static SBI_LIST_HEAD(data_list);
+static DECLARE_BITMAP(data_idx_bmap, SBI_DOMAIN_MAX_DATA_PTRS);
+
+void *sbi_domain_data_ptr(struct sbi_domain *dom, struct sbi_domain_data *data)
+{
+	if (dom && data && data->data_idx < SBI_DOMAIN_MAX_DATA_PTRS)
+		return dom->data_priv.idx_to_data_ptr[data->data_idx];
+
+	return NULL;
+}
+
+static int domain_setup_data_one(struct sbi_domain *dom,
+				 struct sbi_domain_data *data)
+{
+	struct sbi_domain_data_priv *priv = &dom->data_priv;
+	void *data_ptr;
+	int rc;
+
+	if (priv->idx_to_data_ptr[data->data_idx])
+		return SBI_EALREADY;
+
+	data_ptr = sbi_zalloc(data->data_size);
+	if (!data_ptr) {
+		sbi_domain_cleanup_data(dom);
+		return SBI_ENOMEM;
+	}
+
+	if (data->data_setup) {
+		rc = data->data_setup(dom, data, data_ptr);
+		if (rc) {
+			sbi_free(data_ptr);
+			return rc;
+		}
+	}
+
+	priv->idx_to_data_ptr[data->data_idx] = data_ptr;
+	return 0;
+}
+
+static void domain_cleanup_data_one(struct sbi_domain *dom,
+				    struct sbi_domain_data *data)
+{
+	struct sbi_domain_data_priv *priv = &dom->data_priv;
+	void *data_ptr;
+
+	data_ptr = priv->idx_to_data_ptr[data->data_idx];
+	if (!data_ptr)
+		return;
+
+	if (data->data_cleanup)
+		data->data_cleanup(dom, data, data_ptr);
+
+	sbi_free(data_ptr);
+	priv->idx_to_data_ptr[data->data_idx] = NULL;
+}
+
+int sbi_domain_setup_data(struct sbi_domain *dom)
+{
+	struct sbi_domain_data *data;
+	int rc;
+
+	if (!dom)
+		return SBI_EINVAL;
+
+	sbi_list_for_each_entry(data, &data_list, head) {
+		rc = domain_setup_data_one(dom, data);
+		if (rc) {
+			sbi_domain_cleanup_data(dom);
+			return rc;
+		}
+	}
+
+	return 0;
+}
+
+void sbi_domain_cleanup_data(struct sbi_domain *dom)
+{
+	struct sbi_domain_data *data;
+
+	if (!dom)
+		return;
+
+	sbi_list_for_each_entry(data, &data_list, head)
+		domain_cleanup_data_one(dom, data);
+}
+
+int sbi_domain_register_data(struct sbi_domain_data *data)
+{
+	struct sbi_domain *dom;
+	u32 data_idx;
+	int rc;
+
+	if (!data || !data->data_size)
+		return SBI_EINVAL;
+
+	for (data_idx = 0; data_idx < SBI_DOMAIN_MAX_DATA_PTRS; data_idx++) {
+		if (!bitmap_test(data_idx_bmap, data_idx))
+			break;
+	}
+	if (SBI_DOMAIN_MAX_DATA_PTRS <= data_idx)
+		return SBI_ENOSPC;
+	bitmap_set(data_idx_bmap, data_idx, 1);
+
+	data->data_idx = data_idx;
+	sbi_list_add_tail(&data->head, &data_list);
+
+	sbi_domain_for_each(dom) {
+		rc = domain_setup_data_one(dom, data);
+		if (rc) {
+			sbi_domain_unregister_data(data);
+			return rc;
+		}
+	}
+
+	return 0;
+}
+
+void sbi_domain_unregister_data(struct sbi_domain_data *data)
+{
+	struct sbi_domain *dom;
+
+	sbi_domain_for_each(dom)
+		domain_cleanup_data_one(dom, data);
+
+	sbi_list_del(&data->head);
+	bitmap_clear(data_idx_bmap, data->data_idx, 1);
+}
-- 
2.43.0

[PATCH 7/9] lib: sbi: Introduce domain data

[Yu-Chien Peter Lin]

Hi Anup,

On Mon, Sep 23, 2024 at 05:26:58PM +0530, Anup Patel wrote:
> Different parts of OpenSBI require their own per-domain data so
> introduce domain data (or sbi_domain_data) which can be registered
> by any part of OpenSBI. Using the domain data, the domain framework
> will create a data pointer for every domain which can be used to
> maintain some per-domain state.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  include/sbi/sbi_domain.h      |   3 +
>  include/sbi/sbi_domain_data.h |  93 +++++++++++++++++++++++
>  lib/sbi/objects.mk            |   1 +
>  lib/sbi/sbi_domain.c          |  11 +++
>  lib/sbi/sbi_domain_data.c     | 138 ++++++++++++++++++++++++++++++++++
>  5 files changed, 246 insertions(+)
>  create mode 100755 include/sbi/sbi_domain_data.h
>  create mode 100755 lib/sbi/sbi_domain_data.c

The file permission is executable.


Reviewed-by: Yu Chien Peter Lin <peterlin@andestech.com>

Best regards,
Peter Lin

> diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h
> index 22aa8e50..60d7a776 100644
> --- a/include/sbi/sbi_domain.h
> +++ b/include/sbi/sbi_domain.h
> @@ -15,6 +15,7 @@
>  #include <sbi/sbi_types.h>
>  #include <sbi/sbi_hartmask.h>
>  #include <sbi/sbi_domain_context.h>
> +#include <sbi/sbi_domain_data.h>
>  
>  struct sbi_scratch;
>  
> @@ -163,6 +164,8 @@ struct sbi_domain_memregion {
>  struct sbi_domain {
>  	/** Node in linked list of domains */
>  	struct sbi_dlist node;
> +	/** Internal state of per-domain data */
> +	struct sbi_domain_data_priv data_priv;
>  	/** Logical index of this domain */
>  	u32 index;
>  	/** HARTs assigned to this domain */
> diff --git a/include/sbi/sbi_domain_data.h b/include/sbi/sbi_domain_data.h
> new file mode 100755
> index 00000000..7eeafdce
> --- /dev/null
> +++ b/include/sbi/sbi_domain_data.h
> @@ -0,0 +1,93 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (c) 2024 Ventana Micro Systems Inc.
> + */
> +
> +#ifndef __SBI_DOMAIN_DATA_H__
> +#define __SBI_DOMAIN_DATA_H__
> +
> +#include <sbi/sbi_types.h>
> +#include <sbi/sbi_list.h>
> +
> +struct sbi_domain;
> +
> +/** Maximum domain data per-domain */
> +#define SBI_DOMAIN_MAX_DATA_PTRS		32
> +
> +/** Representation of per-domain data */
> +struct sbi_domain_data_priv {
> +	/** Array of domain data pointers indexed by domain data identifier */
> +	void *idx_to_data_ptr[SBI_DOMAIN_MAX_DATA_PTRS];
> +};
> +
> +/** Representation of a domain data */
> +struct sbi_domain_data {
> +	/**
> +	 * Head is used for maintaining data list
> +	 *
> +	 * Note: initialized by domain framework
> +	 */
> +	struct sbi_dlist head;
> +	/**
> +	 * Identifier which used to locate per-domain data
> +	 *
> +	 * Note: initialized by domain framework
> +	 */
> +	unsigned long data_idx;
> +	/** Size of per-domain data */
> +	unsigned long data_size;
> +	/** Optional callback to setup domain data */
> +	int (*data_setup)(struct sbi_domain *dom,
> +			  struct sbi_domain_data *data, void *data_ptr);
> +	/** Optional callback to cleanup domain data */
> +	void (*data_cleanup)(struct sbi_domain *dom,
> +			     struct sbi_domain_data *data, void *data_ptr);
> +};
> +
> +/**
> + * Get per-domain data pointer for a given domain
> + * @param dom pointer to domain
> + * @param data pointer to domain data
> + *
> + * @return per-domain data pointer
> + */
> +void *sbi_domain_data_ptr(struct sbi_domain *dom, struct sbi_domain_data *data);
> +
> +/**
> + * Setup all domain data for a domain
> + * @param dom pointer to domain
> + *
> + * @return 0 on success and negative error code on failure
> + *
> + * Note: This function is used internally within domain framework.
> + */
> +int sbi_domain_setup_data(struct sbi_domain *dom);
> +
> +/**
> + * Cleanup all domain data for a domain
> + * @param dom pointer to domain
> + *
> + * Note: This function is used internally within domain framework.
> + */
> +void sbi_domain_cleanup_data(struct sbi_domain *dom);
> +
> +/**
> + * Register a domain data
> + * @param hndl pointer to domain data
> + *
> + * @return 0 on success and negative error code on failure
> + *
> + * Note: This function must be used only in cold boot path.
> + */
> +int sbi_domain_register_data(struct sbi_domain_data *data);
> +
> +/**
> + * Unregister a domain data
> + * @param hndl pointer to domain data
> + *
> + * Note: This function must be used only in cold boot path.
> + */
> +void sbi_domain_unregister_data(struct sbi_domain_data *data);
> +
> +#endif
> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
> index 535aa709..0b114bbd 100644
> --- a/lib/sbi/objects.mk
> +++ b/lib/sbi/objects.mk
> @@ -65,6 +65,7 @@ libsbi-objs-y += sbi_bitmap.o
>  libsbi-objs-y += sbi_bitops.o
>  libsbi-objs-y += sbi_console.o
>  libsbi-objs-y += sbi_domain_context.o
> +libsbi-objs-y += sbi_domain_data.o
>  libsbi-objs-y += sbi_domain.o
>  libsbi-objs-y += sbi_emulate_csr.o
>  libsbi-objs-y += sbi_fifo.o
> diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
> index 04e70d3b..22c5c752 100644
> --- a/lib/sbi/sbi_domain.c
> +++ b/lib/sbi/sbi_domain.c
> @@ -587,6 +587,15 @@ int sbi_domain_register(struct sbi_domain *dom,
>  		}
>  	}
>  
> +	/* Setup data for the discovered domain */
> +	rc = sbi_domain_setup_data(dom);
> +	if (rc) {
> +		sbi_printf("%s: domain data setup failed for %s (error %d)\n",
> +			   __func__, dom->name, rc);
> +		sbi_list_del(&dom->node);
> +		return rc;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -753,6 +762,8 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
>  	struct sbi_domain_memregion *root_memregs;
>  	const struct sbi_platform *plat = sbi_platform_ptr(scratch);
>  
> +	SBI_INIT_LIST_HEAD(&domain_list);
> +
>  	if (scratch->fw_rw_offset == 0 ||
>  	    (scratch->fw_rw_offset & (scratch->fw_rw_offset - 1)) != 0) {
>  		sbi_printf("%s: fw_rw_offset is not a power of 2 (0x%lx)\n",
> diff --git a/lib/sbi/sbi_domain_data.c b/lib/sbi/sbi_domain_data.c
> new file mode 100755
> index 00000000..04f0edf9
> --- /dev/null
> +++ b/lib/sbi/sbi_domain_data.c
> @@ -0,0 +1,138 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (c) 2024 Ventana Micro Systems Inc.
> + */
> +
> +#include <sbi/sbi_bitmap.h>
> +#include <sbi/sbi_domain.h>
> +#include <sbi/sbi_error.h>
> +#include <sbi/sbi_heap.h>
> +
> +static SBI_LIST_HEAD(data_list);
> +static DECLARE_BITMAP(data_idx_bmap, SBI_DOMAIN_MAX_DATA_PTRS);
> +
> +void *sbi_domain_data_ptr(struct sbi_domain *dom, struct sbi_domain_data *data)
> +{
> +	if (dom && data && data->data_idx < SBI_DOMAIN_MAX_DATA_PTRS)
> +		return dom->data_priv.idx_to_data_ptr[data->data_idx];
> +
> +	return NULL;
> +}
> +
> +static int domain_setup_data_one(struct sbi_domain *dom,
> +				 struct sbi_domain_data *data)
> +{
> +	struct sbi_domain_data_priv *priv = &dom->data_priv;
> +	void *data_ptr;
> +	int rc;
> +
> +	if (priv->idx_to_data_ptr[data->data_idx])
> +		return SBI_EALREADY;
> +
> +	data_ptr = sbi_zalloc(data->data_size);
> +	if (!data_ptr) {
> +		sbi_domain_cleanup_data(dom);
> +		return SBI_ENOMEM;
> +	}
> +
> +	if (data->data_setup) {
> +		rc = data->data_setup(dom, data, data_ptr);
> +		if (rc) {
> +			sbi_free(data_ptr);
> +			return rc;
> +		}
> +	}
> +
> +	priv->idx_to_data_ptr[data->data_idx] = data_ptr;
> +	return 0;
> +}
> +
> +static void domain_cleanup_data_one(struct sbi_domain *dom,
> +				    struct sbi_domain_data *data)
> +{
> +	struct sbi_domain_data_priv *priv = &dom->data_priv;
> +	void *data_ptr;
> +
> +	data_ptr = priv->idx_to_data_ptr[data->data_idx];
> +	if (!data_ptr)
> +		return;
> +
> +	if (data->data_cleanup)
> +		data->data_cleanup(dom, data, data_ptr);
> +
> +	sbi_free(data_ptr);
> +	priv->idx_to_data_ptr[data->data_idx] = NULL;
> +}
> +
> +int sbi_domain_setup_data(struct sbi_domain *dom)
> +{
> +	struct sbi_domain_data *data;
> +	int rc;
> +
> +	if (!dom)
> +		return SBI_EINVAL;
> +
> +	sbi_list_for_each_entry(data, &data_list, head) {
> +		rc = domain_setup_data_one(dom, data);
> +		if (rc) {
> +			sbi_domain_cleanup_data(dom);
> +			return rc;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +void sbi_domain_cleanup_data(struct sbi_domain *dom)
> +{
> +	struct sbi_domain_data *data;
> +
> +	if (!dom)
> +		return;
> +
> +	sbi_list_for_each_entry(data, &data_list, head)
> +		domain_cleanup_data_one(dom, data);
> +}
> +
> +int sbi_domain_register_data(struct sbi_domain_data *data)
> +{
> +	struct sbi_domain *dom;
> +	u32 data_idx;
> +	int rc;
> +
> +	if (!data || !data->data_size)
> +		return SBI_EINVAL;
> +
> +	for (data_idx = 0; data_idx < SBI_DOMAIN_MAX_DATA_PTRS; data_idx++) {
> +		if (!bitmap_test(data_idx_bmap, data_idx))
> +			break;
> +	}
> +	if (SBI_DOMAIN_MAX_DATA_PTRS <= data_idx)
> +		return SBI_ENOSPC;
> +	bitmap_set(data_idx_bmap, data_idx, 1);
> +
> +	data->data_idx = data_idx;
> +	sbi_list_add_tail(&data->head, &data_list);
> +
> +	sbi_domain_for_each(dom) {
> +		rc = domain_setup_data_one(dom, data);
> +		if (rc) {
> +			sbi_domain_unregister_data(data);
> +			return rc;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +void sbi_domain_unregister_data(struct sbi_domain_data *data)
> +{
> +	struct sbi_domain *dom;
> +
> +	sbi_domain_for_each(dom)
> +		domain_cleanup_data_one(dom, data);
> +
> +	sbi_list_del(&data->head);
> +	bitmap_clear(data_idx_bmap, data->data_idx, 1);
> +}

[PATCH 7/9] lib: sbi: Introduce domain data

[Anup Patel]

On Fri, Oct 4, 2024 at 4:27?PM Yu-Chien Peter Lin
<peterlin@andestech.com> wrote:
>
> Hi Anup,
>
> On Mon, Sep 23, 2024 at 05:26:58PM +0530, Anup Patel wrote:
> > Different parts of OpenSBI require their own per-domain data so
> > introduce domain data (or sbi_domain_data) which can be registered
> > by any part of OpenSBI. Using the domain data, the domain framework
> > will create a data pointer for every domain which can be used to
> > maintain some per-domain state.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  include/sbi/sbi_domain.h      |   3 +
> >  include/sbi/sbi_domain_data.h |  93 +++++++++++++++++++++++
> >  lib/sbi/objects.mk            |   1 +
> >  lib/sbi/sbi_domain.c          |  11 +++
> >  lib/sbi/sbi_domain_data.c     | 138 ++++++++++++++++++++++++++++++++++
> >  5 files changed, 246 insertions(+)
> >  create mode 100755 include/sbi/sbi_domain_data.h
> >  create mode 100755 lib/sbi/sbi_domain_data.c
>
> The file permission is executable.

Okay, I will fix file permission in the next revision.

>
>
> Reviewed-by: Yu Chien Peter Lin <peterlin@andestech.com>

Thanks,
Anup

>
> Best regards,
> Peter Lin
>
> > diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h
> > index 22aa8e50..60d7a776 100644
> > --- a/include/sbi/sbi_domain.h
> > +++ b/include/sbi/sbi_domain.h
> > @@ -15,6 +15,7 @@
> >  #include <sbi/sbi_types.h>
> >  #include <sbi/sbi_hartmask.h>
> >  #include <sbi/sbi_domain_context.h>
> > +#include <sbi/sbi_domain_data.h>
> >
> >  struct sbi_scratch;
> >
> > @@ -163,6 +164,8 @@ struct sbi_domain_memregion {
> >  struct sbi_domain {
> >       /** Node in linked list of domains */
> >       struct sbi_dlist node;
> > +     /** Internal state of per-domain data */
> > +     struct sbi_domain_data_priv data_priv;
> >       /** Logical index of this domain */
> >       u32 index;
> >       /** HARTs assigned to this domain */
> > diff --git a/include/sbi/sbi_domain_data.h b/include/sbi/sbi_domain_data.h
> > new file mode 100755
> > index 00000000..7eeafdce
> > --- /dev/null
> > +++ b/include/sbi/sbi_domain_data.h
> > @@ -0,0 +1,93 @@
> > +/*
> > + * SPDX-License-Identifier: BSD-2-Clause
> > + *
> > + * Copyright (c) 2024 Ventana Micro Systems Inc.
> > + */
> > +
> > +#ifndef __SBI_DOMAIN_DATA_H__
> > +#define __SBI_DOMAIN_DATA_H__
> > +
> > +#include <sbi/sbi_types.h>
> > +#include <sbi/sbi_list.h>
> > +
> > +struct sbi_domain;
> > +
> > +/** Maximum domain data per-domain */
> > +#define SBI_DOMAIN_MAX_DATA_PTRS             32
> > +
> > +/** Representation of per-domain data */
> > +struct sbi_domain_data_priv {
> > +     /** Array of domain data pointers indexed by domain data identifier */
> > +     void *idx_to_data_ptr[SBI_DOMAIN_MAX_DATA_PTRS];
> > +};
> > +
> > +/** Representation of a domain data */
> > +struct sbi_domain_data {
> > +     /**
> > +      * Head is used for maintaining data list
> > +      *
> > +      * Note: initialized by domain framework
> > +      */
> > +     struct sbi_dlist head;
> > +     /**
> > +      * Identifier which used to locate per-domain data
> > +      *
> > +      * Note: initialized by domain framework
> > +      */
> > +     unsigned long data_idx;
> > +     /** Size of per-domain data */
> > +     unsigned long data_size;
> > +     /** Optional callback to setup domain data */
> > +     int (*data_setup)(struct sbi_domain *dom,
> > +                       struct sbi_domain_data *data, void *data_ptr);
> > +     /** Optional callback to cleanup domain data */
> > +     void (*data_cleanup)(struct sbi_domain *dom,
> > +                          struct sbi_domain_data *data, void *data_ptr);
> > +};
> > +
> > +/**
> > + * Get per-domain data pointer for a given domain
> > + * @param dom pointer to domain
> > + * @param data pointer to domain data
> > + *
> > + * @return per-domain data pointer
> > + */
> > +void *sbi_domain_data_ptr(struct sbi_domain *dom, struct sbi_domain_data *data);
> > +
> > +/**
> > + * Setup all domain data for a domain
> > + * @param dom pointer to domain
> > + *
> > + * @return 0 on success and negative error code on failure
> > + *
> > + * Note: This function is used internally within domain framework.
> > + */
> > +int sbi_domain_setup_data(struct sbi_domain *dom);
> > +
> > +/**
> > + * Cleanup all domain data for a domain
> > + * @param dom pointer to domain
> > + *
> > + * Note: This function is used internally within domain framework.
> > + */
> > +void sbi_domain_cleanup_data(struct sbi_domain *dom);
> > +
> > +/**
> > + * Register a domain data
> > + * @param hndl pointer to domain data
> > + *
> > + * @return 0 on success and negative error code on failure
> > + *
> > + * Note: This function must be used only in cold boot path.
> > + */
> > +int sbi_domain_register_data(struct sbi_domain_data *data);
> > +
> > +/**
> > + * Unregister a domain data
> > + * @param hndl pointer to domain data
> > + *
> > + * Note: This function must be used only in cold boot path.
> > + */
> > +void sbi_domain_unregister_data(struct sbi_domain_data *data);
> > +
> > +#endif
> > diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
> > index 535aa709..0b114bbd 100644
> > --- a/lib/sbi/objects.mk
> > +++ b/lib/sbi/objects.mk
> > @@ -65,6 +65,7 @@ libsbi-objs-y += sbi_bitmap.o
> >  libsbi-objs-y += sbi_bitops.o
> >  libsbi-objs-y += sbi_console.o
> >  libsbi-objs-y += sbi_domain_context.o
> > +libsbi-objs-y += sbi_domain_data.o
> >  libsbi-objs-y += sbi_domain.o
> >  libsbi-objs-y += sbi_emulate_csr.o
> >  libsbi-objs-y += sbi_fifo.o
> > diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
> > index 04e70d3b..22c5c752 100644
> > --- a/lib/sbi/sbi_domain.c
> > +++ b/lib/sbi/sbi_domain.c
> > @@ -587,6 +587,15 @@ int sbi_domain_register(struct sbi_domain *dom,
> >               }
> >       }
> >
> > +     /* Setup data for the discovered domain */
> > +     rc = sbi_domain_setup_data(dom);
> > +     if (rc) {
> > +             sbi_printf("%s: domain data setup failed for %s (error %d)\n",
> > +                        __func__, dom->name, rc);
> > +             sbi_list_del(&dom->node);
> > +             return rc;
> > +     }
> > +
> >       return 0;
> >  }
> >
> > @@ -753,6 +762,8 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
> >       struct sbi_domain_memregion *root_memregs;
> >       const struct sbi_platform *plat = sbi_platform_ptr(scratch);
> >
> > +     SBI_INIT_LIST_HEAD(&domain_list);
> > +
> >       if (scratch->fw_rw_offset == 0 ||
> >           (scratch->fw_rw_offset & (scratch->fw_rw_offset - 1)) != 0) {
> >               sbi_printf("%s: fw_rw_offset is not a power of 2 (0x%lx)\n",
> > diff --git a/lib/sbi/sbi_domain_data.c b/lib/sbi/sbi_domain_data.c
> > new file mode 100755
> > index 00000000..04f0edf9
> > --- /dev/null
> > +++ b/lib/sbi/sbi_domain_data.c
> > @@ -0,0 +1,138 @@
> > +/*
> > + * SPDX-License-Identifier: BSD-2-Clause
> > + *
> > + * Copyright (c) 2024 Ventana Micro Systems Inc.
> > + */
> > +
> > +#include <sbi/sbi_bitmap.h>
> > +#include <sbi/sbi_domain.h>
> > +#include <sbi/sbi_error.h>
> > +#include <sbi/sbi_heap.h>
> > +
> > +static SBI_LIST_HEAD(data_list);
> > +static DECLARE_BITMAP(data_idx_bmap, SBI_DOMAIN_MAX_DATA_PTRS);
> > +
> > +void *sbi_domain_data_ptr(struct sbi_domain *dom, struct sbi_domain_data *data)
> > +{
> > +     if (dom && data && data->data_idx < SBI_DOMAIN_MAX_DATA_PTRS)
> > +             return dom->data_priv.idx_to_data_ptr[data->data_idx];
> > +
> > +     return NULL;
> > +}
> > +
> > +static int domain_setup_data_one(struct sbi_domain *dom,
> > +                              struct sbi_domain_data *data)
> > +{
> > +     struct sbi_domain_data_priv *priv = &dom->data_priv;
> > +     void *data_ptr;
> > +     int rc;
> > +
> > +     if (priv->idx_to_data_ptr[data->data_idx])
> > +             return SBI_EALREADY;
> > +
> > +     data_ptr = sbi_zalloc(data->data_size);
> > +     if (!data_ptr) {
> > +             sbi_domain_cleanup_data(dom);
> > +             return SBI_ENOMEM;
> > +     }
> > +
> > +     if (data->data_setup) {
> > +             rc = data->data_setup(dom, data, data_ptr);
> > +             if (rc) {
> > +                     sbi_free(data_ptr);
> > +                     return rc;
> > +             }
> > +     }
> > +
> > +     priv->idx_to_data_ptr[data->data_idx] = data_ptr;
> > +     return 0;
> > +}
> > +
> > +static void domain_cleanup_data_one(struct sbi_domain *dom,
> > +                                 struct sbi_domain_data *data)
> > +{
> > +     struct sbi_domain_data_priv *priv = &dom->data_priv;
> > +     void *data_ptr;
> > +
> > +     data_ptr = priv->idx_to_data_ptr[data->data_idx];
> > +     if (!data_ptr)
> > +             return;
> > +
> > +     if (data->data_cleanup)
> > +             data->data_cleanup(dom, data, data_ptr);
> > +
> > +     sbi_free(data_ptr);
> > +     priv->idx_to_data_ptr[data->data_idx] = NULL;
> > +}
> > +
> > +int sbi_domain_setup_data(struct sbi_domain *dom)
> > +{
> > +     struct sbi_domain_data *data;
> > +     int rc;
> > +
> > +     if (!dom)
> > +             return SBI_EINVAL;
> > +
> > +     sbi_list_for_each_entry(data, &data_list, head) {
> > +             rc = domain_setup_data_one(dom, data);
> > +             if (rc) {
> > +                     sbi_domain_cleanup_data(dom);
> > +                     return rc;
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +void sbi_domain_cleanup_data(struct sbi_domain *dom)
> > +{
> > +     struct sbi_domain_data *data;
> > +
> > +     if (!dom)
> > +             return;
> > +
> > +     sbi_list_for_each_entry(data, &data_list, head)
> > +             domain_cleanup_data_one(dom, data);
> > +}
> > +
> > +int sbi_domain_register_data(struct sbi_domain_data *data)
> > +{
> > +     struct sbi_domain *dom;
> > +     u32 data_idx;
> > +     int rc;
> > +
> > +     if (!data || !data->data_size)
> > +             return SBI_EINVAL;
> > +
> > +     for (data_idx = 0; data_idx < SBI_DOMAIN_MAX_DATA_PTRS; data_idx++) {
> > +             if (!bitmap_test(data_idx_bmap, data_idx))
> > +                     break;
> > +     }
> > +     if (SBI_DOMAIN_MAX_DATA_PTRS <= data_idx)
> > +             return SBI_ENOSPC;
> > +     bitmap_set(data_idx_bmap, data_idx, 1);
> > +
> > +     data->data_idx = data_idx;
> > +     sbi_list_add_tail(&data->head, &data_list);
> > +
> > +     sbi_domain_for_each(dom) {
> > +             rc = domain_setup_data_one(dom, data);
> > +             if (rc) {
> > +                     sbi_domain_unregister_data(data);
> > +                     return rc;
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +void sbi_domain_unregister_data(struct sbi_domain_data *data)
> > +{
> > +     struct sbi_domain *dom;
> > +
> > +     sbi_domain_for_each(dom)
> > +             domain_cleanup_data_one(dom, data);
> > +
> > +     sbi_list_del(&data->head);
> > +     bitmap_clear(data_idx_bmap, data->data_idx, 1);
> > +}

[PATCH 8/9] lib: sbi_domain: Use domain data support for per-domain hart context

[Anup Patel]

The per-domain hartindex_to_context_table[] is yet another per-domain
data required for implementing hart entry into (or exit from) domain.

Use the recently added domain data support for per-domain hart context
so that a dedicated hartindex_to_context_table[] in struct sbi_domain
is not needed.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 include/sbi/sbi_domain.h         |   2 -
 include/sbi/sbi_domain_context.h |  57 +++--------------
 lib/sbi/sbi_domain.c             |   9 ++-
 lib/sbi/sbi_domain_context.c     | 104 +++++++++++++++++++++++++++----
 4 files changed, 110 insertions(+), 62 deletions(-)

diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h
index 60d7a776..1ecf3116 100644
--- a/include/sbi/sbi_domain.h
+++ b/include/sbi/sbi_domain.h
@@ -176,8 +176,6 @@ struct sbi_domain {
 	char name[64];
 	/** Possible HARTs in this domain */
 	const struct sbi_hartmask *possible_harts;
-	/** Contexts for possible HARTs indexed by hartindex */
-	struct sbi_context *hartindex_to_context_table[SBI_HARTMASK_MAX_BITS];
 	/** Array of memory regions terminated by a region with order zero */
 	struct sbi_domain_memregion *regions;
 	/** HART id of the HART booting this domain */
diff --git a/include/sbi/sbi_domain_context.h b/include/sbi/sbi_domain_context.h
index 3f43b622..31a3a7f8 100755
--- a/include/sbi/sbi_domain_context.h
+++ b/include/sbi/sbi_domain_context.h
@@ -8,56 +8,9 @@
 #define __SBI_DOMAIN_CONTEXT_H__
 
 #include <sbi/sbi_types.h>
-#include <sbi/sbi_trap.h>
 
 struct sbi_domain;
 
-/** Context representation for a hart within a domain */
-struct sbi_context {
-	/** Trap-related states such as GPRs, mepc, and mstatus */
-	struct sbi_trap_context trap_ctx;
-
-	/** Supervisor status register */
-	unsigned long sstatus;
-	/** Supervisor interrupt enable register */
-	unsigned long sie;
-	/** Supervisor trap vector base address register */
-	unsigned long stvec;
-	/** Supervisor scratch register for temporary storage */
-	unsigned long sscratch;
-	/** Supervisor exception program counter register */
-	unsigned long sepc;
-	/** Supervisor cause register */
-	unsigned long scause;
-	/** Supervisor trap value register */
-	unsigned long stval;
-	/** Supervisor interrupt pending register */
-	unsigned long sip;
-	/** Supervisor address translation and protection register */
-	unsigned long satp;
-	/** Counter-enable register */
-	unsigned long scounteren;
-	/** Supervisor environment configuration register */
-	unsigned long senvcfg;
-
-	/** Reference to the owning domain */
-	struct sbi_domain *dom;
-	/** Previous context (caller) to jump to during context exits */
-	struct sbi_context *prev_ctx;
-	/** Is context initialized and runnable */
-	bool initialized;
-};
-
-/** Get the context pointer for a given hart index and domain */
-#define sbi_hartindex_to_domain_context(__hartindex, __d) \
-	(__d)->hartindex_to_context_table[__hartindex]
-
-/** Macro to obtain the current hart's context pointer */
-#define sbi_domain_context_thishart_ptr()                  \
-	sbi_hartindex_to_domain_context(                   \
-		sbi_hartid_to_hartindex(current_hartid()), \
-		sbi_domain_thishart_ptr())
-
 /**
  * Enter a specific domain context synchronously
  * @param dom pointer to domain
@@ -75,4 +28,14 @@ int sbi_domain_context_enter(struct sbi_domain *dom);
  */
 int sbi_domain_context_exit(void);
 
+/**
+ * Initialize domain context support
+ *
+ * @return 0 on success and negative error code on failure
+ */
+int sbi_domain_context_init(void);
+
+/* Deinitialize domain context support */
+void sbi_domain_context_deinit(void);
+
 #endif // __SBI_DOMAIN_CONTEXT_H__
diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
index 22c5c752..ee6e2569 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -781,11 +781,16 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
 	if (!domain_hart_ptr_offset)
 		return SBI_ENOMEM;
 
+	/* Initialize domain context support */
+	rc = sbi_domain_context_init();
+	if (rc)
+		goto fail_free_domain_hart_ptr_offset;
+
 	root_memregs = sbi_calloc(sizeof(*root_memregs), ROOT_REGION_MAX + 1);
 	if (!root_memregs) {
 		sbi_printf("%s: no memory for root regions\n", __func__);
 		rc = SBI_ENOMEM;
-		goto fail_free_domain_hart_ptr_offset;
+		goto fail_deinit_context;
 	}
 	root.regions = root_memregs;
 
@@ -850,6 +855,8 @@ fail_free_root_hmask:
 	sbi_free(root_hmask);
 fail_free_root_memregs:
 	sbi_free(root_memregs);
+fail_deinit_context:
+	sbi_domain_context_deinit();
 fail_free_domain_hart_ptr_offset:
 	sbi_scratch_free_offset(domain_hart_ptr_offset);
 	return rc;
diff --git a/lib/sbi/sbi_domain_context.c b/lib/sbi/sbi_domain_context.c
index 29e2d280..b70db41e 100755
--- a/lib/sbi/sbi_domain_context.c
+++ b/lib/sbi/sbi_domain_context.c
@@ -14,6 +14,76 @@
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 #include <sbi/sbi_domain.h>
+#include <sbi/sbi_trap.h>
+
+/** Context representation for a hart within a domain */
+struct hart_context {
+	/** Trap-related states such as GPRs, mepc, and mstatus */
+	struct sbi_trap_context trap_ctx;
+
+	/** Supervisor status register */
+	unsigned long sstatus;
+	/** Supervisor interrupt enable register */
+	unsigned long sie;
+	/** Supervisor trap vector base address register */
+	unsigned long stvec;
+	/** Supervisor scratch register for temporary storage */
+	unsigned long sscratch;
+	/** Supervisor exception program counter register */
+	unsigned long sepc;
+	/** Supervisor cause register */
+	unsigned long scause;
+	/** Supervisor trap value register */
+	unsigned long stval;
+	/** Supervisor interrupt pending register */
+	unsigned long sip;
+	/** Supervisor address translation and protection register */
+	unsigned long satp;
+	/** Counter-enable register */
+	unsigned long scounteren;
+	/** Supervisor environment configuration register */
+	unsigned long senvcfg;
+
+	/** Reference to the owning domain */
+	struct sbi_domain *dom;
+	/** Previous context (caller) to jump to during context exits */
+	struct hart_context *prev_ctx;
+	/** Is context initialized and runnable */
+	bool initialized;
+};
+
+struct domain_context_priv {
+	/** Contexts for possible HARTs indexed by hartindex */
+	struct hart_context *hartindex_to_context_table[SBI_HARTMASK_MAX_BITS];
+};
+
+static struct sbi_domain_data dcpriv = {
+	.data_size = sizeof(struct domain_context_priv),
+};
+
+static inline struct hart_context *hart_context_get(struct sbi_domain *dom,
+						    u32 hartindex)
+{
+	struct domain_context_priv *dcp = sbi_domain_data_ptr(dom, &dcpriv);
+
+	return (dcp && hartindex < SBI_HARTMASK_MAX_BITS) ?
+		dcp->hartindex_to_context_table[hartindex] : NULL;
+}
+
+static void hart_context_set(struct sbi_domain *dom, u32 hartindex,
+			     struct hart_context *hc)
+{
+	struct domain_context_priv *dcp = sbi_domain_data_ptr(dom, &dcpriv);
+
+	if (dcp && hartindex < SBI_HARTMASK_MAX_BITS) {
+		dcp->hartindex_to_context_table[hartindex] = hc;
+	}
+}
+
+/** Macro to obtain the current hart's context pointer */
+#define hart_context_thishart_get()					\
+	hart_context_get(sbi_domain_thishart_ptr(),			\
+			 sbi_hartid_to_hartindex(current_hartid()))
 
 /**
  * Switches the HART context from the current domain to the target domain.
@@ -23,8 +93,8 @@
  * @param ctx pointer to the current HART context
  * @param dom_ctx pointer to the target domain context
  */
-static void switch_to_next_domain_context(struct sbi_context *ctx,
-					  struct sbi_context *dom_ctx)
+static void switch_to_next_domain_context(struct hart_context *ctx,
+					  struct hart_context *dom_ctx)
 {
 	u32 hartindex = sbi_hartid_to_hartindex(current_hartid());
 	struct sbi_trap_context *trap_ctx;
@@ -89,9 +159,9 @@ static void switch_to_next_domain_context(struct sbi_context *ctx,
 
 int sbi_domain_context_enter(struct sbi_domain *dom)
 {
-	struct sbi_context *ctx = sbi_domain_context_thishart_ptr();
-	struct sbi_context *dom_ctx = sbi_hartindex_to_domain_context(
-		sbi_hartid_to_hartindex(current_hartid()), dom);
+	struct hart_context *ctx = hart_context_thishart_get();
+	struct hart_context *dom_ctx = hart_context_get(dom,
+				sbi_hartid_to_hartindex(current_hartid()));
 
 	/* Validate the domain context existence */
 	if (!dom_ctx)
@@ -109,8 +179,8 @@ int sbi_domain_context_exit(void)
 {
 	u32 hartindex = sbi_hartid_to_hartindex(current_hartid());
 	struct sbi_domain *dom;
-	struct sbi_context *ctx = sbi_domain_context_thishart_ptr();
-	struct sbi_context *dom_ctx, *tmp;
+	struct hart_context *ctx = hart_context_thishart_get();
+	struct hart_context *dom_ctx, *tmp;
 
 	/*
 	 * If it's first time to call `exit` on the current hart, no
@@ -123,16 +193,16 @@ int sbi_domain_context_exit(void)
 							 dom->possible_harts))
 				continue;
 
-			dom_ctx = sbi_zalloc(sizeof(struct sbi_context));
+			dom_ctx = sbi_zalloc(sizeof(struct hart_context));
 			if (!dom_ctx)
 				return SBI_ENOMEM;
 
 			/* Bind context and domain */
 			dom_ctx->dom				   = dom;
-			dom->hartindex_to_context_table[hartindex] = dom_ctx;
+			hart_context_set(dom, hartindex, dom_ctx);
 		}
 
-		ctx = sbi_domain_context_thishart_ptr();
+		ctx = hart_context_thishart_get();
 	}
 
 	dom_ctx = ctx->prev_ctx;
@@ -144,7 +214,7 @@ int sbi_domain_context_exit(void)
 			if (dom == &root || dom == sbi_domain_thishart_ptr())
 				continue;
 
-			tmp = sbi_hartindex_to_domain_context(hartindex, dom);
+			tmp = hart_context_get(dom, hartindex);
 			if (tmp && !tmp->initialized) {
 				dom_ctx = tmp;
 				break;
@@ -154,9 +224,19 @@ int sbi_domain_context_exit(void)
 
 	/* Take the root domain context if fail to find */
 	if (!dom_ctx)
-		dom_ctx = sbi_hartindex_to_domain_context(hartindex, &root);
+		dom_ctx = hart_context_get(&root, hartindex);
 
 	switch_to_next_domain_context(ctx, dom_ctx);
 
 	return 0;
 }
+
+int sbi_domain_context_init(void)
+{
+	return sbi_domain_register_data(&dcpriv);
+}
+
+void sbi_domain_context_deinit(void)
+{
+	sbi_domain_unregister_data(&dcpriv);
+}
-- 
2.43.0

[PATCH 8/9] lib: sbi_domain: Use domain data support for per-domain hart context

[Yu-Chien Peter Lin]

Hi Anup,

I noticed a few minor non-functional issues.

On Mon, Sep 23, 2024 at 05:26:59PM +0530, Anup Patel wrote:
> The per-domain hartindex_to_context_table[] is yet another per-domain
> data required for implementing hart entry into (or exit from) domain.
> 
> Use the recently added domain data support for per-domain hart context
> so that a dedicated hartindex_to_context_table[] in struct sbi_domain
> is not needed.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  include/sbi/sbi_domain.h         |   2 -
>  include/sbi/sbi_domain_context.h |  57 +++--------------
>  lib/sbi/sbi_domain.c             |   9 ++-
>  lib/sbi/sbi_domain_context.c     | 104 +++++++++++++++++++++++++++----
>  4 files changed, 110 insertions(+), 62 deletions(-)
> 
> diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h
> index 60d7a776..1ecf3116 100644
> --- a/include/sbi/sbi_domain.h
> +++ b/include/sbi/sbi_domain.h
> @@ -176,8 +176,6 @@ struct sbi_domain {
>  	char name[64];
>  	/** Possible HARTs in this domain */
>  	const struct sbi_hartmask *possible_harts;
> -	/** Contexts for possible HARTs indexed by hartindex */
> -	struct sbi_context *hartindex_to_context_table[SBI_HARTMASK_MAX_BITS];
>  	/** Array of memory regions terminated by a region with order zero */
>  	struct sbi_domain_memregion *regions;
>  	/** HART id of the HART booting this domain */
> diff --git a/include/sbi/sbi_domain_context.h b/include/sbi/sbi_domain_context.h
> index 3f43b622..31a3a7f8 100755
> --- a/include/sbi/sbi_domain_context.h
> +++ b/include/sbi/sbi_domain_context.h
> @@ -8,56 +8,9 @@
>  #define __SBI_DOMAIN_CONTEXT_H__
>  
>  #include <sbi/sbi_types.h>
> -#include <sbi/sbi_trap.h>
>  
>  struct sbi_domain;
>  
> -/** Context representation for a hart within a domain */
> -struct sbi_context {
> -	/** Trap-related states such as GPRs, mepc, and mstatus */
> -	struct sbi_trap_context trap_ctx;
> -
> -	/** Supervisor status register */
> -	unsigned long sstatus;
> -	/** Supervisor interrupt enable register */
> -	unsigned long sie;
> -	/** Supervisor trap vector base address register */
> -	unsigned long stvec;
> -	/** Supervisor scratch register for temporary storage */
> -	unsigned long sscratch;
> -	/** Supervisor exception program counter register */
> -	unsigned long sepc;
> -	/** Supervisor cause register */
> -	unsigned long scause;
> -	/** Supervisor trap value register */
> -	unsigned long stval;
> -	/** Supervisor interrupt pending register */
> -	unsigned long sip;
> -	/** Supervisor address translation and protection register */
> -	unsigned long satp;
> -	/** Counter-enable register */
> -	unsigned long scounteren;
> -	/** Supervisor environment configuration register */
> -	unsigned long senvcfg;
> -
> -	/** Reference to the owning domain */
> -	struct sbi_domain *dom;
> -	/** Previous context (caller) to jump to during context exits */
> -	struct sbi_context *prev_ctx;
> -	/** Is context initialized and runnable */
> -	bool initialized;
> -};
> -
> -/** Get the context pointer for a given hart index and domain */
> -#define sbi_hartindex_to_domain_context(__hartindex, __d) \
> -	(__d)->hartindex_to_context_table[__hartindex]
> -
> -/** Macro to obtain the current hart's context pointer */
> -#define sbi_domain_context_thishart_ptr()                  \
> -	sbi_hartindex_to_domain_context(                   \
> -		sbi_hartid_to_hartindex(current_hartid()), \
> -		sbi_domain_thishart_ptr())
> -
>  /**
>   * Enter a specific domain context synchronously
>   * @param dom pointer to domain
> @@ -75,4 +28,14 @@ int sbi_domain_context_enter(struct sbi_domain *dom);
>   */
>  int sbi_domain_context_exit(void);
>  
> +/**
> + * Initialize domain context support
> + *
> + * @return 0 on success and negative error code on failure
> + */
> +int sbi_domain_context_init(void);
> +
> +/* Deinitialize domain context support */
> +void sbi_domain_context_deinit(void);
> +
>  #endif // __SBI_DOMAIN_CONTEXT_H__
> diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
> index 22c5c752..ee6e2569 100644
> --- a/lib/sbi/sbi_domain.c
> +++ b/lib/sbi/sbi_domain.c
> @@ -781,11 +781,16 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
>  	if (!domain_hart_ptr_offset)
>  		return SBI_ENOMEM;
>  
> +	/* Initialize domain context support */
> +	rc = sbi_domain_context_init();
> +	if (rc)
> +		goto fail_free_domain_hart_ptr_offset;
> +
>  	root_memregs = sbi_calloc(sizeof(*root_memregs), ROOT_REGION_MAX + 1);
>  	if (!root_memregs) {
>  		sbi_printf("%s: no memory for root regions\n", __func__);
>  		rc = SBI_ENOMEM;
> -		goto fail_free_domain_hart_ptr_offset;
> +		goto fail_deinit_context;
>  	}
>  	root.regions = root_memregs;
>  
> @@ -850,6 +855,8 @@ fail_free_root_hmask:
>  	sbi_free(root_hmask);
>  fail_free_root_memregs:
>  	sbi_free(root_memregs);
> +fail_deinit_context:
> +	sbi_domain_context_deinit();
>  fail_free_domain_hart_ptr_offset:
>  	sbi_scratch_free_offset(domain_hart_ptr_offset);
>  	return rc;
> diff --git a/lib/sbi/sbi_domain_context.c b/lib/sbi/sbi_domain_context.c
> index 29e2d280..b70db41e 100755
> --- a/lib/sbi/sbi_domain_context.c
> +++ b/lib/sbi/sbi_domain_context.c
> @@ -14,6 +14,76 @@
>  #include <sbi/sbi_scratch.h>
>  #include <sbi/sbi_string.h>
>  #include <sbi/sbi_domain.h>
> +#include <sbi/sbi_trap.h>
> +
> +/** Context representation for a hart within a domain */
> +struct hart_context {
> +	/** Trap-related states such as GPRs, mepc, and mstatus */
> +	struct sbi_trap_context trap_ctx;
> +
> +	/** Supervisor status register */
> +	unsigned long sstatus;
> +	/** Supervisor interrupt enable register */
> +	unsigned long sie;
> +	/** Supervisor trap vector base address register */
> +	unsigned long stvec;
> +	/** Supervisor scratch register for temporary storage */
> +	unsigned long sscratch;
> +	/** Supervisor exception program counter register */
> +	unsigned long sepc;
> +	/** Supervisor cause register */
> +	unsigned long scause;
> +	/** Supervisor trap value register */
> +	unsigned long stval;
> +	/** Supervisor interrupt pending register */
> +	unsigned long sip;
> +	/** Supervisor address translation and protection register */
> +	unsigned long satp;
> +	/** Counter-enable register */
> +	unsigned long scounteren;
> +	/** Supervisor environment configuration register */
> +	unsigned long senvcfg;
> +
> +	/** Reference to the owning domain */
> +	struct sbi_domain *dom;
> +	/** Previous context (caller) to jump to during context exits */
> +	struct hart_context *prev_ctx;
> +	/** Is context initialized and runnable */
> +	bool initialized;
> +};
> +
> +struct domain_context_priv {
> +	/** Contexts for possible HARTs indexed by hartindex */
> +	struct hart_context *hartindex_to_context_table[SBI_HARTMASK_MAX_BITS];
> +};
> +
> +static struct sbi_domain_data dcpriv = {
> +	.data_size = sizeof(struct domain_context_priv),
> +};
> +
> +static inline struct hart_context *hart_context_get(struct sbi_domain *dom,
> +						    u32 hartindex)
> +{
> +	struct domain_context_priv *dcp = sbi_domain_data_ptr(dom, &dcpriv);
> +
> +	return (dcp && hartindex < SBI_HARTMASK_MAX_BITS) ?
> +		dcp->hartindex_to_context_table[hartindex] : NULL;
> +}
> +
> +static void hart_context_set(struct sbi_domain *dom, u32 hartindex,
> +			     struct hart_context *hc)
> +{
> +	struct domain_context_priv *dcp = sbi_domain_data_ptr(dom, &dcpriv);
> +
> +	if (dcp && hartindex < SBI_HARTMASK_MAX_BITS) {
> +		dcp->hartindex_to_context_table[hartindex] = hc;
> +	}
> +}
> +
> +/** Macro to obtain the current hart's context pointer */
> +#define hart_context_thishart_get()					\
> +	hart_context_get(sbi_domain_thishart_ptr(),			\
> +			 sbi_hartid_to_hartindex(current_hartid()))

Perhaps we could simplify this by using current_hartindex().

>  
>  /**
>   * Switches the HART context from the current domain to the target domain.
> @@ -23,8 +93,8 @@
>   * @param ctx pointer to the current HART context
>   * @param dom_ctx pointer to the target domain context
>   */
> -static void switch_to_next_domain_context(struct sbi_context *ctx,
> -					  struct sbi_context *dom_ctx)
> +static void switch_to_next_domain_context(struct hart_context *ctx,
> +					  struct hart_context *dom_ctx)
>  {
>  	u32 hartindex = sbi_hartid_to_hartindex(current_hartid());
>  	struct sbi_trap_context *trap_ctx;
> @@ -89,9 +159,9 @@ static void switch_to_next_domain_context(struct sbi_context *ctx,
>  
>  int sbi_domain_context_enter(struct sbi_domain *dom)
>  {
> -	struct sbi_context *ctx = sbi_domain_context_thishart_ptr();
> -	struct sbi_context *dom_ctx = sbi_hartindex_to_domain_context(
> -		sbi_hartid_to_hartindex(current_hartid()), dom);
> +	struct hart_context *ctx = hart_context_thishart_get();
> +	struct hart_context *dom_ctx = hart_context_get(dom,
> +				sbi_hartid_to_hartindex(current_hartid()));
>  
>  	/* Validate the domain context existence */
>  	if (!dom_ctx)
> @@ -109,8 +179,8 @@ int sbi_domain_context_exit(void)
>  {
>  	u32 hartindex = sbi_hartid_to_hartindex(current_hartid());
>  	struct sbi_domain *dom;
> -	struct sbi_context *ctx = sbi_domain_context_thishart_ptr();
> -	struct sbi_context *dom_ctx, *tmp;
> +	struct hart_context *ctx = hart_context_thishart_get();
> +	struct hart_context *dom_ctx, *tmp;
>  
>  	/*
>  	 * If it's first time to call `exit` on the current hart, no
> @@ -123,16 +193,16 @@ int sbi_domain_context_exit(void)
>  							 dom->possible_harts))
>  				continue;
>  
> -			dom_ctx = sbi_zalloc(sizeof(struct sbi_context));
> +			dom_ctx = sbi_zalloc(sizeof(struct hart_context));
>  			if (!dom_ctx)
>  				return SBI_ENOMEM;
>  
>  			/* Bind context and domain */
>  			dom_ctx->dom				   = dom;

The indentation might need to be updated.

Reviewed-by: Yu Chien Peter Lin <peterlin@andestech.com>

Best regards,
Peter Lin

> -			dom->hartindex_to_context_table[hartindex] = dom_ctx;
> +			hart_context_set(dom, hartindex, dom_ctx);
>  		}
>  
> -		ctx = sbi_domain_context_thishart_ptr();
> +		ctx = hart_context_thishart_get();
>  	}
>  
>  	dom_ctx = ctx->prev_ctx;
> @@ -144,7 +214,7 @@ int sbi_domain_context_exit(void)
>  			if (dom == &root || dom == sbi_domain_thishart_ptr())
>  				continue;
>  
> -			tmp = sbi_hartindex_to_domain_context(hartindex, dom);
> +			tmp = hart_context_get(dom, hartindex);
>  			if (tmp && !tmp->initialized) {
>  				dom_ctx = tmp;
>  				break;
> @@ -154,9 +224,19 @@ int sbi_domain_context_exit(void)
>  
>  	/* Take the root domain context if fail to find */
>  	if (!dom_ctx)
> -		dom_ctx = sbi_hartindex_to_domain_context(hartindex, &root);
> +		dom_ctx = hart_context_get(&root, hartindex);
>  
>  	switch_to_next_domain_context(ctx, dom_ctx);
>  
>  	return 0;
>  }
> +
> +int sbi_domain_context_init(void)
> +{
> +	return sbi_domain_register_data(&dcpriv);
> +}
> +
> +void sbi_domain_context_deinit(void)
> +{
> +	sbi_domain_unregister_data(&dcpriv);
> +}

[PATCH 9/9] docs: Remove hartindex_to_context_table from domain_support.md

[Anup Patel]

The hartindex_to_context_table field is no longer part of sbi_domain
so remove related documentation from domain_support.md.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 docs/domain_support.md | 2 --
 1 file changed, 2 deletions(-)

diff --git a/docs/domain_support.md b/docs/domain_support.md
index 1a400685..93186c4a 100644
--- a/docs/domain_support.md
+++ b/docs/domain_support.md
@@ -41,7 +41,6 @@ has following details:
 * **name** - Name of this domain
 * **assigned_harts** - HARTs assigned to this domain
 * **possible_harts** - HARTs possible in this domain
-* **hartindex_to_context_table** - Contexts corresponding to possible HARTs
 * **regions** - Array of memory regions terminated by a memory region
   with order zero
 * **boot_hartid** - HART id of the HART booting this domain. The domain
@@ -81,7 +80,6 @@ following manner:
   platform support
 * **possible_harts** - All valid HARTs of a RISC-V platform are possible
   HARTs of the ROOT domain
-* **hartindex_to_context_table** - Contexts corresponding to ROOT domain's possible HARTs
 * **regions** - Two memory regions available to the ROOT domain:
   **A)** A memory region to protect OpenSBI firmware from S-mode and U-mode
   **B)** A memory region of **order=__riscv_xlen** allowing S-mode and
-- 
2.43.0