[coreboot-gerrit] Change in coreboot[master]: soc/intel/broadwell: Refactor MRC entry

[Angel Pons (Code Review)]

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47000 )

Change subject: soc/intel/broadwell: Refactor MRC entry
......................................................................

soc/intel/broadwell: Refactor MRC entry

Twist and bend the code so that it looks more like Haswell.

Change-Id: I03eda7efe022ec8d7b684926c28d0e3b546165b2
Signed-off-by: Angel Pons 
---
M src/soc/intel/broadwell/raminit.c
A src/soc/intel/broadwell/raminit.h
M src/soc/intel/broadwell/romstage.c
M src/soc/intel/broadwell/romstage.h
4 files changed, 107 insertions(+), 71 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/47000/1

diff --git a/src/soc/intel/broadwell/raminit.c b/src/soc/intel/broadwell/raminit.c
index f011679..5339daa 100644
--- a/src/soc/intel/broadwell/raminit.c
+++ b/src/soc/intel/broadwell/raminit.c
@@ -9,18 +9,50 @@
 #include <memory_info.h>
 #include <mrc_cache.h>
 #include <string.h>
-#if CONFIG(EC_GOOGLE_CHROMEEC)
-#include <ec/google/chromeec/ec.h>
-#include <ec/google/chromeec/ec_commands.h>
-#endif
-#include <vendorcode/google/chromeos/chromeos.h>
-#include <soc/intel/broadwell/memmap.h>
-#include <soc/intel/broadwell/pch/pm.h>
+#include <security/vboot/vboot_common.h>
 #include <soc/intel/broadwell/romstage.h>

#include "haswell.h"
 #include "pei_data.h"
 #include "pei_wrapper.h"
+#include "raminit.h"
+
+#define MRC_CACHE_VERSION 0
+
+void save_mrc_data(struct pei_data *pei_data)
+{
+	printk(BIOS_DEBUG, "MRC data at %p %d bytes\n", pei_data->data_to_save,
+	       pei_data->data_to_save_size);
+
+	if (pei_data->data_to_save == NULL || !pei_data->data_to_save_size)
+		return;
+
+	/* Save the MRC S3 restore data to cbmem */
+	mrc_cache_stash_data(MRC_TRAINING_DATA, MRC_CACHE_VERSION, pei_data->data_to_save,
+			     pei_data->data_to_save_size);
+}
+
+static void prepare_mrc_cache(struct pei_data *pei_data)
+{
+	size_t mrc_size;
+
+	/* Preset just in case there is an error */
+	pei_data->saved_data = NULL;
+	pei_data->saved_data_size = 0;
+
+	pei_data->saved_data =
+		mrc_cache_current_mmap_leak(MRC_TRAINING_DATA, MRC_CACHE_VERSION, &mrc_size);
+
+	if (!pei_data->saved_data) {
+		printk(BIOS_DEBUG, "No MRC cache found.\n");
+		return;
+	}
+
+	pei_data->saved_data_size = mrc_size;
+
+	printk(BIOS_DEBUG, "MRC data at %p %d bytes\n", pei_data->saved_data,
+	       pei_data->saved_data_size);
+}

static const char *const ecc_decoder[] = {
    "inactive",
@@ -29,10 +61,7 @@
    "active",
 };

-/*
- * Dump in the log memory controller configuration as read from the memory
- * controller registers.
- */
+/* Print out the memory controller configuration, as per the values in its registers. */
 static void report_memory_config(void)
 {
    int i;
@@ -72,47 +101,39 @@
    }
 }

-/*
+/**
  * Find PEI executable in coreboot filesystem and execute it.
+ *
+ * @param pei_data: configuration data for UEFI PEI reference code
  */
