Attention is currently required from: Hsuan Ting Chen.
Hello Hsuan Ting Chen,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/57048
to review the following change.
Change subject: [wip]vboot_logic: Set VB2_CONTEXT_EC_TRUSTED in verstage_main
......................................................................
[wip]vboot_logic: Set VB2_CONTEXT_EC_TRUSTED in verstage_main
We will introduce a new ctx field stores the current bootmode in
crrev/c/2944250 (ctx->bootmode), which will be leveraged in both
vboot flow and elog_add_boot_reason in coreboot.
In current steps of deciding bootmode, a function vb2ex_ec_trusted
is required. This function checks gpio EC_IN_RW pin and will return
'trusted' only if EC is not in RW. Therefore, we need to implement
similar utilities in coreboot.
We will deprecate vb2ex_ec_trusted and use the flag,
VB2_CONTEXT_EC_TRUSTED, in vboot, vb2api_fw_phase1 and set that flag
in coreboot, verstage_main.
Also add a help function get_ec_in_rw which needed to be implemented
per mainboard.
WIP:
Only support volteer, trogdor now.
BUG=b:177196147, b:181931817
BRANCH=none
TEST=TBD
Signed-off-by: Hsuan Ting Chen <roccochen(a)chromium.org>
Change-Id: I479c8f80e45cc524ba87db4293d19b29bdfa2192
---
M src/include/bootmode.h
M src/mainboard/google/trogdor/chromeos.c
M src/mainboard/google/volteer/chromeos.c
M src/security/vboot/bootmode.c
M src/security/vboot/vboot_logic.c
5 files changed, 22 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/57048/1
diff --git a/src/include/bootmode.h b/src/include/bootmode.h
index aadecba..6cfd751 100644
--- a/src/include/bootmode.h
+++ b/src/include/bootmode.h
@@ -11,6 +11,7 @@
int clear_recovery_mode_switch(void);
int get_wipeout_mode_switch(void);
int get_lid_switch(void);
+int get_ec_in_rw(void);
/* Return 1 if display initialization is required. 0 if not. */
int display_init_required(void);
diff --git a/src/mainboard/google/trogdor/chromeos.c b/src/mainboard/google/trogdor/chromeos.c
index c218388..8f86073 100644
--- a/src/mainboard/google/trogdor/chromeos.c
+++ b/src/mainboard/google/trogdor/chromeos.c
@@ -48,3 +48,9 @@
{
return gpio_irq_status(GPIO_H1_AP_INT);
}
+
+int get_ec_in_rw(void)
+{
+ /* Read GPIO_EC_IN_RW. */
+ return gpio_get(GPIO_EC_IN_RW);
+}
diff --git a/src/mainboard/google/volteer/chromeos.c b/src/mainboard/google/volteer/chromeos.c
index abd50c5..baa5913 100644
--- a/src/mainboard/google/volteer/chromeos.c
+++ b/src/mainboard/google/volteer/chromeos.c
@@ -32,3 +32,9 @@
gpios = variant_cros_gpios(&num);
chromeos_acpi_gpio_generate(gpios, num);
}
+
+int get_ec_in_rw(void)
+{
+ /* Read GPIO_EC_IN_RW. */
+ return gpio_get(GPIO_EC_IN_RW);
+}
diff --git a/src/security/vboot/bootmode.c b/src/security/vboot/bootmode.c
index 6c05109..0c18674 100644
--- a/src/security/vboot/bootmode.c
+++ b/src/security/vboot/bootmode.c
@@ -57,6 +57,11 @@
return 0;
}
+int __weak get_ec_in_rw(void)
+{
+ return 1;
+}
+
#if CONFIG(VBOOT_NO_BOARD_SUPPORT)
/**
* TODO: Create flash protection interface which implements get_write_protect_state.
diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c
index 5ea4916..03701fc 100644
--- a/src/security/vboot/vboot_logic.c
+++ b/src/security/vboot/vboot_logic.c
@@ -321,6 +321,10 @@
if (!CONFIG(VBOOT_MUST_REQUEST_DISPLAY) || CONFIG(VBOOT_ALWAYS_ENABLE_DISPLAY))
ctx->flags |= VB2_CONTEXT_DISPLAY_INIT;
+ /* EC is trusted if EC is not in RW */
+ if (!get_ec_in_rw())
+ ctx->flags |= VB2_CONTEXT_EC_TRUSTED;
+
/* Do early init (set up secdata and NVRAM, load GBB) */
printk(BIOS_INFO, "Phase 1\n");
rv = vb2api_fw_phase1(ctx);
--
To view, visit https://review.coreboot.org/c/coreboot/+/57048
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I479c8f80e45cc524ba87db4293d19b29bdfa2192
Gerrit-Change-Number: 57048
Gerrit-PatchSet: 1
Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org>
Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Jeff Chase, Tim Wawrzynczak, Edward O'Callaghan.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57037 )
Change subject: mb/google/hatch: don't override variant_ramstage_init() in baseboard
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> Instead of adding the same file to all puff variants, why not call `mainboard_puff_ramstage_init()` […]
This is what I'm thinking of, to be precise: https://paste.flashrom.org/view.php?id=3503
--
To view, visit https://review.coreboot.org/c/coreboot/+/57037
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic5e787c3d4c2f2c62a0dc868997b6e4c3da84c43
Gerrit-Change-Number: 57037
Gerrit-PatchSet: 1
Gerrit-Owner: Jeff Chase <jnchase(a)google.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Joe Tessler <jrt(a)google.com>
Gerrit-Attention: Jeff Chase <jnchase(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Thu, 19 Aug 2021 10:12:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Jeff Chase, Tim Wawrzynczak, Edward O'Callaghan.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57037 )
Change subject: mb/google/hatch: don't override variant_ramstage_init() in baseboard
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Instead of adding the same file to all puff variants, why not call `mainboard_puff_ramstage_init()` from `src/mainboard/google/hatch/ramstage.c` before or after the `variant_ramstage_init()` call?
if (CONFIG(BOARD_GOOGLE_BASEBOARD_PUFF))
mainboard_puff_ramstage_init();
variant_ramstage_init();
--
To view, visit https://review.coreboot.org/c/coreboot/+/57037
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic5e787c3d4c2f2c62a0dc868997b6e4c3da84c43
Gerrit-Change-Number: 57037
Gerrit-PatchSet: 1
Gerrit-Owner: Jeff Chase <jnchase(a)google.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Joe Tessler <jrt(a)google.com>
Gerrit-Attention: Jeff Chase <jnchase(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Thu, 19 Aug 2021 10:07:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/57046 )
Change subject: [RFC] PCI: Introduce `enable_mmconf()` function
......................................................................
[RFC] PCI: Introduce `enable_mmconf()` function
On platforms with PCI MMCONF support, MMCONF needs to be configured as
per the CONFIG_MMCONF_BASE_ADDRESS and CONFIG_MMCONF_BUS_NUMBER Kconfig
symbols. This is usually done as early as possible because the default
PCI config space functions rely on MMCONF being enabled.
TBD: Figure out a common place to enable MMCONF from.
Change-Id: I3f573f1c0d9a4cc3c25fe907d12778cd5d93241d
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
---
M src/device/Makefile.inc
A src/device/pci_mmio_cfg.c
M src/include/device/pci_mmio_cfg.h
M src/lib/bootblock.c
4 files changed, 17 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/57046/1
diff --git a/src/device/Makefile.inc b/src/device/Makefile.inc
index 70013c9..f18c048 100644
--- a/src/device/Makefile.inc
+++ b/src/device/Makefile.inc
@@ -35,6 +35,8 @@
ramstage-y += pci_ops.c
smm-y += pci_ops.c
+bootblock-$(CONFIG_MMCONF_SUPPORT) += pci_mmio_cfg.c
+
ramstage-$(CONFIG_PCIX_PLUGIN_SUPPORT) += pcix_device.c
ramstage-$(CONFIG_PCIEXP_PLUGIN_SUPPORT) += pciexp_device.c
ramstage-$(CONFIG_CARDBUS_PLUGIN_SUPPORT) += cardbus_device.c
diff --git a/src/device/pci_mmio_cfg.c b/src/device/pci_mmio_cfg.c
new file mode 100644
index 0000000..8394e57
--- /dev/null
+++ b/src/device/pci_mmio_cfg.c
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <device/pci_ops.h>
+
+/* TODO: Remove once all platforms have been updated */
+__weak void enable_mmconf(void)
+{
+}
diff --git a/src/include/device/pci_mmio_cfg.h b/src/include/device/pci_mmio_cfg.h
index 8798405..b4faa73 100644
--- a/src/include/device/pci_mmio_cfg.h
+++ b/src/include/device/pci_mmio_cfg.h
@@ -105,6 +105,9 @@
#error "CONFIG_MMCONF_LENGTH does not correspond with CONFIG_MMCONF_BUS_NUMBER!"
#endif
+/* Defined in chipset code */
+void enable_mmconf(void);
+
/* Avoid name collisions as different stages have different signature
* for these functions. The _s_ stands for simple, fundamental IO or
* MMIO variant.
diff --git a/src/lib/bootblock.c b/src/lib/bootblock.c
index 5989964..1e08e70 100644
--- a/src/lib/bootblock.c
+++ b/src/lib/bootblock.c
@@ -5,6 +5,7 @@
#include <bootblock_common.h>
#include <console/console.h>
#include <delay.h>
+#include <device/pci_ops.h>
#include <metadata_hash.h>
#include <option.h>
#include <post.h>
@@ -41,6 +42,9 @@
timestamp_add_now(TS_START_BOOTBLOCK);
+ if (CONFIG(MMCONF_SUPPORT))
+ enable_mmconf();
+
bootblock_soc_early_init();
bootblock_mainboard_early_init();
--
To view, visit https://review.coreboot.org/c/coreboot/+/57046
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3f573f1c0d9a4cc3c25fe907d12778cd5d93241d
Gerrit-Change-Number: 57046
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newchange
Attention is currently required from: Sean Rhodes.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/57044
to look at the new patch set (#2).
Change subject: ec/starlabs/it8987: Adjust ACPI to be more complaint
......................................................................
ec/starlabs/it8987: Adjust ACPI to be more complaint
Fix build warning when EC binary not included when it shouldn't be
Make LID method compliant with SW_LID (Fixes two backlight controls)
Remove power button device
Add Kconfig option for including EC binary
Signed-off-by: Sean Rhodes <sean(a)starlabs.systems>
Change-Id: Id16881fd7901a2eaafc6108676ff429fb71c3571
---
M src/ec/starlabs/it8987/Kconfig
M src/ec/starlabs/it8987/Makefile.inc
M src/ec/starlabs/it8987/acpi/ac.asl
M src/ec/starlabs/it8987/acpi/battery.asl
M src/ec/starlabs/it8987/acpi/ec.asl
M src/ec/starlabs/it8987/acpi/hid.asl
M src/ec/starlabs/it8987/acpi/keyboard.asl
M src/ec/starlabs/it8987/acpi/lid.asl
M src/ec/starlabs/it8987/acpi/thermal.asl
M src/ec/starlabs/it8987/ec.c
M src/ec/starlabs/it8987/ec.h
11 files changed, 122 insertions(+), 320 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/57044/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/57044
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id16881fd7901a2eaafc6108676ff429fb71c3571
Gerrit-Change-Number: 57044
Gerrit-PatchSet: 2
Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-MessageType: newpatchset