[coreboot-gerrit] Change in coreboot[master]: x86/x86/car: Remove PLATFORM_USES_FSP1_0 layering violation

Anonymous Coward (Code Review) gerrit at coreboot.org
Sat Mar 18 21:39:24 CET 2017


Anonymous Coward #1001579 has uploaded a new change for review. ( https://review.coreboot.org/18890 )

Change subject: x86/x86/car: Remove PLATFORM_USES_FSP1_0 layering violation
......................................................................

x86/x86/car: Remove PLATFORM_USES_FSP1_0 layering violation

This was a hack to enable FSP 1.0 support, but was never implemented
properly. Also, the way FSP 1.0 works, it makes too many controlling
assumptions about the underlying platform, and is fundamentally
incompatible with our boot flow.

The correct way to hanle this would have been to provide hooks that
FSP 1.0 platforms would use to handle the situation elegantly. Don't
bother refactoring the code, but just remove the layering violation.

Change-Id: I54e4e1ec7ac4b1555d91d5e2f29cd1702eb332f3
Signed-off-by: Adurb Akhbar <aakhbar at mail.com>
---
M src/cpu/x86/car.c
1 file changed, 9 insertions(+), 26 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/18890/1

diff --git a/src/cpu/x86/car.c b/src/cpu/x86/car.c
index 606a35e..d4a2088 100644
--- a/src/cpu/x86/car.c
+++ b/src/cpu/x86/car.c
@@ -20,9 +20,6 @@
 #include <arch/early_variables.h>
 #include <symbols.h>
 
-#if IS_ENABLED(CONFIG_PLATFORM_USES_FSP1_0)
-#include <drivers/intel/fsp1_0/fsp_util.h>
-#endif
 typedef void (* const car_migration_func_t)(void);
 
 extern car_migration_func_t _car_migrate_start;
@@ -46,8 +43,8 @@
 {
 	char *migrated_base = NULL;
 	int offset;
-	void *_car_start = _car_relocatable_data_start;
-	void *_car_end = _car_relocatable_data_end;
+	void * _car_start = _car_relocatable_data_start;
+	void * _car_end = _car_relocatable_data_end;
 
 	/* If the cache-as-ram has not been migrated return the pointer
 	 * passed in. */
@@ -61,18 +58,11 @@
 		return var;
 	}
 
-#if IS_ENABLED(CONFIG_PLATFORM_USES_FSP1_0)
-	migrated_base = (char *)find_saved_temp_mem(
-			*(void **)CBMEM_FSP_HOB_PTR);
-	/* FSP 1.0 migrates the entire DCACHE RAM */
-	offset = (char *)var - (char *)CONFIG_DCACHE_RAM_BASE;
-#else
 	migrated_base = cbmem_find(CBMEM_ID_CAR_GLOBALS);
 	offset = (char *)var - (char *)_car_start;
-#endif
 
 	if (migrated_base == NULL)
-		die("CAR: Could not find migration base!\n");
+		die( "CAR: Could not find migration base!\n");
 
 	return &migrated_base[offset];
 }
@@ -83,23 +73,17 @@
  */
 void *car_sync_var_ptr(void *var)
 {
-	void **mig_var = car_get_var_ptr(var);
-	void *_car_start = _car_relocatable_data_start;
-	void *_car_end = _car_relocatable_data_end;
+	void ** mig_var = car_get_var_ptr(var);
+	void * _car_start = _car_relocatable_data_start;
+	void * _car_end = _car_relocatable_data_end;
 
 	/* Not moved or migrated yet. */
 	if (mig_var == var)
 		return mig_var;
 
-	/*
-	 * Migrate the cbmem console pointer for FSP 1.0 platforms. Otherwise,
-	 * keep console buffer in CAR until cbmemc_reinit() moves it.
-	 */
-	if (*mig_var == _preram_cbmem_console) {
-		if (IS_ENABLED(CONFIG_PLATFORM_USES_FSP1_0))
-			*mig_var += (char *)mig_var - (char *)var;
+	/* Keep console buffer in CAR until cbmemc_reinit() moves it. */
+	if (*mig_var == _preram_cbmem_console)
 		return mig_var;
-	}
 
 	/* It's already pointing outside car.global_data. */
 	if (*mig_var < _car_start || *mig_var > _car_end)
@@ -137,7 +121,6 @@
 
 static void car_migrate_variables(int is_recovery)
 {
-	if (!IS_ENABLED(PLATFORM_USES_FSP1_0))
-		do_car_migrate_variables();
+	do_car_migrate_variables();
 }
 ROMSTAGE_CBMEM_INIT_HOOK(car_migrate_variables)

-- 
To view, visit https://review.coreboot.org/18890
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I54e4e1ec7ac4b1555d91d5e2f29cd1702eb332f3
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Anonymous Coward #1001579



More information about the coreboot-gerrit mailing list