[coreboot-gerrit] New patch to review for coreboot: ifdtool: Properly set + decode flmstr regs for IFD v2

Patrick Georgi (pgeorgi@google.com) gerrit at coreboot.org
Tue Sep 15 19:40:04 CET 2015


Patrick Georgi (pgeorgi at google.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/11658

-gerrit

commit 3f788da25cd79a349d07279cd24a7fbb16a11c4d
Author: Shawn Nematbakhsh <shawnn at chromium.org>
Date:   Thu Sep 10 19:07:13 2015 -0700

    ifdtool: Properly set + decode flmstr regs for IFD v2
    
    flmstr register bits have slightly different meaning for IFD v2.
    
    BUG=chrome-os-partner:45091, chrome-os-partner:43461
    TEST=Run `ifdtool -d image.bin` on IFD v1 locked squawks image:
    
    Found Master Section
    FLMSTR1:   0x0a0b0000 (Host CPU/BIOS)
      Platform Data Region Write Access: disabled
      GbE Region Write Access:           enabled
      Intel ME Region Write Access:      disabled
      Host CPU/BIOS Region Write Access: enabled
      Flash Descriptor Write Access:     disabled
      Platform Data Region Read Access:  disabled
      GbE Region Read Access:            enabled
      Intel ME Region Read Access:       disabled
      Host CPU/BIOS Region Read Access:  enabled
      Flash Descriptor Read Access:      enabled
      Requester ID:                      0x0000
    
    FLMSTR2:   0x0c0d0000 (Intel ME)
      Platform Data Region Write Access: disabled
      GbE Region Write Access:           enabled
      Intel ME Region Write Access:      enabled
      Host CPU/BIOS Region Write Access: disabled
      Flash Descriptor Write Access:     disabled
      Platform Data Region Read Access:  disabled
      GbE Region Read Access:            enabled
      Intel ME Region Read Access:       enabled
      Host CPU/BIOS Region Read Access:  disabled
      Flash Descriptor Read Access:      enabled
      Requester ID:                      0x0000
    
    FLMSTR3:   0x08080118 (GbE)
      Platform Data Region Write Access: disabled
      GbE Region Write Access:           enabled
      Intel ME Region Write Access:      disabled
      Host CPU/BIOS Region Write Access: disabled
      Flash Descriptor Write Access:     disabled
      Platform Data Region Read Access:  disabled
      GbE Region Read Access:            enabled
      Intel ME Region Read Access:       disabled
      Host CPU/BIOS Region Read Access:  disabled
      Flash Descriptor Read Access:      disabled
      Requester ID:                      0x0118
    
    Then, run `ifdtool -l image.bin` and verify newly locked image is identical.
    Next, run `ifdtool -l image.bin` on unlocked glados image. Verify that locked
    and unlocked regions are identical to above.
    Finally, burn glados image, run `flashrom -V`, and verify ME regions is
    locked and descriptor region is RO.
    BRANCH=None
    
    Change-Id: I8a65bdc5edd0d888138b88c1189f8badd1404b64
    Signed-off-by: Patrick Georgi <pgeorgi at chromium.org>
    Original-Commit-Id: 11c434835a66a50ab2c0c01a084edc96cbe052da
    Original-Signed-off-by: Shawn Nematbakhsh <shawnn at chromium.org>
    Original-Change-Id: I875dfce6f5cf57831714702872bfe636f8f953f4
    Original-Reviewed-on: https://chromium-review.googlesource.com/298968
    Original-Commit-Ready: Shawn N <shawnn at chromium.org>
    Original-Tested-by: Shawn N <shawnn at chromium.org>
    Original-Reviewed-by: Aaron Durbin <adurbin at chromium.org>
---
 util/ifdtool/ifdtool.c | 91 +++++++++++++++++++++++++++++++++++++++-----------
 util/ifdtool/ifdtool.h |  9 +++++
 2 files changed, 80 insertions(+), 20 deletions(-)

diff --git a/util/ifdtool/ifdtool.c b/util/ifdtool/ifdtool.c
index e6ed110..feaffa8 100644
--- a/util/ifdtool/ifdtool.c
+++ b/util/ifdtool/ifdtool.c
@@ -443,36 +443,50 @@ static void dump_fpsba(fpsba_t * fpsba)
 
 static void decode_flmstr(uint32_t flmstr)
 {
+	int wr_shift, rd_shift;
+	if (ifd_version >= IFD_VERSION_2) {
+		wr_shift = FLMSTR_WR_SHIFT_V2;
+		rd_shift = FLMSTR_RD_SHIFT_V2;
+	} else {
+		wr_shift = FLMSTR_WR_SHIFT_V1;
+		rd_shift = FLMSTR_RD_SHIFT_V1;
+	}
+
+	/* EC region access only available on v2+ */
 	if (ifd_version >= IFD_VERSION_2)
 		printf("  EC Region Write Access:            %s\n",
-		       (flmstr & (1 << 29)) ? "enabled" : "disabled");
+		       (flmstr & (1 << (wr_shift + 8))) ?
+		       "enabled" : "disabled");
 	printf("  Platform Data Region Write Access: %s\n",
-		(flmstr & (1 << 28)) ? "enabled" : "disabled");
+		(flmstr & (1 << (wr_shift + 4))) ? "enabled" : "disabled");
 	printf("  GbE Region Write Access:           %s\n",
-		(flmstr & (1 << 27)) ? "enabled" : "disabled");
+		(flmstr & (1 << (wr_shift + 3))) ? "enabled" : "disabled");
 	printf("  Intel ME Region Write Access:      %s\n",
-		(flmstr & (1 << 26)) ? "enabled" : "disabled");
+		(flmstr & (1 << (wr_shift + 2))) ? "enabled" : "disabled");
 	printf("  Host CPU/BIOS Region Write Access: %s\n",
-		(flmstr & (1 << 25)) ? "enabled" : "disabled");
+		(flmstr & (1 << (wr_shift + 1))) ? "enabled" : "disabled");
 	printf("  Flash Descriptor Write Access:     %s\n",
-		(flmstr & (1 << 24)) ? "enabled" : "disabled");
+		(flmstr & (1 << wr_shift)) ? "enabled" : "disabled");
 
 	if (ifd_version >= IFD_VERSION_2)
 		printf("  EC Region Read Access:             %s\n",
-		       (flmstr & (1 << 21)) ? "enabled" : "disabled");
+		       (flmstr & (1 << (rd_shift + 8))) ?
+		       "enabled" : "disabled");
 	printf("  Platform Data Region Read Access:  %s\n",
-		(flmstr & (1 << 20)) ? "enabled" : "disabled");
+		(flmstr & (1 << (rd_shift + 4))) ? "enabled" : "disabled");
 	printf("  GbE Region Read Access:            %s\n",
-		(flmstr & (1 << 19)) ? "enabled" : "disabled");
+		(flmstr & (1 << (rd_shift + 3))) ? "enabled" : "disabled");
 	printf("  Intel ME Region Read Access:       %s\n",
-		(flmstr & (1 << 18)) ? "enabled" : "disabled");
+		(flmstr & (1 << (rd_shift + 2))) ? "enabled" : "disabled");
 	printf("  Host CPU/BIOS Region Read Access:  %s\n",
-		(flmstr & (1 << 17)) ? "enabled" : "disabled");
+		(flmstr & (1 << (rd_shift + 1))) ? "enabled" : "disabled");
 	printf("  Flash Descriptor Read Access:      %s\n",
-		(flmstr & (1 << 16)) ? "enabled" : "disabled");
+		(flmstr & (1 << rd_shift)) ? "enabled" : "disabled");
 
-	printf("  Requester ID:                      0x%04x\n\n",
-		flmstr & 0xffff);
+	/* Requestor ID doesn't exist for ifd 2 */
+	if (ifd_version < IFD_VERSION_2)
+		printf("  Requester ID:                      0x%04x\n\n",
+			flmstr & 0xffff);
 }
 
 static void dump_fmba(fmba_t * fmba)
@@ -740,14 +754,43 @@ static void set_em100_mode(char *filename, char *image, int size)
 
 static void lock_descriptor(char *filename, char *image, int size)
 {
+	int wr_shift, rd_shift;
 	fdbar_t *fdb = find_fd(image, size);
 	fmba_t *fmba = (fmba_t *) (image + (((fdb->flmap1) & 0xff) << 4));
 	/* TODO: Dynamically take Platform Data Region and GbE Region
 	 * into regard.
 	 */
-	fmba->flmstr1 = 0x0a0b0000;
-	fmba->flmstr2 = 0x0c0d0000;
-	fmba->flmstr3 = 0x08080118;
+
+	if (ifd_version >= IFD_VERSION_2) {
+		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;
+	} else {
+		wr_shift = FLMSTR_WR_SHIFT_V1;
+		rd_shift = FLMSTR_RD_SHIFT_V1;
+
+		fmba->flmstr1 = 0;
+		fmba->flmstr2 = 0;
+		/* Requestor ID */
+		fmba->flmstr3 = 0x118;
+	}
+
+	/* CPU/BIOS can read descriptor, BIOS, and GbE. */
+	fmba->flmstr1 |= 0xb << rd_shift;
+	/* CPU/BIOS can write BIOS and GbE. */
+	fmba->flmstr1 |= 0xa << wr_shift;
+	/* ME can read descriptor, ME, and GbE. */
+	fmba->flmstr2 |= 0xd << rd_shift;
+	/* ME can write ME and GbE. */
+	fmba->flmstr2 |= 0xc << wr_shift;
+	/* GbE can write only GbE. */
+	fmba->flmstr3 |= 0x8 << rd_shift;
+	/* GbE can read only GbE. */
+	fmba->flmstr3 |= 0x8 << wr_shift;
 
 	write_image(filename, image, size);
 }
@@ -756,9 +799,17 @@ static void unlock_descriptor(char *filename, char *image, int size)
 {
 	fdbar_t *fdb = find_fd(image, size);
 	fmba_t *fmba = (fmba_t *) (image + (((fdb->flmap1) & 0xff) << 4));
-	fmba->flmstr1 = 0xffff0000;
-	fmba->flmstr2 = 0xffff0000;
-	fmba->flmstr3 = 0x08080118;
+
+	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);
+	} else {
+		fmba->flmstr1 = 0xffff0000;
+		fmba->flmstr2 = 0xffff0000;
+		fmba->flmstr3 = 0x08080118;
+	}
 
 	write_image(filename, image, size);
 }
diff --git a/util/ifdtool/ifdtool.h b/util/ifdtool/ifdtool.h
index 8b13283..195d91c 100644
--- a/util/ifdtool/ifdtool.h
+++ b/util/ifdtool/ifdtool.h
@@ -101,6 +101,15 @@ typedef struct {
 	uint32_t pchstrp17;
 } __attribute__((packed)) fpsba_t;
 
+/*
+ * WR / RD bits start at different locations within the flmstr regs, but
+ * otherwise have identical meaning.
+ */
+#define FLMSTR_WR_SHIFT_V1 24
+#define FLMSTR_WR_SHIFT_V2 20
+#define FLMSTR_RD_SHIFT_V1 16
+#define FLMSTR_RD_SHIFT_V2 8
+
 // master
 typedef struct {
 	uint32_t flmstr1;



More information about the coreboot-gerrit mailing list