Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36144 )
Change subject: [RFC,POF] pass cbmem_top via the stack to ramstage ......................................................................
[RFC,POF] pass cbmem_top via the stack to ramstage
Currently all stages that need cbmem need an implementation of a cbmem_top function. On FSP and AGESA platforms this proves to be painful and using the top of lower memory if often passed via lower memory or via a PCI scratchpad register.
The problem with writing to lower memory is that also need to be written on S3 as one cannot assume it to be still there. Writing things on S3 is always a fragile thing to do.
A very generic solution is to pass cbmem_top via the program argument. It should be possible to implement this solution on every architecture.
TODO this can be done on all arch
TESTED on qemu-x86.
Change-Id: I6d5a366d6f1bc76f26d459628237e6b2c8ae03ea Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/c_start.S M src/arch/x86/cbmem.c M src/lib/prog_loaders.c M src/mainboard/emulation/qemu-i440fx/memmap.c M src/northbridge/intel/i945/memmap.c 5 files changed, 28 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/36144/1
diff --git a/src/arch/x86/c_start.S b/src/arch/x86/c_start.S index 32b848d..6177773 100644 --- a/src/arch/x86/c_start.S +++ b/src/arch/x86/c_start.S @@ -18,6 +18,7 @@ .section .bss, "aw", @nobits .global _stack .global _estack +.global _cbmem_top_ptr
/* Stack alignment is not enforced with rmodule loader, reserve one * extra CPU such that alignment can be enforced on entry. */ @@ -30,6 +31,8 @@ thread_stacks: .space CONFIG_STACK_SIZE*CONFIG_NUM_THREADS #endif +_cbmem_top_ptr: + .long 0
.section ".text._start", "ax", @progbits #ifdef __x86_64__ @@ -59,6 +62,9 @@
cld
+ pop %eax + movl %eax, _cbmem_top_ptr + /** poison the stack. Code should not count on the * stack being full of zeros. This stack poisoning * recently uncovered a bug in the broadcast SIPI diff --git a/src/arch/x86/cbmem.c b/src/arch/x86/cbmem.c index 16c35b5..6b1581c 100644 --- a/src/arch/x86/cbmem.c +++ b/src/arch/x86/cbmem.c @@ -13,6 +13,7 @@
#include <stdlib.h> #include <cbmem.h> +#include <console/console.h>
#if CONFIG(CBMEM_TOP_BACKUP)
@@ -34,3 +35,13 @@ }
#endif /* CBMEM_TOP_BACKUP */ + +extern const uintptr_t _cbmem_top_ptr; + +#if ENV_RAMSTAGE +void *cbmem_top(void) +{ + printk(BIOS_DEBUG, "_cbmem_top_ptr: 0x%08x\n", (u32)_cbmem_top_ptr); + return (void *)_cbmem_top_ptr; +} +#endif diff --git a/src/lib/prog_loaders.c b/src/lib/prog_loaders.c index 2ef6bdf..a806f41 100644 --- a/src/lib/prog_loaders.c +++ b/src/lib/prog_loaders.c @@ -142,6 +142,10 @@
timestamp_add_now(TS_END_COPYRAM);
+ ramstage.arg = cbmem_top(); + + printk(BIOS_DEBUG, "ramstage arg: 0x%08x\n", (u32)ramstage.arg); + prog_run(&ramstage);
fail: diff --git a/src/mainboard/emulation/qemu-i440fx/memmap.c b/src/mainboard/emulation/qemu-i440fx/memmap.c index 8209379..5ec6a9f 100644 --- a/src/mainboard/emulation/qemu-i440fx/memmap.c +++ b/src/mainboard/emulation/qemu-i440fx/memmap.c @@ -16,6 +16,7 @@ #include <cbmem.h> #include <arch/io.h> #include <arch/romstage.h> +#include <console/console.h> #include "memory.h" #include "fw_cfg.h"
@@ -52,6 +53,7 @@ return tomk; }
+#if !ENV_RAMSTAGE void *cbmem_top(void) { uintptr_t top = 0; @@ -60,8 +62,10 @@ if (!top) top = (uintptr_t)qemu_get_memory_size() * 1024;
+ printk(BIOS_DEBUG, "cbmem_top(): 0x%08x\n", (u32)top); return (void *)top; } +#endif
/* Nothing to do, MTRRs are no-op on QEMU. */ void fill_postcar_frame(struct postcar_frame *pcf) diff --git a/src/northbridge/intel/i945/memmap.c b/src/northbridge/intel/i945/memmap.c index 8207d06..c7e5d39 100644 --- a/src/northbridge/intel/i945/memmap.c +++ b/src/northbridge/intel/i945/memmap.c @@ -71,11 +71,14 @@ * 1 MiB alignment. As this may cause very greedy MTRR setup, push * CBMEM top downwards to 4 MiB boundary. */ + +#if !ENV_RAMSTAGE void *cbmem_top(void) { uintptr_t top_of_ram = ALIGN_DOWN(northbridge_get_tseg_base(), 4*MiB); return (void *) top_of_ram; } +#endif
/** Decodes used Graphics Mode Select (GMS) to kilobytes. */ u32 decode_igd_memory_size(const u32 gms)
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36144 )
Change subject: [RFC,POF] pass cbmem_top via the stack to ramstage ......................................................................
Patch Set 1:
This is an alternative to CB:36140
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36144
to look at the new patch set (#2).
Change subject: [RFC,POF] pass cbmem_top via calling arguments to ramstage ......................................................................
[RFC,POF] pass cbmem_top via calling arguments to ramstage
Currently all stages that need cbmem need an implementation of a cbmem_top function. On FSP and AGESA platforms this proves to be painful and a pointer to the top of lower memory if often passed via lower memory (e.g. EBDA) or via a PCI scratchpad register.
The problem with writing to lower memory is that also need to be written on S3 as one cannot assume it to be still there. Writing things on S3 is always a fragile thing to do.
A very generic solution is to pass cbmem_top via the program argument. It should be possible to implement this solution on every architecture.
TESTED on qemu-x86.
Change-Id: I6d5a366d6f1bc76f26d459628237e6b2c8ae03ea Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/c_start.S M src/arch/x86/cbmem.c M src/lib/prog_loaders.c M src/mainboard/emulation/qemu-i440fx/memmap.c M src/northbridge/intel/i945/memmap.c 5 files changed, 27 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/36144/2
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36144
to look at the new patch set (#3).
Change subject: arch/x86: Use a common cbmem_top implementation ......................................................................
arch/x86: Use a common cbmem_top implementation
Currently all stages that need cbmem need an implementation of a cbmem_top function. On FSP and AGESA platforms this proves to be painful and a pointer to the top of lower memory if often passed via lower memory (e.g. EBDA) or via a PCI scratchpad register.
The problem with writing to lower memory is that also need to be written on S3 as one cannot assume it to be still there. Writing things on S3 is always a fragile thing to do.
A very generic solution is to pass cbmem_top via the program argument. It should be possible to implement this solution on every architecture.
Instead trying to figure out which files can be removed from stages and which cbmem_top implementations need with preprocessor, rename all cbmem_top implementation to cbmem_top_romstage.
Mechanisms set in place to pass on information from rom- to ram-stage will be removed in a followup commit.
TESTED on qemu-x86.
Change-Id: I6d5a366d6f1bc76f26d459628237e6b2c8ae03ea Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/Kconfig M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/lib/Makefile.inc M src/mainboard/emulation/qemu-i440fx/memmap.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/memmap.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/picasso/memmap.c M src/soc/amd/stoneyridge/memmap.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/braswell/memmap.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_broadwell_de/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/quark/memmap.c M src/soc/intel/skylake/memmap.c 28 files changed, 30 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/36144/3
Hello Patrick Rudolph, Vanny E, Huang Jin, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Damien Zammit, David Guckian, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36144
to look at the new patch set (#4).
Change subject: arch/x86: Use a common cbmem_top implementation ......................................................................
arch/x86: Use a common cbmem_top implementation
Currently all stages that need cbmem need an implementation of a cbmem_top function. On FSP and AGESA platforms this proves to be painful and a pointer to the top of lower memory if often passed via lower memory (e.g. EBDA) or via a PCI scratchpad register.
The problem with writing to lower memory is that also need to be written on S3 as one cannot assume it to be still there. Writing things on S3 is always a fragile thing to do.
A very generic solution is to pass cbmem_top via the program argument. It should be possible to implement this solution on every architecture.
Instead trying to figure out which files can be removed from stages and which cbmem_top implementations need with preprocessor, rename all cbmem_top implementation to cbmem_top_romstage.
Mechanisms set in place to pass on information from rom- to ram-stage will be removed in a followup commit.
TESTED on qemu-x86.
Change-Id: I6d5a366d6f1bc76f26d459628237e6b2c8ae03ea Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/Kconfig M src/arch/x86/cbmem.c M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/lib/Makefile.inc M src/mainboard/emulation/qemu-i440fx/memmap.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/memmap.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/picasso/memmap.c M src/soc/amd/stoneyridge/memmap.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/braswell/memmap.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_broadwell_de/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/quark/memmap.c M src/soc/intel/skylake/memmap.c 29 files changed, 30 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/36144/4
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36144 )
Change subject: arch/x86: Use a common cbmem_top implementation ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36144/4/src/arch/x86/cbmem.c File src/arch/x86/cbmem.c:
https://review.coreboot.org/c/coreboot/+/36144/4/src/arch/x86/cbmem.c@19 PS4, Line 19: void *cbmem_top_romstage(void) This function isn't declared.
https://review.coreboot.org/c/coreboot/+/36144/4/src/arch/x86/cbmem.c@24 PS4, Line 24: if (ENV_RAMSTAGE && cbmem_top_backup != NULL) Clean this up as well? If this function is only used in romstage then we should remove all of this? Or make it work with the original intent.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36144 )
Change subject: arch/x86: Use a common cbmem_top implementation ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36144/4/src/arch/x86/cbmem.c File src/arch/x86/cbmem.c:
https://review.coreboot.org/c/coreboot/+/36144/4/src/arch/x86/cbmem.c@19 PS4, Line 19: void *cbmem_top_romstage(void)
This function isn't declared.
It is done in the previous patch.
https://review.coreboot.org/c/coreboot/+/36144/4/src/arch/x86/cbmem.c@24 PS4, Line 24: if (ENV_RAMSTAGE && cbmem_top_backup != NULL)
Clean this up as well? If this function is only used in romstage then we should remove all of this? Or make it work with the original intent.
This idea was to clean things up in a separate patch to remove the custom backing up mechanisms. Hence the renaming of all cbmem_top implementation to make the linker happy.
Hello Patrick Rudolph, Vanny E, Huang Jin, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Damien Zammit, David Guckian, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36144
to look at the new patch set (#5).
Change subject: arch/x86: Use a common cbmem_top implementation ......................................................................
arch/x86: Use a common cbmem_top implementation
Currently all stages that need cbmem need an implementation of a cbmem_top function. On FSP and AGESA platforms this proves to be painful and a pointer to the top of lower memory if often passed via lower memory (e.g. EBDA) or via a PCI scratchpad register.
The problem with writing to lower memory is that also need to be written on S3 as one cannot assume it to be still there. Writing things on S3 is always a fragile thing to do.
A very generic solution is to pass cbmem_top via the program argument. It should be possible to implement this solution on every architecture.
Instead trying to figure out which files can be removed from stages and which cbmem_top implementations need with preprocessor, rename all cbmem_top implementation to cbmem_top_romstage.
Mechanisms set in place to pass on information from rom- to ram-stage will be replaced in a followup commit.
TESTED on qemu-x86.
Change-Id: I6d5a366d6f1bc76f26d459628237e6b2c8ae03ea Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/Kconfig M src/arch/x86/cbmem.c M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/lib/Makefile.inc M src/mainboard/emulation/qemu-i440fx/memmap.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/memmap.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/picasso/memmap.c M src/soc/amd/stoneyridge/memmap.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/braswell/memmap.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_broadwell_de/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/quark/memmap.c M src/soc/intel/skylake/memmap.c 29 files changed, 30 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/36144/5
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36144 )
Change subject: arch/x86: Use a common cbmem_top implementation ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36144/4/src/arch/x86/cbmem.c File src/arch/x86/cbmem.c:
https://review.coreboot.org/c/coreboot/+/36144/4/src/arch/x86/cbmem.c@24 PS4, Line 24: if (ENV_RAMSTAGE && cbmem_top_backup != NULL)
Clean this up as well? If this function is only used in romstage then we should remove all of this […]
Done in CB:36358.
Hello Patrick Rudolph, Vanny E, Huang Jin, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Damien Zammit, David Guckian, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36144
to look at the new patch set (#14).
Change subject: arch/x86: Use a common cbmem_top implementation ......................................................................
arch/x86: Use a common cbmem_top implementation
Currently all stages that need cbmem need an implementation of a cbmem_top function. On FSP and AGESA platforms this proves to be painful and a pointer to the top of lower memory if often passed via lower memory (e.g. EBDA) or via a PCI scratchpad register.
The problem with writing to lower memory is that also need to be written on S3 as one cannot assume it to be still there. Writing things on S3 is always a fragile thing to do.
A very generic solution is to pass cbmem_top via the program argument. It should be possible to implement this solution on every architecture.
Instead trying to figure out which files can be removed from stages and which cbmem_top implementations need with preprocessor, rename all cbmem_top implementation to cbmem_top_romstage.
Mechanisms set in place to pass on information from rom- to ram-stage will be replaced in a followup commit.
TESTED on qemu-x86.
Change-Id: I6d5a366d6f1bc76f26d459628237e6b2c8ae03ea Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/Kconfig M src/arch/x86/cbmem.c M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/lib/Makefile.inc M src/mainboard/emulation/qemu-i440fx/memmap.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/memmap.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/picasso/memmap.c M src/soc/amd/stoneyridge/memmap.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/braswell/memmap.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_broadwell_de/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/quark/memmap.c M src/soc/intel/skylake/memmap.c 29 files changed, 30 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/36144/14
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36144 )
Change subject: arch/x86: Use a common cbmem_top implementation ......................................................................
Patch Set 14: Code-Review+1
I'm still thinking we should squash everything together as the sequence of patches doesn't do anything until this one.
Hello Aaron Durbin, Patrick Rudolph, Vanny E, Huang Jin, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Damien Zammit, David Guckian, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36144
to look at the new patch set (#15).
Change subject: arch/x86: Use the stage argument to implement cbmem_top ......................................................................
arch/x86: Use the stage argument to implement cbmem_top
Currently all stages that need cbmem need an implementation of a cbmem_top function. On FSP and AGESA platforms this proves to be painful and a pointer to the top of lower memory if often passed via lower memory (e.g. EBDA) or via a PCI scratchpad register.
The problem with writing to lower memory is that also need to be written on S3 as one cannot assume it to be still there. Writing things on S3 is always a fragile thing to do.
A very generic solution is to pass cbmem_top via the program argument. It should be possible to implement this solution on every architecture.
Instead trying to figure out which files can be removed from stages and which cbmem_top implementations need with preprocessor, rename all cbmem_top implementation to cbmem_top_romstage.
TESTED on qemu-x86.
Change-Id: I6d5a366d6f1bc76f26d459628237e6b2c8ae03ea Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/Kconfig M src/arch/x86/Makefile.inc M src/arch/x86/c_start.S M src/arch/x86/cbmem.c M src/arch/x86/exit_car.S M src/lib/Makefile.inc M src/northbridge/intel/e7505/Makefile.inc M src/northbridge/intel/fsp_rangeley/Makefile.inc M src/northbridge/intel/i440bx/Makefile.inc M src/soc/intel/quark/Makefile.inc 10 files changed, 20 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/36144/15
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36144 )
Change subject: arch/x86: Use the stage argument to implement cbmem_top ......................................................................
Patch Set 15:
(2 comments)
Patch Set 14: Code-Review+1
I'm still thinking we should squash everything together as the sequence of patches doesn't do anything until this one.
Done. Now this implementation is used everywhere and there is a Kconfig option to use the populated global variable.
https://review.coreboot.org/c/coreboot/+/36144/4/src/arch/x86/cbmem.c File src/arch/x86/cbmem.c:
https://review.coreboot.org/c/coreboot/+/36144/4/src/arch/x86/cbmem.c@19 PS4, Line 19: void *cbmem_top_romstage(void)
This function isn't declared. […]
Done
https://review.coreboot.org/c/coreboot/+/36144/4/src/arch/x86/cbmem.c@24 PS4, Line 24: if (ENV_RAMSTAGE && cbmem_top_backup != NULL)
Done in CB:36358.
Done
Hello Aaron Durbin, Patrick Rudolph, Vanny E, Huang Jin, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Damien Zammit, David Guckian, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36144
to look at the new patch set (#18).
Change subject: arch/x86: Use the stage argument to implement cbmem_top ......................................................................
arch/x86: Use the stage argument to implement cbmem_top
Currently all stages that need cbmem need an implementation of a cbmem_top function. On FSP and AGESA platforms this proves to be painful and a pointer to the top of lower memory if often passed via lower memory (e.g. EBDA) or via a PCI scratchpad register.
The problem with writing to lower memory is that also need to be written on S3 as one cannot assume it to be still there. Writing things on S3 is always a fragile thing to do.
A very generic solution is to pass cbmem_top via the program argument. It should be possible to implement this solution on every architecture.
Instead trying to figure out which files can be removed from stages and which cbmem_top implementations need with preprocessor, rename all cbmem_top implementation to cbmem_top_romstage.
TESTED on qemu-x86.
Change-Id: I6d5a366d6f1bc76f26d459628237e6b2c8ae03ea Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/Kconfig M src/arch/x86/Makefile.inc M src/arch/x86/c_start.S M src/arch/x86/cbmem.c M src/arch/x86/exit_car.S M src/northbridge/intel/e7505/Makefile.inc M src/northbridge/intel/fsp_rangeley/Makefile.inc M src/northbridge/intel/i440bx/Makefile.inc M src/soc/intel/quark/Makefile.inc 9 files changed, 18 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/36144/18
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36144 )
Change subject: arch/x86: Use the stage argument to implement cbmem_top ......................................................................
Patch Set 18: Code-Review+2
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36144 )
Change subject: arch/x86: Use the stage argument to implement cbmem_top ......................................................................
Patch Set 18: Code-Review+2
Hello Aaron Durbin, Patrick Rudolph, Vanny E, Huang Jin, Marshall Dawson, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Damien Zammit, David Guckian, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36144
to look at the new patch set (#19).
Change subject: arch/x86: Use the stage argument to implement cbmem_top ......................................................................
arch/x86: Use the stage argument to implement cbmem_top
Currently all stages that need cbmem need an implementation of a cbmem_top function. On FSP and AGESA platforms this proves to be painful and a pointer to the top of lower memory if often passed via lower memory (e.g. EBDA) or via a PCI scratchpad register.
The problem with writing to lower memory is that also need to be written on S3 as one cannot assume it to be still there. Writing things on S3 is always a fragile thing to do.
A very generic solution is to pass cbmem_top via the program argument. It should be possible to implement this solution on every architecture.
Instead trying to figure out which files can be removed from stages and which cbmem_top implementations need with preprocessor, rename all cbmem_top implementation to cbmem_top_romstage.
TESTED on qemu-x86.
Change-Id: I6d5a366d6f1bc76f26d459628237e6b2c8ae03ea Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/Kconfig M src/arch/x86/Makefile.inc M src/arch/x86/c_start.S M src/arch/x86/cbmem.c M src/arch/x86/exit_car.S M src/northbridge/intel/e7505/Makefile.inc M src/northbridge/intel/fsp_rangeley/Makefile.inc M src/northbridge/intel/i440bx/Makefile.inc M src/soc/intel/quark/Makefile.inc 9 files changed, 18 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/36144/19
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36144 )
Change subject: arch/x86: Use the stage argument to implement cbmem_top ......................................................................
Patch Set 19:
I lost the +2 in a rebase. Can you reinstate?
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36144 )
Change subject: arch/x86: Use the stage argument to implement cbmem_top ......................................................................
Patch Set 19: Code-Review+2
Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36144 )
Change subject: arch/x86: Use the stage argument to implement cbmem_top ......................................................................
arch/x86: Use the stage argument to implement cbmem_top
Currently all stages that need cbmem need an implementation of a cbmem_top function. On FSP and AGESA platforms this proves to be painful and a pointer to the top of lower memory if often passed via lower memory (e.g. EBDA) or via a PCI scratchpad register.
The problem with writing to lower memory is that also need to be written on S3 as one cannot assume it to be still there. Writing things on S3 is always a fragile thing to do.
A very generic solution is to pass cbmem_top via the program argument. It should be possible to implement this solution on every architecture.
Instead trying to figure out which files can be removed from stages and which cbmem_top implementations need with preprocessor, rename all cbmem_top implementation to cbmem_top_romstage.
TESTED on qemu-x86.
Change-Id: I6d5a366d6f1bc76f26d459628237e6b2c8ae03ea Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/36144 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/arch/x86/Kconfig M src/arch/x86/Makefile.inc M src/arch/x86/c_start.S M src/arch/x86/cbmem.c M src/arch/x86/exit_car.S M src/northbridge/intel/e7505/Makefile.inc M src/northbridge/intel/fsp_rangeley/Makefile.inc M src/northbridge/intel/i440bx/Makefile.inc M src/soc/intel/quark/Makefile.inc 9 files changed, 18 insertions(+), 21 deletions(-)
Approvals: build bot (Jenkins): Verified Marshall Dawson: Looks good to me, approved
diff --git a/src/arch/x86/Kconfig b/src/arch/x86/Kconfig index 502e774..8ce5977 100644 --- a/src/arch/x86/Kconfig +++ b/src/arch/x86/Kconfig @@ -16,6 +16,7 @@ default n select PCI select RELOCATABLE_MODULES + select RAMSTAGE_CBMEM_TOP_ARG
# stage selectors for x86
diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc index 8d00174..447fd57 100644 --- a/src/arch/x86/Makefile.inc +++ b/src/arch/x86/Makefile.inc @@ -259,7 +259,6 @@ postcar-$(CONFIG_HAVE_ACPI_RESUME) += acpi_s3.c postcar-y += gdt_init.S postcar-y += cbfs_and_run.c -postcar-y += cbmem.c postcar-$(CONFIG_EARLY_EBDA_INIT) += ebda.c postcar-$(CONFIG_IDT_IN_EVERY_STAGE) += exception.c postcar-$(CONFIG_IDT_IN_EVERY_STAGE) += idt.S @@ -299,7 +298,6 @@ ramstage-$(CONFIG_HAVE_ACPI_RESUME) += acpi_s3.c ramstage-$(CONFIG_ACPI_BERT) += acpi_bert_storage.c ramstage-y += c_start.S -ramstage-y += cbmem.c ramstage-y += cpu.c ramstage-y += ebda.c ramstage-y += exception.c diff --git a/src/arch/x86/c_start.S b/src/arch/x86/c_start.S index 43d7802..bd99c21 100644 --- a/src/arch/x86/c_start.S +++ b/src/arch/x86/c_start.S @@ -60,6 +60,14 @@
cld
+#ifdef __x86_64__ + mov %rdi, _cbmem_top_ptr +#else + /* The return argument is at 0(%esp), the calling argument at 4(%esp) */ + movl 4(%esp), %eax + movl %eax, _cbmem_top_ptr +#endif + /** poison the stack. Code should not count on the * stack being full of zeros. This stack poisoning * recently uncovered a bug in the broadcast SIPI diff --git a/src/arch/x86/cbmem.c b/src/arch/x86/cbmem.c index f7c58a4..fc85bc6 100644 --- a/src/arch/x86/cbmem.c +++ b/src/arch/x86/cbmem.c @@ -18,19 +18,8 @@
void *cbmem_top_chipset(void) { - static void *cbmem_top_backup; - void *top_backup; - - if (ENV_RAMSTAGE && cbmem_top_backup != NULL) - return cbmem_top_backup; - /* Top of CBMEM is at highest usable DRAM address below 4GiB. */ - top_backup = (void *)restore_top_of_low_cacheable(); - - if (ENV_RAMSTAGE) - cbmem_top_backup = top_backup; - - return top_backup; + return (void *)restore_top_of_low_cacheable(); }
#endif /* CBMEM_TOP_BACKUP */ diff --git a/src/arch/x86/exit_car.S b/src/arch/x86/exit_car.S index 679e335e..8c28784 100644 --- a/src/arch/x86/exit_car.S +++ b/src/arch/x86/exit_car.S @@ -31,6 +31,14 @@ /* Migrate GDT to this text segment */ call gdt_init
+#ifdef __x86_64__ + mov %rdi, _cbmem_top_ptr +#else + /* The return argument is at 0(%esp), the calling argument at 4(%esp) */ + movl 4(%esp), %eax + movl %eax, _cbmem_top_ptr +#endif + /* chipset_teardown_car() is expected to disable cache-as-ram. */ call chipset_teardown_car
diff --git a/src/northbridge/intel/e7505/Makefile.inc b/src/northbridge/intel/e7505/Makefile.inc index 4eda3d1..9b68e13 100644 --- a/src/northbridge/intel/e7505/Makefile.inc +++ b/src/northbridge/intel/e7505/Makefile.inc @@ -6,5 +6,4 @@ romstage-y += raminit.c romstage-y += memmap.c
-postcar-y += memmap.c endif diff --git a/src/northbridge/intel/fsp_rangeley/Makefile.inc b/src/northbridge/intel/fsp_rangeley/Makefile.inc index a167c23..f02e3c4 100644 --- a/src/northbridge/intel/fsp_rangeley/Makefile.inc +++ b/src/northbridge/intel/fsp_rangeley/Makefile.inc @@ -18,7 +18,6 @@
subdirs-y += fsp ramstage-y += northbridge.c -ramstage-y += memmap.c
ramstage-y += acpi.c ramstage-y += port_access.c diff --git a/src/northbridge/intel/i440bx/Makefile.inc b/src/northbridge/intel/i440bx/Makefile.inc index 2c503c6..355d9b2 100644 --- a/src/northbridge/intel/i440bx/Makefile.inc +++ b/src/northbridge/intel/i440bx/Makefile.inc @@ -17,12 +17,9 @@ ifeq ($(CONFIG_NORTHBRIDGE_INTEL_I440BX),y)
ramstage-y += northbridge.c -ramstage-y += memmap.c
romstage-y += raminit.c romstage-$(CONFIG_DEBUG_RAM_SETUP) += debug.c romstage-y += memmap.c
-postcar-y += memmap.c - endif diff --git a/src/soc/intel/quark/Makefile.inc b/src/soc/intel/quark/Makefile.inc index cff0891..3a58cc9 100644 --- a/src/soc/intel/quark/Makefile.inc +++ b/src/soc/intel/quark/Makefile.inc @@ -41,7 +41,6 @@
postcar-y += fsp_params.c postcar-y += i2c.c -postcar-y += memmap.c postcar-y += reg_access.c postcar-y += tsc_freq.c postcar-$(CONFIG_ENABLE_BUILTIN_HSUART1) += uart_common.c @@ -53,7 +52,6 @@ ramstage-y += gpio_i2c.c ramstage-y += i2c.c ramstage-y += lpc.c -ramstage-y += memmap.c ramstage-y += northcluster.c ramstage-y += reg_access.c ramstage-y += reset.c
-
Aaron Durbin (Code Review) -
Arthur Heymans (Code Review)
-
Marshall Dawson (Code Review)