[coreboot-gerrit] Change in coreboot[master]: vboot: Remove CHIPSET_PROVIDES_VERSTAGE_MAIN_SYMBOL Kconfig ...

[Julius Werner (Code Review)]

Hello Aaron Durbin, Patrick Georgi, Furquan Shaikh,

I'd like you to do a code review.  Please visit

    https://review.coreboot.org/18978

to review the following change.


Change subject: vboot: Remove CHIPSET_PROVIDES_VERSTAGE_MAIN_SYMBOL Kconfig option
......................................................................

vboot: Remove CHIPSET_PROVIDES_VERSTAGE_MAIN_SYMBOL Kconfig option

CHIPSET_PROVIDES_VERSTAGE_MAIN_SYMBOL allows the SoC directory to
provide its own main() symbol that can execute code before the generic
verstage code runs. We have now established in other places (e.g. T210
ramstage) a sort of convention that SoCs which need to run code in any
stage before main() should just override stage_entry() instead. This
patch aligns the verstage with that model and gets rid of the extra
Kconfig option. This also removes the need for aliasing between main()
and verstage(). Like other stages the main verstage code is now just in
main() and can be called from stage_entry().

Change-Id: If42c9c4fbab51fbd474e1530023a30b69495d1d6
Signed-off-by: Julius Werner <jwerner at chromium.org>
---
M src/arch/x86/verstage.c
M src/soc/nvidia/tegra124/Kconfig
M src/soc/nvidia/tegra124/verstage.c
M src/vboot/Kconfig
M src/vboot/vboot_common.h
M src/vboot/verstage.c
6 files changed, 7 insertions(+), 21 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/18978/1

diff --git a/src/arch/x86/verstage.c b/src/arch/x86/verstage.c
index 7987e1c..d24866b 100644
--- a/src/arch/x86/verstage.c
+++ b/src/arch/x86/verstage.c
@@ -14,10 +14,10 @@
  */
 
 #include <arch/cpu.h>
-#include <vendorcode/google/chromeos/chromeos.h>
+#include <main_decl.h>
 
 /* Provide an entry point for verstage when it's a separate stage. */
 asmlinkage void car_stage_entry(void)
 {
-	verstage();
+	main();
 }
diff --git a/src/soc/nvidia/tegra124/Kconfig b/src/soc/nvidia/tegra124/Kconfig
index 227efca..fdbbc7f 100644
--- a/src/soc/nvidia/tegra124/Kconfig
+++ b/src/soc/nvidia/tegra124/Kconfig
@@ -20,7 +20,6 @@
 	select VBOOT_OPROM_MATTERS
 	select VBOOT_STARTS_IN_BOOTBLOCK
 	select SEPARATE_VERSTAGE
-	select CHIPSET_PROVIDES_VERSTAGE_MAIN_SYMBOL
 
 config TEGRA124_MODEL_TD570D
 	bool "TD570D"
diff --git a/src/soc/nvidia/tegra124/verstage.c b/src/soc/nvidia/tegra124/verstage.c
index 9eee064..526f14e 100644
--- a/src/soc/nvidia/tegra124/verstage.c
+++ b/src/soc/nvidia/tegra124/verstage.c
@@ -45,9 +45,9 @@
 	early_mainboard_init();
 }
 
-void main(void)
+void stage_entry(void)
 {
 	asm volatile ("bl arm_init_caches"
 		      : : : "r0", "r1", "r2", "r3", "r4", "r5", "ip");
-	verstage();
+	main();
 }
diff --git a/src/vboot/Kconfig b/src/vboot/Kconfig
index e67c108..2b6cde1 100644
--- a/src/vboot/Kconfig
+++ b/src/vboot/Kconfig
@@ -95,13 +95,6 @@
 	  reused by the succeeding stage. This is useful if a RAM space is too
 	  small to fit both the verstage and the succeeding stage.
 
-config CHIPSET_PROVIDES_VERSTAGE_MAIN_SYMBOL
-	bool "The chipset provides the main() entry point for verstage"
-	default n
-	depends on SEPARATE_VERSTAGE
-	help
-	  The chipset code provides their own main() entry point.
-
 config VBOOT_DYNAMIC_WORK_BUFFER
 	bool "Vboot's work buffer is dynamically allocated."
 	default y if ARCH_ROMSTAGE_X86_32 && !SEPARATE_VERSTAGE
diff --git a/src/vboot/vboot_common.h b/src/vboot/vboot_common.h
index 956b54c..aa01f28 100644
--- a/src/vboot/vboot_common.h
+++ b/src/vboot/vboot_common.h
@@ -97,11 +97,10 @@
 
 /* ============================= VERSTAGE ================================== */
 /*
- * Main logic for verified boot. verstage() is the stage entry point
- * while the verstage_main() is just the core logic.
+ * Main logic for verified boot. verstage_main() is just the core vboot logic.
+ * If the verstage is a separate stage, it should be entered via main().
  */
 void verstage_main(void);
-void verstage(void);
 void verstage_mainboard_init(void);
 
 /* Check boot modes */
diff --git a/src/vboot/verstage.c b/src/vboot/verstage.c
index 0ec9ca6..64fadc7 100644
--- a/src/vboot/verstage.c
+++ b/src/vboot/verstage.c
@@ -24,7 +24,7 @@
 	/* Default empty implementation. */
 }
 
-void verstage(void)
+void main(void)
 {
 	console_init();
 	exception_init();
@@ -37,8 +37,3 @@
 		hlt();
 	}
 }
-
-#if !IS_ENABLED(CONFIG_CHIPSET_PROVIDES_VERSTAGE_MAIN_SYMBOL)
-/* This is for boards that rely on main() for an entry point of a stage. */
-void main(void) __attribute__((alias ("verstage")));
-#endif

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If42c9c4fbab51fbd474e1530023a30b69495d1d6
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi at google.com>