-void raminit(struct pei_data *pei_data)
+void sdram_initialize(struct pei_data *pei_data)
 {
-	size_t mrc_size;
-	struct memory_info *mem_info;
    pei_wrapper_entry_t entry;
-	int ret;
-	struct cbfsf f;
+
    uint32_t type = CBFS_TYPE_MRC;
+	struct cbfsf f;

-	broadwell_fill_pei_data(pei_data);
+	printk(BIOS_DEBUG, "Starting UEFI PEI System Agent\n");

-	if (CONFIG(BROADWELL_VBOOT_IN_BOOTBLOCK) &&
-	    vboot_recovery_mode_enabled()) {
+	if (CONFIG(BROADWELL_VBOOT_IN_BOOTBLOCK) && vboot_recovery_mode_enabled()) {
    	/* Recovery mode does not use MRC cache */
    	printk(BIOS_DEBUG, "Recovery mode: not using MRC cache.\n");
    } else {
    	/* Assume boot device is memory mapped. */
    	assert(CONFIG(BOOT_DEVICE_MEMORY_MAPPED));

-		pei_data->saved_data =
-			mrc_cache_current_mmap_leak(MRC_TRAINING_DATA, 0,
-						    &mrc_size);
-		if (pei_data->saved_data) {
-			/* MRC cache found */
-			pei_data->saved_data_size = mrc_size;
-		} else if (pei_data->boot_mode == ACPI_S3) {
-			/* Waking from S3 and no cache. */
-			printk(BIOS_DEBUG,
-			       "No MRC cache found in S3 resume path.\n");
-			post_code(POST_RESUME_FAILURE);
-			system_reset();
-		} else {
-			printk(BIOS_DEBUG, "No MRC cache found.\n");
-		}
+		prepare_mrc_cache(pei_data);
+	}
+
+	/* If MRC data is not found, we cannot continue S3 resume */
+	if (pei_data->boot_mode == ACPI_S3 && !pei_data->saved_data) {
+		post_code(POST_RESUME_FAILURE);
+		printk(BIOS_DEBUG, "Giving up in %s: No MRC data\n", __func__);
+		system_reset();
    }

/*
-	 * Do not use saved pei data.  Can be set by mainboard romstage
+	 * Do not use saved pei data. Can be set by mainboard romstage
     * to force a full train of memory on every boot.
     */
    if (pei_data->disable_saved_data) {
@@ -124,52 +145,37 @@
    /* Determine if mrc.bin is in the cbfs. */
    if (cbfs_locate_file_in_region(&f, "COREBOOT", "mrc.bin", &type) < 0)
    	die("mrc.bin not found!");
+
    /* We don't care about leaking the mapping */
-	entry = (pei_wrapper_entry_t)rdev_mmap_full(&f.data);
-	if (entry == NULL) {
-		printk(BIOS_DEBUG, "Couldn't find mrc.bin\n");
-		return;
+	entry = rdev_mmap_full(&f.data);
+	if (entry) {
+		int rv = entry(pei_data);
+
+		if (rv < 0)
+			die("pei_data version mismatch\n");
+	} else {
+		die("UEFI PEI System Agent not found.\n");
    }

-	printk(BIOS_DEBUG, "Starting Memory Reference Code\n");
-
-	ret = entry(pei_data);
-	if (ret < 0)
-		die("pei_data version mismatch\n");
-
-	/* Print the MRC version after executing the UEFI PEI stage. */
+	/* Print the MRC version after executing the UEFI PEI stage */
    u32 version = MCHBAR32(MRC_REVISION);
    printk(BIOS_DEBUG, "MRC Version %d.%d.%d Build %d\n",
    	(version >> 24) & 0xff, (version >> 16) & 0xff,
    	(version >>  8) & 0xff, (version >>  0) & 0xff);

report_memory_config();
+}

-	if (pei_data->boot_mode != ACPI_S3) {
-		cbmem_initialize_empty();
-	} else if (cbmem_initialize()) {
-		printk(BIOS_DEBUG, "Failed to recover CBMEM in S3 resume.\n");
-		/* Failed S3 resume, reset to come up cleanly */
-		system_reset();
-	}
+void setup_sdram_meminfo(struct pei_data *pei_data)
+{
+	struct memory_info *mem_info;

-	printk(BIOS_DEBUG, "MRC data at %p %d bytes\n", pei_data->data_to_save,
-	       pei_data->data_to_save_size);
-
-	if (pei_data->data_to_save != NULL && pei_data->data_to_save_size > 0)
-		mrc_cache_stash_data(MRC_TRAINING_DATA, 0,
-					pei_data->data_to_save,
-					pei_data->data_to_save_size);
-
-	printk(BIOS_DEBUG, "create cbmem for dimm information\n");
    mem_info = cbmem_add(CBMEM_ID_MEMINFO, sizeof(struct memory_info));
-
-	if (!mem_info) {
-		printk(BIOS_ERR, "Error! Failed to add mem_info to cbmem\n");
-		return;
-	}
+	if (!mem_info)
+		die("Failed to add memory info to CBMEM.\n");

memset(mem_info, 0, sizeof(*mem_info));
+
    /* Translate pei_memory_info struct data into memory_info struct */
    mem_info->dimm_cnt = pei_data->meminfo.dimm_cnt;
    for (int i = 0; i < MIN(DIMM_INFO_TOTAL, PEI_DIMM_INFO_TOTAL); i++) {
diff --git a/src/soc/intel/broadwell/raminit.h b/src/soc/intel/broadwell/raminit.h
new file mode 100644
index 0000000..e2363cb
--- /dev/null
+++ b/src/soc/intel/broadwell/raminit.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef RAMINIT_H
+#define RAMINIT_H
+
+#include <stdint.h>
+#include "pei_data.h"
+
+void sdram_initialize(struct pei_data *pei_data);
+void setup_sdram_meminfo(struct pei_data *pei_data);
+
+/* save_mrc_data() must be called after cbmem has been initialized. */
+void save_mrc_data(struct pei_data *pei_data);
+
+#endif	/* RAMINIT_H */
diff --git a/src/soc/intel/broadwell/romstage.c b/src/soc/intel/broadwell/romstage.c
index bea9feb..c47d60f 100644
--- a/src/soc/intel/broadwell/romstage.c
+++ b/src/soc/intel/broadwell/romstage.c
@@ -2,6 +2,8 @@

#include <acpi/acpi.h>
 #include <arch/romstage.h>
+#include <cbmem.h>
+#include <cf9_reset.h>
 #include <console/console.h>
 #include <cpu/intel/haswell/haswell.h>
 #include <elog.h>
@@ -17,6 +19,7 @@
 #include "pch/gpio.h"
 #include "pch/me.h"
 #include "pch/pch.h"
+#include "raminit.h"

/* Entry from cpu/intel/car/romstage.c. */
 void mainboard_romstage_entry(void)
@@ -64,11 +67,25 @@
    /* Save ME HSIO version */
    intel_me_hsio_version(&power_state->hsio_version, &power_state->hsio_checksum);

+	broadwell_fill_pei_data(&pei_data);
+
    /* Initialize RAM */
-	raminit(&pei_data);
+	sdram_initialize(&pei_data);

timestamp_add_now(TS_AFTER_INITRAM);

+	if (!s3resume) {
+		cbmem_initialize_empty();
+		/* Save data returned from MRC on non-S3 resumes. */
+		save_mrc_data(&pei_data);
+	} else if (cbmem_initialize()) {
+		printk(BIOS_DEBUG, "Failed to recover CBMEM in S3 resume.\n");
+		/* Failed S3 resume, reset to come up cleanly */
+		system_reset();
+	}
+
+	setup_sdram_meminfo(&pei_data);
+
    romstage_handoff_init(s3resume);

mainboard_post_raminit(s3resume);
diff --git a/src/soc/intel/broadwell/romstage.h b/src/soc/intel/broadwell/romstage.h
index d788155..d364317 100644
--- a/src/soc/intel/broadwell/romstage.h
+++ b/src/soc/intel/broadwell/romstage.h
@@ -8,8 +8,6 @@
 void mainboard_pre_raminit(struct pei_data *pei_data);
 void mainboard_post_raminit(const int s3resume);

-void raminit(struct pei_data *pei_data);
-
 void report_platform_info(void);

#endif