platform/lib: Allow the OS to map the regions that are protected by PMP

This is achieved by removing the 'no-map' property from the
'reserved-memory' node when PMP is present, otherwise we keep it as it
offers a small protection if the OS does not map this region at all.
A new callback in platform_override is introduced and allows to fixup the
device-tree. It is used here to override this new default behaviour on
SiFive Fu540 platforms that has an erratum that prevents S-mode software
to access a PMP protected region using 1GB page table mapping.

If PMP is present, telling the OS not to map the reserved regions does not
add much protection since it only avoids access to regions that are already
protected by PMP. But by not allowing the OS to map those regions, it
creates holes in the OS system memory map and prevents the use of
hugepages which would generate, among other benefits, less TLB miss.

Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
Reviewed-by: Anup Patel <anup.patel@wdc.com>

include/sbi_utils/fdt/fdt_fixup.h

@@ -47,6 +47,20 @@ void fdt_plic_fixup(void *fdt, const char *compat);
  */
 int fdt_reserved_memory_fixup(void *fdt);
 
+/**
+ * Fix up the reserved memory subnodes in the device tree
+ *
+ * This routine adds the no-map property to the reserved memory subnodes so
+ * that the OS does not map those PMP protected memory regions.
+ *
+ * Platform codes must call this helper in their final_init() after fdt_fixups()
+ * if the OS should not map the PMP protected reserved regions.
+ *
+ * @param fdt: device tree blob
+ * @return zero on success and -ve on failure
+ */
+int fdt_reserved_memory_nomap_fixup(void *fdt);
+
 /**
  * General device tree fix-up
  *

lib/utils/fdt/fdt_fixup.c

@@ -70,7 +70,7 @@ void fdt_plic_fixup(void *fdt, const char *compat)
 
 static int fdt_resv_memory_update_node(void *fdt, unsigned long addr,
 				       unsigned long size, int index,
-				       int parent)
+				       int parent, bool no_map)
 {
 	int na = fdt_address_cells(fdt, 0);
 	int ns = fdt_size_cells(fdt, 0);
@@ -99,14 +99,16 @@ static int fdt_resv_memory_update_node(void *fdt, unsigned long addr,
 	if (subnode < 0)
 		return subnode;
 
-	/*
-	 * Tell operating system not to create a virtual
-	 * mapping of the region as part of its standard
-	 * mapping of system memory.
-	 */
-	err = fdt_setprop_empty(fdt, subnode, "no-map");
-	if (err < 0)
-		return err;
+	if (no_map) {
+		/*
+		 * Tell operating system not to create a virtual
+		 * mapping of the region as part of its standard
+		 * mapping of system memory.
+		 */
+		err = fdt_setprop_empty(fdt, subnode, "no-map");
+		if (err < 0)
+			return err;
+	}
 
 	/* encode the <reg> property value */
 	val = reg;
@@ -199,7 +201,8 @@ int fdt_reserved_memory_fixup(void *fdt)
 		 */
 		addr = scratch->fw_start & ~(scratch->fw_size - 1UL);
 		size = (1UL << log2roundup(scratch->fw_size));
-		return fdt_resv_memory_update_node(fdt, addr, size, 0, parent);
+		return fdt_resv_memory_update_node(fdt, addr, size,
+						   0, parent, true);
 	}
 
 	for (i = 0, j = 0; i < sbi_hart_pmp_count(scratch); i++) {
@@ -211,13 +214,37 @@ int fdt_reserved_memory_fixup(void *fdt)
 		if (prot & (PMP_R | PMP_W | PMP_X))
 			continue;
 
-		fdt_resv_memory_update_node(fdt, addr, size, j, parent);
+		fdt_resv_memory_update_node(fdt, addr, size, j, parent, false);
 		j++;
 	}
 
 	return 0;
 }
 
+int fdt_reserved_memory_nomap_fixup(void *fdt)
+{
+	int parent, subnode;
+	int err;
+
+	/* Locate the reserved memory node */
+	parent = fdt_path_offset(fdt, "/reserved-memory");
+	if (parent < 0)
+		return parent;
+
+	fdt_for_each_subnode(subnode, fdt, parent) {
+		/*
+		 * Tell operating system not to create a virtual
+		 * mapping of the region as part of its standard
+		 * mapping of system memory.
+		 */
+		err = fdt_setprop_empty(fdt, subnode, "no-map");
+		if (err < 0)
+			return err;
+	}
+
+	return 0;
+}
+
 void fdt_fixups(void *fdt)
 {
 	fdt_plic_fixup(fdt, "riscv,plic0");

platform/generic/include/platform_override.h

@@ -21,6 +21,7 @@ struct platform_override {
 	void (*early_exit)(const struct fdt_match *match);
 	void (*final_exit)(const struct fdt_match *match);
 	int (*system_reset)(u32 reset_type, const struct fdt_match *match);
+	int (*fdt_fixup)(void *fdt, const struct fdt_match *match);
 };
 
 #endif

platform/generic/platform.c

@@ -153,6 +153,12 @@ static int generic_final_init(bool cold_boot)
 	fdt_cpu_fixup(fdt);
 	fdt_fixups(fdt);
 
+	if (generic_plat && generic_plat->fdt_fixup) {
+		rc = generic_plat->fdt_fixup(fdt, generic_plat_match);
+		if (rc)
+			return rc;
+	}
+
 	return 0;
 }
 

platform/generic/sifive_fu540.c

@@ -9,6 +9,7 @@
 
 #include <platform_override.h>
 #include <sbi_utils/fdt/fdt_helper.h>
+#include <sbi_utils/fdt/fdt_fixup.h>
 
 static u64 sifive_fu540_tlbr_flush_limit(const struct fdt_match *match)
 {
@@ -19,6 +20,18 @@ static u64 sifive_fu540_tlbr_flush_limit(const struct fdt_match *match)
 	return 0;
 }
 
+static int sifive_fu540_fdt_fixup(void *fdt, const struct fdt_match *match)
+{
+	/*
+	 * SiFive Freedom U540 has an erratum that prevents S-mode software
+	 * to access a PMP protected region using 1GB page table mapping, so
+	 * always add the no-map attribute on this platform.
+	 */
+	fdt_reserved_memory_nomap_fixup(fdt);
+
+	return 0;
+}
+
 static const struct fdt_match sifive_fu540_match[] = {
 	{ .compatible = "sifive,fu540" },
 	{ .compatible = "sifive,fu540g" },
@@ -30,4 +43,5 @@ static const struct fdt_match sifive_fu540_match[] = {
 const struct platform_override sifive_fu540 = {
 	.match_table = sifive_fu540_match,
 	.tlbr_flush_limit = sifive_fu540_tlbr_flush_limit,
+	.fdt_fixup = sifive_fu540_fdt_fixup,
 };

platform/sifive/fu540/platform.c

@@ -63,6 +63,13 @@ static void fu540_modify_dt(void *fdt)
 	fdt_cpu_fixup(fdt);
 
 	fdt_fixups(fdt);
+
+	/*
+	 * SiFive Freedom U540 has an erratum that prevents S-mode software
+	 * to access a PMP protected region using 1GB page table mapping, so
+	 * always add the no-map attribute on this platform.
+	 */
+	fdt_reserved_memory_nomap_fixup(fdt);
 }
 
 static int fu540_final_init(bool cold_boot)