Split region permissions into M-mode and SU-mode

PATCH New Himanshu Chauhan 2 participants 12 messages Jan 9, 2023

Share & tools

Himanshu Chauhan

author

2023-01-09 05:20 UTC #0 lore raw

v2 cover Split region permissions into M-mode and SU-mode

This is first series in the series of forthcoming patches that would add support for different access permission for different sections of OpenSBI (RO, RX, RW). Eventually, this would lead to support of smepmp extension.

This patch series splits the current R/W/X permissions to M-mode R/W/X and SU-mode R/W/X. The split is in such a way that current permission semantics are not broken

Changes in v2:
1. Fixed the grammatical mistakes in the commit log.
2. Removed the white space new line from sbi_domain.h
3. Added enforce permission bit[6] details to documentation

Himanshu Chauhan (9):
include: sbi: Fine grain the permissions for M and SU modes
lib: sbi: Use finer permission semantics for address validation
lib: sbi: Add permissions for the firmware start till end
lib: sbi: Use finer permission sematics to decide on PMP bits
lib: sbi: Modify the boot time region flag prints
lib: utils: Use SU-{R/W/X} flags for region permissions during parsing
lib: utils: Disallow non-root domains from adding M-mode regions
lib: utils: Add M-mode {R/W} flags to the MMIO regions
docs: Update domain's region permissions and requirements

 docs/domain_support.md          | 12 +++++++--
 include/sbi/sbi_domain.h        | 44 +++++++++++++++++++++++++++----
 lib/sbi/sbi_domain.c            | 46 +++++++++++++++++++++++----------
 lib/sbi/sbi_hart.c              | 16 ++++++++----
 lib/utils/fdt/fdt_domain.c      | 20 +++++++++++---
 lib/utils/ipi/aclint_mswi.c     |  4 ++-
 lib/utils/irqchip/aplic.c       |  4 ++-
 lib/utils/irqchip/imsic.c       |  4 ++-
 lib/utils/timer/aclint_mtimer.c | 16 +++++++++---
 9 files changed, 131 insertions(+), 35 deletions(-)

Signature · 1 line

2.34.1

Himanshu Chauhan

author

2023-01-09 05:20 UTC #1 lore raw

PATCH v2 1/9 include: sbi: Fine grain the permissions for M and SU modes

Split the permissions for M-mode and SU-mode. This would help if different sections of OpenSBI need to be given different permissions and if M-mode has different permisssions than the SU-mode over a region.

Signed-off-by Himanshu Chauhan <[email protected]>

Reviewed-by Anup Patel <[email protected]>

---
 include/sbi/sbi_domain.h | 40 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 35 insertions(+), 5 deletions(-)

include/sbi/sbi_domain.h +35 -5

index f0d9289..7b94ce4 100644
@@ -36,11 +36,41 @@ struct sbi_domain_memregion {
 	 */
 	unsigned long base;
 	/** Flags representing memory region attributes */
-#define SBI_DOMAIN_MEMREGION_READABLE		(1UL << 0)
-#define SBI_DOMAIN_MEMREGION_WRITEABLE		(1UL << 1)
-#define SBI_DOMAIN_MEMREGION_EXECUTABLE		(1UL << 2)
-#define SBI_DOMAIN_MEMREGION_MMODE		(1UL << 3)
-#define SBI_DOMAIN_MEMREGION_ACCESS_MASK	(0xfUL)
+#define SBI_DOMAIN_MEMREGION_M_READABLE		(1UL << 0)
+#define SBI_DOMAIN_MEMREGION_M_WRITABLE		(1UL << 1)
+#define SBI_DOMAIN_MEMREGION_M_EXECUTABLE	(1UL << 2)
+#define SBI_DOMAIN_MEMREGION_SU_READABLE	(1UL << 3)
+#define SBI_DOMAIN_MEMREGION_SU_WRITABLE	(1UL << 4)
+#define SBI_DOMAIN_MEMREGION_SU_EXECUTABLE	(1UL << 5)
+
+	/** Bit to control if permissions are enforced on all modes */
+#define SBI_DOMAIN_MEMREGION_ENF_PERMISSIONS	(1UL << 6)
+
+#define SBI_DOMAIN_MEMREGION_M_RWX		(SBI_DOMAIN_MEMREGION_M_READABLE | \
+						 SBI_DOMAIN_MEMREGION_M_WRITABLE | \
+						 SBI_DOMAIN_MEMREGION_M_EXECUTABLE)
+
+	/* Unrestricted M-mode accesses but enfoced on SU-mode */
+#define SBI_DOMAIN_MEMREGION_READABLE		(SBI_DOMAIN_MEMREGION_SU_READABLE | \
+						 SBI_DOMAIN_MEMREGION_M_RWX)
+#define SBI_DOMAIN_MEMREGION_WRITEABLE		(SBI_DOMAIN_MEMREGION_SU_WRITABLE | \
+						 SBI_DOMAIN_MEMREGION_M_RWX)
+#define SBI_DOMAIN_MEMREGION_EXECUTABLE		(SBI_DOMAIN_MEMREGION_SU_EXECUTABLE | \
+						 SBI_DOMAIN_MEMREGION_M_RWX)
+
+	/* Enforced accesses across all modes */
+#define SBI_DOMAIN_MEMREGION_ENF_READABLE	(SBI_DOMAIN_MEMREGION_SU_READABLE | \
+						 SBI_DOMAIN_MEMREGION_M_READABLE)
+#define SBI_DOMAIN_MEMREGION_ENF_WRITABLE	(SBI_DOMAIN_MEMREGION_SU_WRITABLE | \
+						 SBI_DOMAIN_MEMREGION_M_WRITABLE)
+#define SBI_DOMAIN_MEMREGION_ENF_EXECUTABLE	(SBI_DOMAIN_MEMREGION_SU_EXECUTABLE | \
+						 SBI_DOMAIN_MEMREGION_M_EXECUTABLE)
+
+#define SBI_DOMAIN_MEMREGION_ACCESS_MASK	(0x3fUL)
+#define SBI_DOMAIN_MEMREGION_M_ACCESS_MASK	(0x7UL)
+#define SBI_DOMAIN_MEMREGION_SU_ACCESS_MASK	(0x38UL)
+
+#define SBI_DOMAIN_MEMREGION_SU_ACCESS_SHIFT	(3)
 
 #define SBI_DOMAIN_MEMREGION_MMIO		(1UL << 31)
 	unsigned long flags;

Signature · 1 line

2.34.1

Himanshu Chauhan

author

2023-01-09 05:20 UTC #2 lore raw

PATCH v2 2/9 lib: sbi: Use finer permission semantics for address validation

In reply to Himanshu Chauhan

Use the fine grained permisssion semantics for address validation of a given region.

Signed-off-by Himanshu Chauhan <[email protected]>

Reviewed-by Anup Patel <[email protected]>

---
 lib/sbi/sbi_domain.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

lib/sbi/sbi_domain.c +16 -7

index 19e2029..7663244 100644
@@ -107,24 +107,33 @@ bool sbi_domain_check_addr(const struct sbi_domain *dom,
 {
 	bool rmmio, mmio = false;
 	struct sbi_domain_memregion *reg;
-	unsigned long rstart, rend, rflags, rwx = 0;
+	unsigned long rstart, rend, rflags, rwx = 0, rrwx = 0;
 
 	if (!dom)
 		return false;
 
+	/*
+	 * Use M_{R/W/X} bits because the SU-bits are at the
+	 * same relative offsets. If the mode is not M, the SU
+	 * bits will fall at same offsets after the shift.
+	 */
 	if (access_flags & SBI_DOMAIN_READ)
-		rwx |= SBI_DOMAIN_MEMREGION_READABLE;
+		rwx |= SBI_DOMAIN_MEMREGION_M_READABLE;
+
 	if (access_flags & SBI_DOMAIN_WRITE)
-		rwx |= SBI_DOMAIN_MEMREGION_WRITEABLE;
+		rwx |= SBI_DOMAIN_MEMREGION_M_WRITABLE;
+
 	if (access_flags & SBI_DOMAIN_EXECUTE)
-		rwx |= SBI_DOMAIN_MEMREGION_EXECUTABLE;
+		rwx |= SBI_DOMAIN_MEMREGION_M_EXECUTABLE;
+
 	if (access_flags & SBI_DOMAIN_MMIO)
 		mmio = true;
 
 	sbi_domain_for_each_memregion(dom, reg) {
 		rflags = reg->flags;
-		if (mode == PRV_M && !(rflags & SBI_DOMAIN_MEMREGION_MMODE))
-			continue;
+		rrwx = (mode == PRV_M ? (rflags & SBI_DOMAIN_MEMREGION_M_ACCESS_MASK)
+			: (rflags & SBI_DOMAIN_MEMREGION_SU_ACCESS_MASK)
+			>> SBI_DOMAIN_MEMREGION_SU_ACCESS_SHIFT);
 
 		rstart = reg->base;
 		rend = (reg->order < __riscv_xlen) ?

@@ -133,7 +142,7 @@ bool sbi_domain_check_addr(const struct sbi_domain *dom,
 			rmmio = (rflags & SBI_DOMAIN_MEMREGION_MMIO) ? true : false;
 			if (mmio != rmmio)
 				return false;
-			return ((rflags & rwx) == rwx) ? true : false;
+			return ((rrwx & rwx) == rwx) ? true : false;
 		}
 	}

Signature · 1 line

2.34.1

Himanshu Chauhan

author

2023-01-09 05:20 UTC #3 lore raw

PATCH v2 3/9 lib: sbi: Add permissions for the firmware start till end

In reply to Himanshu Chauhan

Change the zero flag to M-mode R/W/X flag for the firmware region.

Signed-off-by Himanshu Chauhan <[email protected]>

Reviewed-by Anup Patel <[email protected]>

---
 lib/sbi/sbi_domain.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

lib/sbi/sbi_domain.c +2 -1

index 7663244..b36a9d5 100644
@@ -628,7 +628,8 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
 	const struct sbi_platform *plat = sbi_platform_ptr(scratch);
 
 	/* Root domain firmware memory region */
-	sbi_domain_memregion_init(scratch->fw_start, scratch->fw_size, 0,
+	sbi_domain_memregion_init(scratch->fw_start, scratch->fw_size,
+				  SBI_DOMAIN_MEMREGION_M_RWX,
 				  &root_fw_region);
 	domain_memregion_initfw(&root_memregs[root_memregs_count++]);

Signature · 1 line

2.34.1

Himanshu Chauhan

author

2023-01-09 05:20 UTC #4 lore raw

PATCH v2 4/9 lib: sbi: Use finer permission sematics to decide on PMP bits

In reply to Himanshu Chauhan

Use the fine grained permission bits to decide if the region permissions are to be enforced on all modes. Also use the new permission bits for deciding on R/W/X bits in pmpcfg register.

Signed-off-by Himanshu Chauhan <[email protected]>

Reviewed-by Anup Patel <[email protected]>

---
 lib/sbi/sbi_hart.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

lib/sbi/sbi_hart.c +11 -5

index 5447c52..2ded55b 100644
@@ -303,14 +303,20 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch)
 			break;
 
 		pmp_flags = 0;
-		if (reg->flags & SBI_DOMAIN_MEMREGION_READABLE)
+
+		/*
+		 * If permissions are to be enforced for all modes on this region,
+		 * the lock bit should be set.
+		 */
+		if (reg->flags & SBI_DOMAIN_MEMREGION_ENF_PERMISSIONS)
+			pmp_flags |= PMP_L;
+
+		if (reg->flags & SBI_DOMAIN_MEMREGION_SU_READABLE)
 			pmp_flags |= PMP_R;
-		if (reg->flags & SBI_DOMAIN_MEMREGION_WRITEABLE)
+		if (reg->flags & SBI_DOMAIN_MEMREGION_SU_WRITABLE)
 			pmp_flags |= PMP_W;
-		if (reg->flags & SBI_DOMAIN_MEMREGION_EXECUTABLE)
+		if (reg->flags & SBI_DOMAIN_MEMREGION_SU_EXECUTABLE)
 			pmp_flags |= PMP_X;
-		if (reg->flags & SBI_DOMAIN_MEMREGION_MMODE)
-			pmp_flags |= PMP_L;
 
 		pmp_addr =  reg->base >> PMP_SHIFT;
 		if (pmp_gran_log2 <= reg->order && pmp_addr < pmp_addr_max)

Signature · 1 line

2.34.1

Himanshu Chauhan

author

2023-01-09 05:20 UTC #5 lore raw

PATCH v2 5/9 lib: sbi: Modify the boot time region flag prints

In reply to Himanshu Chauhan

With the finer permission semantics, the region access permissions must be displayed separately for M and SU mode.

Signed-off-by Himanshu Chauhan <[email protected]>

Reviewed-by Anup Patel <[email protected]>

---
 lib/sbi/sbi_domain.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

lib/sbi/sbi_domain.c +15 -5

index b36a9d5..f6addc4 100644
@@ -344,15 +344,25 @@ void sbi_domain_dump(const struct sbi_domain *dom, const char *suffix)
 			   dom->index, i, suffix, rstart, rend);
 
 		k = 0;
-		if (reg->flags & SBI_DOMAIN_MEMREGION_MMODE)
-			sbi_printf("%cM", (k++) ? ',' : '(');
+
+		sbi_printf("M: ");
 		if (reg->flags & SBI_DOMAIN_MEMREGION_MMIO)
 			sbi_printf("%cI", (k++) ? ',' : '(');
-		if (reg->flags & SBI_DOMAIN_MEMREGION_READABLE)
+		if (reg->flags & SBI_DOMAIN_MEMREGION_M_READABLE)
+			sbi_printf("%cR", (k++) ? ',' : '(');
+		if (reg->flags & SBI_DOMAIN_MEMREGION_M_WRITABLE)
+			sbi_printf("%cW", (k++) ? ',' : '(');
+		if (reg->flags & SBI_DOMAIN_MEMREGION_M_EXECUTABLE)
+			sbi_printf("%cX", (k++) ? ',' : '(');
+		sbi_printf("%s ", (k++) ? ")" : "()");
+
+		k = 0;
+		sbi_printf("S/U: ");
+		if (reg->flags & SBI_DOMAIN_MEMREGION_SU_READABLE)
 			sbi_printf("%cR", (k++) ? ',' : '(');
-		if (reg->flags & SBI_DOMAIN_MEMREGION_WRITEABLE)
+		if (reg->flags & SBI_DOMAIN_MEMREGION_SU_WRITABLE)
 			sbi_printf("%cW", (k++) ? ',' : '(');
-		if (reg->flags & SBI_DOMAIN_MEMREGION_EXECUTABLE)
+		if (reg->flags & SBI_DOMAIN_MEMREGION_SU_EXECUTABLE)
 			sbi_printf("%cX", (k++) ? ',' : '(');
 		sbi_printf("%s\n", (k++) ? ")" : "()");

Signature · 1 line

2.34.1

Himanshu Chauhan

author

2023-01-09 05:20 UTC #6 lore raw

PATCH v2 6/9 lib: utils: Use SU-{R/W/X} flags for region permissions during parsing

In reply to Himanshu Chauhan

Use the newer SU-{R/W/X} flags for checking and assigning region permissions.

Signed-off-by Himanshu Chauhan <[email protected]>

Reviewed-by Anup Patel <[email protected]>

---
 lib/utils/fdt/fdt_domain.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

lib/utils/fdt/fdt_domain.c +3 -3

index 35462a2..45612ef 100644
@@ -338,9 +338,9 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)
 	 * 2) mmio regions protecting M-mode only mmio devices
 	 */
 	sbi_domain_for_each_memregion(&root, reg) {
-		if ((reg->flags & SBI_DOMAIN_MEMREGION_READABLE) ||
-		    (reg->flags & SBI_DOMAIN_MEMREGION_WRITEABLE) ||
-		    (reg->flags & SBI_DOMAIN_MEMREGION_EXECUTABLE))
+		if ((reg->flags & SBI_DOMAIN_MEMREGION_SU_READABLE) ||
+		    (reg->flags & SBI_DOMAIN_MEMREGION_SU_WRITABLE) ||
+		    (reg->flags & SBI_DOMAIN_MEMREGION_SU_EXECUTABLE))
 			continue;
 		if (FDT_DOMAIN_REGION_MAX_COUNT <= val32)
 			return SBI_EINVAL;

Signature · 1 line

2.34.1

Himanshu Chauhan

author

2023-01-09 05:20 UTC #7 lore raw

PATCH v2 7/9 lib: utils: Disallow non-root domains from adding M-mode regions

In reply to Himanshu Chauhan

The M-mode regions can only be added to the root domain. The non-root domains shouldn't be able to add them from FDT.

Signed-off-by Himanshu Chauhan <[email protected]>

Reviewed-by Anup Patel <[email protected]>

---
 include/sbi/sbi_domain.h   |  4 ++++
 lib/utils/fdt/fdt_domain.c | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

include/sbi/sbi_domain.h +4 -0

index 7b94ce4..bd346b6 100644
@@ -50,6 +50,10 @@ struct sbi_domain_memregion {
 						 SBI_DOMAIN_MEMREGION_M_WRITABLE | \
 						 SBI_DOMAIN_MEMREGION_M_EXECUTABLE)
 
+#define SBI_DOMAIN_MEMREGION_SU_RWX		(SBI_DOMAIN_MEMREGION_SU_READABLE | \
+						 SBI_DOMAIN_MEMREGION_SU_WRITABLE | \
+						 SBI_DOMAIN_MEMREGION_SU_EXECUTABLE)
+
 	/* Unrestricted M-mode accesses but enfoced on SU-mode */
 #define SBI_DOMAIN_MEMREGION_READABLE		(SBI_DOMAIN_MEMREGION_SU_READABLE | \
 						 SBI_DOMAIN_MEMREGION_M_RWX)

lib/utils/fdt/fdt_domain.c +14 -0

index 45612ef..2b51a8e 100644
@@ -239,6 +239,20 @@ static int __fdt_parse_region(void *fdt, int domain_offset,
 	u32 *region_count = opaque;
 	struct sbi_domain_memregion *region;
 
+	/*
+	 * Non-root domains cannot add a region with only M-mode
+	 * access permissions. M-mode regions can only be part of
+	 * root domain.
+	 *
+	 * SU permission bits can't be all zeroes and M-mode permission
+	 * bits must be all set.
+	 */
+	if (!((region_access & SBI_DOMAIN_MEMREGION_SU_ACCESS_MASK)
+	     & SBI_DOMAIN_MEMREGION_SU_RWX)
+	    && ((region_access & SBI_DOMAIN_MEMREGION_M_ACCESS_MASK)
+		& SBI_DOMAIN_MEMREGION_M_RWX))
+		return SBI_EINVAL;
+
 	/* Find next region of the domain */
 	if (FDT_DOMAIN_REGION_MAX_COUNT <= *region_count)
 		return SBI_EINVAL;

Signature · 1 line

2.34.1

Himanshu Chauhan

author

2023-01-09 05:20 UTC #8 lore raw

PATCH v2 8/9 lib: utils: Add M-mode {R/W} flags to the MMIO regions

In reply to Himanshu Chauhan

Add the M-mode readable/writable flags to mmio regions of various drivers.

Signed-off-by Himanshu Chauhan <[email protected]>

Reviewed-by Anup Patel <[email protected]>

---
 lib/utils/ipi/aclint_mswi.c     |  4 +++-
 lib/utils/irqchip/aplic.c       |  4 +++-
 lib/utils/irqchip/imsic.c       |  4 +++-
 lib/utils/timer/aclint_mtimer.c | 16 ++++++++++++----
 4 files changed, 21 insertions(+), 7 deletions(-)

lib/utils/ipi/aclint_mswi.c +3 -1

index 832e223..c714a89 100644
@@ -88,7 +88,9 @@ int aclint_mswi_cold_init(struct aclint_mswi_data *mswi)
 		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, &reg);
+					  (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;

lib/utils/irqchip/aplic.c +3 -1

index 0a8469b..858e9b2 100644
@@ -269,7 +269,9 @@ int aplic_cold_irqchip_init(struct aplic_data *aplic)
 	    (last_deleg_irq == aplic->num_source) &&
 	    (first_deleg_irq == 1))) {
 		sbi_domain_memregion_init(aplic->addr, aplic->size,
-					  SBI_DOMAIN_MEMREGION_MMIO, &reg);
+					  (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;

lib/utils/irqchip/imsic.c +3 -1

index 98f2cb6..ada4f3b 100644
@@ -313,7 +313,9 @@ int imsic_cold_irqchip_init(struct imsic_data *imsic)
 	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, &reg);
+					  (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;

lib/utils/timer/aclint_mtimer.c +12 -4

index 1846a9a..84ded4e 100644
@@ -188,26 +188,34 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
 		rc = sbi_domain_root_add_memrange(mt->mtimecmp_addr,
 					mt->mtime_size + mt->mtimecmp_size,
 					MTIMER_REGION_ALIGN,
-					SBI_DOMAIN_MEMREGION_MMIO);
+					(SBI_DOMAIN_MEMREGION_MMIO |
+					 SBI_DOMAIN_MEMREGION_M_READABLE |
+					 SBI_DOMAIN_MEMREGION_M_WRITABLE));
 		if (rc)
 			return rc;
 	} else if (mt->mtimecmp_addr == (mt->mtime_addr + mt->mtime_size)) {
 		rc = sbi_domain_root_add_memrange(mt->mtime_addr,
 					mt->mtime_size + mt->mtimecmp_size,
 					MTIMER_REGION_ALIGN,
-					SBI_DOMAIN_MEMREGION_MMIO);
+					(SBI_DOMAIN_MEMREGION_MMIO |
+					 SBI_DOMAIN_MEMREGION_M_READABLE |
+					 SBI_DOMAIN_MEMREGION_M_WRITABLE));
 		if (rc)
 			return rc;
 	} else {
 		rc = sbi_domain_root_add_memrange(mt->mtime_addr,
 						mt->mtime_size, MTIMER_REGION_ALIGN,
-						SBI_DOMAIN_MEMREGION_MMIO);
+						(SBI_DOMAIN_MEMREGION_MMIO |
+						 SBI_DOMAIN_MEMREGION_M_READABLE |
+						 SBI_DOMAIN_MEMREGION_M_WRITABLE));
 		if (rc)
 			return rc;
 
 		rc = sbi_domain_root_add_memrange(mt->mtimecmp_addr,
 						mt->mtimecmp_size, MTIMER_REGION_ALIGN,
-						SBI_DOMAIN_MEMREGION_MMIO);
+						(SBI_DOMAIN_MEMREGION_MMIO |
+						 SBI_DOMAIN_MEMREGION_M_READABLE |
+						 SBI_DOMAIN_MEMREGION_M_WRITABLE));
 		if (rc)
 			return rc;
 	}

Signature · 1 line

2.34.1

Himanshu Chauhan

author

2023-01-09 05:20 UTC #9 lore raw

PATCH v2 9/9 docs: Update domain's region permissions and requirements

In reply to Himanshu Chauhan

Updated the various permissions bits available for domains defined in DT node and restrictions on them.

Signed-off-by Himanshu Chauhan <[email protected]>

---
 docs/domain_support.md | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

docs/domain_support.md +10 -2

index 8963b57..11a4dee 100644
@@ -160,8 +160,16 @@ The DT properties of a domain instance DT node are as follows:
 * **regions** (Optional) - The list of domain memory region DT node phandle
   and access permissions for the domain instance. Each list entry is a pair
   of DT node phandle and access permissions. The access permissions are
-  represented as a 32bit bitmask having bits: **readable** (BIT[0]),
-  **writeable** (BIT[1]), **executable** (BIT[2]), and **m-mode** (BIT[3]).
+  represented as a 32bit bitmask having bits: **M readable** (BIT[0]),
+  **M writeable** (BIT[1]), **M executable** (BIT[2]), **SU readable**
+  (BIT[3]), **SU writable** (BIT[4]), and **SU executable** (BIT[5]).
+  The enforce permission bit (BIT[6]), if set, will lock the permissions
+  in the PMP. This will enforce the permissions on M-mode as well which
+  otherwise will have unrestricted access. This bit must be used with
+  caution because no changes can be made to a PMP entry once its locked
+  until the hart is reset.
+  Any region of a domain defined in DT node cannot have only M-bits set
+  in access permissions i.e. it cannot be an m-mode only accessible region.
 * **boot-hart** (Optional) - The DT node phandle of the HART booting the
   domain instance. If coldboot HART is assigned to the domain instance then
   this DT property is ignored and the coldboot HART is assumed to be the

Signature · 1 line

2.34.1

Anup Patel

2023-01-09 11:45 UTC #10 lore raw

PATCH v2 9/9 docs: Update domain's region permissions and requirements

Quoted 5 lines · Himanshu Chauhan

Updated the various permissions bits available for domains defined in DT node and restrictions on them.
Signed-off-by Himanshu Chauhan <[email protected]>

Looks good to me.

Reviewed-by Anup Patel <[email protected]>

Regards,
Anup

Quoted 35 lines

---
 docs/domain_support.md | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

docs/domain_support.md +10 -3

index 8963b57..11a4dee 100644
@@ -160,8 +160,16 @@ The DT properties of a domain instance DT node are as follows:
 * **regions** (Optional) - The list of domain memory region DT node phandle
   and access permissions for the domain instance. Each list entry is a pair
   of DT node phandle and access permissions. The access permissions are
-  represented as a 32bit bitmask having bits: **readable** (BIT[0]),
-  **writeable** (BIT[1]), **executable** (BIT[2]), and **m-mode** (BIT[3]).
+  represented as a 32bit bitmask having bits: **M readable** (BIT[0]),
+  **M writeable** (BIT[1]), **M executable** (BIT[2]), **SU readable**
+  (BIT[3]), **SU writable** (BIT[4]), and **SU executable** (BIT[5]).
+  The enforce permission bit (BIT[6]), if set, will lock the permissions
+  in the PMP. This will enforce the permissions on M-mode as well which
+  otherwise will have unrestricted access. This bit must be used with
+  caution because no changes can be made to a PMP entry once its locked
+  until the hart is reset.
+  Any region of a domain defined in DT node cannot have only M-bits set
+  in access permissions i.e. it cannot be an m-mode only accessible region.
 * **boot-hart** (Optional) - The DT node phandle of the HART booting the
   domain instance. If coldboot HART is assigned to the domain instance then
   this DT property is ignored and the coldboot HART is assumed to be the
--

2.34.1

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

Anup Patel

Applied

2023-01-09 12:34 UTC #11 lore raw

v2 cover Split region permissions into M-mode and SU-mode

In reply to Himanshu Chauhan

Quoted 15 lines · Himanshu Chauhan

This is first series in the series of forthcoming patches that would add support for different access permission for different sections of OpenSBI (RO, RX, RW). Eventually, this would lead to support of smepmp extension.
This patch series splits the current R/W/X permissions to M-mode R/W/X and SU-mode R/W/X. The split is in such a way that current permission semantics are not broken
Changes in v2:
1. Fixed the grammatical mistakes in the commit log.
2. Removed the white space new line from sbi_domain.h
3. Added enforce permission bit[6] details to documentation

Quoted 10 lines

Himanshu Chauhan (9):
include: sbi: Fine grain the permissions for M and SU modes
lib: sbi: Use finer permission semantics for address validation
lib: sbi: Add permissions for the firmware start till end
lib: sbi: Use finer permission sematics to decide on PMP bits
lib: sbi: Modify the boot time region flag prints
lib: utils: Use SU-{R/W/X} flags for region permissions during parsing
lib: utils: Disallow non-root domains from adding M-mode regions
lib: utils: Add M-mode {R/W} flags to the MMIO regions
docs: Update domain's region permissions and requirements

I have tested this series with two domains (trusted and untrusted) on QEMU sifive_u machine.

Tested-by Anup Patel <[email protected]>

Applied this series to the riscv/opensbi repo.

Thanks,
Anup

Quoted 20 lines

 docs/domain_support.md          | 12 +++++++--
 include/sbi/sbi_domain.h        | 44 +++++++++++++++++++++++++++----
 lib/sbi/sbi_domain.c            | 46 +++++++++++++++++++++++----------
 lib/sbi/sbi_hart.c              | 16 ++++++++----
 lib/utils/fdt/fdt_domain.c      | 20 +++++++++++---
 lib/utils/ipi/aclint_mswi.c     |  4 ++-
 lib/utils/irqchip/aplic.c       |  4 ++-
 lib/utils/irqchip/imsic.c       |  4 ++-
 lib/utils/timer/aclint_mtimer.c | 16 +++++++++---
 9 files changed, 131 insertions(+), 35 deletions(-)

--
2.34.1

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