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(a)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)
--
To view, visit https://review.coreboot.org/c/coreboot/+/36144
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6d5a366d6f1bc76f26d459628237e6b2c8ae03ea
Gerrit-Change-Number: 36144
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newchange
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36591 )
Change subject: console: serial: add optional delay to wait for BMC serial routing
......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36591/3/src/console/init.c
File src/console/init.c:
https://review.coreboot.org/c/coreboot/+/36591/3/src/console/init.c@79
PS3, Line 79: if (!car_get_var(console_inited))
: mdelay(CONFIG_CONSOLE_DELAY_MS);
This is run on every stage... You only need to wait once before console starts working I presume?
--
To view, visit https://review.coreboot.org/c/coreboot/+/36591
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7d6599b76384fc94a00a9cfc1794ebfe34863ff9
Gerrit-Change-Number: 36591
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Jacob Garber <jgarber1(a)ualberta.ca>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sun, 03 Nov 2019 11:05:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36591 )
Change subject: console: serial: add optional delay to wait for BMC serial routing
......................................................................
Patch Set 3:
(1 comment)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/36591/1/src/console/Kconfig
File src/console/Kconfig:
https://review.coreboot.org/c/coreboot/+/36591/1/src/console/Kconfig@161
PS1, Line 161: depends on BOOTBLOCK_CONSOLE
> You could potentially read-write-poll the UART baudrate divisor register. […]
@Kyösti no, UART works, but the BMC redirects it to one of it's five UART outputs, so this wouldn't help at all
--
To view, visit https://review.coreboot.org/c/coreboot/+/36591
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7d6599b76384fc94a00a9cfc1794ebfe34863ff9
Gerrit-Change-Number: 36591
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Jacob Garber <jgarber1(a)ualberta.ca>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sun, 03 Nov 2019 11:00:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Comment-In-Reply-To: Patrick Rudolph <siro(a)das-labor.org>
Comment-In-Reply-To: Michael Niewöhner
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36570 )
Change subject: soc/intel/common: pmclib: make use of the new ETR address API
......................................................................
Patch Set 4:
alternative approach in CB:35451
--
To view, visit https://review.coreboot.org/c/coreboot/+/36570
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I49f59efb4a7c7d3d629ac54a7922bbcc8a87714d
Gerrit-Change-Number: 36570
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sun, 03 Nov 2019 10:54:58 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36558 )
Change subject: arch/riscv: Pass cbmem_top to ramstage via calling argument
......................................................................
Patch Set 3:
> Patch Set 3:
>
> Have a look at Stage handoff protocol here:
> https://doc.coreboot.org/arch/riscv/index.html
>
> a1 contains a pointer to fdt and not to cbmem_top.
That is not the case anymore when calling ramstage, because cbmem_top is put in the program argument. See run_ramstage() in lib/prog_loaders.c. Besides, I'm sure that the stage_entry() function does not properly contain the calling argument that supposedly contains a pointer to the fdt (or cbmem_top in this case) in ramstage.
--
To view, visit https://review.coreboot.org/c/coreboot/+/36558
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5c74cd5d3ee292931c5bbd2e4075f88381429f72
Gerrit-Change-Number: 36558
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com>
Gerrit-Comment-Date: Sun, 03 Nov 2019 10:51:14 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36570 )
Change subject: soc/intel/common: pmclib: make use of the new ETR address API
......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36570/4/src/soc/intel/common/block…
File src/soc/intel/common/block/pmc/pmclib.c:
https://review.coreboot.org/c/coreboot/+/36570/4/src/soc/intel/common/block…
PS4, Line 434: soc_read_pmc_etr_addr();
> IMO, that would be overkill. If it were NULL or unaligned, it wouldn't do […]
I guess when this was null, we had a bigger problem and wouldn't even reach that code
--
To view, visit https://review.coreboot.org/c/coreboot/+/36570
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I49f59efb4a7c7d3d629ac54a7922bbcc8a87714d
Gerrit-Change-Number: 36570
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sun, 03 Nov 2019 10:21:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36592 )
Change subject: mb/supermicro/x11-lga1151-series: use new console delay option
......................................................................
Uploaded patch set 2: Patch Set 1 was rebased.
--
To view, visit https://review.coreboot.org/c/coreboot/+/36592
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8bf4ef7ad9beea7b3dc22e1567623a423597eff9
Gerrit-Change-Number: 36592
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sun, 03 Nov 2019 10:12:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36591 )
Change subject: console: serial: add optional delay to wait for BMC serial routing
......................................................................
Uploaded patch set 2.
--
To view, visit https://review.coreboot.org/c/coreboot/+/36591
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7d6599b76384fc94a00a9cfc1794ebfe34863ff9
Gerrit-Change-Number: 36591
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Jacob Garber <jgarber1(a)ualberta.ca>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sun, 03 Nov 2019 10:12:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment