Skip to contentSkip to conversation

Make PMP encoding usable for non-hart PMPs

PATCH Applied Nicholas Piggin 2 participants 10 messages Apr 30 – Jun 10 RISC-V

lore
Share & tools
lore
#0 lore raw

cover Make PMP encoding usable for non-hart PMPs

The Tenstorrent RISC-V IOMMU implements PMPs with MMRs in the same format as specified in the RISC-V ISA for the hart CSRs. This series splits out the PMP encoding functions so it can be shared by both users.

I moved this series out of the Tenstorrent Atlantis platform series that included IOMMU support, tried to do better at splitting it up, tidied and improved code.

The new user of the API can be seen here for reference (will not quite compile with this new series):

https://lore.kernel.org/opensbi/[email protected]/

I will post that one out again after the base Atlantis platform and this series is merged.

Thanks,
Nick

Nicholas Piggin (7):
lib: sbi: Introduce pmp_t type
lib: sbi: split PMP encoding and CSR access
lib: sbi: Move RISC-V PMP encoding functions to sbi_pmp.c
lib: sbi: Add PMP CSR read and write accessors
lib: sbi: Add pmp_is_enabled() helper
lib: sbi: Add hart_ prefix to PMP functions
lib: sbi: Move hart PMP functions to sbi_hart_pmp.c

 include/sbi/riscv_asm.h          |  12 ---
 include/sbi/sbi_hart_pmp.h       |  12 +++
 include/sbi/sbi_pmp.h            |  23 +++++
 lib/sbi/objects.mk               |   1 +
 lib/sbi/riscv_asm.c              | 162 +------------------------------
 lib/sbi/sbi_hart_pmp.c           | 128 ++++++++++++++++++++++--
 lib/sbi/sbi_pmp.c                | 105 ++++++++++++++++++++
 platform/generic/eswin/eic770x.c |  36 +++----
 8 files changed, 279 insertions(+), 200 deletions(-)
 create mode 100644 include/sbi/sbi_pmp.h
 create mode 100644 lib/sbi/sbi_pmp.c
Signature · 7 lines
2.53.0


-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi
#1 lore raw

PATCH 1/7 lib: sbi: Introduce pmp_t type

To help abstract details of PMP encoding and access, add a new pmp_t type which contains address and cfg in the format of the riscv CSRs. There is no functional change.

---
 lib/sbi/riscv_asm.c | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)
lib/sbi/riscv_asm.c +26 -15
index 3e44320f..acb6b5e6 100644
@@ -272,6 +272,12 @@ void csr_write_num(int csr_num, unsigned long val)
 #undef switchcase_csr_write
 }
 
+struct pmp {
+	unsigned long addr;
+	u8 cfg;
+};
+typedef struct pmp pmp_t;
+
 static unsigned long ctz(unsigned long x)
 {
 	unsigned long ret = 0;
@@ -335,7 +341,8 @@ int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
 {
 	int pmpcfg_csr, pmpcfg_shift, pmpaddr_csr;
 	unsigned long cfgmask, pmpcfg;
-	unsigned long addrmask, pmpaddr;
+	unsigned long addrmask;
+	pmp_t pmp;
 
 	/* check parameters */
 	if (n >= PMP_COUNT || log2len > __riscv_xlen || log2len < PMP_SHIFT)
@@ -356,25 +363,26 @@ int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
 	/* encode PMP config */
 	prot &= ~PMP_A;
 	prot |= (log2len == PMP_SHIFT) ? PMP_A_NA4 : PMP_A_NAPOT;
-	cfgmask = ~(0xffUL << pmpcfg_shift);
-	pmpcfg	= (csr_read_num(pmpcfg_csr) & cfgmask);
-	pmpcfg |= ((prot << pmpcfg_shift) & ~cfgmask);
+	pmp.cfg = prot;
 
 	/* encode PMP address */
 	if (log2len == PMP_SHIFT) {
-		pmpaddr = (addr >> PMP_SHIFT);
+		pmp.addr = (addr >> PMP_SHIFT);
 	} else {
 		if (log2len == __riscv_xlen) {
-			pmpaddr = -1UL;
+			pmp.addr = -1UL;
 		} else {
 			addrmask = (1UL << (log2len - PMP_SHIFT)) - 1;
-			pmpaddr	 = ((addr >> PMP_SHIFT) & ~addrmask);
-			pmpaddr |= (addrmask >> 1);
+			pmp.addr = ((addr >> PMP_SHIFT) & ~addrmask);
+			pmp.addr |= (addrmask >> 1);
 		}
 	}
 
 	/* write csrs */
-	csr_write_num(pmpaddr_csr, pmpaddr);
+	csr_write_num(pmpaddr_csr, pmp.addr);
+	cfgmask = ~(0xffUL << pmpcfg_shift);
+	pmpcfg  = (csr_read_num(pmpcfg_csr) & cfgmask);
+	pmpcfg |= (((unsigned long)pmp.cfg << pmpcfg_shift) & ~cfgmask);
 	csr_write_num(pmpcfg_csr, pmpcfg);
 
 	return 0;
@@ -384,8 +392,9 @@ int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
 	    unsigned long *log2len)
 {
 	int pmpcfg_csr, pmpcfg_shift, pmpaddr_csr;
-	unsigned long cfgmask, pmpcfg, prot;
+	unsigned long cfgmask, prot;
 	unsigned long t1, addr, len;
+	pmp_t pmp;
 
 	/* check parameters */
 	if (n >= PMP_COUNT || !prot_out || !addr_out || !log2len)
@@ -404,14 +413,16 @@ int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
 #endif
 	pmpaddr_csr = CSR_PMPADDR0 + n;
 
-	/* decode PMP config */
 	cfgmask = (0xffUL << pmpcfg_shift);
-	pmpcfg	= csr_read_num(pmpcfg_csr) & cfgmask;
-	prot	= pmpcfg >> pmpcfg_shift;
+	pmp.cfg = (csr_read_num(pmpcfg_csr) & cfgmask) >> pmpcfg_shift;
+	pmp.addr = csr_read_num(pmpaddr_csr);
+
+	/* decode PMP config */
+	prot = pmp.cfg;
 
 	/* decode PMP address */
 	if ((prot & PMP_A) == PMP_A_NAPOT) {
-		addr = csr_read_num(pmpaddr_csr);
+		addr = pmp.addr;
 		if (addr == -1UL) {
 			addr	= 0;
 			len	= __riscv_xlen;
@@ -421,7 +432,7 @@ int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
 			len	= (t1 + PMP_SHIFT + 1);
 		}
 	} else {
-		addr	= csr_read_num(pmpaddr_csr) << PMP_SHIFT;
+		addr	= pmp.addr << PMP_SHIFT;
 		len	= PMP_SHIFT;
 	}
 
Signature · 7 lines
2.53.0


-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi
#2 lore raw

PATCH 2/7 lib: sbi: split PMP encoding and CSR access

In reply to Nicholas Piggin

Allow PMP encoding functions to be shared with non-hart PMP manipulation by splitting encoding / decoding and hart PMP CSR access into their own functions.

---
 lib/sbi/riscv_asm.c | 119 +++++++++++++++++++++++++++-----------------
 1 file changed, 73 insertions(+), 46 deletions(-)
lib/sbi/riscv_asm.c +73 -46
index acb6b5e6..e33e3c36 100644
@@ -336,70 +336,50 @@ int is_pmp_entry_mapped(unsigned long entry)
 	return false;
 }
 
-int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
-	    unsigned long log2len)
+static int pmp_encode(pmp_t *pmp, unsigned long prot, unsigned long addr,
+		      unsigned long log2len)
 {
-	int pmpcfg_csr, pmpcfg_shift, pmpaddr_csr;
-	unsigned long cfgmask, pmpcfg;
-	unsigned long addrmask;
-	pmp_t pmp;
-
 	/* check parameters */
-	if (n >= PMP_COUNT || log2len > __riscv_xlen || log2len < PMP_SHIFT)
+	if (log2len > __riscv_xlen || log2len < PMP_SHIFT)
 		return SBI_EINVAL;
 
-	/* calculate PMP register and offset */
-#if __riscv_xlen == 32
-	pmpcfg_csr   = CSR_PMPCFG0 + (n >> 2);
-	pmpcfg_shift = (n & 3) << 3;
-#elif __riscv_xlen == 64
-	pmpcfg_csr   = (CSR_PMPCFG0 + (n >> 2)) & ~1;
-	pmpcfg_shift = (n & 7) << 3;
-#else
-# error "Unexpected __riscv_xlen"
-#endif
-	pmpaddr_csr = CSR_PMPADDR0 + n;
-
 	/* encode PMP config */
 	prot &= ~PMP_A;
 	prot |= (log2len == PMP_SHIFT) ? PMP_A_NA4 : PMP_A_NAPOT;
-	pmp.cfg = prot;
+	pmp->cfg = prot;
 
 	/* encode PMP address */
 	if (log2len == PMP_SHIFT) {
-		pmp.addr = (addr >> PMP_SHIFT);
+		pmp->addr = (addr >> PMP_SHIFT);
 	} else {
 		if (log2len == __riscv_xlen) {
-			pmp.addr = -1UL;
+			pmp->addr = -1UL;
 		} else {
+			unsigned long addrmask;
 			addrmask = (1UL << (log2len - PMP_SHIFT)) - 1;
-			pmp.addr = ((addr >> PMP_SHIFT) & ~addrmask);
-			pmp.addr |= (addrmask >> 1);
+			pmp->addr = ((addr >> PMP_SHIFT) & ~addrmask);
+			pmp->addr |= (addrmask >> 1);
 		}
 	}
 
-	/* write csrs */
-	csr_write_num(pmpaddr_csr, pmp.addr);
-	cfgmask = ~(0xffUL << pmpcfg_shift);
-	pmpcfg  = (csr_read_num(pmpcfg_csr) & cfgmask);
-	pmpcfg |= (((unsigned long)pmp.cfg << pmpcfg_shift) & ~cfgmask);
-	csr_write_num(pmpcfg_csr, pmpcfg);
-
-	return 0;
+	return SBI_OK;
 }
 
-int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
-	    unsigned long *log2len)
+int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
+	    unsigned long log2len)
 {
 	int pmpcfg_csr, pmpcfg_shift, pmpaddr_csr;
-	unsigned long cfgmask, prot;
-	unsigned long t1, addr, len;
+	unsigned long cfgmask, pmpcfg;
 	pmp_t pmp;
+	int rc;
 
 	/* check parameters */
-	if (n >= PMP_COUNT || !prot_out || !addr_out || !log2len)
+	if (n >= PMP_COUNT)
 		return SBI_EINVAL;
-	*prot_out = *addr_out = *log2len = 0;
+
+	rc = pmp_encode(&pmp, prot, addr, log2len);
+	if (rc)
+		return rc;
 
 	/* calculate PMP register and offset */
 #if __riscv_xlen == 32
@@ -413,16 +393,33 @@ int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
 #endif
 	pmpaddr_csr = CSR_PMPADDR0 + n;
 
-	cfgmask = (0xffUL << pmpcfg_shift);
-	pmp.cfg = (csr_read_num(pmpcfg_csr) & cfgmask) >> pmpcfg_shift;
-	pmp.addr = csr_read_num(pmpaddr_csr);
+	/* write csrs */
+	csr_write_num(pmpaddr_csr, pmp.addr);
+	cfgmask = ~(0xffUL << pmpcfg_shift);
+	pmpcfg  = (csr_read_num(pmpcfg_csr) & cfgmask);
+	pmpcfg |= (((unsigned long)pmp.cfg << pmpcfg_shift) & ~cfgmask);
+	csr_write_num(pmpcfg_csr, pmpcfg);
+
+	return 0;
+}
+
+static int pmp_decode(pmp_t *pmp, unsigned long *prot_out, unsigned long *addr_out,
+		      unsigned long *log2len)
+{
+	unsigned long prot;
+	unsigned long t1, addr, len;
+
+	/* check parameters */
+	if (!prot_out || !addr_out || !log2len)
+		return SBI_EINVAL;
+	*prot_out = *addr_out = *log2len = 0;
 
 	/* decode PMP config */
-	prot = pmp.cfg;
+	prot = pmp->cfg;
 
 	/* decode PMP address */
 	if ((prot & PMP_A) == PMP_A_NAPOT) {
-		addr = pmp.addr;
+		addr = pmp->addr;
 		if (addr == -1UL) {
 			addr	= 0;
 			len	= __riscv_xlen;
@@ -432,7 +429,7 @@ int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
 			len	= (t1 + PMP_SHIFT + 1);
 		}
 	} else {
-		addr	= pmp.addr << PMP_SHIFT;
+		addr	= pmp->addr << PMP_SHIFT;
 		len	= PMP_SHIFT;
 	}
 
@@ -441,5 +438,35 @@ int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
 	*addr_out    = addr;
 	*log2len     = len;
 
-	return 0;
+	return SBI_OK;
+}
+
+int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
+	    unsigned long *log2len)
+{
+	int pmpcfg_csr, pmpcfg_shift, pmpaddr_csr;
+	unsigned long cfgmask;
+	pmp_t pmp;
+
+	/* check parameters */
+	if (n >= PMP_COUNT)
+		return SBI_EINVAL;
+
+	/* calculate PMP register and offset */
+#if __riscv_xlen == 32
+	pmpcfg_csr   = CSR_PMPCFG0 + (n >> 2);
+	pmpcfg_shift = (n & 3) << 3;
+#elif __riscv_xlen == 64
+	pmpcfg_csr   = (CSR_PMPCFG0 + (n >> 2)) & ~1;
+	pmpcfg_shift = (n & 7) << 3;
+#else
+# error "Unexpected __riscv_xlen"
+#endif
+	pmpaddr_csr = CSR_PMPADDR0 + n;
+
+	cfgmask = (0xffUL << pmpcfg_shift);
+	pmp.cfg = (csr_read_num(pmpcfg_csr) & cfgmask) >> pmpcfg_shift;
+	pmp.addr = csr_read_num(pmpaddr_csr);
+
+	return pmp_decode(&pmp, prot_out, addr_out, log2len);
 }
Signature · 7 lines
2.53.0


-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi
#3 lore raw

PATCH 3/7 lib: sbi: Move RISC-V PMP encoding functions to sbi_pmp.c

In reply to Nicholas Piggin

Create a new file for handling the RISC-V PMP format and the new pmp_t type, as opposed to hart PMP CSR specific access.

---
 include/sbi/sbi_pmp.h | 22 ++++++++++
 lib/sbi/objects.mk    |  1 +
 lib/sbi/riscv_asm.c   | 89 +--------------------------------------
 lib/sbi/sbi_pmp.c     | 96 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 120 insertions(+), 88 deletions(-)
 create mode 100644 include/sbi/sbi_pmp.h
 create mode 100644 lib/sbi/sbi_pmp.c
include/sbi/sbi_pmp.h new file +22 -0
new file mode 100644
index 00000000..66a5cc75
@@ -0,0 +1,22 @@
+/*
+ * SPDX-FileCopyrightText: (c) 2025-2026 Tenstorrent USA, Inc.
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#ifndef __SBI_PMP_H__
+#define __SBI_PMP_H__
+
+#include <sbi/sbi_types.h>
+
+struct pmp {
+	unsigned long addr;
+	u8 cfg;
+};
+typedef struct pmp pmp_t;
+
+int pmp_encode(pmp_t *pmp, unsigned long prot, unsigned long addr,
+	       unsigned long log2len);
+int pmp_decode(pmp_t *pmp, unsigned long *prot, unsigned long *addr,
+	       unsigned long *log2len);
+
+#endif
lib/sbi/objects.mk +1 -0
index 07d13229..0e29a277 100644
@@ -87,6 +87,7 @@ libsbi-objs-y += sbi_init.o
 libsbi-objs-y += sbi_ipi.o
 libsbi-objs-y += sbi_irqchip.o
 libsbi-objs-y += sbi_platform.o
+libsbi-objs-y += sbi_pmp.o
 libsbi-objs-y += sbi_pmu.o
 libsbi-objs-y += sbi_dbtr.o
 libsbi-objs-y += sbi_mpxy.o
lib/sbi/riscv_asm.c +1 -88

113 lines not on this page · raw message

lib/sbi/sbi_pmp.c new file +96 -0

96 lines not on this page · raw message

Signature · 7 lines
2.53.0


-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi
#4 lore raw

PATCH 3/7 lib: sbi: Move non-HART PMP functions to sbi_pmp.c

In reply to Nicholas Piggin

Create a new file for handling the RISC-V PMP format.

---
 include/sbi/sbi_pmp.h | 22 ++++++++++
 lib/sbi/objects.mk    |  1 +
 lib/sbi/riscv_asm.c   | 89 +--------------------------------------
 lib/sbi/sbi_pmp.c     | 96 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 120 insertions(+), 88 deletions(-)
 create mode 100644 include/sbi/sbi_pmp.h
 create mode 100644 lib/sbi/sbi_pmp.c
include/sbi/sbi_pmp.h new file +22 -0
new file mode 100644
index 00000000..66a5cc75
@@ -0,0 +1,22 @@
+/*
+ * SPDX-FileCopyrightText: (c) 2025-2026 Tenstorrent USA, Inc.
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#ifndef __SBI_PMP_H__
+#define __SBI_PMP_H__
+
+#include <sbi/sbi_types.h>
+
+struct pmp {
+	unsigned long addr;
+	u8 cfg;
+};
+typedef struct pmp pmp_t;
+
+int pmp_encode(pmp_t *pmp, unsigned long prot, unsigned long addr,
+	       unsigned long log2len);
+int pmp_decode(pmp_t *pmp, unsigned long *prot, unsigned long *addr,
+	       unsigned long *log2len);
+
+#endif
lib/sbi/objects.mk +1 -0
index 07d13229..0e29a277 100644
@@ -87,6 +87,7 @@ libsbi-objs-y += sbi_init.o
 libsbi-objs-y += sbi_ipi.o
 libsbi-objs-y += sbi_irqchip.o
 libsbi-objs-y += sbi_platform.o
+libsbi-objs-y += sbi_pmp.o
 libsbi-objs-y += sbi_pmu.o
 libsbi-objs-y += sbi_dbtr.o
 libsbi-objs-y += sbi_mpxy.o
lib/sbi/riscv_asm.c +1 -88

113 lines not on this page · raw message

lib/sbi/sbi_pmp.c new file +96 -0

96 lines not on this page · raw message

Signature · 7 lines
2.53.0


-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi
#5 lore raw

PATCH 4/7 lib: sbi: Add PMP CSR read and write accessors

In reply to Nicholas Piggin

PMPCFG CSR access is non-trivial as it requires shifting and masking, it makes PMP manipulation code simpler if this basic CSR read/write access is abstracted away.

---
 lib/sbi/riscv_asm.c | 114 ++++++++++++++++++++++----------------------
 1 file changed, 58 insertions(+), 56 deletions(-)
lib/sbi/riscv_asm.c +58 -56
index 9b45805c..2fb0f585 100644
@@ -273,14 +273,16 @@ void csr_write_num(int csr_num, unsigned long val)
 #undef switchcase_csr_write
 }
 
-int pmp_disable(unsigned int n)
+static int hart_pmp_read(pmp_t *pmp, unsigned int n)
 {
-	int pmpcfg_csr, pmpcfg_shift;
-	unsigned long cfgmask, pmpcfg;
+	int pmpcfg_csr, pmpcfg_shift, pmpaddr_csr;
+	unsigned long cfgmask;
 
+	/* check parameters */
 	if (n >= PMP_COUNT)
 		return SBI_EINVAL;
 
+	/* calculate PMP register and offset */
 #if __riscv_xlen == 32
 	pmpcfg_csr   = CSR_PMPCFG0 + (n >> 2);
 	pmpcfg_shift = (n & 3) << 3;
@@ -290,48 +292,24 @@ int pmp_disable(unsigned int n)
 #else
 # error "Unexpected __riscv_xlen"
 #endif
+	pmpaddr_csr = CSR_PMPADDR0 + n;
 
-	/* Clear the address matching bits to disable the pmp entry */
-	cfgmask = ~(0xffUL << pmpcfg_shift);
-	pmpcfg	= (csr_read_num(pmpcfg_csr) & cfgmask);
-
-	csr_write_num(pmpcfg_csr, pmpcfg);
+	cfgmask = (0xffUL << pmpcfg_shift);
+	pmp->cfg = (csr_read_num(pmpcfg_csr) & cfgmask) >> pmpcfg_shift;
+	pmp->addr = csr_read_num(pmpaddr_csr);
 
 	return SBI_OK;
 }
 
-int is_pmp_entry_mapped(unsigned long entry)
-{
-	unsigned long prot;
-	unsigned long addr;
-	unsigned long log2len;
-
-	if (pmp_get(entry, &prot, &addr, &log2len) != 0)
-		return false;
-
-	/* If address matching bits are non-zero, the entry is enable */
-	if (prot & PMP_A)
-		return true;
-
-	return false;
-}
-
-int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
-	    unsigned long log2len)
+static int hart_pmp_write(pmp_t *pmp, unsigned int n)
 {
 	int pmpcfg_csr, pmpcfg_shift, pmpaddr_csr;
 	unsigned long cfgmask, pmpcfg;
-	pmp_t pmp;
-	int rc;
 
 	/* check parameters */
 	if (n >= PMP_COUNT)
 		return SBI_EINVAL;
 
-	rc = pmp_encode(&pmp, prot, addr, log2len);
-	if (rc)
-		return rc;
-
 	/* calculate PMP register and offset */
 #if __riscv_xlen == 32
 	pmpcfg_csr   = CSR_PMPCFG0 + (n >> 2);
@@ -345,41 +323,65 @@ int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
 	pmpaddr_csr = CSR_PMPADDR0 + n;
 
 	/* write csrs */
-	csr_write_num(pmpaddr_csr, pmp.addr);
+	csr_write_num(pmpaddr_csr, pmp->addr);
 	cfgmask = ~(0xffUL << pmpcfg_shift);
 	pmpcfg  = (csr_read_num(pmpcfg_csr) & cfgmask);
-	pmpcfg |= (((unsigned long)pmp.cfg << pmpcfg_shift) & ~cfgmask);
+	pmpcfg |= (((unsigned long)pmp->cfg << pmpcfg_shift) & ~cfgmask);
 	csr_write_num(pmpcfg_csr, pmpcfg);
 
-	return 0;
+	return SBI_OK;
 }
 
