Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31902 )
Change subject: soc/intel/cannonlake: Clear PMCON status bits
......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/#/c/31902/4/src/soc/intel/cannonlake/finalize.c
File src/soc/intel/cannonlake/finalize.c:
https://review.coreboot.org/#/c/31902/4/src/soc/intel/cannonlake/finalize.c…
PS4, Line 56: pmc_clear_pmcon_sts
Shouldn't this function live in pmutil.c?
https://review.coreboot.org/#/c/31902/4/src/soc/intel/cannonlake/finalize.c…
PS4, Line 65: reg_val |= (SUS_PWR_FLR | GBL_RST_STS | HOST_RST_STS | PWR_FLR);
Why is this required? Since you are reading GEN_PMCON_A, writing back the same value & ~(MS4V) should take care of clearing any status bits?
--
To view, visit https://review.coreboot.org/c/coreboot/+/31902
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If9863d52ed3c61b6a160df53f023b0787eaaed68
Gerrit-Change-Number: 31902
Gerrit-PatchSet: 4
Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Lijian Zhao <lijian.zhao(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Fri, 22 Mar 2019 00:28:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31978 )
Change subject: Move calls to quick_ram_check() before CBMEM init
......................................................................
Patch Set 4:
Welp, that was a nasty rabbit hole. It turns out that the sudden jump in size has nothing to do with the actual code you're adding, but is triggered by an Arm errata linker workaround that we need to enable for the MT8173 SoC. When a certain pattern of instructions is placed at just the wrong offset, it may cause incorrect behavior and the linker must fix it up. One of the ways it may do that is adding an extra 4K of code.
The weird thing is that the added code isn't actually necessary for the way it is fixed up in our case. I think this is a binutils bug/limitation, I've filed https://sourceware.org/bugzilla/show_bug.cgi?id=24373 about it... but I wouldn't hold my breath about it being fixed anytime soon (it's a rare erratum for an old CPU revision, after all).
Anyway, for now either restricting this code to x86 or changing the MT8173 memlayout to make more space available sounds fine.
--
To view, visit https://review.coreboot.org/c/coreboot/+/31978
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I33d4153a5ce0908b8889517394afb46f1ca28f92
Gerrit-Change-Number: 31978
Gerrit-PatchSet: 4
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Damien Zammit <damien(a)zamaudio.com>
Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com>
Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 21 Mar 2019 21:33:26 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Mathew King has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32019
Change subject: grunt: Mark RW_LEGACY as CBFS
......................................................................
grunt: Mark RW_LEGACY as CBFS
Depthcharge is changing how the RW_LEGACY CBFS is handled for alternate
bootloaders, see https://crrev.com/c/1528550 and
https://crrev.com/c/1530303. This means that RW_LEGACY must be marked as
CBFS in the fmap in order to work. All boards except for kahlee(grunt)
have CBFS marked.
BUG=b:128703316
TEST=Build and ran on grunt along with chromium patches on grunt and was
able to list alternate bootloader with ctrl+l
BRANCH=none
Change-Id: I843d565a9503d27e666a34e59aba263ec490c81f
Signed-off-by: Mathew King <mathewk(a)chromium.org>
---
M src/mainboard/google/kahlee/variants/baseboard/chromeos.fmd
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/32019/1
diff --git a/src/mainboard/google/kahlee/variants/baseboard/chromeos.fmd b/src/mainboard/google/kahlee/variants/baseboard/chromeos.fmd
index 9d89a15..b746545 100644
--- a/src/mainboard/google/kahlee/variants/baseboard/chromeos.fmd
+++ b/src/mainboard/google/kahlee/variants/baseboard/chromeos.fmd
@@ -24,7 +24,7 @@
RW_NVRAM(PRESERVE)@0x467000 0x5000
RW_UNUSED@0x46C000 0x14000
SMMSTORE(PRESERVE)@0x480000 0x20000
- RW_LEGACY@0x4a0000 0x760000
+ RW_LEGACY(CBFS)@0x4a0000 0x760000
WP_RO@0xC00000 0x400000 {
RO_VPD(PRESERVE)@0x0 0x4000
--
To view, visit https://review.coreboot.org/c/coreboot/+/32019
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I843d565a9503d27e666a34e59aba263ec490c81f
Gerrit-Change-Number: 32019
Gerrit-PatchSet: 1
Gerrit-Owner: Mathew King <mathewk(a)chromium.org>
Gerrit-MessageType: newchange
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31978 )
Change subject: Move calls to quick_ram_check() before CBMEM init
......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/#/c/31978/4/src/include/lib.h
File src/include/lib.h:
https://review.coreboot.org/#/c/31978/4/src/include/lib.h@30
PS4, Line 30: void quick_ram_check_or_die(uintptr_t dst);
Add comment that this is just touching 32-bits of memory?
Also, only when memory location is uncacheable does this function do anything. I think that should be documented as well.
--
To view, visit https://review.coreboot.org/c/coreboot/+/31978
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I33d4153a5ce0908b8889517394afb46f1ca28f92
Gerrit-Change-Number: 31978
Gerrit-PatchSet: 4
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Damien Zammit <damien(a)zamaudio.com>
Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com>
Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 21 Mar 2019 18:22:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31978 )
Change subject: Move calls to quick_ram_check() before CBMEM init
......................................................................
Patch Set 4:
(1 comment)
google/rowan is a dead board, feel free to delete it if it causes trouble... but I'm also skeptical of the 4K number. I don't see how this patch could add anywhere close to that much. Maybe a sign that something unexpected is happening.
https://review.coreboot.org/#/c/31978/4/src/lib/ramtest.c
File src/lib/ramtest.c:
https://review.coreboot.org/#/c/31978/4/src/lib/ramtest.c@22
PS4, Line 22: #else
This code doesn't look ready to run on other architectures. On Arm we enable DRAM caches before initializing CBMEM, so this would do nothing. If we want to use this in generic code, we should either create an <arch/...> API for non-temporal read/write that all architectures can implement, or move this onto another API (we already have dcache_clean_invalidate_all() available everywhere, but that's probably a bit too big of a hammer... we could expand the dcache_clean_invalidate_by_mva() APIs from Arm onto other architectures).
--
To view, visit https://review.coreboot.org/c/coreboot/+/31978
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I33d4153a5ce0908b8889517394afb46f1ca28f92
Gerrit-Change-Number: 31978
Gerrit-PatchSet: 4
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Damien Zammit <damien(a)zamaudio.com>
Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com>
Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 21 Mar 2019 18:21:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31978 )
Change subject: Move calls to quick_ram_check() before CBMEM init
......................................................................
Patch Set 4:
> Patch Set 4:
>
> Board google/rowan with chromeos has about 4 KiB free in romstage before this commit, and no longer fits with this. The entire test is kind of redundant, we might as well fail when unable to setup IMD/CBMEM root signatures.
I'm not sure what you mean here. quick ram test takes 4 KiB of code... so we can't unconditionally enable it?
--
To view, visit https://review.coreboot.org/c/coreboot/+/31978
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I33d4153a5ce0908b8889517394afb46f1ca28f92
Gerrit-Change-Number: 31978
Gerrit-PatchSet: 4
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Damien Zammit <damien(a)zamaudio.com>
Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com>
Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 21 Mar 2019 18:17:53 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31978 )
Change subject: Move calls to quick_ram_check() before CBMEM init
......................................................................
Patch Set 4:
Board google/rowan with chromeos has about 4 KiB free in romstage before this commit, and no longer fits with this. The entire test is kind of redundant, we might as well fail when unable to setup IMD/CBMEM root signatures.
--
To view, visit https://review.coreboot.org/c/coreboot/+/31978
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I33d4153a5ce0908b8889517394afb46f1ca28f92
Gerrit-Change-Number: 31978
Gerrit-PatchSet: 4
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 21 Mar 2019 18:09:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Joel Kitching has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31897
Change subject: vboot: remove VBOOT_EC_EFS Kconfig option
......................................................................
vboot: remove VBOOT_EC_EFS Kconfig option
This option has been relocated to depthcharge:
https://crrev.com/c/1523248
BUG=b:124141368, b:124192753
TEST=Build and deploy to eve
TEST=util/lint/checkpatch.pl -g origin/master..HEAD
TEST=util/abuild/abuild -B -e -y -c 50 -p none -x
TEST=make clean && make test-abuild
CQ-DEPEND=CL:1523248
BRANCH=none
Change-Id: I8b3740c8301f9a193f4fce2c6492d9382730faa1
Signed-off-by: Joel Kitching <kitching(a)google.com>
---
M src/mainboard/google/fizz/Kconfig
M src/security/vboot/Kconfig
M src/security/vboot/vboot_handoff.c
3 files changed, 0 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/31897/1
diff --git a/src/mainboard/google/fizz/Kconfig b/src/mainboard/google/fizz/Kconfig
index 6f05c24..3c8215b 100644
--- a/src/mainboard/google/fizz/Kconfig
+++ b/src/mainboard/google/fizz/Kconfig
@@ -39,7 +39,6 @@
config VBOOT
select EC_GOOGLE_CHROMEEC_SWITCHES
- select VBOOT_EC_EFS
select VBOOT_PHYSICAL_REC_SWITCH
select HAS_RECOVERY_MRC_CACHE
select MRC_CLEAR_NORMAL_CACHE_ON_RECOVERY_RETRAIN
diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig
index a382e67..8d3d83f 100644
--- a/src/security/vboot/Kconfig
+++ b/src/security/vboot/Kconfig
@@ -193,15 +193,6 @@
Whether the EC (or PD) is slow to update and needs to display a
screen that informs the user the update is happening.
-config VBOOT_EC_EFS
- bool
- default n
- depends on VBOOT_EC_SOFTWARE_SYNC
- help
- CrosEC can support EFS: Early Firmware Selection. If it's enabled,
- software sync need to also support it. This setting tells vboot to
- perform EFS software sync.
-
config VBOOT_PHYSICAL_DEV_SWITCH
bool
default n
diff --git a/src/security/vboot/vboot_handoff.c b/src/security/vboot/vboot_handoff.c
index 2bb26a8..eaac76a 100644
--- a/src/security/vboot/vboot_handoff.c
+++ b/src/security/vboot/vboot_handoff.c
@@ -85,8 +85,6 @@
vb_sd->flags |= VBSD_EC_SOFTWARE_SYNC;
if (CONFIG(VBOOT_EC_SLOW_UPDATE))
vb_sd->flags |= VBSD_EC_SLOW_UPDATE;
- if (CONFIG(VBOOT_EC_EFS))
- vb_sd->flags |= VBSD_EC_EFS;
}
if (!CONFIG(VBOOT_PHYSICAL_REC_SWITCH))
vb_sd->flags |= VBSD_BOOT_REC_SWITCH_VIRTUAL;
--
To view, visit https://review.coreboot.org/c/coreboot/+/31897
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8b3740c8301f9a193f4fce2c6492d9382730faa1
Gerrit-Change-Number: 31897
Gerrit-PatchSet: 1
Gerrit-Owner: Joel Kitching <kitching(a)google.com>
Gerrit-MessageType: newchange