[coreboot-gerrit] Change in coreboot[master]: arch/x86: Fix car_active for CONFIG_NO_CAR_GLOBAL_MIGRATION

[Furquan Shaikh (Code Review)]

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/29555


Change subject: arch/x86: Fix car_active for CONFIG_NO_CAR_GLOBAL_MIGRATION
......................................................................

arch/x86: Fix car_active for CONFIG_NO_CAR_GLOBAL_MIGRATION

Change 76ab2b7 ("arch/x86: allow global .bss objects without
CAR_GLOBAL") allowed use of global .bss objects and hence moved around
the macros resulting in car_active returning 0 even for those boards
where CAR is actually active but do not require global migration. This
resulted in boards getting stuck when doing a reset in verstage because
the code flow incorrectly assumed that there was no CAR active and
hence triggered a cache invalidate.

This change fixes the above issue by returning 1 for car_active if
ENV_CACHE_AS_RAM is set even if global migration is not required.

BUG=b:109717603
TEST=Verified that board reset does not trigger cache invalidate in
verstage and does not result in board hang.

Change-Id: I182f3e4277c57d6c50f7fcac2be72514896b3c61
Signed-off-by: Furquan Shaikh <furquan at google.com>
---
M src/arch/x86/include/arch/early_variables.h
1 file changed, 17 insertions(+), 1 deletion(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/29555/1

diff --git a/src/arch/x86/include/arch/early_variables.h b/src/arch/x86/include/arch/early_variables.h
index e95a45c..68cb9ff 100644
--- a/src/arch/x86/include/arch/early_variables.h
+++ b/src/arch/x86/include/arch/early_variables.h
@@ -79,12 +79,28 @@
 }
 
 #else
+
+/*
+ * We might end up here if:
+ * 1. ENV_CACHE_AS_RAM is not set for the stage or
+ * 2. ENV_CACHE_AS_RAM is set for the stage but CONFIG_NO_CAR_GLOBAL_MIGRATION
+ * is also set. In this case, there is no need to migrate CAR global
+ * variables. But, since we might still be running out of CAR, car_active needs
+ * to return 1 if ENV_CACHE_AS_RAM is set.
+ */
+
 #define CAR_GLOBAL
 static inline void *car_get_var_ptr(void *var) { return var; }
+
+#if ENV_CACHE_AS_RAM
+static inline int car_active(void) { return 1; }
+#else
 static inline int car_active(void) { return 0; }
+#endif /* ENV_CACHE_AS_RAM */
+
 #define car_get_var(var) (var)
 #define car_sync_var(var) (var)
 #define car_set_var(var, val)	(var) = (val)
-#endif
+#endif /* ENV_CACHE_AS_RAM && !IS_ENABLED(CONFIG_NO_CAR_GLOBAL_MIGRATION) */
 
 #endif

-- 
To view, visit https://review.coreboot.org/29555
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I182f3e4277c57d6c50f7fcac2be72514896b3c61
Gerrit-Change-Number: 29555
Gerrit-PatchSet: 1
Gerrit-Owner: Furquan Shaikh <furquan at google.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20181109/1ef7a8e9/attachment.html>