-int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
-	    unsigned long *log2len)
+int pmp_disable(unsigned int n)
 {
-	int pmpcfg_csr, pmpcfg_shift, pmpaddr_csr;
-	unsigned long cfgmask;
 	pmp_t pmp;
+	int rc;
 
-	/* check parameters */
-	if (n >= PMP_COUNT)
-		return SBI_EINVAL;
+	rc = hart_pmp_read(&pmp, n);
+	if (rc)
+		return rc;
 
-	/* calculate PMP register and offset */
-#if __riscv_xlen == 32
-	pmpcfg_csr   = CSR_PMPCFG0 + (n >> 2);
-	pmpcfg_shift = (n & 3) << 3;
-#elif __riscv_xlen == 64
-	pmpcfg_csr   = (CSR_PMPCFG0 + (n >> 2)) & ~1;
-	pmpcfg_shift = (n & 7) << 3;
-#else
-# error "Unexpected __riscv_xlen"
-#endif
-	pmpaddr_csr = CSR_PMPADDR0 + n;
+	pmp.cfg = 0;
 
-	cfgmask = (0xffUL << pmpcfg_shift);
-	pmp.cfg = (csr_read_num(pmpcfg_csr) & cfgmask) >> pmpcfg_shift;
-	pmp.addr = csr_read_num(pmpaddr_csr);
+	return hart_pmp_write(&pmp, n);
+}
+
+int is_pmp_entry_mapped(unsigned long entry)
+{
+	pmp_t pmp;
+
+	if (hart_pmp_read(&pmp, entry) != SBI_OK)
+		return false;
+
+	/* If address matching bits are non-zero, the entry is enable */
+	if (pmp.cfg & PMP_A)
+		return true;
+
+	return false;
+}
+
+int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
+	    unsigned long log2len)
+{
+	pmp_t pmp;
+	int rc;
+
+	rc = pmp_encode(&pmp, prot, addr, log2len);
+	if (rc)
+		return rc;
+
+	return hart_pmp_write(&pmp, n);
+}
+
+int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
+	    unsigned long *log2len)
+{
+	pmp_t pmp;
+	int rc;
+
+	rc = hart_pmp_read(&pmp, n);
+	if (rc)
+		return rc;
 
 	return pmp_decode(&pmp, prot_out, addr_out, log2len);
 }
Signature · 7 lines
2.53.0


-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi
#6 lore raw

PATCH 5/7 lib: sbi: Add pmp_is_enabled() helper

In reply to Nicholas Piggin
---
 include/sbi/sbi_pmp.h | 1 +
 lib/sbi/riscv_asm.c   | 6 +-----
 lib/sbi/sbi_pmp.c     | 9 +++++++++
 3 files changed, 11 insertions(+), 5 deletions(-)
include/sbi/sbi_pmp.h +1 -0
index 66a5cc75..bce62d22 100644
@@ -14,6 +14,7 @@ struct pmp {
 };
 typedef struct pmp pmp_t;
 
+bool pmp_is_enabled(pmp_t *pmp);
 int pmp_encode(pmp_t *pmp, unsigned long prot, unsigned long addr,
 	       unsigned long log2len);
 int pmp_decode(pmp_t *pmp, unsigned long *prot, unsigned long *addr,
lib/sbi/riscv_asm.c +1 -5
index 2fb0f585..c0ad4dbe 100644
@@ -353,11 +353,7 @@ int is_pmp_entry_mapped(unsigned long entry)
 	if (hart_pmp_read(&pmp, entry) != SBI_OK)
 		return false;
 
-	/* If address matching bits are non-zero, the entry is enable */
-	if (pmp.cfg & PMP_A)
-		return true;
-
-	return false;
+	return pmp_is_enabled(&pmp);
 }
 
 int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
lib/sbi/sbi_pmp.c +9 -0
index 91eab87f..75b18764 100644
@@ -28,6 +28,15 @@ static unsigned long ctz(unsigned long x)
 	return ret;
 }
 
+bool pmp_is_enabled(pmp_t *pmp)
+{
+	/* If address matching bits are non-zero, the entry is enable */
+	if (pmp->cfg & PMP_A)
+		return true;
+
+	return false;
+}
+
 int pmp_encode(pmp_t *pmp, unsigned long prot, unsigned long addr,
 	       unsigned long log2len)
 {
Signature · 7 lines
2.53.0


-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi
#7 lore raw

PATCH 6/7 lib: sbi: Add hart_ prefix to PMP functions

In reply to Nicholas Piggin

PMP functions that deal with hart PMP CSRs are given a hart_ prefix, to distinguish from RISC-V PMP encoding functions.

The is_pmp_entry_mapped() function is changed a little more, to align with other PMP conventions, and made to return a bool to make it more obvious that it returns a bool and not an SBI_ return code.

---
 include/sbi/riscv_asm.h          | 10 +++++----
 lib/sbi/riscv_asm.c              | 10 ++++-----
 lib/sbi/sbi_hart_pmp.c           | 18 ++++++++--------
 platform/generic/eswin/eic770x.c | 36 ++++++++++++++++----------------
 4 files changed, 38 insertions(+), 36 deletions(-)
include/sbi/riscv_asm.h +6 -4
index 0cf3fc37..f845320b 100644
@@ -81,6 +81,8 @@
 
 #ifndef __ASSEMBLER__
 
+#include <sbi/sbi_types.h>
+
 #define csr_swap(csr, val)                                              \
 	({                                                              \
 		register unsigned long __v = (unsigned long)(val);      \
@@ -210,15 +212,15 @@ int misa_xlen(void);
 void misa_string(int xlen, char *out, unsigned int out_sz);
 
 /* Disable pmp entry at a given index */
-int pmp_disable(unsigned int n);
+int hart_pmp_disable(unsigned int n);
 
 /* Check if the matching field is set */
-int is_pmp_entry_mapped(unsigned long entry);
+bool hart_is_pmp_enabled(unsigned int n);
 
-int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
+int hart_pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
 	    unsigned long log2len);
 
-int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
+int hart_pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
 	    unsigned long *log2len);
 
 #endif /* !__ASSEMBLER__ */
lib/sbi/riscv_asm.c +5 -5
index c0ad4dbe..98a93462 100644
@@ -332,7 +332,7 @@ static int hart_pmp_write(pmp_t *pmp, unsigned int n)
 	return SBI_OK;
 }
 
-int pmp_disable(unsigned int n)
+int hart_pmp_disable(unsigned int n)
 {
 	pmp_t pmp;
 	int rc;
@@ -346,17 +346,17 @@ int pmp_disable(unsigned int n)
 	return hart_pmp_write(&pmp, n);
 }
 
-int is_pmp_entry_mapped(unsigned long entry)
+bool hart_is_pmp_enabled(unsigned int n)
 {
 	pmp_t pmp;
 
-	if (hart_pmp_read(&pmp, entry) != SBI_OK)
+	if (hart_pmp_read(&pmp, n) != SBI_OK)
 		return false;
 
 	return pmp_is_enabled(&pmp);
 }
 
-int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
+int hart_pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
 	    unsigned long log2len)
 {
 	pmp_t pmp;
@@ -369,7 +369,7 @@ int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
 	return hart_pmp_write(&pmp, n);
 }
 
-int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
+int hart_pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
 	    unsigned long *log2len)
 {
 	pmp_t pmp;
lib/sbi/sbi_hart_pmp.c +9 -9
index 02a3b3c4..8b7b091c 100644
@@ -100,7 +100,7 @@ static void sbi_hart_smepmp_set(struct sbi_scratch *scratch,
 		sbi_platform_pmp_set(sbi_platform_ptr(scratch),
 				     pmp_idx, reg->flags, pmp_flags,
 				     reg->base, reg->order);
-		pmp_set(pmp_idx, pmp_flags, reg->base, reg->order);
+		hart_pmp_set(pmp_idx, pmp_flags, reg->base, reg->order);
 	} else {
 		sbi_printf("Can not configure pmp for domain %s because"
 			   " memory region address 0x%lx or size 0x%lx "
@@ -139,7 +139,7 @@ static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch)
 	csr_set(CSR_MSECCFG, MSECCFG_RLB);
 
 	/* Disable the reserved entry */
-	pmp_disable(SBI_SMEPMP_RESV_ENTRY);
+	hart_pmp_disable(SBI_SMEPMP_RESV_ENTRY);
 
 	/* Program M-only regions when MML is not set. */
 	pmp_idx = 0;
@@ -206,7 +206,7 @@ static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch)
 	}
 	/* Disable remaining PMP entries */
 	for(; pmp_idx < pmp_count; pmp_idx++)
-		pmp_disable(pmp_idx);
+		hart_pmp_disable(pmp_idx);
 
 	/*
 	 * All entries are programmed.
@@ -224,7 +224,7 @@ static int sbi_hart_smepmp_map_range(struct sbi_scratch *scratch,
 	unsigned int pmp_flags = (PMP_W | PMP_X);
 	unsigned long order, base = 0;
 
-	if (is_pmp_entry_mapped(SBI_SMEPMP_RESV_ENTRY))
+	if (hart_is_pmp_enabled(SBI_SMEPMP_RESV_ENTRY))
 		return SBI_ENOSPC;
 
 	for (order = MAX(sbi_hart_pmp_log2gran(scratch), log2roundup(size));
@@ -244,7 +244,7 @@ static int sbi_hart_smepmp_map_range(struct sbi_scratch *scratch,
 	sbi_platform_pmp_set(sbi_platform_ptr(scratch), SBI_SMEPMP_RESV_ENTRY,
 			     SBI_DOMAIN_MEMREGION_SHARED_SURW_MRW,
 			     pmp_flags, base, order);
-	pmp_set(SBI_SMEPMP_RESV_ENTRY, pmp_flags, base, order);
+	hart_pmp_set(SBI_SMEPMP_RESV_ENTRY, pmp_flags, base, order);
 
 	return SBI_OK;
 }
@@ -253,7 +253,7 @@ static int sbi_hart_smepmp_unmap_range(struct sbi_scratch *scratch,
 				       unsigned long addr, unsigned long size)
 {
 	sbi_platform_pmp_disable(sbi_platform_ptr(scratch), SBI_SMEPMP_RESV_ENTRY);
-	return pmp_disable(SBI_SMEPMP_RESV_ENTRY);
+	return hart_pmp_disable(SBI_SMEPMP_RESV_ENTRY);
 }
 
 static int sbi_hart_oldpmp_configure(struct sbi_scratch *scratch)
@@ -281,7 +281,7 @@ static int sbi_hart_oldpmp_configure(struct sbi_scratch *scratch)
 			sbi_platform_pmp_set(sbi_platform_ptr(scratch),
 					     pmp_idx, reg->flags, pmp_flags,
 					     reg->base, reg->order);
-			pmp_set(pmp_idx++, pmp_flags, reg->base, reg->order);
+			hart_pmp_set(pmp_idx++, pmp_flags, reg->base, reg->order);
 		} else {
 			sbi_printf("Can not configure pmp for domain %s because"
 				   " memory region address 0x%lx or size 0x%lx "
@@ -291,7 +291,7 @@ static int sbi_hart_oldpmp_configure(struct sbi_scratch *scratch)
 	}
 	/* Disable remaining PMP entries */
 	for(; pmp_idx < pmp_count; pmp_idx++)
-		pmp_disable(pmp_idx);
+		hart_pmp_disable(pmp_idx);
 
 	sbi_hart_pmp_fence();
 	return 0;
@@ -307,7 +307,7 @@ static void sbi_hart_pmp_unconfigure(struct sbi_scratch *scratch)
 			continue;
 
 		sbi_platform_pmp_disable(sbi_platform_ptr(scratch), i);
-		pmp_disable(i);
+		hart_pmp_disable(i);
 	}
 }
 
platform/generic/eswin/eic770x.c +18 -18
index 7330df9f..dfca0c41 100644
@@ -254,21 +254,21 @@ static int eswin_eic7700_final_init(bool cold_boot)
 				   __func__);
 			return SBI_EFAIL;
 		}
-		pmp_set(pmp_idx++, sbi_domain_get_oldpmp_flags(reg),
-			reg->base, reg->order);
+		hart_pmp_set(pmp_idx++, sbi_domain_get_oldpmp_flags(reg),
+			     reg->base, reg->order);
 	}
 
-	pmp_set(PMP_RESERVED_A, PMP_L, EIC770X_L3_ZERO_REMOTE,
-			   log2roundup(EIC770X_L3_ZERO_SIZE));
+	hart_pmp_set(PMP_RESERVED_A, PMP_L, EIC770X_L3_ZERO_REMOTE,
+		     log2roundup(EIC770X_L3_ZERO_SIZE));
 	/**
 	 * Enable P550 internal + System Port, so OpenSBI can access
 	 * CLINT/PLIC/UART. Might be overwritten in pmp_configure.
 	 */
-	pmp_set(PMP_FREE_A_START + PMP_FREE_A_COUNT - 1, 0, 0,
-		log2roundup(EIC770X_MEMPORT_BASE));
+	hart_pmp_set(PMP_FREE_A_START + PMP_FREE_A_COUNT - 1, 0, 0,
+		     log2roundup(EIC770X_MEMPORT_BASE));
 
-	pmp_set(PMP_RESERVED_B, PMP_L, 0,
-		log2roundup(EIC770X_MEMPORT_LIMIT));
+	hart_pmp_set(PMP_RESERVED_B, PMP_L, 0,
+		     log2roundup(EIC770X_MEMPORT_LIMIT));
 	/**
 	 * These must come after the setup of PMP, as we are about to
 	 * enable speculation and HW prefetcher bits
@@ -321,13 +321,13 @@ static int eswin_eic7700_pmp_configure(struct sbi_scratch *scratch)
 		if (pmp_idx >= pmp_max)
 			goto no_more_pmp;
 
-		pmp_set(pmp_idx++, sbi_domain_get_oldpmp_flags(reg),
-			reg->base, reg->order);
+		hart_pmp_set(pmp_idx++, sbi_domain_get_oldpmp_flags(reg),
+			     reg->base, reg->order);
 		prev = reg;
 	}
 	/* Disable the rest */
 	while (pmp_idx < pmp_max)
-		pmp_disable(pmp_idx++);
+		hart_pmp_disable(pmp_idx++);
 
 	/* Process the second free range B [7-7] */
 	pmp_idx = PMP_FREE_B_START,
@@ -340,12 +340,12 @@ static int eswin_eic7700_pmp_configure(struct sbi_scratch *scratch)
 		if (pmp_idx >= pmp_max)
 			goto no_more_pmp;
 
-		pmp_set(pmp_idx++, sbi_domain_get_oldpmp_flags(reg),
-			reg->base, reg->order);
+		hart_pmp_set(pmp_idx++, sbi_domain_get_oldpmp_flags(reg),
+			     reg->base, reg->order);
 	}
 	/* Disable the rest */
 	while (pmp_idx < pmp_max)
-		pmp_disable(pmp_idx++);
+		hart_pmp_disable(pmp_idx++);
 
 	sbi_hart_pmp_fence();
 	return 0;
@@ -357,14 +357,14 @@ no_more_pmp:
 static void eswin_eic7700_pmp_unconfigure(struct sbi_scratch *scratch)
 {
 	/* Enable P550 internal + System Port */
-	pmp_set(PMP_FREE_A_START + PMP_FREE_A_COUNT - 1, 0, 0,
-		log2roundup(EIC770X_MEMPORT_BASE));
+	hart_pmp_set(PMP_FREE_A_START + PMP_FREE_A_COUNT - 1, 0, 0,
+		     log2roundup(EIC770X_MEMPORT_BASE));
 
 	for (unsigned int i = 0; i < PMP_FREE_A_COUNT - 1; i++)
-		pmp_disable(i + PMP_FREE_A_START);
+		hart_pmp_disable(i + PMP_FREE_A_START);
 
 	for (unsigned int i = 0; i < PMP_FREE_B_COUNT; i++)
-		pmp_disable(i + PMP_FREE_B_START);
+		hart_pmp_disable(i + PMP_FREE_B_START);
 }
 
 static struct sbi_hart_protection eswin_eic7700_pmp_protection = {
Signature · 7 lines
2.53.0


-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi
#8 lore raw

PATCH 7/7 lib: sbi: Move hart PMP functions to sbi_hart_pmp.c

In reply to Nicholas Piggin

The sbi_hart_pmp.c looks like a good place for the hart PMP CSR access functions.

---
 include/sbi/riscv_asm.h    |  14 -----
 include/sbi/sbi_hart_pmp.h |  12 ++++
 lib/sbi/riscv_asm.c        | 109 ------------------------------------
 lib/sbi/sbi_hart_pmp.c     | 110 +++++++++++++++++++++++++++++++++++++
 4 files changed, 122 insertions(+), 123 deletions(-)
include/sbi/riscv_asm.h +0 -14
index f845320b..7765edb7 100644
@@ -81,8 +81,6 @@
 
 #ifndef __ASSEMBLER__
 
-#include <sbi/sbi_types.h>
-
 #define csr_swap(csr, val)                                              \
 	({                                                              \
 		register unsigned long __v = (unsigned long)(val);      \
@@ -211,18 +209,6 @@ int misa_xlen(void);
 /* Get RISC-V ISA string representation */
 void misa_string(int xlen, char *out, unsigned int out_sz);
 
-/* Disable pmp entry at a given index */
-int hart_pmp_disable(unsigned int n);
-
-/* Check if the matching field is set */
-bool hart_is_pmp_enabled(unsigned int n);
-
-int hart_pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
-	    unsigned long log2len);
-
-int hart_pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
-	    unsigned long *log2len);
-
 #endif /* !__ASSEMBLER__ */
 
 #endif
include/sbi/sbi_hart_pmp.h +12 -0
index a7765d17..3e0219ac 100644
@@ -9,6 +9,18 @@
 
 #include <sbi/sbi_types.h>
 
+/* Disable pmp entry at a given index */
+int hart_pmp_disable(unsigned int n);
+
+/* Check if the matching field is set */
+bool hart_is_pmp_enabled(unsigned int n);
+
+int hart_pmp_set(unsigned int n, unsigned long prot,
+		 unsigned long addr, unsigned long log2len);
+
+int hart_pmp_get(unsigned int n, unsigned long *prot_out,
+		 unsigned long *addr_out, unsigned long *log2len);
+
 struct sbi_scratch;
 
 unsigned int sbi_hart_pmp_count(struct sbi_scratch *scratch);
lib/sbi/riscv_asm.c +0 -109

112 lines not on this page · raw message

lib/sbi/sbi_hart_pmp.c +110 -0

119 lines not on this page · raw message

Signature · 7 lines
2.53.0


-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi
Reviewed-by on the v1 series Applied
#9 lore raw
In reply to Nicholas Piggin
Quoted 21 lines · Nicholas Piggin

The Tenstorrent RISC-V IOMMU implements PMPs with MMRs in the same format as specified in the RISC-V ISA for the hart CSRs. This series splits out the PMP encoding functions so it can be shared by both users.

I moved this series out of the Tenstorrent Atlantis platform series that included IOMMU support, tried to do better at splitting it up, tidied and improved code.

The new user of the API can be seen here for reference (will not quite compile with this new series):

https://lore.kernel.org/opensbi/[email protected]/

I will post that one out again after the base Atlantis platform and this series is merged.

Thanks,
Nick

Quoted 8 lines

Nicholas Piggin (7):
lib: sbi: Introduce pmp_t type
lib: sbi: split PMP encoding and CSR access
lib: sbi: Move RISC-V PMP encoding functions to sbi_pmp.c
lib: sbi: Add PMP CSR read and write accessors
lib: sbi: Add pmp_is_enabled() helper
lib: sbi: Add hart_ prefix to PMP functions
lib: sbi: Move hart PMP functions to sbi_hart_pmp.c

The only minor comment is to use "sbi_" prefix for various public functions in sbi_pmp.h and sbi_hart_pmp.h otherwise this looks good to me. I will take care of it at the time of merging.

Applied this series to the riscv/opensbi repo.

Thanks,
Anup

Quoted 21 lines
 include/sbi/riscv_asm.h          |  12 ---
 include/sbi/sbi_hart_pmp.h       |  12 +++
 include/sbi/sbi_pmp.h            |  23 +++++
 lib/sbi/objects.mk               |   1 +
 lib/sbi/riscv_asm.c              | 162 +------------------------------
 lib/sbi/sbi_hart_pmp.c           | 128 ++++++++++++++++++++++--
 lib/sbi/sbi_pmp.c                | 105 ++++++++++++++++++++
 platform/generic/eswin/eic770x.c |  36 +++----
 8 files changed, 279 insertions(+), 200 deletions(-)
 create mode 100644 include/sbi/sbi_pmp.h
 create mode 100644 lib/sbi/sbi_pmp.c

--
2.53.0

--
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi

Signature · 3 lines
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi