Change in coreboot[master]: arch/x86: Move generic functions from postcar_loader.c to stage_loader.c
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34751 )
Change subject: arch/x86: Move generic functions from postcar_loader.c to stage_loader.c ......................................................................
arch/x86: Move generic functions from postcar_loader.c to stage_loader.c
This patch moves few generic functions (finalize_load and stack_push) from postcar_loader.c to stage_loader.c file so that other callers can also use these functions.
Change-Id: I4d2200d95e68c0eb01681b8f9b71b3d30d122c43 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/Makefile.inc A src/arch/x86/include/arch/stage_loader.h M src/arch/x86/postcar_loader.c A src/arch/x86/stage_loader.c 4 files changed, 62 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/34751/1
diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc index 32e0173..2fad75e 100644 --- a/src/arch/x86/Makefile.inc +++ b/src/arch/x86/Makefile.inc @@ -239,6 +239,7 @@ romstage-y += memset.c romstage-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c romstage-y += postcar_loader.c +romstage-y += stage_loader.c romstage-$(CONFIG_COLLECT_TIMESTAMPS_TSC) += timestamp.c romstage-$(CONFIG_ARCH_ROMSTAGE_X86_32) += walkcbfs.S
diff --git a/src/arch/x86/include/arch/stage_loader.h b/src/arch/x86/include/arch/stage_loader.h new file mode 100644 index 0000000..23ac6e9 --- /dev/null +++ b/src/arch/x86/include/arch/stage_loader.h @@ -0,0 +1,32 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Intel Corp. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef STAGE_LOADER_H +#define STAGE_LOADER_H + +#include <arch/cpu.h> + +static inline void stack_push(struct postcar_frame *pcf, uint32_t val) +{ + uint32_t *ptr; + + pcf->stack -= sizeof(val); + ptr = (void *)pcf->stack; + *ptr = val; +} + +void finalize_load(uintptr_t *stack_top_ptr, uintptr_t stack_top); + +#endif /* STAGE_LOADER_H */ diff --git a/src/arch/x86/postcar_loader.c b/src/arch/x86/postcar_loader.c index 35e139f..3f42298 100644 --- a/src/arch/x86/postcar_loader.c +++ b/src/arch/x86/postcar_loader.c @@ -13,7 +13,7 @@ * GNU General Public License for more details. */
-#include <arch/cpu.h> +#include <arch/stage_loader.h> #include <cbmem.h> #include <console/console.h> #include <cpu/cpu.h> @@ -25,15 +25,6 @@ #include <stage_cache.h> #include <timestamp.h>
-static inline void stack_push(struct postcar_frame *pcf, uint32_t val) -{ - uint32_t *ptr; - - pcf->stack -= sizeof(val); - ptr = (void *)pcf->stack; - *ptr = val; -} - static void postcar_frame_prepare(struct postcar_frame *pcf) { msr_t msr; @@ -131,17 +122,6 @@ return (void *) pcf->stack; }
-static void finalize_load(uintptr_t *stack_top_ptr, uintptr_t stack_top) -{ - *stack_top_ptr = stack_top; - /* - * Signal to rest of system that another update was made to the - * postcar program prior to running it. - */ - prog_segment_loaded((uintptr_t)stack_top_ptr, sizeof(uintptr_t), - SEG_FINAL); -} - static void load_postcar_cbfs(struct prog *prog, struct postcar_frame *pcf) { struct rmod_stage_load rsl = { diff --git a/src/arch/x86/stage_loader.c b/src/arch/x86/stage_loader.c new file mode 100644 index 0000000..ca6a987 --- /dev/null +++ b/src/arch/x86/stage_loader.c @@ -0,0 +1,28 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Intel Corp. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <arch/stage_loader.h> +#include <stage_cache.h> + +void finalize_load(uintptr_t *stack_top_ptr, uintptr_t stack_top) +{ + *stack_top_ptr = stack_top; + /* + * Signal to rest of system that another update was made to the + * postcar program prior to running it. + */ + prog_segment_loaded((uintptr_t)stack_top_ptr, sizeof(uintptr_t), + SEG_FINAL); +}
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34751
to look at the new patch set (#4).
Change subject: arch/x86: Add new API for run_ramstage_phase() ......................................................................
arch/x86: Add new API for run_ramstage_phase()
This patch creates common static function to accomodate run_ramstage_phase() in same way run_postcar_phase() is working.
Change-Id: I4d2200d95e68c0eb01681b8f9b71b3d30d122c43 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/include/arch/cpu.h M src/arch/x86/postcar_loader.c 2 files changed, 33 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/34751/4
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34751 )
Change subject: arch/x86: Add new API for run_ramstage_phase() ......................................................................
Patch Set 4:
(1 comment)
I think you do need
https://review.coreboot.org/c/coreboot/+/34751/4/src/arch/x86/postcar_loader... File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/c/coreboot/+/34751/4/src/arch/x86/postcar_loader... PS4, Line 200: { You still need to make sure relocatable ramstage is selected. I think you can do that with a _staticassert here.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34751 )
Change subject: arch/x86: Add new API for run_ramstage_phase() ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34751/4/src/arch/x86/postcar_loader... File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/c/coreboot/+/34751/4/src/arch/x86/postcar_loader... PS4, Line 200: {
You still need to make sure relocatable ramstage is selected. […]
Done
Hello Aaron Durbin, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34751
to look at the new patch set (#5).
Change subject: arch/x86: Add new API for run_ramstage_phase() ......................................................................
arch/x86: Add new API for run_ramstage_phase()
This patch creates common static function to accomodate run_ramstage_phase() in same way run_postcar_phase() is working.
Change-Id: I4d2200d95e68c0eb01681b8f9b71b3d30d122c43 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/include/arch/cpu.h M src/arch/x86/postcar_loader.c 2 files changed, 35 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/34751/5
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34751 )
Change subject: arch/x86: Add new API for run_ramstage_phase() ......................................................................
Patch Set 5: Code-Review+1
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34751 )
Change subject: arch/x86: Add new API for run_ramstage_phase() ......................................................................
Patch Set 6:
Patch Set 5: Code-Review+1
removed _staticassert due to compilation error here https://qa.coreboot.org/job/coreboot-gerrit/101009/testReport/junit/(root)/b...
Hello Aaron Durbin, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34751
to look at the new patch set (#7).
Change subject: arch/x86: Add new API for run_ramstage_phase() ......................................................................
arch/x86: Add new API for run_ramstage_phase()
This patch creates common static function to accomodate run_ramstage_phase() in same way run_postcar_phase() is working.
Change-Id: I4d2200d95e68c0eb01681b8f9b71b3d30d122c43 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/include/arch/cpu.h M src/arch/x86/postcar_loader.c 2 files changed, 35 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/34751/7
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34751 )
Change subject: arch/x86: Add new API for run_ramstage_phase() ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34751/7/src/arch/x86/postcar_loader... File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/c/coreboot/+/34751/7/src/arch/x86/postcar_loader... PS7, Line 229: die("RELOCATABLE_RAMSTAGE must be selected!"); You could turn this into an #error as well instead of a runtime die(). I was thinking static assert would work -- but only if things are properly guarded. At that point #error is what we want.
Hello Aaron Durbin, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34751
to look at the new patch set (#8).
Change subject: arch/x86: Add new API for run_ramstage_phase() ......................................................................
arch/x86: Add new API for run_ramstage_phase()
This patch creates common static function to accomodate run_ramstage_phase() in same way run_postcar_phase() is working.
Change-Id: I4d2200d95e68c0eb01681b8f9b71b3d30d122c43 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/include/arch/cpu.h M src/arch/x86/postcar_loader.c 2 files changed, 41 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/34751/8
Hello Aaron Durbin, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34751
to look at the new patch set (#9).
Change subject: arch/x86: Add new API for run_ramstage_phase() ......................................................................
arch/x86: Add new API for run_ramstage_phase()
This patch creates common static function to accomodate run_ramstage_phase() in same way run_postcar_phase() is working.
Change-Id: I4d2200d95e68c0eb01681b8f9b71b3d30d122c43 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/include/arch/cpu.h M src/arch/x86/postcar_loader.c 2 files changed, 41 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/34751/9
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34751 )
Change subject: arch/x86: Add new API for run_ramstage_phase() ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34751/7/src/arch/x86/postcar_loader... File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/c/coreboot/+/34751/7/src/arch/x86/postcar_loader... PS7, Line 229: die("RELOCATABLE_RAMSTAGE must be selected!");
You could turn this into an #error as well instead of a runtime die(). […]
Done
Hello Aaron Durbin, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34751
to look at the new patch set (#10).
Change subject: arch/x86: Add new API for run_ramstage_phase() ......................................................................
arch/x86: Add new API for run_ramstage_phase()
This patch creates common static function to accomodate run_ramstage_phase() in same way run_postcar_phase() is working.
Also adds required provision in postcar_loader code to pick ramstage directly from romstage and avoid postcar as intermediate stage for car tear down.
When a platform is not using postcar stage it will use ramstage, hence it's by definition not tearing down cache-as-ram from within romstage prior to loading ramstage. Because of this property there's no need to migrate CAR_GLOBAL variables to cbmem.
Change-Id: I4d2200d95e68c0eb01681b8f9b71b3d30d122c43 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/include/arch/cpu.h M src/arch/x86/postcar_loader.c 2 files changed, 46 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/34751/10
Hello Aaron Durbin, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34751
to look at the new patch set (#11).
Change subject: arch/x86: Add new API for run_ramstage_phase() ......................................................................
arch/x86: Add new API for run_ramstage_phase()
This patch creates common static function to accomodate run_ramstage_phase() in same way run_postcar_phase() is working.
Also adds required provision in postcar_loader code to pick ramstage directly from romstage and avoid postcar as intermediate stage for car tear down.
When a platform is not using postcar stage it will use ramstage, hence it's by definition not tearing down cache-as-ram from within romstage prior to loading ramstage. Because of this property there's no need to migrate CAR_GLOBAL variables to cbmem.
Change-Id: I4d2200d95e68c0eb01681b8f9b71b3d30d122c43 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/postcar_loader.c 1 file changed, 46 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/34751/11
Hello Aaron Durbin, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34751
to look at the new patch set (#12).
Change subject: arch/x86: Add new API for run_ramstage_phase() ......................................................................
arch/x86: Add new API for run_ramstage_phase()
This patch creates common static function to accomodate run_ramstage_phase() in same way run_postcar_phase() is working.
Change-Id: I4d2200d95e68c0eb01681b8f9b71b3d30d122c43 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/postcar_loader.c 1 file changed, 43 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/34751/12
Hello Aaron Durbin, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34751
to look at the new patch set (#13).
Change subject: arch/x86: Refactor run_postcar_phase() implementation ......................................................................
arch/x86: Refactor run_postcar_phase() implementation
This patch creates static function to accomodate other stages (probably ramstage) in same way run_postcar_phase() is working.
Change-Id: I4d2200d95e68c0eb01681b8f9b71b3d30d122c43 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/postcar_loader.c 1 file changed, 26 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/34751/13
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34751 )
Change subject: arch/x86: Refactor run_postcar_phase() implementation ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34751/14/src/arch/x86/postcar_loade... File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/c/coreboot/+/34751/14/src/arch/x86/postcar_loade... PS14, Line 233: struct prog prog why isn't this being passed by reference?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34751 )
Change subject: arch/x86: Refactor run_postcar_phase() implementation ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34751/14/src/arch/x86/postcar_loade... File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/c/coreboot/+/34751/14/src/arch/x86/postcar_loade... PS14, Line 233: struct prog prog
why isn't this being passed by reference?
Done
Hello Aaron Durbin, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34751
to look at the new patch set (#15).
Change subject: arch/x86: Refactor run_postcar_phase() implementation ......................................................................
arch/x86: Refactor run_postcar_phase() implementation
This patch creates static function to accomodate other stages (probably ramstage) in same way run_postcar_phase() is working.
Change-Id: I4d2200d95e68c0eb01681b8f9b71b3d30d122c43 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/postcar_loader.c 1 file changed, 26 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/34751/15
Hello Aaron Durbin, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34751
to look at the new patch set (#17).
Change subject: arch/x86: Refactor run_postcar_phase() implementation ......................................................................
arch/x86: Refactor run_postcar_phase() implementation
This patch creates static function to accomodate other stages (probably ramstage) in same way run_postcar_phase() is working.
Change-Id: I4d2200d95e68c0eb01681b8f9b71b3d30d122c43 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/arch/x86/postcar_loader.c 1 file changed, 26 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/34751/17
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34751 )
Change subject: arch/x86: Refactor run_postcar_phase() implementation ......................................................................
Patch Set 17:
@Aaron, can you please help to take a look
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/34751?usp=email )
Change subject: arch/x86: Refactor run_postcar_phase() implementation ......................................................................
Abandoned
This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.
-
Aaron Durbin (Code Review) -
Martin L Roth (Code Review)
-
Subrata Banik (Code Review)