lib/utils/fdt: Reserve enough FDT space for fdt_cpu_fixup

The fdt_cpu_fixup() function uses a hardcoded +32 bytes reservation when calling fdt_open_into() to make room for disabling HART DT nodes and appending the "zicntr" isa-extension. This is insufficient on systems where many HARTs are disabled: each disabled HART needs 4 bytes ("okay" -> "disabled"), and each zicntr append needs ~16 bytes. With the current limit only ~8 HARTs can be disabled before FDT_ERR_NOSPACE (-3) is returned.

Replace the hardcoded reservation with a dynamic calculation based on SBI_HARTMASK_MAX_BITS, which defines the maximum number of HARTs OpenSBI supports. Each HART may need up to ~20 bytes in the worst case, so reserve SBI_HARTMASK_MAX_BITS * 20 bytes.

Also remove the now-redundant per-HART fdt_open_into() call inside the zicntr loop since the single upfront reservation is sufficient.

---
 lib/utils/fdt/fdt_fixup.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

index b0ed20c8..bace2277 100644
@@ -124,7 +124,13 @@ void fdt_cpu_fixup(void *fdt)
 			  sbi_hart_has_csr(scratch, SBI_HART_CSR_CYCLE) &&
 			  sbi_hart_has_csr(scratch, SBI_HART_CSR_INSTRET);
 
-	err = fdt_open_into(fdt, fdt, fdt_totalsize(fdt) + 32);
+	/*
+	 * Reserve enough FDT space for the worst case:
+	 *  - Each disabled hart: 4 bytes ("okay" -> "disabled")
+	 *  - Each zicntr append: ~16 bytes (property + string)
+	 */
+	err = fdt_open_into(fdt, fdt, fdt_totalsize(fdt) +
+			    SBI_HARTMASK_MAX_BITS * 20);
 	if (err < 0)
 		return;
 

@@ -163,14 +169,9 @@ void fdt_cpu_fixup(void *fdt)
 		 * property if there hasn't been already one.
 		 */
 		if (extensions &&
-		    !fdt_stringlist_contains(extensions, len, "zicntr")) {
-			err = fdt_open_into(fdt, fdt, fdt_totalsize(fdt) + 16);
-			if (err)
-				continue;
-
+		    !fdt_stringlist_contains(extensions, len, "zicntr"))
 			fdt_appendprop_string(fdt, cpu_offset,
 					      "riscv,isa-extensions", "zicntr");
-		}
 	}
 }
 

2.50.1


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

The fdt_cpu_fixup() function uses a hardcoded +32 bytes reservation when calling fdt_open_into() to make room for disabling HART DT nodes and appending the "zicntr" isa-extension. This is insufficient on systems where many HARTs are disabled: each disabled HART needs 4 bytes ("okay" -> "disabled"), and each zicntr append needs ~16 bytes. With the current limit only ~8 HARTs can be disabled before FDT_ERR_NOSPACE (-3) is returned.

Replace the hardcoded reservation with a dynamic calculation based on SBI_HARTMASK_MAX_BITS, which defines the maximum number of HARTs OpenSBI supports. Each HART may need up to ~20 bytes in the worst case, so reserve SBI_HARTMASK_MAX_BITS * 20 bytes.

Also remove the now-redundant per-HART fdt_open_into() call inside the zicntr loop since the single upfront reservation is sufficient.

---
 lib/utils/fdt/fdt_fixup.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

index b0ed20c8..bace2277 100644
@@ -124,7 +124,13 @@ void fdt_cpu_fixup(void *fdt)
 			  sbi_hart_has_csr(scratch, SBI_HART_CSR_CYCLE) &&
 			  sbi_hart_has_csr(scratch, SBI_HART_CSR_INSTRET);
 
-	err = fdt_open_into(fdt, fdt, fdt_totalsize(fdt) + 32);
+	/*
+	 * Reserve enough FDT space for the worst case:
+	 *  - Each disabled hart: 4 bytes ("okay" -> "disabled")
+	 *  - Each zicntr append: ~16 bytes (property + string)
+	 */
+	err = fdt_open_into(fdt, fdt, fdt_totalsize(fdt) +
+			    SBI_HARTMASK_MAX_BITS * 20);
 	if (err < 0)
 		return;
 

@@ -163,14 +169,9 @@ void fdt_cpu_fixup(void *fdt)
 		 * property if there hasn't been already one.
 		 */
 		if (extensions &&
-		    !fdt_stringlist_contains(extensions, len, "zicntr")) {
-			err = fdt_open_into(fdt, fdt, fdt_totalsize(fdt) + 16);
-			if (err)
-				continue;
-
+		    !fdt_stringlist_contains(extensions, len, "zicntr"))
 			fdt_appendprop_string(fdt, cpu_offset,
 					      "riscv,isa-extensions", "zicntr");
-		}
 	}
 }
 

2.50.1


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

Hello everyone,

Friendly ping on this patch. Is there any comment on this fix? Thanks.

To summarize, this patch fixes a real issue where fdt_cpu_fixup() reserves only a hardcoded +32 bytes when disabling HART DT nodes and appending the "zicntr" isa-extension. On systems with many disabled HARTs, fdt_setprop_string() silently returns -FDT_ERR_NOSPACE after roughly 8 HARTs, leaving the remaining CPU nodes unexpectedly enabled in the DTB passed to the OS.

The fix replaces the hardcoded reservation with a dynamic calculation based on SBI_HARTMASK_MAX_BITS and removes the now-redundant per-HART fdt_open_into() call in the zicntr loop.

Thanks,
Pei

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

Gentle ping. Is there any comment on this fix?

Thanks,
Pei

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