* [PATCH 0/3] Initial ESWIN/EIC7700 support
@ 2025-11-11 3:41 Bo Gan
2025-11-11 3:41 ` [PATCH 1/3] lib: sbi: allow platform to override PMP configuration Bo Gan
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Bo Gan @ 2025-11-11 3:41 UTC (permalink / raw)
To: opensbi; +Cc: linmin, pinkesh.vaghela, gaohan, samuel
EIC7700 is the SoC used in HiFive P550 and Milk-V Megrez. This SoC is
currently one of the only off-the-shelf board/chips that support H
extension, although it's v0.6.1. It also supports pre-ratified N-trace.
Add support for it so people can benefit from latest OpenSBI features.
The device-tree of HiFive P550 has been upstreamed to Linux:
https://lore.kernel.org/all/20250825132427.1618089-1-pinkesh.vaghela@einfochips.com/
However U-boot is not, and there are bugs in vendor U-boot device-tree,
and also inconsistencies between the two. Thus, this patch is coded with
the upstreamed device-tree as the reference, but tested with the patched
vendor U-boot device tree as `FW_FDT_PATH`. The patched vendor U-boot is
hosted here: https://github.com/ganboing/u-boot-eic7x/tree/eic7x-dt-fix
Refer to PATCH 3/3 for the instructions on building the firmware blob
and launch it through UART boot.
The major complication of this chip is that it requires certain memory
regions to be blocked with PMP entries to prevent speculative execution
or HW prefetcher from touching them to avoid bus errors. The logic of
handling PMP is therefore enhanced to support TOR entries and allow the
platform to override PMP configuration. The platform code then properly
set the SoC/Chip specific PMP entries alongside with the ones passed by
generic code in lib/.
Bo Gan (3):
lib: sbi: allow platform to override PMP configuration
lib: sbi: Add pmp_set_tor for setting TOR regions
platform: generic: eswin: add EIC7700
include/sbi/riscv_asm.h | 4 +
include/sbi/sbi_hart.h | 9 ++
include/sbi/sbi_platform.h | 33 +++++
lib/sbi/riscv_asm.c | 75 ++++++++---
lib/sbi/sbi_hart.c | 27 ++--
platform/generic/Kconfig | 5 +
platform/generic/configs/defconfig | 1 +
platform/generic/eswin/Kconfig | 29 ++++
platform/generic/eswin/eic770x.c | 165 +++++++++++++++++++++++
platform/generic/eswin/objects.mk | 11 ++
platform/generic/include/eswin/eic770x.h | 49 +++++++
11 files changed, 383 insertions(+), 25 deletions(-)
create mode 100644 platform/generic/eswin/Kconfig
create mode 100644 platform/generic/eswin/eic770x.c
create mode 100644 platform/generic/eswin/objects.mk
create mode 100644 platform/generic/include/eswin/eic770x.h
--
2.34.1
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] lib: sbi: allow platform to override PMP configuration
2025-11-11 3:41 [PATCH 0/3] Initial ESWIN/EIC7700 support Bo Gan
@ 2025-11-11 3:41 ` Bo Gan
2025-11-11 5:45 ` Xiang W
2025-11-11 3:41 ` [PATCH 2/3] lib: sbi: Add pmp_set_tor for setting TOR regions Bo Gan
2025-11-11 3:41 ` [PATCH 3/3] platform: generic: eswin: add EIC7700 Bo Gan
2 siblings, 1 reply; 11+ messages in thread
From: Bo Gan @ 2025-11-11 3:41 UTC (permalink / raw)
To: opensbi; +Cc: linmin, pinkesh.vaghela, gaohan, samuel
Platform sometimes wants to override the entire PMP configuration phase,
not just performing additional work for each individual PMP entry. This
allows platforms to insert SoC/core specific PMP entries in a way that
works together with the existing ones set by lib/ code, not conflicting.
platform can also choose to merge or skip memory regions in a reasonable
way in case there's a shortage of PMP entries.
In addition, `sbi_hart_oldpmp_configure` is made public and a callback
function `skip` is added, so platform code can built on top of it to
simplify PMP configuration override.
Signed-off-by: Bo Gan <ganboing@gmail.com>
---
include/sbi/sbi_hart.h | 9 +++++++++
include/sbi/sbi_platform.h | 33 +++++++++++++++++++++++++++++++++
lib/sbi/sbi_hart.c | 27 +++++++++++++++++++--------
3 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
index e66dd52f..e5a221bd 100644
--- a/include/sbi/sbi_hart.h
+++ b/include/sbi/sbi_hart.h
@@ -133,6 +133,7 @@ struct sbi_hart_features {
};
struct sbi_scratch;
+struct sbi_domain_memregion;
int sbi_hart_reinit(struct sbi_scratch *scratch);
int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot);
@@ -148,6 +149,14 @@ 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_oldpmp_configure(struct sbi_scratch *scratch,
+ unsigned int pmp_start,
+ unsigned int pmp_count,
+ unsigned int pmp_log2gran,
+ unsigned long pmp_addr_max,
+ bool (*skip)(struct sbi_domain_memregion *reg,
+ void *data),
+ void *data);
int sbi_hart_map_saddr(unsigned long base, unsigned long size);
int sbi_hart_unmap_saddr(void);
int sbi_hart_priv_version(struct sbi_scratch *scratch);
diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
index d75c12de..c6fc137f 100644
--- a/include/sbi/sbi_platform.h
+++ b/include/sbi/sbi_platform.h
@@ -146,6 +146,11 @@ struct sbi_platform_operations {
unsigned long log2len);
/** platform specific pmp disable on current HART */
void (*pmp_disable)(unsigned int n);
+
+ /** platform pmp configure override on current HART */
+ int (*pmp_configure)(unsigned int pmp_count,
+ unsigned int pmp_log2gran,
+ unsigned long pmp_addr_max);
};
/** Platform default per-HART stack size for exception/interrupt handling */
@@ -666,6 +671,34 @@ static inline void sbi_platform_pmp_disable(const struct sbi_platform *plat,
sbi_platform_ops(plat)->pmp_disable(n);
}
+/**
+ * Check if platform wants to override PMP configuration
+ *
+ * @param plat pointer to struct sbi_platform
+ */
+static inline bool sbi_platform_pmp_override(const struct sbi_platform *plat)
+{
+ return plat && sbi_platform_ops(plat)->pmp_configure;
+}
+
+/**
+ * Platform PMP configuration override
+ *
+ * @param plat pointer to struct sbi_platform
+ * @param pmp_count number of PMP entries
+ * @param pmp_log2gran PMP granularity
+ * @param pmp_addr_max largest value pmpaddr(x) can hold
+ */
+static inline int sbi_platform_pmp_configure(const struct sbi_platform *plat,
+ unsigned int pmp_count,
+ unsigned int pmp_log2gran,
+ unsigned long pmp_addr_max)
+{
+ return sbi_platform_ops(plat)->pmp_configure(pmp_count,
+ pmp_log2gran,
+ pmp_addr_max);
+}
+
#endif
#endif
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index a91703b4..4425f36b 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -433,18 +433,24 @@ static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch,
return 0;
}
-static int sbi_hart_oldpmp_configure(struct sbi_scratch *scratch,
- unsigned int pmp_count,
- unsigned int pmp_log2gran,
- unsigned long pmp_addr_max)
+int sbi_hart_oldpmp_configure(struct sbi_scratch *scratch,
+ unsigned int pmp_start,
+ unsigned int pmp_count,
+ unsigned int pmp_log2gran,
+ unsigned long pmp_addr_max,
+ bool (*skip)(struct sbi_domain_memregion *reg,
+ void *data),
+ void *data)
{
struct sbi_domain_memregion *reg;
struct sbi_domain *dom = sbi_domain_thishart_ptr();
- unsigned int pmp_idx = 0;
+ unsigned int pmp_idx = pmp_start;
unsigned int pmp_flags;
unsigned long pmp_addr;
sbi_domain_for_each_memregion(dom, reg) {
+ if (skip && skip(reg, data))
+ continue;
if (!is_valid_pmp_idx(pmp_count, pmp_idx))
return SBI_EFAIL;
@@ -534,6 +540,7 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch)
unsigned int pmp_bits, pmp_log2gran;
unsigned int pmp_count = sbi_hart_pmp_count(scratch);
unsigned long pmp_addr_max;
+ const struct sbi_platform *plat = sbi_platform_ptr(scratch);
if (!pmp_count)
return 0;
@@ -542,12 +549,16 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch)
pmp_bits = sbi_hart_pmp_addrbits(scratch) - 1;
pmp_addr_max = (1UL << pmp_bits) | ((1UL << pmp_bits) - 1);
- if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMEPMP))
+ if (sbi_platform_pmp_override(plat))
+ rc = sbi_platform_pmp_configure(plat, pmp_count,
+ pmp_log2gran, pmp_addr_max);
+ else if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMEPMP))
rc = sbi_hart_smepmp_configure(scratch, pmp_count,
pmp_log2gran, pmp_addr_max);
else
- rc = sbi_hart_oldpmp_configure(scratch, pmp_count,
- pmp_log2gran, pmp_addr_max);
+ rc = sbi_hart_oldpmp_configure(scratch, 0, pmp_count,
+ pmp_log2gran, pmp_addr_max,
+ NULL, NULL);
/*
* As per section 3.7.2 of privileged specification v1.12,
--
2.34.1
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] lib: sbi: Add pmp_set_tor for setting TOR regions
2025-11-11 3:41 [PATCH 0/3] Initial ESWIN/EIC7700 support Bo Gan
2025-11-11 3:41 ` [PATCH 1/3] lib: sbi: allow platform to override PMP configuration Bo Gan
@ 2025-11-11 3:41 ` Bo Gan
2025-11-11 5:45 ` Xiang W
2025-11-11 3:41 ` [PATCH 3/3] platform: generic: eswin: add EIC7700 Bo Gan
2 siblings, 1 reply; 11+ messages in thread
From: Bo Gan @ 2025-11-11 3:41 UTC (permalink / raw)
To: opensbi; +Cc: linmin, pinkesh.vaghela, gaohan, samuel
TOR can be utilized to cover memory regions that are not aligned with
their sizes. Given that the address matching is formed bt 2 consecutive
pmpaddr, i.e., pmpaddr(i-1) and pmpaddr(i), TOR should not be used
generically to avoid pmpaddr conflict with other NA4/NAPOT regions.
It's advised to only use them in platform code where the caller can
ensure the index and order of every pmp region especially when there's
a mixture of TOR/NA4/NAPOT.
Signed-off-by: Bo Gan <ganboing@gmail.com>
---
include/sbi/riscv_asm.h | 4 +++
lib/sbi/riscv_asm.c | 75 +++++++++++++++++++++++++++++++----------
2 files changed, 62 insertions(+), 17 deletions(-)
diff --git a/include/sbi/riscv_asm.h b/include/sbi/riscv_asm.h
index ef48dc89..c23feab6 100644
--- a/include/sbi/riscv_asm.h
+++ b/include/sbi/riscv_asm.h
@@ -218,6 +218,10 @@ int is_pmp_entry_mapped(unsigned long entry);
int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
unsigned long log2len);
+/* Top of range (TOR) matching mode. pmpaddr(n-1) will also be changed */
+int pmp_set_tor(unsigned int n, unsigned long prot, unsigned long addr,
+ unsigned long size);
+
int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
unsigned long *log2len);
diff --git a/lib/sbi/riscv_asm.c b/lib/sbi/riscv_asm.c
index 3e44320f..8f64f2b5 100644
--- a/lib/sbi/riscv_asm.c
+++ b/lib/sbi/riscv_asm.c
@@ -330,16 +330,10 @@ 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 void pmp_set_prot(unsigned int n, unsigned long prot)
{
- int pmpcfg_csr, pmpcfg_shift, pmpaddr_csr;
+ int pmpcfg_csr, pmpcfg_shift;
unsigned long cfgmask, pmpcfg;
- unsigned long addrmask, pmpaddr;
-
- /* check parameters */
- if (n >= PMP_COUNT || log2len > __riscv_xlen || log2len < PMP_SHIFT)
- return SBI_EINVAL;
/* calculate PMP register and offset */
#if __riscv_xlen == 32
@@ -351,15 +345,29 @@ int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
#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;
cfgmask = ~(0xffUL << pmpcfg_shift);
pmpcfg = (csr_read_num(pmpcfg_csr) & cfgmask);
pmpcfg |= ((prot << pmpcfg_shift) & ~cfgmask);
+ csr_write_num(pmpcfg_csr, pmpcfg);
+}
+
+static void pmp_set_addr(unsigned int n, unsigned long pmpaddr)
+{
+ int pmpaddr_csr = CSR_PMPADDR0 + n;
+
+ csr_write_num(pmpaddr_csr, pmpaddr);
+}
+
+int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
+ unsigned long log2len)
+{
+ unsigned long addrmask, pmpaddr;
+
+ /* check parameters */
+ if (n >= PMP_COUNT || log2len > __riscv_xlen || log2len < PMP_SHIFT)
+ return SBI_EINVAL;
+
/* encode PMP address */
if (log2len == PMP_SHIFT) {
pmpaddr = (addr >> PMP_SHIFT);
@@ -373,10 +381,41 @@ 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;
+
/* write csrs */
- csr_write_num(pmpaddr_csr, pmpaddr);
- csr_write_num(pmpcfg_csr, pmpcfg);
+ pmp_set_addr(n, pmpaddr);
+ pmp_set_prot(n, prot);
+ return 0;
+}
+
+int pmp_set_tor(unsigned int n, unsigned long prot, unsigned long addr,
+ unsigned long size)
+{
+ unsigned long pmpaddr, pmpaddrp;
+
+ /* check parameters */
+ if (n >= PMP_COUNT)
+ return SBI_EINVAL;
+
+ if (n == 0 && addr != 0)
+ return SBI_EINVAL;
+
+ /* encode PMP address */
+ pmpaddrp = addr >> PMP_SHIFT;
+ pmpaddr = (addr + size) >> PMP_SHIFT;
+ /* encode PMP config */
+ prot &= ~PMP_A;
+ prot |= PMP_A_TOR;
+
+ /* write csrs */
+ if (n)
+ pmp_set_addr(n - 1, pmpaddrp);
+ pmp_set_addr(n, pmpaddr);
+ pmp_set_prot(n, prot);
return 0;
}
@@ -420,10 +459,12 @@ int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
addr = (addr & ~((1UL << t1) - 1)) << PMP_SHIFT;
len = (t1 + PMP_SHIFT + 1);
}
- } else {
+ } else if ((prot & PMP_A) == PMP_A_NA4) {
addr = csr_read_num(pmpaddr_csr) << PMP_SHIFT;
len = PMP_SHIFT;
- }
+ } else
+ /* Error out for TOR region */
+ return SBI_EINVAL;
/* return details */
*prot_out = prot;
--
2.34.1
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] platform: generic: eswin: add EIC7700
2025-11-11 3:41 [PATCH 0/3] Initial ESWIN/EIC7700 support Bo Gan
2025-11-11 3:41 ` [PATCH 1/3] lib: sbi: allow platform to override PMP configuration Bo Gan
2025-11-11 3:41 ` [PATCH 2/3] lib: sbi: Add pmp_set_tor for setting TOR regions Bo Gan
@ 2025-11-11 3:41 ` Bo Gan
2 siblings, 0 replies; 11+ messages in thread
From: Bo Gan @ 2025-11-11 3:41 UTC (permalink / raw)
To: opensbi; +Cc: linmin, pinkesh.vaghela, gaohan, samuel
Initial platform support for ESWIN Computing EIC7700 based on public SoC
datasheet[1] and tested on HiFive Premier P550. Vendor U-boot/Linux boots
fine, and I've tested Geekbench 6.5.0 Preview and got scores on par with
the vendor OpenSBI. System shutdown is not implemented due to the lack of
public doc regarding the UART communication protocol between the SoC and
the onboard BMC, which controls ATX power on HiFive P550. [2]
The files and functions are intentionally named as eic770x in many places
for future enhancements to support the 2-cluster/die version of the same
SoC, namely EIC7702, seen on DC-ROMA AI PC FML13V03. (Out of scope)
There're 4 build time tunable parameters added to Kconfig, and the values
are set in custom CSRs during boot:
- ESWIN_EIC770X_FEAT0_CFG
- ESWIN_EIC770X_FEAT1_CFG
- ESWIN_EIC770X_L1_HWPF_CFG
- ESWIN_EIC770X_L2_HWPF_CFG
The default values are dumped when running SiFive's vendor OpenSBI, and
with my own tuning to address the perf regression reported by drmpeg [3]
To build the firmware+u-boot blob, Use the following, and docs [4] for
testing it with UART boot without flashing:
make FW_TEXT_START=0x80000000 \
FW_PAYLOAD_OFFSET=0x200000 \
FW_PAYLOAD_PATH=u-boot-nodtb.bin \
FW_PAYLOAD_FDT_ADDR=0xf8000000 \
FW_FDT_PATH=u-boot.dtb
[1] https://github.com/eswincomputing/EIC7700X-SoC-Technical-Reference-Manual
[2] https://www.sifive.com/boards/hifive-premier-p550#documentation
[3] https://forums.sifive.com/t/low-1-core-stream-bandwidth/7274/15
[4] https://github.com/ganboing/EIC770x-Docs/blob/main/p550/bootchain/UART-Boot.md
Signed-off-by: Bo Gan <ganboing@gmail.com>
---
platform/generic/Kconfig | 5 +
platform/generic/configs/defconfig | 1 +
platform/generic/eswin/Kconfig | 29 ++++
platform/generic/eswin/eic770x.c | 165 +++++++++++++++++++++++
platform/generic/eswin/objects.mk | 11 ++
platform/generic/include/eswin/eic770x.h | 49 +++++++
6 files changed, 260 insertions(+)
create mode 100644 platform/generic/eswin/Kconfig
create mode 100644 platform/generic/eswin/eic770x.c
create mode 100644 platform/generic/eswin/objects.mk
create mode 100644 platform/generic/include/eswin/eic770x.h
diff --git a/platform/generic/Kconfig b/platform/generic/Kconfig
index aedc59a5..047e13ca 100644
--- a/platform/generic/Kconfig
+++ b/platform/generic/Kconfig
@@ -88,7 +88,12 @@ config PLATFORM_SPACEMIT_K1
select FDT_HSM_SPACEMIT
default n
+config PLATFORM_ESWIN_EIC770X
+ bool "ESWIN EIC770X support"
+ default n
+
source "$(OPENSBI_SRC_DIR)/platform/generic/andes/Kconfig"
+source "$(OPENSBI_SRC_DIR)/platform/generic/eswin/Kconfig"
source "$(OPENSBI_SRC_DIR)/platform/generic/thead/Kconfig"
endif
diff --git a/platform/generic/configs/defconfig b/platform/generic/configs/defconfig
index 9e040ac3..60c5449e 100644
--- a/platform/generic/configs/defconfig
+++ b/platform/generic/configs/defconfig
@@ -10,6 +10,7 @@ CONFIG_PLATFORM_STARFIVE_JH7110=y
CONFIG_PLATFORM_THEAD=y
CONFIG_PLATFORM_MIPS_P8700=y
CONFIG_PLATFORM_SPACEMIT_K1=y
+CONFIG_PLATFORM_ESWIN_EIC770X=y
CONFIG_FDT_CACHE=y
CONFIG_FDT_CACHE_SIFIVE_CCACHE=y
CONFIG_FDT_CPPC=y
diff --git a/platform/generic/eswin/Kconfig b/platform/generic/eswin/Kconfig
new file mode 100644
index 00000000..84d0f43a
--- /dev/null
+++ b/platform/generic/eswin/Kconfig
@@ -0,0 +1,29 @@
+# SPDX-License-Identifier: BSD-2-Clause
+
+config ESWIN_EIC770X_FEAT0_CFG
+ int "ESWIN EIC7700X Feature Disable 0 CSR Configuration"
+ default 0x4000
+ help
+ CSR Value to initialize EIC770X_FEAT0 (0x7c1) with.
+ Refer to EIC770X SoC TRM for recommendations.
+
+config ESWIN_EIC770X_FEAT1_CFG
+ int "ESWIN EIC7700X Feature Disable 1 CSR Configuration"
+ default 0x80
+ help
+ CSR Value to initialize EIC770X_FEAT1 (0x7c2) with.
+ Refer to EIC770X SoC TRM for recommendations.
+
+config ESWIN_EIC770X_L1_HWPF_CFG
+ int "ESWIN EIC7700X L1 HW Prefetcher CSR Configuration"
+ default 0x1005c1be649
+ help
+ CSR Value to initialize EIC770X_L1_HWPF (0x7c3) with.
+ Refer to EIC770X SoC TRM for recommendations.
+
+config ESWIN_EIC770X_L2_HWPF_CFG
+ int "ESWIN EIC7700X L2 HW Prefetcher CSR Configuration"
+ default 0x929f
+ help
+ CSR Value to initialize EIC770X_L2_HWPF (0x7c4) with.
+ Refer to EIC770X SoC TRM for recommendations.
diff --git a/platform/generic/eswin/eic770x.c b/platform/generic/eswin/eic770x.c
new file mode 100644
index 00000000..a4a11186
--- /dev/null
+++ b/platform/generic/eswin/eic770x.c
@@ -0,0 +1,165 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Authors:
+ * Bo Gan <ganboing@gmail.com>
+ *
+ */
+
+#include <platform_override.h>
+#include <sbi/riscv_io.h>
+#include <sbi/sbi_console.h>
+#include <sbi/sbi_system.h>
+#include <sbi/sbi_math.h>
+#include <eswin/eic770x.h>
+
+static int eic770x_system_reset_check(u32 type, u32 reason)
+{
+ switch (type) {
+ case SBI_SRST_RESET_TYPE_COLD_REBOOT:
+ case SBI_SRST_RESET_TYPE_WARM_REBOOT:
+ return 1;
+ default:
+ return 0;
+ }
+}
+
+static void eic770x_system_reset(u32 type, u32 reason)
+{
+ switch (type) {
+ case SBI_SRST_RESET_TYPE_COLD_REBOOT:
+ case SBI_SRST_RESET_TYPE_WARM_REBOOT:
+ writel(EIC770X_SYSCRG_RST_VAL, (void *)EIC770X_SYSCRG_RST);
+ }
+
+ sbi_printf("%s: Unable to reset system\n", __func__);
+ sbi_hart_hang();
+}
+
+static struct sbi_system_reset_device eic770x_reset = {
+ .name = "eic770x_reset",
+ .system_reset_check = eic770x_system_reset_check,
+ .system_reset = eic770x_system_reset
+};
+
+static int eswin_eic770x_early_init(bool cold_boot)
+{
+ if (cold_boot) {
+ /* Enable bus blocker */
+ writel(1, (void*)EIC770X_TL64D2D_OUT);
+ writel(1, (void*)EIC770X_TL256D2D_OUT);
+ writel(1, (void*)EIC770X_TL256D2D_IN);
+ asm volatile ("fence o, rw");
+ }
+
+ return generic_early_init(cold_boot);
+}
+
+static int eswin_eic770x_final_init(bool cold_boot)
+{
+ if (cold_boot)
+ sbi_system_reset_add_device(&eic770x_reset);
+
+ /**
+ * On EIC770X, we need to use PMP to block Memory Port regions that
+ * doesn't belong to the current hart's cluster in order to prevent
+ * speculative accesses or HW prefetcher (perhaps?) from generating
+ * bus error:
+ *
+ * bus error of cause event: 9, accrued: 0x220,
+ * physical address: 0x24ffffffa0
+ */
+#define PMP_ENTRY_FW 0
+#define PMP_RESERVED 4
+ if (hart_cluster(current_hartid())) {
+ /* On cluster 1: block entire memory port except for DRAM */
+ pmp_set(1, PMP_R | PMP_W | PMP_X,
+ EIC770X_D1_MEM_BASE, log2roundup(EIC770X_D1_MEM_SIZE));
+ /* Use TOR as MEMPORT_BASE is not aligned with SIZE */
+ pmp_set_tor(3, PMP_L,
+ EIC770X_MEMPORT_BASE, EIC770X_MEMPORT_SIZE);
+ } else {
+ /* On cluster 0: block entire memory port except for DRAM */
+ _Static_assert(EIC770X_MEMPORT_BASE == EIC770X_D0_MEM_BASE,
+ "Should not assume D0_MEM is at MEMPORT_BASE");
+ /* Use TORs as MEMPORT/D0_MEM_BASE is not aligned with SIZE */
+ pmp_set_tor(2, PMP_R | PMP_W | PMP_X,
+ EIC770X_MEMPORT_BASE, EIC770X_D0_MEM_SIZE);
+ pmp_set_tor(3, PMP_L,
+ EIC770X_MEMPORT_BASE + EIC770X_D0_MEM_SIZE,
+ EIC770X_MEMPORT_SIZE - EIC770X_D0_MEM_SIZE);
+ }
+ /**
+ * These must come after the setup of PMP, as we are about to
+ * enable speculation and HW prefetcher bits
+ */
+ csr_write(EIC770X_CSR_FEAT0, CONFIG_ESWIN_EIC770X_FEAT0_CFG);
+ csr_write(EIC770X_CSR_FEAT1, CONFIG_ESWIN_EIC770X_FEAT1_CFG);
+ csr_write(EIC770X_CSR_L1_HWPF, CONFIG_ESWIN_EIC770X_L1_HWPF_CFG);
+ csr_write(EIC770X_CSR_L2_HWPF, CONFIG_ESWIN_EIC770X_L2_HWPF_CFG);
+
+ return generic_final_init(cold_boot);
+}
+
+static bool eswin_eic770x_pmp_skip(struct sbi_domain_memregion *reg,
+ void *data)
+{
+ if (SBI_DOMAIN_MEMREGION_IS_FIRMWARE(reg->flags))
+ /* Already handled separately below */
+ return true;
+
+ /**
+ * If we are still in short of PMP entries, we can skip
+ * more, such as UART/PLIC for harts in root domain, as
+ * they are already covered by the defult [0, ~0) memory
+ * region. Right now we are OK (barely), as we used up
+ * all PMP entries.
+ */
+ return false;
+}
+
+static int eswin_eic770x_pmp_configure(unsigned int pmp_count,
+ unsigned int pmp_log2gran,
+ unsigned long pmp_addr_max)
+{
+ struct sbi_domain_memregion temp_memreg;
+ struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
+
+ /* Use a single memory region to cover opensbi FW */
+ sbi_domain_memregion_init(scratch->fw_start, scratch->fw_size,
+ (SBI_DOMAIN_MEMREGION_M_READABLE |
+ SBI_DOMAIN_MEMREGION_M_WRITABLE |
+ SBI_DOMAIN_MEMREGION_M_EXECUTABLE |
+ SBI_DOMAIN_MEMREGION_FW),
+ &temp_memreg);
+ /* This saves 1 PMP entry */
+ pmp_set(PMP_ENTRY_FW, 0, temp_memreg.base, temp_memreg.order);
+
+ return sbi_hart_oldpmp_configure(sbi_scratch_thishart_ptr(),
+ PMP_RESERVED,
+ pmp_count,
+ pmp_log2gran,
+ pmp_addr_max,
+ eswin_eic770x_pmp_skip,
+ NULL);
+}
+
+static int eswin_eic7700_platform_init(const void *fdt, int nodeoff,
+ const struct fdt_match *match)
+{
+ generic_platform_ops.early_init = eswin_eic770x_early_init;
+ generic_platform_ops.final_init = eswin_eic770x_final_init;
+ generic_platform_ops.pmp_configure = eswin_eic770x_pmp_configure;
+
+ return 0;
+}
+
+static const struct fdt_match eswin_eic7700_match[] = {
+ { .compatible = "eswin,eic7700" },
+ { },
+};
+
+const struct fdt_driver eswin_eic7700 = {
+ .match_table = eswin_eic7700_match,
+ .init = eswin_eic7700_platform_init,
+};
diff --git a/platform/generic/eswin/objects.mk b/platform/generic/eswin/objects.mk
new file mode 100644
index 00000000..6942a107
--- /dev/null
+++ b/platform/generic/eswin/objects.mk
@@ -0,0 +1,11 @@
+#
+# SPDX-License-Identifier: BSD-2-Clause
+#
+# Copyright (C) 2025 Bo Gan <ganboing@gmail.com>
+#
+
+carray-platform_override_modules-$(CONFIG_PLATFORM_ESWIN_EIC770X) += eswin_eic7700
+platform-objs-$(CONFIG_PLATFORM_ESWIN_EIC770X) += eswin/eic770x.o
+
+FW_PAYLOAD=y
+FW_PAYLOAD_OFFSET=0x200000
diff --git a/platform/generic/include/eswin/eic770x.h b/platform/generic/include/eswin/eic770x.h
new file mode 100644
index 00000000..cec676da
--- /dev/null
+++ b/platform/generic/include/eswin/eic770x.h
@@ -0,0 +1,49 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2025 Bo Gan <ganboing@gmail.com>
+ *
+ */
+
+#ifndef __EIC770X_H__
+#define __EIC770X_H__
+
+/* CSRs */
+#define EIC770X_CSR_BRPREDICT 0x7c0
+#define EIC770X_CSR_FEAT0 0x7c1
+#define EIC770X_CSR_FEAT1 0x7c2
+#define EIC770X_CSR_L1_HWPF 0x7c3
+#define EIC770X_CSR_L2_HWPF 0x7c4
+
+/* Hart ID to cluster/core conversion */
+#define CPU_CORE_BITS 2
+#define CPU_CORE_MASK ((1 << CPU_CORE_BITS) - 1)
+#define CPU_CLUSTER_SHIFT CPU_CORE_BITS
+#define CPU_CLUSTER_BITS 1
+#define CPU_CLUSTER_MASK ((1 << CPU_CLUSTER_SHIFT) - 1)
+
+#define hart_core(i) ((i) & CPU_CORE_MASK)
+#define hart_cluster(i) (((i) >> CPU_CLUSTER_SHIFT) & CPU_CLUSTER_MASK)
+#define current_hart_core() hart_core(current_hartid())
+#define current_hart_cluster() hart_cluster(current_hartid())
+
+/* P550 Internal, System Ports and MMIO */
+#define EIC770X_P550_INTERNAL (0x0 + 0x20000000UL * current_hart_cluster())
+#define EIC770X_TL64D2D_OUT (EIC770X_P550_INTERNAL + 0x200000)
+#define EIC770X_TL256D2D_OUT (EIC770X_P550_INTERNAL + 0x202000)
+#define EIC770X_TL256D2D_IN (EIC770X_P550_INTERNAL + 0x204000)
+
+#define EIC770X_SYSPORT0_BASE (0x40000000UL + 0x20000000UL * current_hart_cluster())
+#define EIC770X_SYSCRG (EIC770X_SYSPORT0_BASE + 0x11828000UL)
+#define EIC770X_SYSCRG_RST (EIC770X_SYSCRG + 0x300UL)
+#define EIC770X_SYSCRG_RST_VAL 0x1AC0FFE6UL
+
+/* Memory Ports */
+#define EIC770X_MEMPORT_BASE 0x0080000000UL // 2G
+#define EIC770X_MEMPORT_SIZE 0x7f80000000UL // +510G
+#define EIC770X_D0_MEM_BASE 0x0080000000UL // 2G
+#define EIC770X_D0_MEM_SIZE 0x0f80000000UL // +62G
+#define EIC770X_D1_MEM_BASE 0x2000000000UL // 128G
+#define EIC770X_D1_MEM_SIZE 0x1000000000UL // +64G
+
+#endif
--
2.34.1
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] lib: sbi: Add pmp_set_tor for setting TOR regions
2025-11-11 3:41 ` [PATCH 2/3] lib: sbi: Add pmp_set_tor for setting TOR regions Bo Gan
@ 2025-11-11 5:45 ` Xiang W
2025-11-11 9:45 ` Bo Gan
0 siblings, 1 reply; 11+ messages in thread
From: Xiang W @ 2025-11-11 5:45 UTC (permalink / raw)
To: Bo Gan, opensbi; +Cc: linmin, pinkesh.vaghela, gaohan, samuel
在 2025-11-10一的 19:41 -0800，Bo Gan写道：
> TOR can be utilized to cover memory regions that are not aligned with
> their sizes. Given that the address matching is formed bt 2 consecutive
> pmpaddr, i.e., pmpaddr(i-1) and pmpaddr(i), TOR should not be used
> generically to avoid pmpaddr conflict with other NA4/NAPOT regions.
>
> It's advised to only use them in platform code where the caller can
> ensure the index and order of every pmp region especially when there's
> a mixture of TOR/NA4/NAPOT.
>
> Signed-off-by: Bo Gan <ganboing@gmail.com>
Some suggestions:
1. Merge pmp_set_addr/pmp_set_port into
void static void pmp_set_raw(unsigned int n,
unsigned long port, unsigned long pmpaddr)
2. Modify the declaration of pmp_set to
int pmp_set(bool is_tor, unsigned int n, unsigned long port,
unsigned long addr, unsigned long opaque)
Regards,
Xiang W
> ---
> include/sbi/riscv_asm.h | 4 +++
> lib/sbi/riscv_asm.c | 75 +++++++++++++++++++++++++++++++----------
> 2 files changed, 62 insertions(+), 17 deletions(-)
>
> diff --git a/include/sbi/riscv_asm.h b/include/sbi/riscv_asm.h
> index ef48dc89..c23feab6 100644
> --- a/include/sbi/riscv_asm.h
> +++ b/include/sbi/riscv_asm.h
> @@ -218,6 +218,10 @@ int is_pmp_entry_mapped(unsigned long entry);
> int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
> unsigned long log2len);
>
> +/* Top of range (TOR) matching mode. pmpaddr(n-1) will also be changed */
> +int pmp_set_tor(unsigned int n, unsigned long prot, unsigned long addr,
> + unsigned long size);
> +
> int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
> unsigned long *log2len);
>
> diff --git a/lib/sbi/riscv_asm.c b/lib/sbi/riscv_asm.c
> index 3e44320f..8f64f2b5 100644
> --- a/lib/sbi/riscv_asm.c
> +++ b/lib/sbi/riscv_asm.c
> @@ -330,16 +330,10 @@ 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 void pmp_set_prot(unsigned int n, unsigned long prot)
> {
> - int pmpcfg_csr, pmpcfg_shift, pmpaddr_csr;
> + int pmpcfg_csr, pmpcfg_shift;
> unsigned long cfgmask, pmpcfg;
> - unsigned long addrmask, pmpaddr;
> -
> - /* check parameters */
> - if (n >= PMP_COUNT || log2len > __riscv_xlen || log2len < PMP_SHIFT)
> - return SBI_EINVAL;
>
> /* calculate PMP register and offset */
> #if __riscv_xlen == 32
> @@ -351,15 +345,29 @@ int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
> #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;
> cfgmask = ~(0xffUL << pmpcfg_shift);
> pmpcfg = (csr_read_num(pmpcfg_csr) & cfgmask);
> pmpcfg |= ((prot << pmpcfg_shift) & ~cfgmask);
>
> + csr_write_num(pmpcfg_csr, pmpcfg);
> +}
> +
> +static void pmp_set_addr(unsigned int n, unsigned long pmpaddr)
> +{
> + int pmpaddr_csr = CSR_PMPADDR0 + n;
> +
> + csr_write_num(pmpaddr_csr, pmpaddr);
> +}
> +
> +int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
> + unsigned long log2len)
> +{
> + unsigned long addrmask, pmpaddr;
> +
> + /* check parameters */
> + if (n >= PMP_COUNT || log2len > __riscv_xlen || log2len < PMP_SHIFT)
> + return SBI_EINVAL;
> +
> /* encode PMP address */
> if (log2len == PMP_SHIFT) {
> pmpaddr = (addr >> PMP_SHIFT);
> @@ -373,10 +381,41 @@ 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;
> +
> /* write csrs */
> - csr_write_num(pmpaddr_csr, pmpaddr);
> - csr_write_num(pmpcfg_csr, pmpcfg);
> + pmp_set_addr(n, pmpaddr);
> + pmp_set_prot(n, prot);
> + return 0;
> +}
> +
> +int pmp_set_tor(unsigned int n, unsigned long prot, unsigned long addr,
> + unsigned long size)
> +{
> + unsigned long pmpaddr, pmpaddrp;
> +
> + /* check parameters */
> + if (n >= PMP_COUNT)
> + return SBI_EINVAL;
> +
> + if (n == 0 && addr != 0)
> + return SBI_EINVAL;
> +
> + /* encode PMP address */
> + pmpaddrp = addr >> PMP_SHIFT;
> + pmpaddr = (addr + size) >> PMP_SHIFT;
>
> + /* encode PMP config */
> + prot &= ~PMP_A;
> + prot |= PMP_A_TOR;
> +
> + /* write csrs */
> + if (n)
> + pmp_set_addr(n - 1, pmpaddrp);
> + pmp_set_addr(n, pmpaddr);
> + pmp_set_prot(n, prot);
> return 0;
> }
>
> @@ -420,10 +459,12 @@ int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
> addr = (addr & ~((1UL << t1) - 1)) << PMP_SHIFT;
> len = (t1 + PMP_SHIFT + 1);
> }
> - } else {
> + } else if ((prot & PMP_A) == PMP_A_NA4) {
> addr = csr_read_num(pmpaddr_csr) << PMP_SHIFT;
> len = PMP_SHIFT;
> - }
> + } else
> + /* Error out for TOR region */
> + return SBI_EINVAL;
>
> /* return details */
> *prot_out = prot;
> --
> 2.34.1
>
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] lib: sbi: allow platform to override PMP configuration
2025-11-11 3:41 ` [PATCH 1/3] lib: sbi: allow platform to override PMP configuration Bo Gan
@ 2025-11-11 5:45 ` Xiang W
2025-11-11 9:18 ` Bo Gan
0 siblings, 1 reply; 11+ messages in thread
From: Xiang W @ 2025-11-11 5:45 UTC (permalink / raw)
To: Bo Gan, opensbi; +Cc: linmin, pinkesh.vaghela, gaohan, samuel
在 2025-11-10一的 19:41 -0800，Bo Gan写道：
> Platform sometimes wants to override the entire PMP configuration phase,
> not just performing additional work for each individual PMP entry. This
> allows platforms to insert SoC/core specific PMP entries in a way that
> works together with the existing ones set by lib/ code, not conflicting.
> platform can also choose to merge or skip memory regions in a reasonable
> way in case there's a shortage of PMP entries.
>
> In addition, `sbi_hart_oldpmp_configure` is made public and a callback
> function `skip` is added, so platform code can built on top of it to
> simplify PMP configuration override.
>
> Signed-off-by: Bo Gan <ganboing@gmail.com>
> ---
> include/sbi/sbi_hart.h | 9 +++++++++
> include/sbi/sbi_platform.h | 33 +++++++++++++++++++++++++++++++++
> lib/sbi/sbi_hart.c | 27 +++++++++++++++++++--------
> 3 files changed, 61 insertions(+), 8 deletions(-)
>
> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> index e66dd52f..e5a221bd 100644
> --- a/include/sbi/sbi_hart.h
> +++ b/include/sbi/sbi_hart.h
> @@ -133,6 +133,7 @@ struct sbi_hart_features {
> };
>
> struct sbi_scratch;
> +struct sbi_domain_memregion;
>
> int sbi_hart_reinit(struct sbi_scratch *scratch);
> int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot);
> @@ -148,6 +149,14 @@ 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_oldpmp_configure(struct sbi_scratch *scratch,
> + unsigned int pmp_start,
It is recommended to change the declaration from pmp_start to pmp_reserved.
Otherwise, it might give the misleading impression that the last pmp is
pmp_start + pmp_count - 1.
Regards,
Xiang W
> + unsigned int pmp_count,
> + unsigned int pmp_log2gran,
> + unsigned long pmp_addr_max,
> + bool (*skip)(struct sbi_domain_memregion *reg,
> + void *data),
> + void *data);
> int sbi_hart_map_saddr(unsigned long base, unsigned long size);
> int sbi_hart_unmap_saddr(void);
> int sbi_hart_priv_version(struct sbi_scratch *scratch);
> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> index d75c12de..c6fc137f 100644
> --- a/include/sbi/sbi_platform.h
> +++ b/include/sbi/sbi_platform.h
> @@ -146,6 +146,11 @@ struct sbi_platform_operations {
> unsigned long log2len);
> /** platform specific pmp disable on current HART */
> void (*pmp_disable)(unsigned int n);
> +
> + /** platform pmp configure override on current HART */
> + int (*pmp_configure)(unsigned int pmp_count,
> + unsigned int pmp_log2gran,
> + unsigned long pmp_addr_max);
> };
>
> /** Platform default per-HART stack size for exception/interrupt handling */
> @@ -666,6 +671,34 @@ static inline void sbi_platform_pmp_disable(const struct sbi_platform *plat,
> sbi_platform_ops(plat)->pmp_disable(n);
> }
>
> +/**
> + * Check if platform wants to override PMP configuration
> + *
> + * @param plat pointer to struct sbi_platform
> + */
> +static inline bool sbi_platform_pmp_override(const struct sbi_platform *plat)
> +{
> + return plat && sbi_platform_ops(plat)->pmp_configure;
> +}
> +
> +/**
> + * Platform PMP configuration override
> + *
> + * @param plat pointer to struct sbi_platform
> + * @param pmp_count number of PMP entries
> + * @param pmp_log2gran PMP granularity
> + * @param pmp_addr_max largest value pmpaddr(x) can hold
> + */
> +static inline int sbi_platform_pmp_configure(const struct sbi_platform *plat,
> + unsigned int pmp_count,
> + unsigned int pmp_log2gran,
> + unsigned long pmp_addr_max)
> +{
> + return sbi_platform_ops(plat)->pmp_configure(pmp_count,
> + pmp_log2gran,
> + pmp_addr_max);
> +}
> +
> #endif
>
> #endif
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index a91703b4..4425f36b 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -433,18 +433,24 @@ static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch,
> return 0;
> }
>
> -static int sbi_hart_oldpmp_configure(struct sbi_scratch *scratch,
> - unsigned int pmp_count,
> - unsigned int pmp_log2gran,
> - unsigned long pmp_addr_max)
> +int sbi_hart_oldpmp_configure(struct sbi_scratch *scratch,
> + unsigned int pmp_start,
> + unsigned int pmp_count,
> + unsigned int pmp_log2gran,
> + unsigned long pmp_addr_max,
> + bool (*skip)(struct sbi_domain_memregion *reg,
> + void *data),
> + void *data)
> {
> struct sbi_domain_memregion *reg;
> struct sbi_domain *dom = sbi_domain_thishart_ptr();
> - unsigned int pmp_idx = 0;
> + unsigned int pmp_idx = pmp_start;
> unsigned int pmp_flags;
> unsigned long pmp_addr;
>
> sbi_domain_for_each_memregion(dom, reg) {
> + if (skip && skip(reg, data))
> + continue;
> if (!is_valid_pmp_idx(pmp_count, pmp_idx))
> return SBI_EFAIL;
>
> @@ -534,6 +540,7 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch)
> unsigned int pmp_bits, pmp_log2gran;
> unsigned int pmp_count = sbi_hart_pmp_count(scratch);
> unsigned long pmp_addr_max;
> + const struct sbi_platform *plat = sbi_platform_ptr(scratch);
>
> if (!pmp_count)
> return 0;
> @@ -542,12 +549,16 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch)
> pmp_bits = sbi_hart_pmp_addrbits(scratch) - 1;
> pmp_addr_max = (1UL << pmp_bits) | ((1UL << pmp_bits) - 1);
>
> - if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMEPMP))
> + if (sbi_platform_pmp_override(plat))
> + rc = sbi_platform_pmp_configure(plat, pmp_count,
> + pmp_log2gran, pmp_addr_max);
> + else if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMEPMP))
> rc = sbi_hart_smepmp_configure(scratch, pmp_count,
> pmp_log2gran, pmp_addr_max);
> else
> - rc = sbi_hart_oldpmp_configure(scratch, pmp_count,
> - pmp_log2gran, pmp_addr_max);
> + rc = sbi_hart_oldpmp_configure(scratch, 0, pmp_count,
> + pmp_log2gran, pmp_addr_max,
> + NULL, NULL);
>
> /*
> * As per section 3.7.2 of privileged specification v1.12,
> --
> 2.34.1
>
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] lib: sbi: allow platform to override PMP configuration
2025-11-11 5:45 ` Xiang W
@ 2025-11-11 9:18 ` Bo Gan
0 siblings, 0 replies; 11+ messages in thread
From: Bo Gan @ 2025-11-11 9:18 UTC (permalink / raw)
To: Xiang W, Bo Gan, opensbi; +Cc: linmin, pinkesh.vaghela, gaohan, samuel
Hi Xiang,
On 11/10/25 21:45, Xiang W wrote:
> 在 2025-11-10一的 19:41 -0800，Bo Gan写道：
>> Platform sometimes wants to override the entire PMP configuration phase,
>> not just performing additional work for each individual PMP entry. This
>> allows platforms to insert SoC/core specific PMP entries in a way that
>> works together with the existing ones set by lib/ code, not conflicting.
>> platform can also choose to merge or skip memory regions in a reasonable
>> way in case there's a shortage of PMP entries.
>>
>> In addition, `sbi_hart_oldpmp_configure` is made public and a callback
>> function `skip` is added, so platform code can built on top of it to
>> simplify PMP configuration override.
>>
>> Signed-off-by: Bo Gan <ganboing@gmail.com>
>> ---
>> include/sbi/sbi_hart.h | 9 +++++++++
>> include/sbi/sbi_platform.h | 33 +++++++++++++++++++++++++++++++++
>> lib/sbi/sbi_hart.c | 27 +++++++++++++++++++--------
>> 3 files changed, 61 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
>> index e66dd52f..e5a221bd 100644
>> --- a/include/sbi/sbi_hart.h
>> +++ b/include/sbi/sbi_hart.h
>> @@ -133,6 +133,7 @@ struct sbi_hart_features {
>> };
>>
>> struct sbi_scratch;
>> +struct sbi_domain_memregion;
>>
>> int sbi_hart_reinit(struct sbi_scratch *scratch);
>> int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot);
>> @@ -148,6 +149,14 @@ 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_oldpmp_configure(struct sbi_scratch *scratch,
>> + unsigned int pmp_start,
> It is recommended to change the declaration from pmp_start to pmp_reserved.
> Otherwise, it might give the misleading impression that the last pmp is
> pmp_start + pmp_count - 1.
>
> Regards,
> Xiang W
Thanks for the review. Yes, this pmp_start/pmp_count is indeed misleading.
I'll address it in the next version.
Bo
>> + unsigned int pmp_count,
>> + unsigned int pmp_log2gran,
>> + unsigned long pmp_addr_max,
>> + bool (*skip)(struct sbi_domain_memregion *reg,
>> + void *data),
>> + void *data);
>> int sbi_hart_map_saddr(unsigned long base, unsigned long size);
>> int sbi_hart_unmap_saddr(void);
>> int sbi_hart_priv_version(struct sbi_scratch *scratch);
>> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
>> index d75c12de..c6fc137f 100644
>> --- a/include/sbi/sbi_platform.h
>> +++ b/include/sbi/sbi_platform.h
>> @@ -146,6 +146,11 @@ struct sbi_platform_operations {
>> unsigned long log2len);
>> /** platform specific pmp disable on current HART */
>> void (*pmp_disable)(unsigned int n);
>> +
>> + /** platform pmp configure override on current HART */
>> + int (*pmp_configure)(unsigned int pmp_count,
>> + unsigned int pmp_log2gran,
>> + unsigned long pmp_addr_max);
>> };
>>
>> /** Platform default per-HART stack size for exception/interrupt handling */
>> @@ -666,6 +671,34 @@ static inline void sbi_platform_pmp_disable(const struct sbi_platform *plat,
>> sbi_platform_ops(plat)->pmp_disable(n);
>> }
>>
>> +/**
>> + * Check if platform wants to override PMP configuration
>> + *
>> + * @param plat pointer to struct sbi_platform
>> + */
>> +static inline bool sbi_platform_pmp_override(const struct sbi_platform *plat)
>> +{
>> + return plat && sbi_platform_ops(plat)->pmp_configure;
>> +}
>> +
>> +/**
>> + * Platform PMP configuration override
>> + *
>> + * @param plat pointer to struct sbi_platform
>> + * @param pmp_count number of PMP entries
>> + * @param pmp_log2gran PMP granularity
>> + * @param pmp_addr_max largest value pmpaddr(x) can hold
>> + */
>> +static inline int sbi_platform_pmp_configure(const struct sbi_platform *plat,
>> + unsigned int pmp_count,
>> + unsigned int pmp_log2gran,
>> + unsigned long pmp_addr_max)
>> +{
>> + return sbi_platform_ops(plat)->pmp_configure(pmp_count,
>> + pmp_log2gran,
>> + pmp_addr_max);
>> +}
>> +
>> #endif
>>
>> #endif
>> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
>> index a91703b4..4425f36b 100644
>> --- a/lib/sbi/sbi_hart.c
>> +++ b/lib/sbi/sbi_hart.c
>> @@ -433,18 +433,24 @@ static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch,
>> return 0;
>> }
>>
>> -static int sbi_hart_oldpmp_configure(struct sbi_scratch *scratch,
>> - unsigned int pmp_count,
>> - unsigned int pmp_log2gran,
>> - unsigned long pmp_addr_max)
>> +int sbi_hart_oldpmp_configure(struct sbi_scratch *scratch,
>> + unsigned int pmp_start,
>> + unsigned int pmp_count,
>> + unsigned int pmp_log2gran,
>> + unsigned long pmp_addr_max,
>> + bool (*skip)(struct sbi_domain_memregion *reg,
>> + void *data),
>> + void *data)
>> {
>> struct sbi_domain_memregion *reg;
>> struct sbi_domain *dom = sbi_domain_thishart_ptr();
>> - unsigned int pmp_idx = 0;
>> + unsigned int pmp_idx = pmp_start;
>> unsigned int pmp_flags;
>> unsigned long pmp_addr;
>>
>> sbi_domain_for_each_memregion(dom, reg) {
>> + if (skip && skip(reg, data))
>> + continue;
>> if (!is_valid_pmp_idx(pmp_count, pmp_idx))
>> return SBI_EFAIL;
>>
>> @@ -534,6 +540,7 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch)
>> unsigned int pmp_bits, pmp_log2gran;
>> unsigned int pmp_count = sbi_hart_pmp_count(scratch);
>> unsigned long pmp_addr_max;
>> + const struct sbi_platform *plat = sbi_platform_ptr(scratch);
>>
>> if (!pmp_count)
>> return 0;
>> @@ -542,12 +549,16 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch)
>> pmp_bits = sbi_hart_pmp_addrbits(scratch) - 1;
>> pmp_addr_max = (1UL << pmp_bits) | ((1UL << pmp_bits) - 1);
>>
>> - if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMEPMP))
>> + if (sbi_platform_pmp_override(plat))
>> + rc = sbi_platform_pmp_configure(plat, pmp_count,
>> + pmp_log2gran, pmp_addr_max);
>> + else if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMEPMP))
>> rc = sbi_hart_smepmp_configure(scratch, pmp_count,
>> pmp_log2gran, pmp_addr_max);
>> else
>> - rc = sbi_hart_oldpmp_configure(scratch, pmp_count,
>> - pmp_log2gran, pmp_addr_max);
>> + rc = sbi_hart_oldpmp_configure(scratch, 0, pmp_count,
>> + pmp_log2gran, pmp_addr_max,
>> + NULL, NULL);
>>
>> /*
>> * As per section 3.7.2 of privileged specification v1.12,
>> --
>> 2.34.1
>>
>
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] lib: sbi: Add pmp_set_tor for setting TOR regions
2025-11-11 5:45 ` Xiang W
@ 2025-11-11 9:45 ` Bo Gan
2025-11-11 10:35 ` Xiang W
0 siblings, 1 reply; 11+ messages in thread
From: Bo Gan @ 2025-11-11 9:45 UTC (permalink / raw)
To: Xiang W, Bo Gan, opensbi; +Cc: linmin, pinkesh.vaghela, gaohan, samuel
Hi Xiang,
On 11/10/25 21:45, Xiang W wrote:
> 在 2025-11-10一的 19:41 -0800，Bo Gan写道：
>> TOR can be utilized to cover memory regions that are not aligned with
>> their sizes. Given that the address matching is formed bt 2 consecutive
>> pmpaddr, i.e., pmpaddr(i-1) and pmpaddr(i), TOR should not be used
>> generically to avoid pmpaddr conflict with other NA4/NAPOT regions.
>>
>> It's advised to only use them in platform code where the caller can
>> ensure the index and order of every pmp region especially when there's
>> a mixture of TOR/NA4/NAPOT.
>>
>> Signed-off-by: Bo Gan <ganboing@gmail.com>
>
> Some suggestions:
> 1. Merge pmp_set_addr/pmp_set_port into
> void static void pmp_set_raw(unsigned int n,
> unsigned long port, unsigned long pmpaddr)
>
> 2. Modify the declaration of pmp_set to
> int pmp_set(bool is_tor, unsigned int n, unsigned long port,
> unsigned long addr, unsigned long opaque)
>
> Regards,
> Xiang W
I assume you meant port -> prot. For 1, I don't quite like it because it
kind of assumes you always change a pmpaddr[i]/pmpcfg[i] pair. In the TOR
case, often times you change pmpaddr[i-1] and pmpaddr[i], then pmpcfg[i].
An edge case is when addr == 0 and n == 0, for it we only update pmpaddr0,
then pmpcfg0. Perhaps I'm mistaken, and you are suggesting `pmp_set_raw`
can handle different cases based on whether prot is TOR? Then I'd say it
probably makes the code more complicated than necessary, as the check can
already be made on the caller side, or no TOR check is necessary with my
different function approach.
For 2, my rationale on having a different function is that I want to keep
all existing pmp_set the same way, and try to discourage the use of TOR.
Maybe I should have added a warning in the comment in header file. The
reason is because TOR makes the PMP entries dependent on preceding ones,
so care must be taken when programming them. The caller must have global
knowledge of potentially all PMP entries to be programmed in order to use
it properly. I don't feel comfortable unifying to the same interface for
TOR and NA4/NAPOT.
Let me know what you think. I'm happy to listen. Thanks.
Bo
>> ---
>> include/sbi/riscv_asm.h | 4 +++
>> lib/sbi/riscv_asm.c | 75 +++++++++++++++++++++++++++++++----------
>> 2 files changed, 62 insertions(+), 17 deletions(-)
>>
>> diff --git a/include/sbi/riscv_asm.h b/include/sbi/riscv_asm.h
>> index ef48dc89..c23feab6 100644
>> --- a/include/sbi/riscv_asm.h
>> +++ b/include/sbi/riscv_asm.h
>> @@ -218,6 +218,10 @@ int is_pmp_entry_mapped(unsigned long entry);
>> int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
>> unsigned long log2len);
>>
>> +/* Top of range (TOR) matching mode. pmpaddr(n-1) will also be changed */
>> +int pmp_set_tor(unsigned int n, unsigned long prot, unsigned long addr,
>> + unsigned long size);
>> +
>> int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
>> unsigned long *log2len);
>>
>> diff --git a/lib/sbi/riscv_asm.c b/lib/sbi/riscv_asm.c
>> index 3e44320f..8f64f2b5 100644
>> --- a/lib/sbi/riscv_asm.c
>> +++ b/lib/sbi/riscv_asm.c
>> @@ -330,16 +330,10 @@ 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 void pmp_set_prot(unsigned int n, unsigned long prot)
>> {
>> - int pmpcfg_csr, pmpcfg_shift, pmpaddr_csr;
>> + int pmpcfg_csr, pmpcfg_shift;
>> unsigned long cfgmask, pmpcfg;
>> - unsigned long addrmask, pmpaddr;
>> -
>> - /* check parameters */
>> - if (n >= PMP_COUNT || log2len > __riscv_xlen || log2len < PMP_SHIFT)
>> - return SBI_EINVAL;
>>
>> /* calculate PMP register and offset */
>> #if __riscv_xlen == 32
>> @@ -351,15 +345,29 @@ int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
>> #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;
>> cfgmask = ~(0xffUL << pmpcfg_shift);
>> pmpcfg = (csr_read_num(pmpcfg_csr) & cfgmask);
>> pmpcfg |= ((prot << pmpcfg_shift) & ~cfgmask);
>>
>> + csr_write_num(pmpcfg_csr, pmpcfg);
>> +}
>> +
>> +static void pmp_set_addr(unsigned int n, unsigned long pmpaddr)
>> +{
>> + int pmpaddr_csr = CSR_PMPADDR0 + n;
>> +
>> + csr_write_num(pmpaddr_csr, pmpaddr);
>> +}
>> +
>> +int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
>> + unsigned long log2len)
>> +{
>> + unsigned long addrmask, pmpaddr;
>> +
>> + /* check parameters */
>> + if (n >= PMP_COUNT || log2len > __riscv_xlen || log2len < PMP_SHIFT)
>> + return SBI_EINVAL;
>> +
>> /* encode PMP address */
>> if (log2len == PMP_SHIFT) {
>> pmpaddr = (addr >> PMP_SHIFT);
>> @@ -373,10 +381,41 @@ 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;
>> +
>> /* write csrs */
>> - csr_write_num(pmpaddr_csr, pmpaddr);
>> - csr_write_num(pmpcfg_csr, pmpcfg);
>> + pmp_set_addr(n, pmpaddr);
>> + pmp_set_prot(n, prot);
>> + return 0;
>> +}
>> +
>> +int pmp_set_tor(unsigned int n, unsigned long prot, unsigned long addr,
>> + unsigned long size)
>> +{
>> + unsigned long pmpaddr, pmpaddrp;
>> +
>> + /* check parameters */
>> + if (n >= PMP_COUNT)
>> + return SBI_EINVAL;
>> +
>> + if (n == 0 && addr != 0)
>> + return SBI_EINVAL;
>> +
>> + /* encode PMP address */
>> + pmpaddrp = addr >> PMP_SHIFT;
>> + pmpaddr = (addr + size) >> PMP_SHIFT;
>>
>> + /* encode PMP config */
>> + prot &= ~PMP_A;
>> + prot |= PMP_A_TOR;
>> +
>> + /* write csrs */
>> + if (n)
>> + pmp_set_addr(n - 1, pmpaddrp);
>> + pmp_set_addr(n, pmpaddr);
>> + pmp_set_prot(n, prot);
>> return 0;
>> }
>>
>> @@ -420,10 +459,12 @@ int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
>> addr = (addr & ~((1UL << t1) - 1)) << PMP_SHIFT;
>> len = (t1 + PMP_SHIFT + 1);
>> }
>> - } else {
>> + } else if ((prot & PMP_A) == PMP_A_NA4) {
>> addr = csr_read_num(pmpaddr_csr) << PMP_SHIFT;
>> len = PMP_SHIFT;
>> - }
>> + } else
>> + /* Error out for TOR region */
>> + return SBI_EINVAL;
>>
>> /* return details */
>> *prot_out = prot;
>> --
>> 2.34.1
>>
>
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] lib: sbi: Add pmp_set_tor for setting TOR regions
2025-11-11 9:45 ` Bo Gan
@ 2025-11-11 10:35 ` Xiang W
2025-11-12 10:50 ` Bo Gan
0 siblings, 1 reply; 11+ messages in thread
From: Xiang W @ 2025-11-11 10:35 UTC (permalink / raw)
To: Bo Gan, opensbi; +Cc: linmin, pinkesh.vaghela, gaohan, samuel
在 2025-11-11二的 01:45 -0800，Bo Gan写道：
> Hi Xiang,
>
> On 11/10/25 21:45, Xiang W wrote:
> > 在 2025-11-10一的 19:41 -0800，Bo Gan写道：
> > > TOR can be utilized to cover memory regions that are not aligned with
> > > their sizes. Given that the address matching is formed bt 2 consecutive
> > > pmpaddr, i.e., pmpaddr(i-1) and pmpaddr(i), TOR should not be used
> > > generically to avoid pmpaddr conflict with other NA4/NAPOT regions.
> > >
> > > It's advised to only use them in platform code where the caller can
> > > ensure the index and order of every pmp region especially when there's
> > > a mixture of TOR/NA4/NAPOT.
> > >
> > > Signed-off-by: Bo Gan <ganboing@gmail.com>
> >
> > Some suggestions:
> > 1. Merge pmp_set_addr/pmp_set_port into
> > void static void pmp_set_raw(unsigned int n,
> > unsigned long port, unsigned long pmpaddr)
> >
> > 2. Modify the declaration of pmp_set to
> > int pmp_set(bool is_tor, unsigned int n, unsigned long port,
> > unsigned long addr, unsigned long opaque)
> >
> > Regards,
> > Xiang W
>
> I assume you meant port -> prot. For 1, I don't quite like it because it
> kind of assumes you always change a pmpaddr[i]/pmpcfg[i] pair. In the TOR
> case, often times you change pmpaddr[i-1] and pmpaddr[i], then pmpcfg[i].
> An edge case is when addr == 0 and n == 0, for it we only update pmpaddr0,
> then pmpcfg0. Perhaps I'm mistaken, and you are suggesting `pmp_set_raw`
> can handle different cases based on whether prot is TOR? Then I'd say it
> probably makes the code more complicated than necessary, as the check can
> already be made on the caller side, or no TOR check is necessary with my
> different function approach.
The function pmp_set_raw is used to set pmpaddr[i]/pmpcfg[i].
Regarding the edge case you mentioned, calling pmp_set_raw once is sufficient.
For regular Tor types, call it twice:
pmp_set_raw(n-1, 0, addr >> PMP_SHIFT)
pmp_set_raw(n,prot, (addr + size) >> PMP_SHIFT)
>
> For 2, my rationale on having a different function is that I want to keep
> all existing pmp_set the same way, and try to discourage the use of TOR.
> Maybe I should have added a warning in the comment in header file. The
> reason is because TOR makes the PMP entries dependent on preceding ones,
> so care must be taken when programming them. The caller must have global
> knowledge of potentially all PMP entries to be programmed in order to use
> it properly. I don't feel comfortable unifying to the same interface for
> TOR and NA4/NAPOT.
The names pmp_set and pmp_set_tor give the impression that pmp_set is a
general-purpose method, while pmp_set_tor is for handling special cases.
In reality, pmp_set is not a general-purpose method.
Regards,
Xiang W
>
> Let me know what you think. I'm happy to listen. Thanks.
>
> Bo
>
> > > ---
> > > include/sbi/riscv_asm.h | 4 +++
> > > lib/sbi/riscv_asm.c | 75 +++++++++++++++++++++++++++++++----------
> > > 2 files changed, 62 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/include/sbi/riscv_asm.h b/include/sbi/riscv_asm.h
> > > index ef48dc89..c23feab6 100644
> > > --- a/include/sbi/riscv_asm.h
> > > +++ b/include/sbi/riscv_asm.h
> > > @@ -218,6 +218,10 @@ int is_pmp_entry_mapped(unsigned long entry);
> > > int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
> > > unsigned long log2len);
> > >
> > > +/* Top of range (TOR) matching mode. pmpaddr(n-1) will also be changed */
> > > +int pmp_set_tor(unsigned int n, unsigned long prot, unsigned long addr,
> > > + unsigned long size);
> > > +
> > > int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
> > > unsigned long *log2len);
> > >
> > > diff --git a/lib/sbi/riscv_asm.c b/lib/sbi/riscv_asm.c
> > > index 3e44320f..8f64f2b5 100644
> > > --- a/lib/sbi/riscv_asm.c
> > > +++ b/lib/sbi/riscv_asm.c
> > > @@ -330,16 +330,10 @@ 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 void pmp_set_prot(unsigned int n, unsigned long prot)
> > > {
> > > - int pmpcfg_csr, pmpcfg_shift, pmpaddr_csr;
> > > + int pmpcfg_csr, pmpcfg_shift;
> > > unsigned long cfgmask, pmpcfg;
> > > - unsigned long addrmask, pmpaddr;
> > > -
> > > - /* check parameters */
> > > - if (n >= PMP_COUNT || log2len > __riscv_xlen || log2len < PMP_SHIFT)
> > > - return SBI_EINVAL;
> > >
> > > /* calculate PMP register and offset */
> > > #if __riscv_xlen == 32
> > > @@ -351,15 +345,29 @@ int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
> > > #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;
> > > cfgmask = ~(0xffUL << pmpcfg_shift);
> > > pmpcfg = (csr_read_num(pmpcfg_csr) & cfgmask);
> > > pmpcfg |= ((prot << pmpcfg_shift) & ~cfgmask);
> > >
> > > + csr_write_num(pmpcfg_csr, pmpcfg);
> > > +}
> > > +
> > > +static void pmp_set_addr(unsigned int n, unsigned long pmpaddr)
> > > +{
> > > + int pmpaddr_csr = CSR_PMPADDR0 + n;
> > > +
> > > + csr_write_num(pmpaddr_csr, pmpaddr);
> > > +}
> > > +
> > > +int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
> > > + unsigned long log2len)
> > > +{
> > > + unsigned long addrmask, pmpaddr;
> > > +
> > > + /* check parameters */
> > > + if (n >= PMP_COUNT || log2len > __riscv_xlen || log2len < PMP_SHIFT)
> > > + return SBI_EINVAL;
> > > +
> > > /* encode PMP address */
> > > if (log2len == PMP_SHIFT) {
> > > pmpaddr = (addr >> PMP_SHIFT);
> > > @@ -373,10 +381,41 @@ 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;
> > > +
> > > /* write csrs */
> > > - csr_write_num(pmpaddr_csr, pmpaddr);
> > > - csr_write_num(pmpcfg_csr, pmpcfg);
> > > + pmp_set_addr(n, pmpaddr);
> > > + pmp_set_prot(n, prot);
> > > + return 0;
> > > +}
> > > +
> > > +int pmp_set_tor(unsigned int n, unsigned long prot, unsigned long addr,
> > > + unsigned long size)
> > > +{
> > > + unsigned long pmpaddr, pmpaddrp;
> > > +
> > > + /* check parameters */
> > > + if (n >= PMP_COUNT)
> > > + return SBI_EINVAL;
> > > +
> > > + if (n == 0 && addr != 0)
> > > + return SBI_EINVAL;
> > > +
> > > + /* encode PMP address */
> > > + pmpaddrp = addr >> PMP_SHIFT;
> > > + pmpaddr = (addr + size) >> PMP_SHIFT;
> > >
> > > + /* encode PMP config */
> > > + prot &= ~PMP_A;
> > > + prot |= PMP_A_TOR;
> > > +
> > > + /* write csrs */
> > > + if (n)
> > > + pmp_set_addr(n - 1, pmpaddrp);
> > > + pmp_set_addr(n, pmpaddr);
> > > + pmp_set_prot(n, prot);
> > > return 0;
> > > }
> > >
> > > @@ -420,10 +459,12 @@ int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
> > > addr = (addr & ~((1UL << t1) - 1)) << PMP_SHIFT;
> > > len = (t1 + PMP_SHIFT + 1);
> > > }
> > > - } else {
> > > + } else if ((prot & PMP_A) == PMP_A_NA4) {
> > > addr = csr_read_num(pmpaddr_csr) << PMP_SHIFT;
> > > len = PMP_SHIFT;
> > > - }
> > > + } else
> > > + /* Error out for TOR region */
> > > + return SBI_EINVAL;
> > >
> > > /* return details */
> > > *prot_out = prot;
> > > --
> > > 2.34.1
> > >
> >
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] lib: sbi: Add pmp_set_tor for setting TOR regions
2025-11-11 10:35 ` Xiang W
@ 2025-11-12 10:50 ` Bo Gan
2025-11-12 11:29 ` Xiang W
0 siblings, 1 reply; 11+ messages in thread
From: Bo Gan @ 2025-11-12 10:50 UTC (permalink / raw)
To: Xiang W, Bo Gan, opensbi; +Cc: linmin, pinkesh.vaghela, gaohan, samuel
On 11/11/25 02:35, Xiang W wrote:
> 在 2025-11-11二的 01:45 -0800，Bo Gan写道：
>> Hi Xiang,
>>
>> On 11/10/25 21:45, Xiang W wrote:
>>> 在 2025-11-10一的 19:41 -0800，Bo Gan写道：
>>>> TOR can be utilized to cover memory regions that are not aligned with
>>>> their sizes. Given that the address matching is formed bt 2 consecutive
>>>> pmpaddr, i.e., pmpaddr(i-1) and pmpaddr(i), TOR should not be used
>>>> generically to avoid pmpaddr conflict with other NA4/NAPOT regions.
>>>>
>>>> It's advised to only use them in platform code where the caller can
>>>> ensure the index and order of every pmp region especially when there's
>>>> a mixture of TOR/NA4/NAPOT.
>>>>
>>>> Signed-off-by: Bo Gan <ganboing@gmail.com>
>>>
>>> Some suggestions:
>>> 1. Merge pmp_set_addr/pmp_set_port into
>>> void static void pmp_set_raw(unsigned int n,
>>> unsigned long port, unsigned long pmpaddr)
>>>
>>> 2. Modify the declaration of pmp_set to
>>> int pmp_set(bool is_tor, unsigned int n, unsigned long port,
>>> unsigned long addr, unsigned long opaque)
>>>
>>> Regards,
>>> Xiang W
>>
>> I assume you meant port -> prot. For 1, I don't quite like it because it
>> kind of assumes you always change a pmpaddr[i]/pmpcfg[i] pair. In the TOR
>> case, often times you change pmpaddr[i-1] and pmpaddr[i], then pmpcfg[i].
>> An edge case is when addr == 0 and n == 0, for it we only update pmpaddr0,
>> then pmpcfg0. Perhaps I'm mistaken, and you are suggesting `pmp_set_raw`
>> can handle different cases based on whether prot is TOR? Then I'd say it
>> probably makes the code more complicated than necessary, as the check can
>> already be made on the caller side, or no TOR check is necessary with my
>> different function approach.
> The function pmp_set_raw is used to set pmpaddr[i]/pmpcfg[i].
>
> Regarding the edge case you mentioned, calling pmp_set_raw once is sufficient.
> For regular Tor types, call it twice:
>
> pmp_set_raw(n-1, 0, addr >> PMP_SHIFT)
> pmp_set_raw(n,prot, (addr + size) >> PMP_SHIFT)
>
We shouldn't do pmp_set_raw(n-1, 0, addr >> PMP_SHIFT), because it'll zero-
out the corresponding pmpcfg of n-1.
This is actually the tricky part of TOR. You can have adjacent TOR regions
that share the same pmpaddr, i.e., the same pmpaddr forms the upper-bound
of previous region and lower-bound of current region. If we disallow this
sharing, then potentially we'll end up with 2x the number of TOR regions.
Thus, if lib/ code were to use TOR, either it would have to coordinate
neighboring PMP entries, or a single PMP entry should own 2 pmpcfg/pmpaddr
register pairs.
>>
>> For 2, my rationale on having a different function is that I want to keep
>> all existing pmp_set the same way, and try to discourage the use of TOR.
>> Maybe I should have added a warning in the comment in header file. The
>> reason is because TOR makes the PMP entries dependent on preceding ones,
>> so care must be taken when programming them. The caller must have global
>> knowledge of potentially all PMP entries to be programmed in order to use
>> it properly. I don't feel comfortable unifying to the same interface for
>> TOR and NA4/NAPOT.
> The names pmp_set and pmp_set_tor give the impression that pmp_set is a
> general-purpose method, while pmp_set_tor is for handling special cases.
> In reality, pmp_set is not a general-purpose method.
>
Given those scenarios I mentioned earlier, I'd like to avoid making it
overly complicated. I.e., lib/ code never set or get TOR regions, and can
still safely assume the 1:1 correspondence of pmp index to pmpcfg/pmpaddr
pair. Everything related to TOR is only used by platform pmp_configure
override. Thus, the different function name, so the caller of set_tor can
be easily identified.
Bo
> Regards,
> Xiang W
>>
>> Let me know what you think. I'm happy to listen. Thanks.
>>
>> Bo
>>
>>>> ---
>>>> include/sbi/riscv_asm.h | 4 +++
>>>> lib/sbi/riscv_asm.c | 75 +++++++++++++++++++++++++++++++----------
>>>> 2 files changed, 62 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/include/sbi/riscv_asm.h b/include/sbi/riscv_asm.h
>>>> index ef48dc89..c23feab6 100644
>>>> --- a/include/sbi/riscv_asm.h
>>>> +++ b/include/sbi/riscv_asm.h
>>>> @@ -218,6 +218,10 @@ int is_pmp_entry_mapped(unsigned long entry);
>>>> int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
>>>> unsigned long log2len);
>>>>
>>>> +/* Top of range (TOR) matching mode. pmpaddr(n-1) will also be changed */
>>>> +int pmp_set_tor(unsigned int n, unsigned long prot, unsigned long addr,
>>>> + unsigned long size);
>>>> +
>>>> int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
>>>> unsigned long *log2len);
>>>>
>>>> diff --git a/lib/sbi/riscv_asm.c b/lib/sbi/riscv_asm.c
>>>> index 3e44320f..8f64f2b5 100644
>>>> --- a/lib/sbi/riscv_asm.c
>>>> +++ b/lib/sbi/riscv_asm.c
>>>> @@ -330,16 +330,10 @@ 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 void pmp_set_prot(unsigned int n, unsigned long prot)
>>>> {
>>>> - int pmpcfg_csr, pmpcfg_shift, pmpaddr_csr;
>>>> + int pmpcfg_csr, pmpcfg_shift;
>>>> unsigned long cfgmask, pmpcfg;
>>>> - unsigned long addrmask, pmpaddr;
>>>> -
>>>> - /* check parameters */
>>>> - if (n >= PMP_COUNT || log2len > __riscv_xlen || log2len < PMP_SHIFT)
>>>> - return SBI_EINVAL;
>>>>
>>>> /* calculate PMP register and offset */
>>>> #if __riscv_xlen == 32
>>>> @@ -351,15 +345,29 @@ int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
>>>> #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;
>>>> cfgmask = ~(0xffUL << pmpcfg_shift);
>>>> pmpcfg = (csr_read_num(pmpcfg_csr) & cfgmask);
>>>> pmpcfg |= ((prot << pmpcfg_shift) & ~cfgmask);
>>>>
>>>> + csr_write_num(pmpcfg_csr, pmpcfg);
>>>> +}
>>>> +
>>>> +static void pmp_set_addr(unsigned int n, unsigned long pmpaddr)
>>>> +{
>>>> + int pmpaddr_csr = CSR_PMPADDR0 + n;
>>>> +
>>>> + csr_write_num(pmpaddr_csr, pmpaddr);
>>>> +}
>>>> +
>>>> +int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
>>>> + unsigned long log2len)
>>>> +{
>>>> + unsigned long addrmask, pmpaddr;
>>>> +
>>>> + /* check parameters */
>>>> + if (n >= PMP_COUNT || log2len > __riscv_xlen || log2len < PMP_SHIFT)
>>>> + return SBI_EINVAL;
>>>> +
>>>> /* encode PMP address */
>>>> if (log2len == PMP_SHIFT) {
>>>> pmpaddr = (addr >> PMP_SHIFT);
>>>> @@ -373,10 +381,41 @@ 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;
>>>> +
>>>> /* write csrs */
>>>> - csr_write_num(pmpaddr_csr, pmpaddr);
>>>> - csr_write_num(pmpcfg_csr, pmpcfg);
>>>> + pmp_set_addr(n, pmpaddr);
>>>> + pmp_set_prot(n, prot);
>>>> + return 0;
>>>> +}
>>>> +
>>>> +int pmp_set_tor(unsigned int n, unsigned long prot, unsigned long addr,
>>>> + unsigned long size)
>>>> +{
>>>> + unsigned long pmpaddr, pmpaddrp;
>>>> +
>>>> + /* check parameters */
>>>> + if (n >= PMP_COUNT)
>>>> + return SBI_EINVAL;
>>>> +
>>>> + if (n == 0 && addr != 0)
>>>> + return SBI_EINVAL;
>>>> +
>>>> + /* encode PMP address */
>>>> + pmpaddrp = addr >> PMP_SHIFT;
>>>> + pmpaddr = (addr + size) >> PMP_SHIFT;
>>>>
>>>> + /* encode PMP config */
>>>> + prot &= ~PMP_A;
>>>> + prot |= PMP_A_TOR;
>>>> +
>>>> + /* write csrs */
>>>> + if (n)
>>>> + pmp_set_addr(n - 1, pmpaddrp);
>>>> + pmp_set_addr(n, pmpaddr);
>>>> + pmp_set_prot(n, prot);
>>>> return 0;
>>>> }
>>>>
>>>> @@ -420,10 +459,12 @@ int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
>>>> addr = (addr & ~((1UL << t1) - 1)) << PMP_SHIFT;
>>>> len = (t1 + PMP_SHIFT + 1);
>>>> }
>>>> - } else {
>>>> + } else if ((prot & PMP_A) == PMP_A_NA4) {
>>>> addr = csr_read_num(pmpaddr_csr) << PMP_SHIFT;
>>>> len = PMP_SHIFT;
>>>> - }
>>>> + } else
>>>> + /* Error out for TOR region */
>>>> + return SBI_EINVAL;
>>>>
>>>> /* return details */
>>>> *prot_out = prot;
>>>> --
>>>> 2.34.1
>>>>
>>>
>
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] lib: sbi: Add pmp_set_tor for setting TOR regions
2025-11-12 10:50 ` Bo Gan
@ 2025-11-12 11:29 ` Xiang W
0 siblings, 0 replies; 11+ messages in thread
From: Xiang W @ 2025-11-12 11:29 UTC (permalink / raw)
To: Bo Gan, opensbi; +Cc: linmin, pinkesh.vaghela, gaohan, samuel
在 2025-11-12三的 02:50 -0800，Bo Gan写道：
> On 11/11/25 02:35, Xiang W wrote:
> > 在 2025-11-11二的 01:45 -0800，Bo Gan写道：
> > > Hi Xiang,
> > >
> > > On 11/10/25 21:45, Xiang W wrote:
> > > > 在 2025-11-10一的 19:41 -0800，Bo Gan写道：
> > > > > TOR can be utilized to cover memory regions that are not aligned with
> > > > > their sizes. Given that the address matching is formed bt 2 consecutive
> > > > > pmpaddr, i.e., pmpaddr(i-1) and pmpaddr(i), TOR should not be used
> > > > > generically to avoid pmpaddr conflict with other NA4/NAPOT regions.
> > > > >
> > > > > It's advised to only use them in platform code where the caller can
> > > > > ensure the index and order of every pmp region especially when there's
> > > > > a mixture of TOR/NA4/NAPOT.
> > > > >
> > > > > Signed-off-by: Bo Gan <ganboing@gmail.com>
> > > >
> > > > Some suggestions:
> > > > 1. Merge pmp_set_addr/pmp_set_port into
> > > > void static void pmp_set_raw(unsigned int n,
> > > > unsigned long port, unsigned long pmpaddr)
> > > >
> > > > 2. Modify the declaration of pmp_set to
> > > > int pmp_set(bool is_tor, unsigned int n, unsigned long port,
> > > > unsigned long addr, unsigned long opaque)
> > > >
> > > > Regards,
> > > > Xiang W
> > >
> > > I assume you meant port -> prot. For 1, I don't quite like it because it
> > > kind of assumes you always change a pmpaddr[i]/pmpcfg[i] pair. In the TOR
> > > case, often times you change pmpaddr[i-1] and pmpaddr[i], then pmpcfg[i].
> > > An edge case is when addr == 0 and n == 0, for it we only update pmpaddr0,
> > > then pmpcfg0. Perhaps I'm mistaken, and you are suggesting `pmp_set_raw`
> > > can handle different cases based on whether prot is TOR? Then I'd say it
> > > probably makes the code more complicated than necessary, as the check can
> > > already be made on the caller side, or no TOR check is necessary with my
> > > different function approach.
> > The function pmp_set_raw is used to set pmpaddr[i]/pmpcfg[i].
> >
> > Regarding the edge case you mentioned, calling pmp_set_raw once is sufficient.
> > For regular Tor types, call it twice:
> >
> > pmp_set_raw(n-1, 0, addr >> PMP_SHIFT)
> > pmp_set_raw(n,prot, (addr + size) >> PMP_SHIFT)
> >
>
> We shouldn't do pmp_set_raw(n-1, 0, addr >> PMP_SHIFT), because it'll zero-
> out the corresponding pmpcfg of n-1.
>
> This is actually the tricky part of TOR. You can have adjacent TOR regions
> that share the same pmpaddr, i.e., the same pmpaddr forms the upper-bound
> of previous region and lower-bound of current region. If we disallow this
> sharing, then potentially we'll end up with 2x the number of TOR regions.
> Thus, if lib/ code were to use TOR, either it would have to coordinate
> neighboring PMP entries, or a single PMP entry should own 2 pmpcfg/pmpaddr
> register pairs.
Thank you for your detailed explanation.
>
> > >
> > > For 2, my rationale on having a different function is that I want to keep
> > > all existing pmp_set the same way, and try to discourage the use of TOR.
> > > Maybe I should have added a warning in the comment in header file. The
> > > reason is because TOR makes the PMP entries dependent on preceding ones,
> > > so care must be taken when programming them. The caller must have global
> > > knowledge of potentially all PMP entries to be programmed in order to use
> > > it properly. I don't feel comfortable unifying to the same interface for
> > > TOR and NA4/NAPOT.
> > The names pmp_set and pmp_set_tor give the impression that pmp_set is a
> > general-purpose method, while pmp_set_tor is for handling special cases.
> > In reality, pmp_set is not a general-purpose method.
> >
>
> Given those scenarios I mentioned earlier, I'd like to avoid making it
> overly complicated. I.e., lib/ code never set or get TOR regions, and can
> still safely assume the 1:1 correspondence of pmp index to pmpcfg/pmpaddr
> pair. Everything related to TOR is only used by platform pmp_configure
> override. Thus, the different function name, so the caller of set_tor can
> be easily identified.
I agree that we should avoid unnecessary complexity and keep the lib/ code
from directly handling TOR regions. To streamline things further, I suggest
moving all TOR-related configurations to the platform code. This will help
maintain clarity and allow the lib/ code to focus on its main responsibilities
without the added burden of managing TOR settings.
Regards,
Xiang W
>
> Bo
>
> > Regards,
> > Xiang W
> > >
> > > Let me know what you think. I'm happy to listen. Thanks.
> > >
> > > Bo
> > >
> > > > > ---
> > > > > include/sbi/riscv_asm.h | 4 +++
> > > > > lib/sbi/riscv_asm.c | 75 +++++++++++++++++++++++++++++++----------
> > > > > 2 files changed, 62 insertions(+), 17 deletions(-)
> > > > >
> > > > > diff --git a/include/sbi/riscv_asm.h b/include/sbi/riscv_asm.h
> > > > > index ef48dc89..c23feab6 100644
> > > > > --- a/include/sbi/riscv_asm.h
> > > > > +++ b/include/sbi/riscv_asm.h
> > > > > @@ -218,6 +218,10 @@ int is_pmp_entry_mapped(unsigned long entry);
> > > > > int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
> > > > > unsigned long log2len);
> > > > >
> > > > > +/* Top of range (TOR) matching mode. pmpaddr(n-1) will also be changed */
> > > > > +int pmp_set_tor(unsigned int n, unsigned long prot, unsigned long addr,
> > > > > + unsigned long size);
> > > > > +
> > > > > int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
> > > > > unsigned long *log2len);
> > > > >
> > > > > diff --git a/lib/sbi/riscv_asm.c b/lib/sbi/riscv_asm.c
> > > > > index 3e44320f..8f64f2b5 100644
> > > > > --- a/lib/sbi/riscv_asm.c
> > > > > +++ b/lib/sbi/riscv_asm.c
> > > > > @@ -330,16 +330,10 @@ 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 void pmp_set_prot(unsigned int n, unsigned long prot)
> > > > > {
> > > > > - int pmpcfg_csr, pmpcfg_shift, pmpaddr_csr;
> > > > > + int pmpcfg_csr, pmpcfg_shift;
> > > > > unsigned long cfgmask, pmpcfg;
> > > > > - unsigned long addrmask, pmpaddr;
> > > > > -
> > > > > - /* check parameters */
> > > > > - if (n >= PMP_COUNT || log2len > __riscv_xlen || log2len < PMP_SHIFT)
> > > > > - return SBI_EINVAL;
> > > > >
> > > > > /* calculate PMP register and offset */
> > > > > #if __riscv_xlen == 32
> > > > > @@ -351,15 +345,29 @@ int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
> > > > > #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;
> > > > > cfgmask = ~(0xffUL << pmpcfg_shift);
> > > > > pmpcfg = (csr_read_num(pmpcfg_csr) & cfgmask);
> > > > > pmpcfg |= ((prot << pmpcfg_shift) & ~cfgmask);
> > > > >
> > > > > + csr_write_num(pmpcfg_csr, pmpcfg);
> > > > > +}
> > > > > +
> > > > > +static void pmp_set_addr(unsigned int n, unsigned long pmpaddr)
> > > > > +{
> > > > > + int pmpaddr_csr = CSR_PMPADDR0 + n;
> > > > > +
> > > > > + csr_write_num(pmpaddr_csr, pmpaddr);
> > > > > +}
> > > > > +
> > > > > +int pmp_set(unsigned int n, unsigned long prot, unsigned long addr,
> > > > > + unsigned long log2len)
> > > > > +{
> > > > > + unsigned long addrmask, pmpaddr;
> > > > > +
> > > > > + /* check parameters */
> > > > > + if (n >= PMP_COUNT || log2len > __riscv_xlen || log2len < PMP_SHIFT)
> > > > > + return SBI_EINVAL;
> > > > > +
> > > > > /* encode PMP address */
> > > > > if (log2len == PMP_SHIFT) {
> > > > > pmpaddr = (addr >> PMP_SHIFT);
> > > > > @@ -373,10 +381,41 @@ 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;
> > > > > +
> > > > > /* write csrs */
> > > > > - csr_write_num(pmpaddr_csr, pmpaddr);
> > > > > - csr_write_num(pmpcfg_csr, pmpcfg);
> > > > > + pmp_set_addr(n, pmpaddr);
> > > > > + pmp_set_prot(n, prot);
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +int pmp_set_tor(unsigned int n, unsigned long prot, unsigned long addr,
> > > > > + unsigned long size)
> > > > > +{
> > > > > + unsigned long pmpaddr, pmpaddrp;
> > > > > +
> > > > > + /* check parameters */
> > > > > + if (n >= PMP_COUNT)
> > > > > + return SBI_EINVAL;
> > > > > +
> > > > > + if (n == 0 && addr != 0)
> > > > > + return SBI_EINVAL;
> > > > > +
> > > > > + /* encode PMP address */
> > > > > + pmpaddrp = addr >> PMP_SHIFT;
> > > > > + pmpaddr = (addr + size) >> PMP_SHIFT;
> > > > >
> > > > > + /* encode PMP config */
> > > > > + prot &= ~PMP_A;
> > > > > + prot |= PMP_A_TOR;
> > > > > +
> > > > > + /* write csrs */
> > > > > + if (n)
> > > > > + pmp_set_addr(n - 1, pmpaddrp);
> > > > > + pmp_set_addr(n, pmpaddr);
> > > > > + pmp_set_prot(n, prot);
> > > > > return 0;
> > > > > }
> > > > >
> > > > > @@ -420,10 +459,12 @@ int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
> > > > > addr = (addr & ~((1UL << t1) - 1)) << PMP_SHIFT;
> > > > > len = (t1 + PMP_SHIFT + 1);
> > > > > }
> > > > > - } else {
> > > > > + } else if ((prot & PMP_A) == PMP_A_NA4) {
> > > > > addr = csr_read_num(pmpaddr_csr) << PMP_SHIFT;
> > > > > len = PMP_SHIFT;
> > > > > - }
> > > > > + } else
> > > > > + /* Error out for TOR region */
> > > > > + return SBI_EINVAL;
> > > > >
> > > > > /* return details */
> > > > > *prot_out = prot;
> > > > > --
> > > > > 2.34.1
> > > > >
> > > >
> >
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-11-12 11:30 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-11 3:41 [PATCH 0/3] Initial ESWIN/EIC7700 support Bo Gan
2025-11-11 3:41 ` [PATCH 1/3] lib: sbi: allow platform to override PMP configuration Bo Gan
2025-11-11 5:45 ` Xiang W
2025-11-11 9:18 ` Bo Gan
2025-11-11 3:41 ` [PATCH 2/3] lib: sbi: Add pmp_set_tor for setting TOR regions Bo Gan
2025-11-11 5:45 ` Xiang W
2025-11-11 9:45 ` Bo Gan
2025-11-11 10:35 ` Xiang W
2025-11-12 10:50 ` Bo Gan
2025-11-12 11:29 ` Xiang W
2025-11-11 3:41 ` [PATCH 3/3] platform: generic: eswin: add EIC7700 Bo Gan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox