ifdtool: Add a list of known platforms that support IFD_VERSION_2

ifdtool has relied on one of the fields within FCBA(read_freq) to
determine whether a platform supports IFD_VERSION_1 or
IFD_VERSION_2. However, newer platforms like GLK and CNL do not have
read_freq field in FCBA and so the value of these bits cannot be used
as an indicator to distinguish IFD versions. In the long run, we need
to re-write ifdtool to have a better mapping of SoC to IFD fields. But
until that is done, this change adds a list of platforms that we know
do not support read_freq field but still use IFD_VERSION_2. This
change also updates GLK and CNL to pass in platform parameter to
ifdtool.

BUG=b:79109029, b:69270831

Change-Id: I36c49f4dcb480ad53b0538ad12292fb94b0e3934
Signed-off-by: Furquan Shaikh <furquan@google.com>
Reviewed-on: https://review.coreboot.org/26023
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>

src/soc/intel/apollolake/Kconfig

@@ -350,6 +350,7 @@ config SMM_RESERVED_SIZE
 
 config IFD_CHIPSET
 	string
+	default "glk" if SOC_INTEL_GLK
 	default "aplk"
 
 config CPU_BCLK_MHZ

src/soc/intel/cannonlake/Kconfig

@@ -114,6 +114,10 @@ config DCACHE_BSP_STACK_SIZE
 	  The amount of anticipated stack usage in CAR by bootblock and
 	  other stages.
 
+config IFD_CHIPSET
+	string
+	default "cnl"
+
 config IED_REGION_SIZE
 	hex
 	default 0x400000

util/ifdtool/ifdtool.c

@@ -125,15 +125,36 @@ static fmsba_t *find_fmsba(char *image, int size)
 	return PTR_IN_RANGE(fmsba, image, size) ? fmsba : NULL;
 }
 
+/*
+ * Some newer platforms have re-defined the FCBA field that was used to
+ * distinguish IFD v1 v/s v2. Define a list of platforms that we know do not
+ * have the required FCBA field, but are IFD v2 and return true if current
+ * platform is one of them.
+ */
+static int is_platform_ifd_2(void)
+{
+	static const int ifd_2_platforms[] = {
+		PLATFORM_GLK,
+		PLATFORM_CNL,
+	};
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(ifd_2_platforms); i++) {
+		if (platform == ifd_2_platforms[i])
+			return 1;
+	}
+
+	return 0;
+}
+
 /*
  * There is no version field in the descriptor so to determine
  * if this is a new descriptor format we check the hardcoded SPI
  * read frequency to see if it is fixed at 20MHz or 17MHz.
  */
-static void check_ifd_version(char *image, int size)
+static int get_ifd_version_from_fcba(char *image, int size)
 {
 	int read_freq;
-
 	const fcba_t *fcba = find_fcba(image, size);
 	if (!fcba)
 		exit(EXIT_FAILURE);
@@ -142,14 +163,10 @@ static void check_ifd_version(char *image, int size)
 
 	switch (read_freq) {
 	case SPI_FREQUENCY_20MHZ:
-		ifd_version = IFD_VERSION_1;
-		max_regions = MAX_REGIONS_OLD;
-		break;
+		return IFD_VERSION_1;
 	case SPI_FREQUENCY_17MHZ:
 	case SPI_FREQUENCY_50MHZ_30MHZ:
-		ifd_version = IFD_VERSION_2;
-		max_regions = MAX_REGIONS;
-		break;
+		return IFD_VERSION_2;
 	default:
 		fprintf(stderr, "Unknown descriptor version: %d\n",
 			read_freq);
@@ -157,6 +174,19 @@ static void check_ifd_version(char *image, int size)
 	}
 }
 
+static void check_ifd_version(char *image, int size)
+{
+	if (is_platform_ifd_2())
+		ifd_version = IFD_VERSION_2;
+	else
+		ifd_version = get_ifd_version_from_fcba(image, size);
+
+	if (ifd_version == IFD_VERSION_1)
+		max_regions = MAX_REGIONS_OLD;
+	else
+		max_regions = MAX_REGIONS;
+}
+
 static region_t get_region(const frba_t *frba, unsigned int region_type)
 {
 	int base_mask;
@@ -816,7 +846,8 @@ static void lock_descriptor(const char *filename, char *image, int size)
 	}
 
 	switch (platform) {
-	case PLATFORM_APOLLOLAKE:
+	case PLATFORM_APL:
+	case PLATFORM_GLK:
 		/* CPU/BIOS can read descriptor and BIOS */
 		fmba->flmstr1 |= 0x3 << rd_shift;
 		/* CPU/BIOS can write BIOS */
@@ -1153,6 +1184,9 @@ static void print_usage(const char *name)
 	       "   -u | --unlock                      Unlock firmware descriptor and ME region\n"
 	       "   -p | --platform                    Add platform-specific quirks\n"
 	       "                                      aplk - Apollo Lake\n"
+	       "                                      cnl - Cannon Lake\n"
+	       "                                      glk - Gemini Lake\n"
+	       "                                      sklkbl - Skylake/Kaby Lake\n"
 	       "   -v | --version:                    print the version\n"
 	       "   -h | --help:                       print this help\n\n"
 	       "<region> is one of Descriptor, BIOS, ME, GbE, Platform\n"
@@ -1342,13 +1376,18 @@ int main(int argc, char *argv[])
 			break;
 		case 'p':
 			if (!strcmp(optarg, "aplk")) {
-				platform = PLATFORM_APOLLOLAKE;
+				platform = PLATFORM_APL;
+			} else if (!strcmp(optarg, "cnl")) {
+				platform = PLATFORM_CNL;
+			} else if (!strcmp(optarg, "glk")) {
+				platform = PLATFORM_GLK;
 			} else if (!strcmp(optarg, "sklkbl")) {
 				platform = PLATFORM_SKLKBL;
 			} else {
 				fprintf(stderr, "Unknown platform: %s\n", optarg);
 				exit(EXIT_FAILURE);
 			}
+			fprintf(stderr, "Platform is: %s\n", optarg);
 			break;
 		case 'v':
 			print_version();

util/ifdtool/ifdtool.h

@@ -22,7 +22,9 @@ enum ifd_version {
 };
 
 enum platform {
-	PLATFORM_APOLLOLAKE,
+	PLATFORM_APL,
+	PLATFORM_CNL,
+	PLATFORM_GLK,
 	PLATFORM_SKLKBL,
 };