[PATCH v2 3/5] firmware: add CONFIG_FIRMWARE_PACKED_RXRW

[Bo Gan <ganboing@gmail.com>]

By default we power-of-2 align the rw sections of firmware. Provide
an option to disable this behavior. Platforms w/o Smepmp won't be
able to enforce RX/RW in M mode, so this 2^n align only perhaps
saves some memory by covering OpenSBI with 2 finer grained PMP
entries, instead of 1. For platforms that are really in short of
PMP, consider enabling this knob.

Signed-off-by: Bo Gan <ganboing@gmail.com>
---
 firmware/Kconfig     | 11 +++++++++++
 firmware/fw_base.ldS | 14 ++++++++++++--
 lib/sbi/sbi_domain.c | 11 +++++++++++
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/firmware/Kconfig b/firmware/Kconfig
index c1fee190..30e4ec7d 100644
--- a/firmware/Kconfig
+++ b/firmware/Kconfig
@@ -25,4 +25,15 @@ config STACK_PROTECTOR_ALL
 	  Turn on the "stack-protector" with "-fstack-protector-all" option.
 	  Like -fstack-protector except that all functions are protected.
 
+config FIRMWARE_PACKED_RXRW
+	bool "Do not power-of-2 align the RW portion"
+	default n
+	help
+	  By default we align the beginning of read-write data sections to
+	  2^n. This facilitates the setting of NAPOT PMP entries to cover
+	  text, rodata, data... sections with proper permissions. For those
+	  platforms that're in short of PMP entries, and not using Smepmp,
+	  they may choose to disable this alignment and the entire firmware
+	  can be covered by a single PMP entry.
+
 endmenu
diff --git a/firmware/fw_base.ldS b/firmware/fw_base.ldS
index 12c7a844..8b60a68a 100644
--- a/firmware/fw_base.ldS
+++ b/firmware/fw_base.ldS
@@ -56,12 +56,22 @@
 
 	/* End of the read-only data sections */
 
+#ifdef CONFIG_FIRMWARE_PACKED_RXRW
+	. = ALIGN(0x1000); /* Ensure next section is page aligned */
+#else
 	/*
-	 * PMP regions must be to be power-of-2. RX/RW will have separate
-	 * regions, so ensure that the split is power-of-2.
+	 * Align the start of RW sections to power-of-2, so READ/EXEC
+	 * and READ/WRITE sections can be covered by different PMP
+	 * entries. Platforms with Smepmp can utilize it to enfore RX/RW
+	 * permission on different portions of the firmware. For platforms
+	 * using traditional PMPs, there's no enforcement of permissions
+	 * in M mode, so NAPOT aligning the RW sections has no security
+	 * benefits other than perhaps saving some memory for SU mode to
+	 * use. It does require 2 PMP entries to cover OpenSBI, not 1.
 	 */
 	. = ALIGN(1 << LOG2CEIL((SIZEOF(.rodata) + SIZEOF(.text)
 				+ SIZEOF(.dynsym) + SIZEOF(.rela.dyn))));
+#endif
 
 	PROVIDE(_fw_rw_start = .);
 
diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
index da0f0557..7307f11e 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -865,6 +865,7 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
 
 	SBI_INIT_LIST_HEAD(&domain_list);
 
+#ifndef CONFIG_FIRMWARE_PACKED_RXRW
 	if (scratch->fw_rw_offset == 0 ||
 	    (scratch->fw_rw_offset & (scratch->fw_rw_offset - 1)) != 0) {
 		sbi_printf("%s: fw_rw_offset is not a power of 2 (0x%lx)\n",
@@ -877,6 +878,7 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
 			   __func__);
 		return SBI_EINVAL;
 	}
+#endif
 
 	domain_hart_ptr_offset = sbi_scratch_alloc_type_offset(void *);
 	if (!domain_hart_ptr_offset)
@@ -904,6 +906,14 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
 	root.possible_harts = root_hmask;
 
 	/* Root domain firmware memory region */
+#ifdef CONFIG_FIRMWARE_PACKED_RXRW
+	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),
+				  &root_memregs[root_memregs_count++]);
+#else
 	sbi_domain_memregion_init(scratch->fw_start, scratch->fw_rw_offset,
 				  (SBI_DOMAIN_MEMREGION_M_READABLE |
 				   SBI_DOMAIN_MEMREGION_M_EXECUTABLE |
@@ -916,6 +926,7 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
 				   SBI_DOMAIN_MEMREGION_M_WRITABLE |
 				   SBI_DOMAIN_MEMREGION_FW),
 				  &root_memregs[root_memregs_count++]);
+#endif
 
 	root.fw_region_inited = true;
 
-- 
2.34.1


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