[coreboot-gerrit] Change in ...coreboot[master]: ifdtool: Enable GbE/PDR region access if they exist

[Duncan Laurie (Code Review)]

Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33273

Change subject: ifdtool: Enable GbE/PDR region access if they exist
......................................................................

ifdtool: Enable GbE/PDR region access if they exist

Instead of assuming GbE region is used and PDR region is not,
check if there is a valid region defined in the descriptor and
set the region access permissions based on that.

This enables the use of the PDR region on the sarien platform,
which also uses the GbE region.

This results in the following example changes:

mb/google/sarien (GbE and PDR)
.      DESC BIOS ME GbE PDR EC
-BIOS  r    rw      rw      r
+BIOS  r    rw      rw  rw  r

mb/google/eve: (no GbE, no PDR)
.      DESC BIOS ME GbE PDR EC
-BIOS  r    rw      rw
+BIOS  r    rw

Change-Id: I7aeffc8f8194638c6012340b43aea8f8460d268a
Signed-off-by: Duncan Laurie 
---
M util/ifdtool/ifdtool.c
M util/ifdtool/ifdtool.h
2 files changed, 40 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/33273/1

diff --git a/util/ifdtool/ifdtool.c b/util/ifdtool/ifdtool.c
index 6c5e784..0204a71 100644
--- a/util/ifdtool/ifdtool.c
+++ b/util/ifdtool/ifdtool.c
@@ -925,15 +925,28 @@
    write_image(filename, image, size);
 }

+static int check_region(const frba_t *frba, unsigned int region_type)
+{
+	region_t region;
+
+	if (!frba)
+		return 0;
+
+	region = get_region(frba, region_type);
+	return !!((region.base < region.limit) && (region.size > 0));
+}
+
 static void lock_descriptor(const char *filename, char *image, int size)
 {
-	int wr_shift, rd_shift;
+	int wr_shift, rd_shift, rw_pdr, rw_gbe;
    fmba_t *fmba = find_fmba(image, size);
+	const frba_t *frba = find_frba(image, size);
    if (!fmba)
    	exit(EXIT_FAILURE);
-	/* TODO: Dynamically take Platform Data Region and GbE Region
-	 * into regard.
-	 */
+
+	/* Check for valid PDR and GBE region */
+	rw_pdr = check_region(frba, REGION_PDR);
+	rw_gbe = check_region(frba, REGION_GBE);

if (ifd_version >= IFD_VERSION_2) {
    	wr_shift = FLMSTR_WR_SHIFT_V2;
@@ -969,10 +982,20 @@
    case PLATFORM_CNL:
    case PLATFORM_ICL:
    case PLATFORM_SKLKBL:
-		/* CPU/BIOS can read descriptor, BIOS, EC and GbE. */
-		fmba->flmstr1 |= 0x10b << rd_shift;
-		/* CPU/BIOS can write BIOS and Gbe. */
-		fmba->flmstr1 |= 0xa << wr_shift;
+		/* CPU/BIOS can read descriptor, BIOS and EC */
+		fmba->flmstr1 |= 0x103 << rd_shift;
+		/* CPU/BIOS can write BIOS. */
+		fmba->flmstr1 |= 0x2 << wr_shift;
+		if (rw_gbe) {
+			/* Enable GBE RW if region is defined */
+			fmba->flmstr1 |= (1 << REGION_GBE) << rd_shift;
+			fmba->flmstr1 |= (1 << REGION_GBE) << wr_shift;
+		}
+		if (rw_pdr) {
+			/* Enable PDR RW if region is defined */
+			fmba->flmstr1 |= (1 << REGION_PDR) << rd_shift;
+			fmba->flmstr1 |= (1 << REGION_PDR) << wr_shift;
+		}
    	/* ME can read descriptor, ME and GbE. */
    	fmba->flmstr2 |= 0xd << rd_shift;
    	/* ME can write ME. */
diff --git a/util/ifdtool/ifdtool.h b/util/ifdtool/ifdtool.h
index 49463b9..6617319 100644
--- a/util/ifdtool/ifdtool.h
+++ b/util/ifdtool/ifdtool.h
@@ -94,6 +94,15 @@
 #define MAX_REGIONS 9
 #define MAX_REGIONS_OLD 5

+enum flash_regions {
+	REGION_DESCRIPTOR,
+	REGION_BIOS,
+	REGION_ME,
+	REGION_GBE,
+	REGION_PDR,
+	REGION_EC = 8,
+};
+
 typedef struct {
    uint32_t flreg[MAX_REGIONS];
 } __attribute__((packed)) frba_t;