[PATCH 0/8] SmePMP bugfixes and improvement

[PATCH 0/8] SmePMP bugfixes and improvement

[Yu-Chien Peter Lin]

This series improves SmePMP related functions and fixes
the access fault during domain context switch when SmePMP
is enabled.

Yu-Chien Peter Lin (8):
  lib: sbi_hart: move sbi_hart_get_smepmp_flags() to sbi_domain
  lib: sbi_domain: allow specifying inaccessible region
  lib: sbi_domain: print unsupported SmePMP permissions
  lib: sbi_hart: add error message for insufficient PMP entries
  lib: sbi_domain_context: skip the reserved entry during domain context
    switch
  lib: sbi_domain: add SBI_DOMAIN_MEMREGION_FW memregion flag
  lib: sbi_domain: ensure consistent firmware PMP entries
  lib: sbi_domain_context: preserve firmware PMP entries during domain
    context switch

 include/sbi/sbi_domain.h     | 11 ++++
 lib/sbi/sbi_domain.c         | 99 +++++++++++++++++++++++++++++++++++-
 lib/sbi/sbi_domain_context.c | 48 +++++++++++++++++
 lib/sbi/sbi_hart.c           | 86 +++++--------------------------
 4 files changed, 170 insertions(+), 74 deletions(-)

-- 
2.39.3


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH 1/8] lib: sbi_hart: move sbi_hart_get_smepmp_flags() to sbi_domain

[Yu-Chien Peter Lin]

Move sbi_hart_get_smepmp_flags() from sbi_hart.c to sbi_domain.c and
rename it to sbi_domain_get_smepmp_flags() to better reflect its
purpose of converting domain memory region flags to PMP configuration.

The function's original parameters (scratch and dom) were unused, so
this refactoring also makes the function cleaner.

Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
---
 include/sbi/sbi_domain.h |  7 +++++
 lib/sbi/sbi_domain.c     | 58 ++++++++++++++++++++++++++++++++++
 lib/sbi/sbi_hart.c       | 67 ++--------------------------------------
 3 files changed, 67 insertions(+), 65 deletions(-)

diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h
index e9cff0b1..8a51f289 100644
--- a/include/sbi/sbi_domain.h
+++ b/include/sbi/sbi_domain.h
@@ -249,6 +249,13 @@ void sbi_domain_memregion_init(unsigned long addr,
 				unsigned long flags,
 				struct sbi_domain_memregion *reg);
 
+/*
+ * Returns Smepmp pmpcfg.LRWX encoding for a given region flag.
+ *
+ * @param reg pointer to memory region with flags encoded with permissions
+ */
+unsigned int sbi_domain_get_smepmp_flags(struct sbi_domain_memregion *reg);
+
 /**
  * Check whether we can access specified address for given mode and
  * memory region flags under a domain
diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
index 461c7e53..1fa56b6a 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -122,6 +122,64 @@ void sbi_domain_memregion_init(unsigned long addr,
 	}
 }
 
+unsigned int sbi_domain_get_smepmp_flags(struct sbi_domain_memregion *reg)
+{
+	unsigned int pmp_flags = 0;
+
+	if (SBI_DOMAIN_MEMREGION_IS_SHARED(reg->flags)) {
+		/* Read only for both M and SU modes */
+		if (SBI_DOMAIN_MEMREGION_IS_SUR_MR(reg->flags))
+			pmp_flags = (PMP_L | PMP_R | PMP_W | PMP_X);
+
+		/* Execute for SU but Read/Execute for M mode */
+		else if (SBI_DOMAIN_MEMREGION_IS_SUX_MRX(reg->flags))
+			/* locked region */
+			pmp_flags = (PMP_L | PMP_W | PMP_X);
+
+		/* Execute only for both M and SU modes */
+		else if (SBI_DOMAIN_MEMREGION_IS_SUX_MX(reg->flags))
+			pmp_flags = (PMP_L | PMP_W);
+
+		/* Read/Write for both M and SU modes */
+		else if (SBI_DOMAIN_MEMREGION_IS_SURW_MRW(reg->flags))
+			pmp_flags = (PMP_W | PMP_X);
+
+		/* Read only for SU mode but Read/Write for M mode */
+		else if (SBI_DOMAIN_MEMREGION_IS_SUR_MRW(reg->flags))
+			pmp_flags = (PMP_W);
+	} else if (SBI_DOMAIN_MEMREGION_M_ONLY_ACCESS(reg->flags)) {
+		/*
+		 * When smepmp is supported and used, M region cannot have RWX
+		 * permissions on any region.
+		 */
+		if ((reg->flags & SBI_DOMAIN_MEMREGION_M_ACCESS_MASK)
+		    == SBI_DOMAIN_MEMREGION_M_RWX) {
+			sbi_printf("%s: M-mode only regions cannot have"
+				   "RWX permissions\n", __func__);
+			return 0;
+		}
+
+		/* M-mode only access regions are always locked */
+		pmp_flags |= PMP_L;
+
+		if (reg->flags & SBI_DOMAIN_MEMREGION_M_READABLE)
+			pmp_flags |= PMP_R;
+		if (reg->flags & SBI_DOMAIN_MEMREGION_M_WRITABLE)
+			pmp_flags |= PMP_W;
+		if (reg->flags & SBI_DOMAIN_MEMREGION_M_EXECUTABLE)
+			pmp_flags |= PMP_X;
+	} else if (SBI_DOMAIN_MEMREGION_SU_ONLY_ACCESS(reg->flags)) {
+		if (reg->flags & SBI_DOMAIN_MEMREGION_SU_READABLE)
+			pmp_flags |= PMP_R;
+		if (reg->flags & SBI_DOMAIN_MEMREGION_SU_WRITABLE)
+			pmp_flags |= PMP_W;
+		if (reg->flags & SBI_DOMAIN_MEMREGION_SU_EXECUTABLE)
+			pmp_flags |= PMP_X;
+	}
+
+	return pmp_flags;
+}
+
 bool sbi_domain_check_addr(const struct sbi_domain *dom,
 			   unsigned long addr, unsigned long mode,
 			   unsigned long access_flags)
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 6a2d7d6f..c5c3ecb4 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -301,69 +301,6 @@ unsigned int sbi_hart_mhpm_bits(struct sbi_scratch *scratch)
 	return hfeatures->mhpm_bits;
 }
 
-/*
- * Returns Smepmp flags for a given domain and region based on permissions.
- */
-static unsigned int sbi_hart_get_smepmp_flags(struct sbi_scratch *scratch,
-					      struct sbi_domain *dom,
-					      struct sbi_domain_memregion *reg)
-{
-	unsigned int pmp_flags = 0;
-
-	if (SBI_DOMAIN_MEMREGION_IS_SHARED(reg->flags)) {
-		/* Read only for both M and SU modes */
-		if (SBI_DOMAIN_MEMREGION_IS_SUR_MR(reg->flags))
-			pmp_flags = (PMP_L | PMP_R | PMP_W | PMP_X);
-
-		/* Execute for SU but Read/Execute for M mode */
-		else if (SBI_DOMAIN_MEMREGION_IS_SUX_MRX(reg->flags))
-			/* locked region */
-			pmp_flags = (PMP_L | PMP_W | PMP_X);
-
-		/* Execute only for both M and SU modes */
-		else if (SBI_DOMAIN_MEMREGION_IS_SUX_MX(reg->flags))
-			pmp_flags = (PMP_L | PMP_W);
-
-		/* Read/Write for both M and SU modes */
-		else if (SBI_DOMAIN_MEMREGION_IS_SURW_MRW(reg->flags))
-			pmp_flags = (PMP_W | PMP_X);
-
-		/* Read only for SU mode but Read/Write for M mode */
-		else if (SBI_DOMAIN_MEMREGION_IS_SUR_MRW(reg->flags))
-			pmp_flags = (PMP_W);
-	} else if (SBI_DOMAIN_MEMREGION_M_ONLY_ACCESS(reg->flags)) {
-		/*
-		 * When smepmp is supported and used, M region cannot have RWX
-		 * permissions on any region.
-		 */
-		if ((reg->flags & SBI_DOMAIN_MEMREGION_M_ACCESS_MASK)
-		    == SBI_DOMAIN_MEMREGION_M_RWX) {
-			sbi_printf("%s: M-mode only regions cannot have"
-				   "RWX permissions\n", __func__);
-			return 0;
-		}
-
-		/* M-mode only access regions are always locked */
-		pmp_flags |= PMP_L;
-
-		if (reg->flags & SBI_DOMAIN_MEMREGION_M_READABLE)
-			pmp_flags |= PMP_R;
-		if (reg->flags & SBI_DOMAIN_MEMREGION_M_WRITABLE)
-			pmp_flags |= PMP_W;
-		if (reg->flags & SBI_DOMAIN_MEMREGION_M_EXECUTABLE)
-			pmp_flags |= PMP_X;
-	} else if (SBI_DOMAIN_MEMREGION_SU_ONLY_ACCESS(reg->flags)) {
-		if (reg->flags & SBI_DOMAIN_MEMREGION_SU_READABLE)
-			pmp_flags |= PMP_R;
-		if (reg->flags & SBI_DOMAIN_MEMREGION_SU_WRITABLE)
-			pmp_flags |= PMP_W;
-		if (reg->flags & SBI_DOMAIN_MEMREGION_SU_EXECUTABLE)
-			pmp_flags |= PMP_X;
-	}
-
-	return pmp_flags;
-}
-
 static void sbi_hart_smepmp_set(struct sbi_scratch *scratch,
 				struct sbi_domain *dom,
 				struct sbi_domain_memregion *reg,
@@ -420,7 +357,7 @@ static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch,
 			continue;
 		}
 
-		pmp_flags = sbi_hart_get_smepmp_flags(scratch, dom, reg);
+		pmp_flags = sbi_domain_get_smepmp_flags(reg);
 		if (!pmp_flags)
 			return 0;
 
@@ -446,7 +383,7 @@ static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch,
 			continue;
 		}
 
-		pmp_flags = sbi_hart_get_smepmp_flags(scratch, dom, reg);
+		pmp_flags = sbi_domain_get_smepmp_flags(reg);
 		if (!pmp_flags)
 			return 0;
 
-- 
2.39.3


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH 2/8] lib: sbi_domain: allow specifying inaccessible region

[Yu-Chien Peter Lin]

According to the RISC‑V Privileged Specification, SmePMP
regions that grant no access in any privilege mode are
valid. Allow such regions to be specified.

Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
---
 lib/sbi/sbi_domain.c | 13 ++++++++++++-
 lib/sbi/sbi_hart.c   |  4 ----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
index 1fa56b6a..e8adf66f 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -126,7 +126,18 @@ unsigned int sbi_domain_get_smepmp_flags(struct sbi_domain_memregion *reg)
 {
 	unsigned int pmp_flags = 0;
 
-	if (SBI_DOMAIN_MEMREGION_IS_SHARED(reg->flags)) {
+	if ((reg->flags & SBI_DOMAIN_MEMREGION_ACCESS_MASK) == 0) {
+		/*
+		 * Inaccessible region for any modes
+		 *
+		 * Note that SmePMP configuration allows two settings
+		 * for such region:
+		 * - pmpcfg.LRWX = 0000 (Inaccessible region)
+		 * - pmpcfg.LRWX = 1000 (Locked inaccessible region)
+		 * Here we use the first one.
+		 */
+		return 0;
+	} else if (SBI_DOMAIN_MEMREGION_IS_SHARED(reg->flags)) {
 		/* Read only for both M and SU modes */
 		if (SBI_DOMAIN_MEMREGION_IS_SUR_MR(reg->flags))
 			pmp_flags = (PMP_L | PMP_R | PMP_W | PMP_X);
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index c5c3ecb4..eab27f48 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -358,8 +358,6 @@ static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch,
 		}
 
 		pmp_flags = sbi_domain_get_smepmp_flags(reg);
-		if (!pmp_flags)
-			return 0;
 
 		sbi_hart_smepmp_set(scratch, dom, reg, pmp_idx++, pmp_flags,
 				    pmp_log2gran, pmp_addr_max);
@@ -384,8 +382,6 @@ static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch,
 		}
 
 		pmp_flags = sbi_domain_get_smepmp_flags(reg);
-		if (!pmp_flags)
-			return 0;
 
 		sbi_hart_smepmp_set(scratch, dom, reg, pmp_idx++, pmp_flags,
 				    pmp_log2gran, pmp_addr_max);
-- 
2.39.3


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH 3/8] lib: sbi_domain: print unsupported SmePMP permissions

[Yu-Chien Peter Lin]

The reg->flag is encoded with 6 bits to specify RWX
permissions for M-mode and S-/U-mode. However, only 16
of the possible encodings are valid on SmePMP.

Previously, specifying an unsupported encoding could fail
silently. Add a warning log when an unsupported permission
encoding is detected, improving visibility for debugging.

Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
---
 lib/sbi/sbi_domain.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
index e8adf66f..d6566b12 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -125,6 +125,7 @@ void sbi_domain_memregion_init(unsigned long addr,
 unsigned int sbi_domain_get_smepmp_flags(struct sbi_domain_memregion *reg)
 {
 	unsigned int pmp_flags = 0;
+	unsigned long rstart, rend;
 
 	if ((reg->flags & SBI_DOMAIN_MEMREGION_ACCESS_MASK) == 0) {
 		/*
@@ -186,6 +187,13 @@ unsigned int sbi_domain_get_smepmp_flags(struct sbi_domain_memregion *reg)
 			pmp_flags |= PMP_W;
 		if (reg->flags & SBI_DOMAIN_MEMREGION_SU_EXECUTABLE)
 			pmp_flags |= PMP_X;
+	} else {
+		rstart = reg->base;
+		rend = (reg->order < __riscv_xlen) ?
+			rstart + ((1UL << reg->order) - 1) : -1UL;
+		sbi_printf("%s: Unsupported Smepmp permissions "
+			   "on region 0x%" PRILX "-0x%" PRILX "\n",
+			   __func__, rstart, rend);
 	}
 
 	return pmp_flags;
-- 
2.39.3


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH 4/8] lib: sbi_hart: add error message for insufficient PMP entries

[Yu-Chien Peter Lin]

Report PMP configuration errors when available PMP entries are
insufficient to adequately protect the system, at least the
security vulnerabilities becomes visible at boot-time.

Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
---
 lib/sbi/sbi_hart.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index eab27f48..f4932909 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -348,8 +348,11 @@ static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch,
 		/* Skip reserved entry */
 		if (pmp_idx == SBI_SMEPMP_RESV_ENTRY)
 			pmp_idx++;
-		if (pmp_count <= pmp_idx)
+		if (pmp_count <= pmp_idx) {
+			sbi_printf("%s: ERR: region %#lx cannot be protected - "
+				   "insufficient PMP entries\n", __func__, reg->base);
 			break;
+		}
 
 		/* Skip shared and SU-only regions */
 		if (!SBI_DOMAIN_MEMREGION_M_ONLY_ACCESS(reg->flags)) {
@@ -372,8 +375,11 @@ static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch,
 		/* Skip reserved entry */
 		if (pmp_idx == SBI_SMEPMP_RESV_ENTRY)
 			pmp_idx++;
-		if (pmp_count <= pmp_idx)
+		if (pmp_count <= pmp_idx) {
+			sbi_printf("%s: ERR: region %#lx cannot be protected - "
+				   "insufficient PMP entries\n", __func__, reg->base);
 			break;
+		}
 
 		/* Skip M-only regions */
 		if (SBI_DOMAIN_MEMREGION_M_ONLY_ACCESS(reg->flags)) {
@@ -407,8 +413,11 @@ static int sbi_hart_oldpmp_configure(struct sbi_scratch *scratch,
 	unsigned long pmp_addr;
 
 	sbi_domain_for_each_memregion(dom, reg) {
-		if (pmp_count <= pmp_idx)
+		if (pmp_count <= pmp_idx) {
+			sbi_printf("%s: ERR: region %#lx cannot be protected - "
+				   "insufficient PMP entries\n", __func__, reg->base);
 			break;
+		}
 
 		pmp_flags = 0;
 
-- 
2.39.3


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH 5/8] lib: sbi_domain_context: skip the reserved entry during domain context switch

[Yu-Chien Peter Lin]

The reserved entry is activated to create shared region between M-mode
and S-mode, it should remain unchanged during domain context switch.

Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
---
 lib/sbi/sbi_domain_context.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/sbi/sbi_domain_context.c b/lib/sbi/sbi_domain_context.c
index fb04d81d..1812c7c9 100644
--- a/lib/sbi/sbi_domain_context.c
+++ b/lib/sbi/sbi_domain_context.c
@@ -116,6 +116,11 @@ static void switch_to_next_domain_context(struct hart_context *ctx,
 
 	/* Reconfigure PMP settings for the new domain */
 	for (int i = 0; i < pmp_count; i++) {
+		/* Skip the reserved entry when SmePMP is enabled */
+		if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMEPMP) &&
+		    (i == SBI_SMEPMP_RESV_ENTRY)) {
+			continue;
+		}
 		sbi_platform_pmp_disable(sbi_platform_thishart_ptr(), i);
 		pmp_disable(i);
 	}
-- 
2.39.3


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH 6/8] lib: sbi_domain: add SBI_DOMAIN_MEMREGION_FW memregion flag

[Yu-Chien Peter Lin]

Add a new memregion flag, SBI_DOMAIN_MEMREGION_FW and mark the
code and data sections of firmware.

Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
---
 include/sbi/sbi_domain.h | 1 +
 lib/sbi/sbi_domain.c     | 8 ++++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h
index 8a51f289..d71354c7 100644
--- a/include/sbi/sbi_domain.h
+++ b/include/sbi/sbi_domain.h
@@ -157,6 +157,7 @@ struct sbi_domain_memregion {
 				 SBI_DOMAIN_MEMREGION_M_EXECUTABLE)
 
 #define SBI_DOMAIN_MEMREGION_MMIO		(1UL << 31)
+#define SBI_DOMAIN_MEMREGION_FW			(1UL << 32)
 	unsigned long flags;
 };
 
diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
index d6566b12..ccfcc7f0 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -541,6 +541,8 @@ void sbi_domain_dump(const struct sbi_domain *dom, const char *suffix)
 		sbi_printf("M: ");
 		if (reg->flags & SBI_DOMAIN_MEMREGION_MMIO)
 			sbi_printf("%cI", (k++) ? ',' : '(');
+		if (reg->flags & SBI_DOMAIN_MEMREGION_FW)
+			sbi_printf("%cF", (k++) ? ',' : '(');
 		if (reg->flags & SBI_DOMAIN_MEMREGION_M_READABLE)
 			sbi_printf("%cR", (k++) ? ',' : '(');
 		if (reg->flags & SBI_DOMAIN_MEMREGION_M_WRITABLE)
@@ -894,13 +896,15 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
 	/* Root domain firmware memory region */
 	sbi_domain_memregion_init(scratch->fw_start, scratch->fw_rw_offset,
 				  (SBI_DOMAIN_MEMREGION_M_READABLE |
-				   SBI_DOMAIN_MEMREGION_M_EXECUTABLE),
+				   SBI_DOMAIN_MEMREGION_M_EXECUTABLE |
+				   SBI_DOMAIN_MEMREGION_FW),
 				  &root_memregs[root_memregs_count++]);
 
 	sbi_domain_memregion_init((scratch->fw_start + scratch->fw_rw_offset),
 				  (scratch->fw_size - scratch->fw_rw_offset),
 				  (SBI_DOMAIN_MEMREGION_M_READABLE |
-				   SBI_DOMAIN_MEMREGION_M_WRITABLE),
+				   SBI_DOMAIN_MEMREGION_M_WRITABLE |
+				   SBI_DOMAIN_MEMREGION_FW),
 				  &root_memregs[root_memregs_count++]);
 
 	root.fw_region_inited = true;
-- 
2.39.3


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH 7/8] lib: sbi_domain: ensure consistent firmware PMP entries

[Yu-Chien Peter Lin]

Currently, during a domain context switch, all PMP entries — including
those granting RW/RX permissions to firmware data and code are cleared
which will lead to access faults.

To configure firmware PMP entries deterministically across switches,
sort regions so that firmware regions come before others.

Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
---
 include/sbi/sbi_domain.h |  3 +++
 lib/sbi/sbi_domain.c     | 14 ++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h
index d71354c7..31237401 100644
--- a/include/sbi/sbi_domain.h
+++ b/include/sbi/sbi_domain.h
@@ -121,6 +121,9 @@ struct sbi_domain_memregion {
 		((__flags & SBI_DOMAIN_MEMREGION_SU_ACCESS_MASK)  &&	\
 		 !(__flags & SBI_DOMAIN_MEMREGION_M_ACCESS_MASK))
 
+#define SBI_DOMAIN_MEMREGION_IS_FIRMWARE(__flags)			\
+		((__flags & SBI_DOMAIN_MEMREGION_FW) ? true : false)	\
+
 /** Bit to control if permissions are enforced on all modes */
 #define SBI_DOMAIN_MEMREGION_ENF_PERMISSIONS	(1UL << 6)
 
diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
index ccfcc7f0..9e284f49 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -295,6 +295,20 @@ static bool is_region_compatible(const struct sbi_domain_memregion *regA,
 static bool is_region_before(const struct sbi_domain_memregion *regA,
 			     const struct sbi_domain_memregion *regB)
 {
+	/*
+	 * Firmware regions are placed before non‑firmware
+	 * regions. This ensures firmware code/data remain
+	 * accessible when SmePMP is enabled. Optional for
+	 * regular PMP, but safe to follow consistently.
+	 */
+	if (SBI_DOMAIN_MEMREGION_IS_FIRMWARE(regA->flags) &&
+	   !SBI_DOMAIN_MEMREGION_IS_FIRMWARE(regB->flags))
+		return true;
+	if (!SBI_DOMAIN_MEMREGION_IS_FIRMWARE(regA->flags) &&
+	    SBI_DOMAIN_MEMREGION_IS_FIRMWARE(regB->flags))
+		return false;
+
+
 	if (regA->order < regB->order)
 		return true;
 
-- 
2.39.3


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH 8/8] lib: sbi_domain_context: preserve firmware PMP entries during domain context switch

[Yu-Chien Peter Lin]

When SmePMP is enabled, clearing firmware PMP entries during a domain
context switch can temporarily revoke access to OpenSBI’s own code and
data, leading to faults.

Keep firmware PMP entries enabled across switches so firmware regions
remain accessible and executable.

Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
---
 lib/sbi/sbi_domain_context.c | 43 ++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/lib/sbi/sbi_domain_context.c b/lib/sbi/sbi_domain_context.c
index 1812c7c9..4ee50ac3 100644
--- a/lib/sbi/sbi_domain_context.c
+++ b/lib/sbi/sbi_domain_context.c
@@ -85,6 +85,45 @@ static void hart_context_set(struct sbi_domain *dom, u32 hartindex,
 	hart_context_get(sbi_domain_thishart_ptr(),			\
 			 current_hartindex())
 
+/**
+ * Determine whether a PMP entry protects firmware memory regions
+ *
+ * @param n PMP entry index
+ * @param dom current domain
+ */
+static bool pmp_is_fw_region(unsigned int n, struct sbi_domain *dom)
+{
+	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
+	unsigned int pmp_count = sbi_hart_pmp_count(scratch);
+	struct sbi_domain_memregion *reg;
+	unsigned long addr, log2len;
+	unsigned long prot, fw_flag;
+	int rc;
+
+	if (!dom || (n >= pmp_count))
+		return false;
+
+	/* Decode the pmpcfg and pmpaddr */
+	rc = pmp_get(n, &prot, &addr, &log2len);
+	if (rc)
+		return false;
+
+	sbi_domain_for_each_memregion(dom, reg) {
+		if ((reg->flags & SBI_DOMAIN_MEMREGION_FW) == 0)
+			continue;
+
+		/* Found a firmware region, assume it is always NAPOT mode */
+		fw_flag = sbi_domain_get_smepmp_flags(reg) | PMP_A_NAPOT;
+		if ((reg->base == addr) &&
+		    (reg->order == log2len) &&
+		    (fw_flag == prot)) {
+			return true;
+		}
+	}
+
+	return false;
+}
+
 /**
  * Switches the HART context from the current domain to the target domain.
  * This includes changing domain assignments and reconfiguring PMP, as well
@@ -121,6 +160,10 @@ static void switch_to_next_domain_context(struct hart_context *ctx,
 		    (i == SBI_SMEPMP_RESV_ENTRY)) {
 			continue;
 		}
+
+		if (pmp_is_fw_region(i, current_dom))
+			continue;
+
 		sbi_platform_pmp_disable(sbi_platform_thishart_ptr(), i);
 		pmp_disable(i);
 	}
-- 
2.39.3


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

RE: [PATCH 6/8] lib: sbi_domain: add SBI_DOMAIN_MEMREGION_FW memregion flag

[Alvin Che-Chia Chang(張哲嘉)]

Hi Peter,

> -----Original Message-----
> From: Yu-Chien Peter Lin <peter.lin@sifive.com>
> Sent: Thursday, August 14, 2025 7:05 PM
> To: opensbi@lists.infradead.org
> Cc: zong.li@sifive.com; greentime.hu@sifive.com; Alvin Che-Chia Chang(張哲
> 嘉) <alvinga@andestech.com>; Yu-Chien Peter Lin <peter.lin@sifive.com>
> Subject: [PATCH 6/8] lib: sbi_domain: add SBI_DOMAIN_MEMREGION_FW
> memregion flag
>
> [EXTERNAL MAIL]
>
> Add a new memregion flag, SBI_DOMAIN_MEMREGION_FW and mark the
> code and data sections of firmware.
>
> Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
> ---
>  include/sbi/sbi_domain.h | 1 +
>  lib/sbi/sbi_domain.c     | 8 ++++++--
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h index
> 8a51f289..d71354c7 100644
> --- a/include/sbi/sbi_domain.h
> +++ b/include/sbi/sbi_domain.h
> @@ -157,6 +157,7 @@ struct sbi_domain_memregion {
>
> SBI_DOMAIN_MEMREGION_M_EXECUTABLE)
>
>  #define SBI_DOMAIN_MEMREGION_MMIO              (1UL << 31)
> +#define SBI_DOMAIN_MEMREGION_FW                        (1UL <<
> 32)

This exceeds "unsigned long" of RV32 which only has 32 bits.
Besides, why we need this new flag?
Isn't SBI_DOMAIN_MEMREGION_M_ONLY_ACCESS(__flags) enough?

>         unsigned long flags;
>  };
>
> diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c index
> d6566b12..ccfcc7f0 100644
> --- a/lib/sbi/sbi_domain.c
> +++ b/lib/sbi/sbi_domain.c
> @@ -541,6 +541,8 @@ void sbi_domain_dump(const struct sbi_domain *dom,
> const char *suffix)
>                 sbi_printf("M: ");
>                 if (reg->flags & SBI_DOMAIN_MEMREGION_MMIO)
>                         sbi_printf("%cI", (k++) ? ',' : '(');
> +               if (reg->flags & SBI_DOMAIN_MEMREGION_FW)
> +                       sbi_printf("%cF", (k++) ? ',' : '(');
>                 if (reg->flags &
> SBI_DOMAIN_MEMREGION_M_READABLE)
>                         sbi_printf("%cR", (k++) ? ',' : '(');
>                 if (reg->flags & SBI_DOMAIN_MEMREGION_M_WRITABLE)
> @@ -894,13 +896,15 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32
> cold_hartid)
>         /* Root domain firmware memory region */
>         sbi_domain_memregion_init(scratch->fw_start,
> scratch->fw_rw_offset,
>
> (SBI_DOMAIN_MEMREGION_M_READABLE |
> -
> SBI_DOMAIN_MEMREGION_M_EXECUTABLE),
> +
> SBI_DOMAIN_MEMREGION_M_EXECUTABLE |
> +                                  SBI_DOMAIN_MEMREGION_FW),
>
> &root_memregs[root_memregs_count++]);
>
>         sbi_domain_memregion_init((scratch->fw_start +
> scratch->fw_rw_offset),
>                                   (scratch->fw_size -
> scratch->fw_rw_offset),
>
> (SBI_DOMAIN_MEMREGION_M_READABLE |
> -
> SBI_DOMAIN_MEMREGION_M_WRITABLE),
> +
> SBI_DOMAIN_MEMREGION_M_WRITABLE |
> +                                  SBI_DOMAIN_MEMREGION_FW),
>
> &root_memregs[root_memregs_count++]);
>
>         root.fw_region_inited = true;
> --
> 2.39.3

CONFIDENTIALITY NOTICE:

This e-mail (and its attachments) may contain confidential and legally privileged information or information protected from disclosure. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the information contained herein is strictly prohibited. In this case, please immediately notify the sender by return e-mail, delete the message (and any accompanying documents) and destroy all printed hard copies. Thank you for your cooperation.

Copyright ANDES TECHNOLOGY CORPORATION - All Rights Reserved.
-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

RE: [PATCH 5/8] lib: sbi_domain_context: skip the reserved entry during domain context switch

[Alvin Che-Chia Chang(張哲嘉)]

Hi Peter,

> -----Original Message-----
> From: Yu-Chien Peter Lin <peter.lin@sifive.com>
> Sent: Thursday, August 14, 2025 7:05 PM
> To: opensbi@lists.infradead.org
> Cc: zong.li@sifive.com; greentime.hu@sifive.com; Alvin Che-Chia Chang(張哲
> 嘉) <alvinga@andestech.com>; Yu-Chien Peter Lin <peter.lin@sifive.com>
> Subject: [PATCH 5/8] lib: sbi_domain_context: skip the reserved entry during
> domain context switch
>
> [EXTERNAL MAIL]
>
> The reserved entry is activated to create shared region between M-mode and
> S-mode, it should remain unchanged during domain context switch.
>
> Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
> ---
>  lib/sbi/sbi_domain_context.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/lib/sbi/sbi_domain_context.c b/lib/sbi/sbi_domain_context.c index
> fb04d81d..1812c7c9 100644
> --- a/lib/sbi/sbi_domain_context.c
> +++ b/lib/sbi/sbi_domain_context.c
> @@ -116,6 +116,11 @@ static void switch_to_next_domain_context(struct
> hart_context *ctx,
>
>         /* Reconfigure PMP settings for the new domain */
>         for (int i = 0; i < pmp_count; i++) {
> +               /* Skip the reserved entry when SmePMP is enabled */
> +               if (sbi_hart_has_extension(scratch,
> SBI_HART_EXT_SMEPMP) &&
> +                   (i == SBI_SMEPMP_RESV_ENTRY)) {
> +                       continue;
> +               }

Is this reserved entry mapped(activated) during domain context switch ?
If my understanding is correct, the mapped reserved entry creates shared region between M-mode and "current supervisor domain".
Now we are performing domain context switch to "next supervisor domain which does not have permission to access that shared region".
In my opinion, this reserved entry should be disabled for security reason.

Maybe OpenSBI should "context-switch" the reserved entry?
If the reserved entry is mapped during the domain A, and system switches to domain B without unmapping the reserved entry.
Does this mean that OpenSBI cannot call sbi_hart_map_saddr() to map shared-region with domain B? Because the function will return SBI_ENOSPC in this case.


Best regards,
Alvin


>                 sbi_platform_pmp_disable(sbi_platform_thishart_ptr(), i);
>                 pmp_disable(i);
>         }
> --
> 2.39.3

CONFIDENTIALITY NOTICE:

This e-mail (and its attachments) may contain confidential and legally privileged information or information protected from disclosure. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the information contained herein is strictly prohibited. In this case, please immediately notify the sender by return e-mail, delete the message (and any accompanying documents) and destroy all printed hard copies. Thank you for your cooperation.

Copyright ANDES TECHNOLOGY CORPORATION - All Rights Reserved.
-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH 6/8] lib: sbi_domain: add SBI_DOMAIN_MEMREGION_FW memregion flag

[Xiang W]

在 2025-08-14四的 19:05 +0800，Yu-Chien Peter Lin写道：
> Add a new memregion flag, SBI_DOMAIN_MEMREGION_FW and mark the
> code and data sections of firmware.
> 
> Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
> ---
>  include/sbi/sbi_domain.h | 1 +
>  lib/sbi/sbi_domain.c     | 8 ++++++--
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h
> index 8a51f289..d71354c7 100644
> --- a/include/sbi/sbi_domain.h
> +++ b/include/sbi/sbi_domain.h
> @@ -157,6 +157,7 @@ struct sbi_domain_memregion {
>  				 SBI_DOMAIN_MEMREGION_M_EXECUTABLE)
>  
>  #define SBI_DOMAIN_MEMREGION_MMIO		(1UL << 31)
> +#define SBI_DOMAIN_MEMREGION_FW			(1UL << 32)
Will overflow under RV32.

Regards,
Xiang W
>  	unsigned long flags;
>  };
>  
> diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
> index d6566b12..ccfcc7f0 100644
> --- a/lib/sbi/sbi_domain.c
> +++ b/lib/sbi/sbi_domain.c
> @@ -541,6 +541,8 @@ void sbi_domain_dump(const struct sbi_domain *dom, const char *suffix)
>  		sbi_printf("M: ");
>  		if (reg->flags & SBI_DOMAIN_MEMREGION_MMIO)
>  			sbi_printf("%cI", (k++) ? ',' : '(');
> +		if (reg->flags & SBI_DOMAIN_MEMREGION_FW)
> +			sbi_printf("%cF", (k++) ? ',' : '(');
>  		if (reg->flags & SBI_DOMAIN_MEMREGION_M_READABLE)
>  			sbi_printf("%cR", (k++) ? ',' : '(');
>  		if (reg->flags & SBI_DOMAIN_MEMREGION_M_WRITABLE)
> @@ -894,13 +896,15 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
>  	/* Root domain firmware memory region */
>  	sbi_domain_memregion_init(scratch->fw_start, scratch->fw_rw_offset,
>  				  (SBI_DOMAIN_MEMREGION_M_READABLE |
> -				   SBI_DOMAIN_MEMREGION_M_EXECUTABLE),
> +				   SBI_DOMAIN_MEMREGION_M_EXECUTABLE |
> +				   SBI_DOMAIN_MEMREGION_FW),
>  				  &root_memregs[root_memregs_count++]);
>  
>  	sbi_domain_memregion_init((scratch->fw_start + scratch->fw_rw_offset),
>  				  (scratch->fw_size - scratch->fw_rw_offset),
>  				  (SBI_DOMAIN_MEMREGION_M_READABLE |
> -				   SBI_DOMAIN_MEMREGION_M_WRITABLE),
> +				   SBI_DOMAIN_MEMREGION_M_WRITABLE |
> +				   SBI_DOMAIN_MEMREGION_FW),
>  				  &root_memregs[root_memregs_count++]);
>  
>  	root.fw_region_inited = true;
> -- 
> 2.39.3
> 


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH 0/8] SmePMP bugfixes and improvement

[Anup Patel]

On Thu, Aug 14, 2025 at 7:46 PM Yu-Chien Peter Lin <peter.lin@sifive.com> wrote:
>
> This series improves SmePMP related functions and fixes
> the access fault during domain context switch when SmePMP
> is enabled.
>
> Yu-Chien Peter Lin (8):
>   lib: sbi_hart: move sbi_hart_get_smepmp_flags() to sbi_domain
>   lib: sbi_domain: allow specifying inaccessible region
>   lib: sbi_domain: print unsupported SmePMP permissions
>   lib: sbi_hart: add error message for insufficient PMP entries
>   lib: sbi_domain_context: skip the reserved entry during domain context
>     switch
>   lib: sbi_domain: add SBI_DOMAIN_MEMREGION_FW memregion flag
>   lib: sbi_domain: ensure consistent firmware PMP entries
>   lib: sbi_domain_context: preserve firmware PMP entries during domain
>     context switch

I totally missed this series because for some strange reason, it is not
showing up in OpenSBI patchwork
(https://patchwork.ozlabs.org/project/opensbi/list/).

Can you please rebase and resend this series ?

Regards,
Anup

>
>  include/sbi/sbi_domain.h     | 11 ++++
>  lib/sbi/sbi_domain.c         | 99 +++++++++++++++++++++++++++++++++++-
>  lib/sbi/sbi_domain_context.c | 48 +++++++++++++++++
>  lib/sbi/sbi_hart.c           | 86 +++++--------------------------
>  4 files changed, 170 insertions(+), 74 deletions(-)
>
> --
> 2.39.3
>
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH 0/8] SmePMP bugfixes and improvement

[Peter Lin]

Hi Anup,

On Mon, Oct 6, 2025 at 1:21 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Thu, Aug 14, 2025 at 7:46 PM Yu-Chien Peter Lin <peter.lin@sifive.com> wrote:
> >
> > This series improves SmePMP related functions and fixes
> > the access fault during domain context switch when SmePMP
> > is enabled.
> >
> > Yu-Chien Peter Lin (8):
> >   lib: sbi_hart: move sbi_hart_get_smepmp_flags() to sbi_domain
> >   lib: sbi_domain: allow specifying inaccessible region
> >   lib: sbi_domain: print unsupported SmePMP permissions
> >   lib: sbi_hart: add error message for insufficient PMP entries
> >   lib: sbi_domain_context: skip the reserved entry during domain context
> >     switch
> >   lib: sbi_domain: add SBI_DOMAIN_MEMREGION_FW memregion flag
> >   lib: sbi_domain: ensure consistent firmware PMP entries
> >   lib: sbi_domain_context: preserve firmware PMP entries during domain
> >     context switch
>
> I totally missed this series because for some strange reason, it is not
> showing up in OpenSBI patchwork
> (https://patchwork.ozlabs.org/project/opensbi/list/).
>
> Can you please rebase and resend this series ?

Yes, I am preparing PATCHv2, thanks.

Regards,
Peter Lin

> Regards,
> Anup
>
> >
> >  include/sbi/sbi_domain.h     | 11 ++++
> >  lib/sbi/sbi_domain.c         | 99 +++++++++++++++++++++++++++++++++++-
> >  lib/sbi/sbi_domain_context.c | 48 +++++++++++++++++
> >  lib/sbi/sbi_hart.c           | 86 +++++--------------------------
> >  4 files changed, 170 insertions(+), 74 deletions(-)
> >
> > --
> > 2.39.3
> >
> >
> > --
> > opensbi mailing list
> > opensbi@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/opensbi

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi