[M] Change in coreboot[main]: util/ifdtool: Add support for extended region read/write access

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/79790?usp=email )

Change subject: util/ifdtool: Add support for extended region read/write access
......................................................................

util/ifdtool: Add support for extended region read/write access

Platforms from CNL onwards support up to 16 flash regions, not 12. The
permissions for regions [15:12] are stored in extended region
read/write access fields in the FLMSTR registers. Currently ifdtool
treats these fields as reserved, so they're not modified when locking or
unlocking.

Add support for extended regions so that they are locked/unlocked by the
--lock/--unlock options. This will make the locked/unlocked descriptors
generated by ifdtool match those generated by mFIT.

BUG=b:270275115
TEST=Without this change:

`ifdtool -lr -p adl` on unlocked image:
Before:
00000080  ff ff ff ff ff ff ff ff  ff ff ff ff 00 00 00 00
00000090  ff ff ff ff
After:
00000080  ff 07 20 00 ff 05 40 00  ff 00 00 00 00 00 00 00
00000090  ff 00 00 00

`ifdtool -u -p adl` on locked image:
Before:
00000080  00 07 20 00 00 05 40 00  00 00 00 00 00 00 00 00
00000090  00 00 00 00
After:
00000080  00 ff ff ff 00 ff ff ff  00 ff ff ff 00 00 00 00
00000090  00 ff ff ff

With this change:

`ifdtool -lr -p adl` on unlocked image:
Before:
00000080  ff ff ff ff ff ff ff ff  ff ff ff ff 00 00 00 00
00000090  ff ff ff ff
After:
00000080  00 07 20 00 00 05 40 00  00 00 00 00 00 00 00 00
00000090  00 00 00 00

`ifdtool -u -p adl` on locked image:
Before:
00000080  00 07 20 00 00 05 40 00  00 00 00 00 00 00 00 00
00000090  00 00 00 00
After:
00000080  ff ff ff ff ff ff ff ff  ff ff ff ff 00 00 00 00
00000090  ff ff ff ff

Change-Id: Iaa43524d91c399a996ade56f2f613b4110a44aad
Signed-off-by: Reka Norman 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/79790
Tested-by: build bot (Jenkins) 
Reviewed-by: Subrata Banik 
---
M util/ifdtool/ifdtool.c
1 file changed, 60 insertions(+), 10 deletions(-)

Approvals:
  Subrata Banik: Looks good to me, approved
  build bot (Jenkins): Verified

diff --git a/util/ifdtool/ifdtool.c b/util/ifdtool/ifdtool.c
index 7d69bcf..99a83e5 100644
--- a/util/ifdtool/ifdtool.c
+++ b/util/ifdtool/ifdtool.c
@@ -1329,6 +1329,36 @@
    return !!((region.base < region.limit) && (region.size > 0));
 }

+/*
+ * Platforms from CNL onwards support up to 16 flash regions, not 12. The
+ * permissions for regions [15:12] are stored in extended region read/write
+ * access fields in the FLMSTR registers.
+ *
+ * FLMSTR with extended regions:
+ *   31:20 Region Write Access
+ *   19:8  Region Read Access
+ *    7:4  Extended Region Write Access
+ *    3:0  Extended Region Read Access
+ *
+ * FLMSTR without extended regions:
+ *   31:20 Region Write Access
+ *   19:8  Region Read Access
+ *    7:0  Reserved
+ */
+static bool platform_has_extended_regions(void)
+{
+	switch (platform) {
+	case PLATFORM_CNL:
+	case PLATFORM_JSL:
+	case PLATFORM_TGL:
+	case PLATFORM_ADL:
+	case PLATFORM_MTL:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static void lock_descriptor(const char *filename, char *image, int size)
 {
    int wr_shift, rd_shift;
@@ -1341,11 +1371,21 @@
    	wr_shift = FLMSTR_WR_SHIFT_V2;
    	rd_shift = FLMSTR_RD_SHIFT_V2;

-		/* Clear non-reserved bits */
-		fmba->flmstr1 &= 0xff;
-		fmba->flmstr2 &= 0xff;
-		fmba->flmstr3 &= 0xff;
-		fmba->flmstr5 &= 0xff;
+		/*
+		 * Clear all read/write access bits. See comment on
+		 * platform_has_extended_regions() for bitfields.
+		 */
+		if (platform_has_extended_regions()) {
+			fmba->flmstr1 = 0;
+			fmba->flmstr2 = 0;
+			fmba->flmstr3 = 0;
+			fmba->flmstr5 = 0;
+		} else {
+			fmba->flmstr1 &= 0xff;
+			fmba->flmstr2 &= 0xff;
+			fmba->flmstr3 &= 0xff;
+			fmba->flmstr5 &= 0xff;
+		}
    } else {
    	wr_shift = FLMSTR_WR_SHIFT_V1;
    	rd_shift = FLMSTR_RD_SHIFT_V1;
@@ -1482,11 +1522,21 @@
    	exit(EXIT_FAILURE);

if (ifd_version >= IFD_VERSION_2) {
-		/* Access bits for each region are read: 19:8 write: 31:20 */
-		fmba->flmstr1 = 0xffffff00 | (fmba->flmstr1 & 0xff);
-		fmba->flmstr2 = 0xffffff00 | (fmba->flmstr2 & 0xff);
-		fmba->flmstr3 = 0xffffff00 | (fmba->flmstr3 & 0xff);
-		fmba->flmstr5 = 0xffffff00 | (fmba->flmstr5 & 0xff);
+		/*
+		 * Set all read/write access bits. See comment on
+		 * platform_has_extended_regions() for bitfields.
+		 */
+		if (platform_has_extended_regions()) {
+			fmba->flmstr1 = 0xffffffff;
+			fmba->flmstr2 = 0xffffffff;
+			fmba->flmstr3 = 0xffffffff;
+			fmba->flmstr5 = 0xffffffff;
+		} else {
+			fmba->flmstr1 = 0xffffff00 | (fmba->flmstr1 & 0xff);
+			fmba->flmstr2 = 0xffffff00 | (fmba->flmstr2 & 0xff);
+			fmba->flmstr3 = 0xffffff00 | (fmba->flmstr3 & 0xff);
+			fmba->flmstr5 = 0xffffff00 | (fmba->flmstr5 & 0xff);
+		}
    } else {
    	fmba->flmstr1 = 0xffff0000;
    	fmba->flmstr2 = 0xffff0000;