[PATCH v2 0/8] SmePMP bugfixes and improvement

[PATCH v2 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: return error when insufficient PMP entries available
  lib: sbi_domain: add SBI_DOMAIN_MEMREGION_FW memregion flag
  lib: sbi_domain: ensure consistent firmware PMP entries
  lib: sbi: sbi_hart: track firmware PMP entries when configuring SmePMP
  lib: sbi_domain_context: preserve firmware PMP entries during domain
    context switch

 include/sbi/sbi_domain.h     |  11 ++++
 include/sbi/sbi_hart.h       |   1 +
 lib/sbi/sbi_domain.c         |  98 ++++++++++++++++++++++++++++-
 lib/sbi/sbi_domain_context.c |   4 ++
 lib/sbi/sbi_hart.c           | 116 ++++++++++++++---------------------
 5 files changed, 158 insertions(+), 72 deletions(-)

-- 
2.48.0


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

[PATCH v2 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.

Also removes unused parameters (scratch and dom).

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..a6ee553b 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);
 
+/**
+ * Return the Smepmp pmpcfg LRWX encoding for the flags in @reg.
+ *
+ * @param reg pointer to memory region; its flags field encodes 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 1b50f671..2dca2699 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.48.0


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

[PATCH v2 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 | 12 +++++++++++-
 lib/sbi/sbi_hart.c   |  4 ----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
index 1fa56b6a..89250d4c 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -126,7 +126,17 @@ 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) {
+		/*
+		 * Region is inaccessible in all privilege modes.
+		 *
+		 * SmePMP allows two encodings for an inaccessible region:
+		 *   - pmpcfg.LRWX = 0000 (Inaccessible region)
+		 *   - pmpcfg.LRWX = 1000 (Locked inaccessible region)
+		 * We use the first encoding here.
+		 */
+		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 2dca2699..d018619b 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.48.0


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

[PATCH v2 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.

Add a warning message when an unsupported permission
encoding is detected.

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 89250d4c..798e68b3 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) {
 		/*
@@ -185,6 +186,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.48.0


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

[PATCH v2 4/8] lib: sbi_hart: return error when insufficient PMP entries available

[Yu-Chien Peter Lin]

Previously, when memory regions exceed available PMP entries,
some regions were silently ignored. If the last entry that covers
the full 64-bit address space is not added to a domain, the next
stage S-mode software won't have permission to access and fetch
instructions from its memory. So return early with error message
to catch such situation.

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

diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index d018619b..032f7dc1 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -324,6 +324,16 @@ static void sbi_hart_smepmp_set(struct sbi_scratch *scratch,
 	}
 }
 
+static bool is_valid_pmp_idx(unsigned int pmp_count, unsigned int pmp_idx)
+{
+	if (pmp_count > pmp_idx)
+		return true;
+
+	sbi_printf("ERR: insufficient PMP entries\n");
+
+	return false;
+}
+
 static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch,
 				     unsigned int pmp_count,
 				     unsigned int pmp_log2gran,
@@ -348,8 +358,8 @@ 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)
-			break;
+		if (!is_valid_pmp_idx(pmp_count, pmp_idx))
+			return SBI_EFAIL;
 
 		/* Skip shared and SU-only regions */
 		if (!SBI_DOMAIN_MEMREGION_M_ONLY_ACCESS(reg->flags)) {
@@ -372,8 +382,8 @@ 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)
-			break;
+		if (!is_valid_pmp_idx(pmp_count, pmp_idx))
+			return SBI_EFAIL;
 
 		/* Skip M-only regions */
 		if (SBI_DOMAIN_MEMREGION_M_ONLY_ACCESS(reg->flags)) {
@@ -407,8 +417,8 @@ 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)
-			break;
+		if (!is_valid_pmp_idx(pmp_count, pmp_idx))
+			return SBI_EFAIL;
 
 		pmp_flags = 0;
 
-- 
2.48.0


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

[PATCH v2 5/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
OpenSBI code and data regions.

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 a6ee553b..9193feb0 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 << 30)
 	unsigned long flags;
 };
 
diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
index 798e68b3..968fe61b 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -540,6 +540,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)
@@ -893,13 +895,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.48.0


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

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

[Yu-Chien Peter Lin]

During domain context switches, all PMP entries are reconfigured
which can clear firmware access permissions, causing M-mode access
faults under SmePMP.

Sort domain regions to place firmware regions first, ensuring
consistent firmware PMP entries so they won't be revoked during
domain context switches.

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 9193feb0..1196d609 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 968fe61b..657de10d 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -294,6 +294,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)
 {
+	/*
+	 * Enforce firmware region ordering for memory access
+	 * under SmePMP.
+	 * Place firmware regions first to ensure consistent
+	 * PMP entries during domain context switches.
+	 */
+	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.48.0


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

[PATCH v2 7/8] lib: sbi: sbi_hart: track firmware PMP entries when configuring SmePMP

[Yu-Chien Peter Lin]

Add fw_smepmp_ids bitmap to track PMP entries that protect firmware
regions. Allow us to preserve these critical entries across domain
transitions and check inconsistent firmware entry allocation.

Also add sbi_hart_pmp_is_fw_region() helper function to query
whether a given PMP entry protects firmware regions.

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

diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
index 82b19dcf..6db4fed7 100644
--- a/include/sbi/sbi_hart.h
+++ b/include/sbi/sbi_hart.h
@@ -142,6 +142,7 @@ unsigned int sbi_hart_pmp_count(struct sbi_scratch *scratch);
 unsigned int sbi_hart_pmp_log2gran(struct sbi_scratch *scratch);
 unsigned int sbi_hart_pmp_addrbits(struct sbi_scratch *scratch);
 unsigned int sbi_hart_mhpm_bits(struct sbi_scratch *scratch);
+bool sbi_hart_smepmp_is_fw_region(unsigned int pmp_idx);
 int sbi_hart_pmp_configure(struct sbi_scratch *scratch);
 int sbi_hart_map_saddr(unsigned long base, unsigned long size);
 int sbi_hart_unmap_saddr(void);
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 032f7dc1..abab8b73 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -30,6 +30,8 @@ extern void __sbi_expected_trap_hext(void);
 void (*sbi_hart_expected_trap)(void) = &__sbi_expected_trap;
 
 static unsigned long hart_features_offset;
+static DECLARE_BITMAP(fw_smepmp_ids, PMP_COUNT);
+static bool fw_smepmp_ids_inited;
 
 static void mstatus_init(struct sbi_scratch *scratch)
 {
@@ -301,6 +303,17 @@ unsigned int sbi_hart_mhpm_bits(struct sbi_scratch *scratch)
 	return hfeatures->mhpm_bits;
 }
 
+bool sbi_hart_smepmp_is_fw_region(unsigned int pmp_idx)
+{
+	if (!fw_smepmp_ids_inited) {
+		sbi_printf("%s: ERR: fw_smepmp_ids uninitialized\n",
+			   __func__);
+		return false;
+	}
+
+	return bitmap_test(fw_smepmp_ids, pmp_idx) ? true : false;
+}
+
 static void sbi_hart_smepmp_set(struct sbi_scratch *scratch,
 				struct sbi_domain *dom,
 				struct sbi_domain_memregion *reg,
@@ -367,12 +380,32 @@ static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch,
 			continue;
 		}
 
+		/*
+		 * Track firmware PMP entries to preserve them during
+		 * domain switches. Under SmePMP, M-mode requires
+		 * explicit PMP entries to access firmware code/data.
+		 * These entries must remain enabled across domain
+		 * context switches to prevent M-mode access faults.
+		 */
+		if (SBI_DOMAIN_MEMREGION_IS_FIRMWARE(reg->flags)) {
+			if (fw_smepmp_ids_inited) {
+				/* Check inconsistent firmware region */
+				if (!sbi_hart_smepmp_is_fw_region(pmp_idx))
+					return SBI_EINVAL;
+			} else {
+				bitmap_set(fw_smepmp_ids, pmp_idx, 1);
+			}
+		}
+
 		pmp_flags = sbi_domain_get_smepmp_flags(reg);
 
 		sbi_hart_smepmp_set(scratch, dom, reg, pmp_idx++, pmp_flags,
 				    pmp_log2gran, pmp_addr_max);
+
 	}
 
+	fw_smepmp_ids_inited = true;
+
 	/* Set the MML to enforce new encoding */
 	csr_set(CSR_MSECCFG, MSECCFG_MML);
 
-- 
2.48.0


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

[PATCH v2 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 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/sbi/sbi_domain_context.c b/lib/sbi/sbi_domain_context.c
index fb04d81d..51534de1 100644
--- a/lib/sbi/sbi_domain_context.c
+++ b/lib/sbi/sbi_domain_context.c
@@ -116,6 +116,10 @@ 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++) {
+		/* Don't revoke firmware access permissions */
+		if (sbi_hart_smepmp_is_fw_region(i))
+			continue;
+
 		sbi_platform_pmp_disable(sbi_platform_thishart_ptr(), i);
 		pmp_disable(i);
 	}
-- 
2.48.0


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

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

[Anup Patel]

On Wed, Oct 8, 2025 at 2:14 PM Yu-Chien Peter Lin <peter.lin@sifive.com> wrote:
>
> 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.
>
> Also removes unused parameters (scratch and dom).
>
> Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>

LGTM.

Reviewed-by: Anup Patel <anup@brainfault.org>

Thanks,
Anup

> ---
>  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..a6ee553b 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);
>
> +/**
> + * Return the Smepmp pmpcfg LRWX encoding for the flags in @reg.
> + *
> + * @param reg pointer to memory region; its flags field encodes 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 1b50f671..2dca2699 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.48.0
>

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

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

[Anup Patel]

On Wed, Oct 8, 2025 at 2:14 PM Yu-Chien Peter Lin <peter.lin@sifive.com> wrote:
>
> 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>

LGTM.

Reviewed-by: Anup Patel <anup@brainfault.org>

Thanks,
Anup

> ---
>  lib/sbi/sbi_domain.c | 12 +++++++++++-
>  lib/sbi/sbi_hart.c   |  4 ----
>  2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
> index 1fa56b6a..89250d4c 100644
> --- a/lib/sbi/sbi_domain.c
> +++ b/lib/sbi/sbi_domain.c
> @@ -126,7 +126,17 @@ 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) {
> +               /*
> +                * Region is inaccessible in all privilege modes.
> +                *
> +                * SmePMP allows two encodings for an inaccessible region:
> +                *   - pmpcfg.LRWX = 0000 (Inaccessible region)
> +                *   - pmpcfg.LRWX = 1000 (Locked inaccessible region)
> +                * We use the first encoding here.
> +                */
> +               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 2dca2699..d018619b 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.48.0
>

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

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

[Anup Patel]

On Wed, Oct 8, 2025 at 2:14 PM Yu-Chien Peter Lin <peter.lin@sifive.com> wrote:
>
> 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.
>
> Add a warning message when an unsupported permission
> encoding is detected.
>
> 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 89250d4c..798e68b3 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) {
>                 /*
> @@ -185,6 +186,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);

It's okay to us upto 100 characters per-line otherwise
it looks good to me. I will take care of this at the time
of merging this patch.

Reviewed-by: Anup Patel <anup@brainfault.org>

Thanks,
Anup

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

Re: [PATCH v2 4/8] lib: sbi_hart: return error when insufficient PMP entries available

[Anup Patel]

On Wed, Oct 8, 2025 at 2:14 PM Yu-Chien Peter Lin <peter.lin@sifive.com> wrote:
>
> Previously, when memory regions exceed available PMP entries,
> some regions were silently ignored. If the last entry that covers
> the full 64-bit address space is not added to a domain, the next
> stage S-mode software won't have permission to access and fetch
> instructions from its memory. So return early with error message
> to catch such situation.
>
> Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
> ---
>  lib/sbi/sbi_hart.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index d018619b..032f7dc1 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -324,6 +324,16 @@ static void sbi_hart_smepmp_set(struct sbi_scratch *scratch,
>         }
>  }
>
> +static bool is_valid_pmp_idx(unsigned int pmp_count, unsigned int pmp_idx)
> +{
> +       if (pmp_count > pmp_idx)
> +               return true;
> +
> +       sbi_printf("ERR: insufficient PMP entries\n");
> +

Redundant newline here. Also, "error:" is better than "ERR:".
I will take care of this at the time of merging this patch.

Reviewed-by: Anup Patel <anup@brainfault.org>

Thanks,
Anup

> +       return false;
> +}
> +
>  static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch,
>                                      unsigned int pmp_count,
>                                      unsigned int pmp_log2gran,
> @@ -348,8 +358,8 @@ 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)
> -                       break;
> +               if (!is_valid_pmp_idx(pmp_count, pmp_idx))
> +                       return SBI_EFAIL;
>
>                 /* Skip shared and SU-only regions */
>                 if (!SBI_DOMAIN_MEMREGION_M_ONLY_ACCESS(reg->flags)) {
> @@ -372,8 +382,8 @@ 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)
> -                       break;
> +               if (!is_valid_pmp_idx(pmp_count, pmp_idx))
> +                       return SBI_EFAIL;
>
>                 /* Skip M-only regions */
>                 if (SBI_DOMAIN_MEMREGION_M_ONLY_ACCESS(reg->flags)) {
> @@ -407,8 +417,8 @@ 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)
> -                       break;
> +               if (!is_valid_pmp_idx(pmp_count, pmp_idx))
> +                       return SBI_EFAIL;
>
>                 pmp_flags = 0;
>
> --
> 2.48.0
>

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

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

[Anup Patel]

On Wed, Oct 8, 2025 at 2:15 PM Yu-Chien Peter Lin <peter.lin@sifive.com> wrote:
>
> Add a new memregion flag, SBI_DOMAIN_MEMREGION_FW and mark the
> OpenSBI code and data regions.
>
> Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>

LGTM.

Reviewed-by: Anup Patel <anup@brainfault.org>

Thanks,
Anup

> ---
>  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 a6ee553b..9193feb0 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 << 30)
>         unsigned long flags;
>  };
>
> diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
> index 798e68b3..968fe61b 100644
> --- a/lib/sbi/sbi_domain.c
> +++ b/lib/sbi/sbi_domain.c
> @@ -540,6 +540,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)
> @@ -893,13 +895,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.48.0
>

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

Re: [PATCH v2 6/8] lib: sbi_domain: ensure consistent firmware PMP entries

[Anup Patel]

On Wed, Oct 8, 2025 at 2:15 PM Yu-Chien Peter Lin <peter.lin@sifive.com> wrote:
>
> During domain context switches, all PMP entries are reconfigured
> which can clear firmware access permissions, causing M-mode access
> faults under SmePMP.
>
> Sort domain regions to place firmware regions first, ensuring
> consistent firmware PMP entries so they won't be revoked during
> domain context switches.
>
> Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>

Reviewed-by: Anup Patel <anup@brainfault.org>

Thanks,
Anup

> ---
>  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 9193feb0..1196d609 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 968fe61b..657de10d 100644
> --- a/lib/sbi/sbi_domain.c
> +++ b/lib/sbi/sbi_domain.c
> @@ -294,6 +294,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)
>  {
> +       /*
> +        * Enforce firmware region ordering for memory access
> +        * under SmePMP.
> +        * Place firmware regions first to ensure consistent
> +        * PMP entries during domain context switches.
> +        */
> +       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;
> +
> +

Redundant newline here otherwise it looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Thanks,
Anup

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

Re: [PATCH v2 7/8] lib: sbi: sbi_hart: track firmware PMP entries when configuring SmePMP

[Anup Patel]

On Wed, Oct 8, 2025 at 2:15 PM Yu-Chien Peter Lin <peter.lin@sifive.com> wrote:
>
> Add fw_smepmp_ids bitmap to track PMP entries that protect firmware
> regions. Allow us to preserve these critical entries across domain
> transitions and check inconsistent firmware entry allocation.
>
> Also add sbi_hart_pmp_is_fw_region() helper function to query

s/sbi_hart_pmp_is_fw_region/sbi_hart_smepmp_is_fw_region/

> whether a given PMP entry protects firmware regions.

s/PMP/SmePMP/

>
> Signed-off-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
> ---
>  include/sbi/sbi_hart.h |  1 +
>  lib/sbi/sbi_hart.c     | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
>
> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> index 82b19dcf..6db4fed7 100644
> --- a/include/sbi/sbi_hart.h
> +++ b/include/sbi/sbi_hart.h
> @@ -142,6 +142,7 @@ unsigned int sbi_hart_pmp_count(struct sbi_scratch *scratch);
>  unsigned int sbi_hart_pmp_log2gran(struct sbi_scratch *scratch);
>  unsigned int sbi_hart_pmp_addrbits(struct sbi_scratch *scratch);
>  unsigned int sbi_hart_mhpm_bits(struct sbi_scratch *scratch);
> +bool sbi_hart_smepmp_is_fw_region(unsigned int pmp_idx);
>  int sbi_hart_pmp_configure(struct sbi_scratch *scratch);
>  int sbi_hart_map_saddr(unsigned long base, unsigned long size);
>  int sbi_hart_unmap_saddr(void);
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 032f7dc1..abab8b73 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -30,6 +30,8 @@ extern void __sbi_expected_trap_hext(void);
>  void (*sbi_hart_expected_trap)(void) = &__sbi_expected_trap;
>
>  static unsigned long hart_features_offset;
> +static DECLARE_BITMAP(fw_smepmp_ids, PMP_COUNT);
> +static bool fw_smepmp_ids_inited;
>
>  static void mstatus_init(struct sbi_scratch *scratch)
>  {
> @@ -301,6 +303,17 @@ unsigned int sbi_hart_mhpm_bits(struct sbi_scratch *scratch)
>         return hfeatures->mhpm_bits;
>  }
>
> +bool sbi_hart_smepmp_is_fw_region(unsigned int pmp_idx)
> +{
> +       if (!fw_smepmp_ids_inited) {
> +               sbi_printf("%s: ERR: fw_smepmp_ids uninitialized\n",
> +                          __func__);

This printf() causes unnecessary prints on HW with legacy PMP so
needs to be removed.

> +               return false;
> +       }
> +
> +       return bitmap_test(fw_smepmp_ids, pmp_idx) ? true : false;
> +}
> +
>  static void sbi_hart_smepmp_set(struct sbi_scratch *scratch,
>                                 struct sbi_domain *dom,
>                                 struct sbi_domain_memregion *reg,
> @@ -367,12 +380,32 @@ static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch,
>                         continue;
>                 }
>
> +               /*
> +                * Track firmware PMP entries to preserve them during
> +                * domain switches. Under SmePMP, M-mode requires
> +                * explicit PMP entries to access firmware code/data.
> +                * These entries must remain enabled across domain
> +                * context switches to prevent M-mode access faults.
> +                */
> +               if (SBI_DOMAIN_MEMREGION_IS_FIRMWARE(reg->flags)) {
> +                       if (fw_smepmp_ids_inited) {
> +                               /* Check inconsistent firmware region */
> +                               if (!sbi_hart_smepmp_is_fw_region(pmp_idx))
> +                                       return SBI_EINVAL;
> +                       } else {
> +                               bitmap_set(fw_smepmp_ids, pmp_idx, 1);
> +                       }
> +               }
> +
>                 pmp_flags = sbi_domain_get_smepmp_flags(reg);
>
>                 sbi_hart_smepmp_set(scratch, dom, reg, pmp_idx++, pmp_flags,
>                                     pmp_log2gran, pmp_addr_max);
> +

Redundant newline.

>         }
>
> +       fw_smepmp_ids_inited = true;
> +
>         /* Set the MML to enforce new encoding */
>         csr_set(CSR_MSECCFG, MSECCFG_MML);
>
> --
> 2.48.0
>

Otherwise, it looks good to me. I will take care of the above
comments at the time of merging.

Reviewed-by: Anup Patel <anup@brainfault.org>

Thanks,
Anup

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

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

[Anup Patel]

On Wed, Oct 8, 2025 at 2:15 PM Yu-Chien Peter Lin <peter.lin@sifive.com> wrote:
>
> 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>

LGTM.

Reviewed-by: Anup Patel <anup@brainfault.org>

Thanks,
Anup

> ---
>  lib/sbi/sbi_domain_context.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/lib/sbi/sbi_domain_context.c b/lib/sbi/sbi_domain_context.c
> index fb04d81d..51534de1 100644
> --- a/lib/sbi/sbi_domain_context.c
> +++ b/lib/sbi/sbi_domain_context.c
> @@ -116,6 +116,10 @@ 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++) {
> +               /* Don't revoke firmware access permissions */
> +               if (sbi_hart_smepmp_is_fw_region(i))
> +                       continue;
> +
>                 sbi_platform_pmp_disable(sbi_platform_thishart_ptr(), i);
>                 pmp_disable(i);
>         }
> --
> 2.48.0
>

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

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

[Anup Patel]

On Wed, Oct 8, 2025 at 2:14 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: return error when insufficient PMP entries available
>   lib: sbi_domain: add SBI_DOMAIN_MEMREGION_FW memregion flag
>   lib: sbi_domain: ensure consistent firmware PMP entries
>   lib: sbi: sbi_hart: track firmware PMP entries when configuring SmePMP
>   lib: sbi_domain_context: preserve firmware PMP entries during domain
>     context switch
>
>  include/sbi/sbi_domain.h     |  11 ++++
>  include/sbi/sbi_hart.h       |   1 +
>  lib/sbi/sbi_domain.c         |  98 ++++++++++++++++++++++++++++-
>  lib/sbi/sbi_domain_context.c |   4 ++
>  lib/sbi/sbi_hart.c           | 116 ++++++++++++++---------------------
>  5 files changed, 158 insertions(+), 72 deletions(-)
>
> --
> 2.48.0
>

Applied this series to the riscv/opensbi repo.

Regards,
Anup

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