Hello Patrick Rudolph, Felix Held, Matt DeVillier, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30385
to look at the new patch set (#45).
Change subject: soc/intel/broadwell: Enable LPC/SIO setup in bootblock
......................................................................
soc/intel/broadwell: Enable LPC/SIO setup in bootblock
This allows for serial console during the bootblock and enables
bootblock console by default.
Change-Id: I7746e4f819486d6142c96bc4c7480076fbfdfbde
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/mainboard/google/jecht/Makefile.inc
A src/mainboard/google/jecht/bootblock.c
M src/soc/intel/broadwell/Kconfig
M src/soc/intel/broadwell/bootblock/pch.c
M src/soc/intel/broadwell/romstage/pch.c
M src/soc/intel/broadwell/romstage/romstage.c
6 files changed, 83 insertions(+), 52 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/30385/45
--
To view, visit https://review.coreboot.org/c/coreboot/+/30385
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7746e4f819486d6142c96bc4c7480076fbfdfbde
Gerrit-Change-Number: 30385
Gerrit-PatchSet: 45
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
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-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Hello Patrick Rudolph, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30384
to look at the new patch set (#44).
Change subject: nb/intel/broadwell: Add an option for where verstage starts
......................................................................
nb/intel/broadwell: Add an option for where verstage starts
Previously broadwell used a romcc bootblock and starting verstage in
romstage was madatory but with C_ENVIRONMENT_BOOTBLOCK it is also
possible to have a separate verstage.
This selects using a separate verstage by default but still keeps the
option around to use verstage in romstage.
With a separate verstage the romstage becomes an RW stage.
The mrc.bin however is only added to the RO COREBOOT fmap region as it
requires to be run at a specific offset. This means that coreboot will
have to jump from a RW region to the RO region for that binary and
back to that RW region after that binary is done initializing the
memory.
Change-Id: I900233cadb3c76da329fb98f93917570e633365f
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/mainboard/google/auron/Makefile.inc
M src/mainboard/google/jecht/Makefile.inc
M src/mainboard/intel/wtm2/Makefile.inc
M src/soc/intel/broadwell/Kconfig
M src/soc/intel/broadwell/Makefile.inc
M src/soc/intel/broadwell/romstage/raminit.c
6 files changed, 35 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/30384/44
--
To view, visit https://review.coreboot.org/c/coreboot/+/30384
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I900233cadb3c76da329fb98f93917570e633365f
Gerrit-Change-Number: 30384
Gerrit-PatchSet: 44
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
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-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Hello Patrick Rudolph, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30383
to look at the new patch set (#40).
Change subject: soc/intel/broadwell: Use C_ENVIRONMENT_BOOTBLOCK
......................................................................
soc/intel/broadwell: Use C_ENVIRONMENT_BOOTBLOCK
This puts the cache-as-ram init in the bootblock.
Before setting up cache as ram the microcode updates are applied.
This removes the possibility for a normal/fallback setup although
implementing this should be quite easy.
Setting up LPC in the bootblock to output console on SuperIOs is not
done in this patch, therefore BOOTBLOCK_CONSOLE is not yet selected.
Change-Id: I44eb6d380dea5b82e3f009a46381a0f611bb7935
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/soc/intel/broadwell/Kconfig
M src/soc/intel/broadwell/Makefile.inc
M src/soc/intel/broadwell/bootblock/cpu.c
M src/soc/intel/broadwell/bootblock/pch.c
M src/soc/intel/broadwell/bootblock/systemagent.c
M src/soc/intel/broadwell/romstage/Makefile.inc
M src/soc/intel/broadwell/romstage/romstage.c
7 files changed, 26 insertions(+), 76 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/30383/40
--
To view, visit https://review.coreboot.org/c/coreboot/+/30383
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I44eb6d380dea5b82e3f009a46381a0f611bb7935
Gerrit-Change-Number: 30383
Gerrit-PatchSet: 40
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
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-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30383 )
Change subject: soc/intel/broadwell: Use C_ENVIRONMENT_BOOTBLOCK
......................................................................
Patch Set 39:
(3 comments)
https://review.coreboot.org/#/c/30383/37/src/soc/intel/broadwell/Kconfig
File src/soc/intel/broadwell/Kconfig:
https://review.coreboot.org/#/c/30383/37/src/soc/intel/broadwell/Kconfig@117
PS37, Line 117: default 0x2000
> No idea actually if the mrc.bin uses our stack. It has a separate heap in DCACHE_RAM_MRC_VAR_SIZE. […]
Well, I don't really think that we could get into trouble
here. And compiling romcc purism/librem_bdw gives me a
romstage stack of `0xec0` bytes. So 0x2000 seems more than
enough.
https://review.coreboot.org/#/c/30383/37/src/soc/intel/broadwell/bootblock/…
File src/soc/intel/broadwell/bootblock/cpu.c:
https://review.coreboot.org/#/c/30383/37/src/soc/intel/broadwell/bootblock/…
PS37, Line 51:
> The only reason this was done, was to speed up finding the microcode updates. cache_as_ram. […]
Ack
https://review.coreboot.org/#/c/30383/37/src/soc/intel/broadwell/romstage/r…
File src/soc/intel/broadwell/romstage/romstage.c:
https://review.coreboot.org/#/c/30383/37/src/soc/intel/broadwell/romstage/r…
PS37, Line 68: struct romstage_params rp = {
: .bist = 0, /* checking done in the bootblock */
: .pei_data = NULL,
: };
> At the moment we try to have the same romstage interface with or without romcc bootblock (which pass […]
Ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/30383
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I44eb6d380dea5b82e3f009a46381a0f611bb7935
Gerrit-Change-Number: 30383
Gerrit-PatchSet: 39
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
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-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Sun, 12 May 2019 11:44:26 +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
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30383 )
Change subject: soc/intel/broadwell: Use C_ENVIRONMENT_BOOTBLOCK
......................................................................
Patch Set 39:
(1 comment)
https://review.coreboot.org/#/c/30383/37/src/soc/intel/broadwell/Kconfig
File src/soc/intel/broadwell/Kconfig:
https://review.coreboot.org/#/c/30383/37/src/soc/intel/broadwell/Kconfig@117
PS37, Line 117: default 0x2000
> It can only be statically decided if there is no dynamic stack […]
No idea actually if the mrc.bin uses our stack. It has a separate heap in DCACHE_RAM_MRC_VAR_SIZE.
It might be a better idea to use cpu/intel/car/romstage.c as it places stack guards.
--
To view, visit https://review.coreboot.org/c/coreboot/+/30383
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I44eb6d380dea5b82e3f009a46381a0f611bb7935
Gerrit-Change-Number: 30383
Gerrit-PatchSet: 39
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
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-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Sun, 12 May 2019 11:02:50 +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
Hello Patrick Rudolph, Felix Held, Matt DeVillier, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30385
to look at the new patch set (#44).
Change subject: soc/intel/broadwell: Enable LPC/SIO setup in bootblock
......................................................................
soc/intel/broadwell: Enable LPC/SIO setup in bootblock
This allows for serial console during the bootblock and enables
bootblock console by default.
Change-Id: I7746e4f819486d6142c96bc4c7480076fbfdfbde
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/mainboard/google/jecht/Makefile.inc
A src/mainboard/google/jecht/bootblock.c
M src/mainboard/google/jecht/romstage.c
M src/soc/intel/broadwell/Kconfig
M src/soc/intel/broadwell/bootblock/pch.c
A src/soc/intel/broadwell/include/soc/bootblock.h
M src/soc/intel/broadwell/romstage/pch.c
M src/soc/intel/broadwell/romstage/romstage.c
8 files changed, 112 insertions(+), 66 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/30385/44
--
To view, visit https://review.coreboot.org/c/coreboot/+/30385
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7746e4f819486d6142c96bc4c7480076fbfdfbde
Gerrit-Change-Number: 30385
Gerrit-PatchSet: 44
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
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-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Hello Patrick Rudolph, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30384
to look at the new patch set (#43).
Change subject: nb/intel/broadwell: Add an option for where verstage starts
......................................................................
nb/intel/broadwell: Add an option for where verstage starts
Previously broadwell used a romcc bootblock and starting verstage in
romstage was madatory but with C_ENVIRONMENT_BOOTBLOCK it is also
possible to have a separate verstage.
This selects using a separate verstage by default but still keeps the
option around to use verstage in romstage.
With a separate verstage the romstage becomes an RW stage.
The mrc.bin however is only added to the RO COREBOOT fmap region as it
requires to be run at a specific offset. This means that coreboot will
have to jump from a RW region to the RO region for that binary and
back to that RW region after that binary is done initializing the
memory.
Change-Id: I900233cadb3c76da329fb98f93917570e633365f
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/mainboard/google/auron/Makefile.inc
M src/mainboard/google/jecht/Makefile.inc
M src/mainboard/intel/wtm2/Makefile.inc
M src/soc/intel/broadwell/Kconfig
M src/soc/intel/broadwell/Makefile.inc
M src/soc/intel/broadwell/romstage/raminit.c
6 files changed, 35 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/30384/43
--
To view, visit https://review.coreboot.org/c/coreboot/+/30384
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I900233cadb3c76da329fb98f93917570e633365f
Gerrit-Change-Number: 30384
Gerrit-PatchSet: 43
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
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-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30383 )
Change subject: soc/intel/broadwell: Use C_ENVIRONMENT_BOOTBLOCK
......................................................................
Patch Set 39:
(1 comment)
https://review.coreboot.org/#/c/30383/37/src/soc/intel/broadwell/Kconfig
File src/soc/intel/broadwell/Kconfig:
https://review.coreboot.org/#/c/30383/37/src/soc/intel/broadwell/Kconfig@117
PS37, Line 117: default 0x2000
> I think the linker tells you if that is not enough?
It can only be statically decided if there is no dynamic stack
usage, e.g. local array with a size given by a function parameter.
I don't know if we got rid of it.
Oh, does the MRC use our stack?
--
To view, visit https://review.coreboot.org/c/coreboot/+/30383
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I44eb6d380dea5b82e3f009a46381a0f611bb7935
Gerrit-Change-Number: 30383
Gerrit-PatchSet: 39
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
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-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Sun, 12 May 2019 10:46:12 +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
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30384 )
Change subject: nb/intel/broadwell: Add an option for where verstage starts
......................................................................
Patch Set 42: Code-Review+2
(2 comments)
https://review.coreboot.org/#/c/30384/42//COMMIT_MSG
Commit Message:
https://review.coreboot.org/#/c/30384/42//COMMIT_MSG@20
PS42, Line 20: memory.
Took me a moment to realize why this is related. Might be worth
to remind the reader that separate verstage implies RW romstage :)
https://review.coreboot.org/#/c/30384/42/src/soc/intel/broadwell/romstage/r…
File src/soc/intel/broadwell/romstage/raminit.c:
https://review.coreboot.org/#/c/30384/42/src/soc/intel/broadwell/romstage/r…
PS42, Line 48: f
rather short identifier ^^
--
To view, visit https://review.coreboot.org/c/coreboot/+/30384
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I900233cadb3c76da329fb98f93917570e633365f
Gerrit-Change-Number: 30384
Gerrit-PatchSet: 42
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
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-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Sun, 12 May 2019 10:37:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment