coreboot-gerrit March 2024

coreboot-gerrit@coreboot.org

1 participants
3668 discussions

[M] Change in coreboot[main]: Support for creating hybrid vboot images
by Vladimir Serbinenko (Code Review) 24 Mar '24

24 Mar '24

Attention is currently required from: Julius Werner, Nick Vaccaro, Yu-Ping Wu. Hello Julius Werner, Nick Vaccaro, Yu-Ping Wu, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81508?usp=email to look at the new patch set (#2). Change subject: Support for creating hybrid vboot images ...................................................................... Support for creating hybrid vboot images This allows creating an image where RO is triggered by default with normal RW secrets locked out. On a signal in CMOS, clear the signal and follow normal RW_A/RW_B path. This allows dual-boot between stock ChromeOS and an alternative payload while keeping compatibility with ChromeOS updates. Change-Id: I9b26c332f5bf6befd62b5930b19d1b20e76261e7 Signed-off-by: Vladimir Serbinenko <phcoder(a)gmail.com> --- M src/drivers/mrc_cache/mrc_cache.c M src/mainboard/google/volteer/Kconfig M src/security/vboot/Kconfig M src/security/vboot/misc.h M src/security/vboot/vboot_common.h M src/security/vboot/vboot_loader.c M src/security/vboot/vboot_logic.c M src/soc/intel/common/block/cse/cse_eop.c 8 files changed, 111 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/81508/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81508?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9b26c332f5bf6befd62b5930b19d1b20e76261e7 Gerrit-Change-Number: 81508 Gerrit-PatchSet: 2 Gerrit-Owner: Vladimir Serbinenko <phcoder(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: Support smmstore in RW_LEGACY
by Vladimir Serbinenko (Code Review) 24 Mar '24

24 Mar '24

Vladimir Serbinenko has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/81507?usp=email ) Change subject: Support smmstore in RW_LEGACY ...................................................................... Support smmstore in RW_LEGACY When creating hybrid image with dual-boot we can't add a new fmap for smmstore. Use a file in RW_LEGACY instead Change-Id: I86a617782f187adcce4429ea41ac40a9df58d6d3 Signed-off-by: Vladimir Serbinenko <phcoder(a)gmail.com> --- M src/drivers/smmstore/store.c 1 file changed, 26 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/81507/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81507?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I86a617782f187adcce4429ea41ac40a9df58d6d3 Gerrit-Change-Number: 81507 Gerrit-PatchSet: 2 Gerrit-Owner: Vladimir Serbinenko <phcoder(a)gmail.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: Disable NULL breakpoint at the end of bootblock
by Vladimir Serbinenko (Code Review) 24 Mar '24

24 Mar '24

Vladimir Serbinenko has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/81506?usp=email ) Change subject: Disable NULL breakpoint at the end of bootblock ...................................................................... Disable NULL breakpoint at the end of bootblock If stage is older then it will not be able to correctly disable it when needed. New stages will reenable breakpoint early Change-Id: I6d83dfd8c84ccdd97c1899f206519ada91c990d5 Signed-off-by: Vladimir Serbinenko <phcoder(a)gmail.com> --- M src/lib/prog_loaders.c 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/81506/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81506?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6d83dfd8c84ccdd97c1899f206519ada91c990d5 Gerrit-Change-Number: 81506 Gerrit-PatchSet: 2 Gerrit-Owner: Vladimir Serbinenko <phcoder(a)gmail.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: Support loading legacy stage
by Vladimir Serbinenko (Code Review) 24 Mar '24

24 Mar '24

Attention is currently required from: Julius Werner. Hello Julius Werner, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81505?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: Support loading legacy stage ...................................................................... Support loading legacy stage This allows to create hybrid images that boot old signed stage image in one of RW_A/RW_B and a newer RO with newer behaviour. This allows to have dual-boot between stock ChromeOS and custom coreboot Change-Id: I4ae29a6227235c86f9f846986c18b361c3b3c78d Signed-off-by: Vladimir Serbinenko <phcoder(a)gmail.com> --- M src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h M src/lib/cbfs.c 2 files changed, 78 insertions(+), 27 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/81505/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81505?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I4ae29a6227235c86f9f846986c18b361c3b3c78d Gerrit-Change-Number: 81505 Gerrit-PatchSet: 2 Gerrit-Owner: Vladimir Serbinenko <phcoder(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: mb/google/{brya,hades}: use soc index for variant_update_power_limits()
by SH Kim (Code Review) 24 Mar '24

24 Mar '24

SH Kim has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81436?usp=email ) Change subject: mb/google/{brya,hades}: use soc index for variant_update_power_limits() ...................................................................... mb/google/{brya,hades}: use soc index for variant_update_power_limits() The power_limits_config variable for ADL/RPL is array data, but we got soc_power_limits_config variable without its index. So correct the code to get the proper pointer of the data for current CPU SKU. BUG=None BRANCH=None TEST=Built and tested the function could adjust PL4 on xol in local. Change-Id: I9f1ba25c2d673fda48babf773208c2f2d2386c53 Signed-off-by: Seunghwan Kim <sh_.kim(a)samsung.corp-partner.google.com> --- M src/mainboard/google/brya/variants/baseboard/brya/ramstage.c M src/mainboard/google/brya/variants/baseboard/hades/ramstage.c 2 files changed, 64 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/81436/1 diff --git a/src/mainboard/google/brya/variants/baseboard/brya/ramstage.c b/src/mainboard/google/brya/variants/baseboard/brya/ramstage.c index 9c2d4aa..2235994 100644 --- a/src/mainboard/google/brya/variants/baseboard/brya/ramstage.c +++ b/src/mainboard/google/brya/variants/baseboard/brya/ramstage.c @@ -11,11 +11,40 @@ WEAK_DEV_PTR(dptf_policy); +static struct soc_power_limits_config* get_soc_power_limit_config(void) +{ + config_t *config = config_of_soc(); + size_t i; + struct device *sa = pcidev_path_on_root(SA_DEVFN_ROOT); + uint16_t sa_pci_id; + u8 tdp; + + if (!sa) + return NULL; + + sa_pci_id = pci_read_config16(sa, PCI_DEVICE_ID); + + tdp = get_cpu_tdp(); + + for (i = 0; i < ARRAY_SIZE(cpuid_to_adl); i++) { + if (sa_pci_id == cpuid_to_adl[i].cpu_id && + tdp == cpuid_to_adl[i].cpu_tdp) { + return &config->power_limits_config[cpuid_to_adl[i].limits]; + } + } + + return NULL; +} + void variant_update_power_limits(const struct cpu_power_limits *limits, size_t num_entries) { if (!num_entries) return; + struct soc_power_limits_config *soc_config = get_soc_power_limit_config(); + if (!soc_config) + return; + const struct device *policy_dev = DEV_PTR(dptf_policy); if (!policy_dev) return; @@ -29,20 +58,18 @@ for (size_t i = 0; i < num_entries; i++) { if (mchid == limits[i].mchid && tdp == limits[i].cpu_tdp) { struct dptf_power_limits *settings = &config->controls.power_limits; - config_t *conf = config_of_soc(); - struct soc_power_limits_config *soc_config = conf->power_limits_config; settings->pl1.min_power = limits[i].pl1_min_power; settings->pl1.max_power = limits[i].pl1_max_power; settings->pl2.min_power = limits[i].pl2_min_power; settings->pl2.max_power = limits[i].pl2_max_power; soc_config->tdp_pl4 = DIV_ROUND_UP(limits[i].pl4_power, - MILLIWATTS_TO_WATTS); + MILLIWATTS_TO_WATTS); printk(BIOS_INFO, "Overriding power limits PL1 (%u, %u) PL2 (%u, %u) PL4 (%u)\n", - limits[i].pl1_min_power, - limits[i].pl1_max_power, - limits[i].pl2_min_power, - limits[i].pl2_max_power, - limits[i].pl4_power); + limits[i].pl1_min_power, + limits[i].pl1_max_power, + limits[i].pl2_min_power, + limits[i].pl2_max_power, + limits[i].pl4_power); } } } diff --git a/src/mainboard/google/brya/variants/baseboard/hades/ramstage.c b/src/mainboard/google/brya/variants/baseboard/hades/ramstage.c index 4858eb7..2235994 100644 --- a/src/mainboard/google/brya/variants/baseboard/hades/ramstage.c +++ b/src/mainboard/google/brya/variants/baseboard/hades/ramstage.c @@ -11,11 +11,40 @@ WEAK_DEV_PTR(dptf_policy); +static struct soc_power_limits_config* get_soc_power_limit_config(void) +{ + config_t *config = config_of_soc(); + size_t i; + struct device *sa = pcidev_path_on_root(SA_DEVFN_ROOT); + uint16_t sa_pci_id; + u8 tdp; + + if (!sa) + return NULL; + + sa_pci_id = pci_read_config16(sa, PCI_DEVICE_ID); + + tdp = get_cpu_tdp(); + + for (i = 0; i < ARRAY_SIZE(cpuid_to_adl); i++) { + if (sa_pci_id == cpuid_to_adl[i].cpu_id && + tdp == cpuid_to_adl[i].cpu_tdp) { + return &config->power_limits_config[cpuid_to_adl[i].limits]; + } + } + + return NULL; +} + void variant_update_power_limits(const struct cpu_power_limits *limits, size_t num_entries) { if (!num_entries) return; + struct soc_power_limits_config *soc_config = get_soc_power_limit_config(); + if (!soc_config) + return; + const struct device *policy_dev = DEV_PTR(dptf_policy); if (!policy_dev) return; @@ -29,8 +58,6 @@ for (size_t i = 0; i < num_entries; i++) { if (mchid == limits[i].mchid && tdp == limits[i].cpu_tdp) { struct dptf_power_limits *settings = &config->controls.power_limits; - config_t *conf = config_of_soc(); - struct soc_power_limits_config *soc_config = conf->power_limits_config; settings->pl1.min_power = limits[i].pl1_min_power; settings->pl1.max_power = limits[i].pl1_max_power; settings->pl2.min_power = limits[i].pl2_min_power; -- To view, visit https://review.coreboot.org/c/coreboot/+/81436?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9f1ba25c2d673fda48babf773208c2f2d2386c53 Gerrit-Change-Number: 81436 Gerrit-PatchSet: 1 Gerrit-Owner: SH Kim <sh_.kim(a)samsung.corp-partner.google.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: doc/getting_started: Update embedded-table examples
by Nicholas Chin (Code Review) 24 Mar '24

24 Mar '24

Attention is currently required from: Nico Huber. Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81503?usp=email ) Change subject: doc/getting_started: Update embedded-table examples ...................................................................... Patch Set 2: (2 comments) Patchset: PS1: > Ok, so then let's keep it simple and keep suggesting to use rST tables? I think that's reasonable, as it keeps everything consistent. Though I think some people might appreciate having the option to use markdown syntax. File Documentation/getting_started/writing_documentation.md: https://review.coreboot.org/c/coreboot/+/81503/comment/38328c9d_f092894d : PS2, Line 147: ```{eval-rst} : .. csv-table:: : :header: "Key", "Value" : :file: keyvalues.csv : ``` This should work, as the contents of an eval-rst block just needs to be valid reST, which doesn't depend on Recommonmark vs MyST. However, MyST also provides more direct integration of Sphinx directives allowing it to call csv-table without enclosing it in an eval-rst block: https://myst-parser.readthedocs.io/en/latest/syntax/tables.html#csv-tables That does make the docs more MyST specific though instead of Markdown + standard reST + minimal implementation specific bridging syntax (`{eval-rst}`), so I'm not sure if it's really worth switching. However, there are other areas where the direct Sphinx directives might be nice, such as using the `:doc:` directive. For example, `northbridge/intel/haswell/known-issues.md` encloses an entire paragraph in `{eval-rst}` just to use that, whereas with the direct syntax, eval-rst isn't needed and `{doc}` can be used directly in place of `:doc:`. -- To view, visit https://review.coreboot.org/c/coreboot/+/81503?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1b0d0b029fd527e0e18c71371806cd248ea0ecdf Gerrit-Change-Number: 81503 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Sun, 24 Mar 2024 23:37:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: reboot_to_cros: Add a tool to reboot into ChromeOS in hybrid config
by Vladimir Serbinenko (Code Review) 24 Mar '24

24 Mar '24

Vladimir Serbinenko has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81511?usp=email ) Change subject: reboot_to_cros: Add a tool to reboot into ChromeOS in hybrid config ...................................................................... reboot_to_cros: Add a tool to reboot into ChromeOS in hybrid config Change-Id: I9b72d3ab380c70fab336b1cd989c70a83964ccd2 Signed-off-by: Vladimir Serbinenko <phcoder(a)gmail.com> --- A util/reboot_to_cros/reboot_to_cros.c 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/81511/1 diff --git a/util/reboot_to_cros/reboot_to_cros.c b/util/reboot_to_cros/reboot_to_cros.c new file mode 100644 index 0000000..3a8b2bc --- /dev/null +++ b/util/reboot_to_cros/reboot_to_cros.c @@ -0,0 +1,9 @@ +#include <unistd.h> +#include <sys/io.h> + +int main() { + iopl(3); + outb(0x77, 0x72); + outb(0xc1, 0x73); + execl ("/usr/sbin/shutdown", "shutdown", "-r", "now", NULL); +} -- To view, visit https://review.coreboot.org/c/coreboot/+/81511?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9b72d3ab380c70fab336b1cd989c70a83964ccd2 Gerrit-Change-Number: 81511 Gerrit-PatchSet: 1 Gerrit-Owner: Vladimir Serbinenko <phcoder(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: Generate CNVS in vboot_hybrid case
by Vladimir Serbinenko (Code Review) 24 Mar '24

24 Mar '24

Vladimir Serbinenko has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81510?usp=email ) Change subject: Generate CNVS in vboot_hybrid case ...................................................................... Generate CNVS in vboot_hybrid case This allows chromeos tools in alternative path Change-Id: I20d04ff1d37701dad3fd3ea414f1d5fcd307c3df Signed-off-by: Vladimir Serbinenko <phcoder(a)gmail.com> --- M src/vendorcode/google/chromeos/gnvs.c 1 file changed, 38 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/81510/1 diff --git a/src/vendorcode/google/chromeos/gnvs.c b/src/vendorcode/google/chromeos/gnvs.c index 3d66aa3..d5105d7 100644 --- a/src/vendorcode/google/chromeos/gnvs.c +++ b/src/vendorcode/google/chromeos/gnvs.c @@ -45,6 +45,10 @@ if (acpi_is_wakeup_s3()) return; +#if CONFIG(VBOOT_HYBRID) + memset(chromeos_acpi, 0, sizeof(*chromeos_acpi)); +#endif + vpd_size = chromeos_vpd_region("RO_VPD", &vpd_base); if (vpd_size && vpd_base) { chromeos_acpi->vpd_ro_base = vpd_base; @@ -56,6 +60,40 @@ chromeos_acpi->vpd_rw_base = vpd_base; chromeos_acpi->vpd_rw_size = vpd_size; } + + /* In case of hybrid we may not have depthcharge, + so fill CBNV ourselves, so that crossystem works properly. */ +#if CONFIG(VBOOT_HYBRID) + chromeos_acpi->vbt0 = 0; /* Boot reason other */ + chromeos_acpi->vbt1 = 0; /* Main fw recovery */ +#if CONFIG(EC_GOOGLE_CHROMEEC) + chromeos_acpi->vbt2 = !google_ec_running_ro(); +#else + chromeos_acpi->vbt2 = 1; /* Assume RW */ +#endif + chromeos_acpi->vbt3 = 0; /* No switches */ + + struct region_device rdev; + if (fmap_locate_area_as_rdev("GBB", &rdev)) { + struct { + uint32_t hwid_offset; + uint32_t hwid_size; + } hwid_pointer = {0, 0}; + rdev_readat(&rdev, &hwid_pointer, 16, 8); + if (hwid_pointer.hwid_size > sizeof(chromeos_acpi->vbt4)) + hwid_pointer.hwid_size = sizeof(chromeos_acpi->vbt4); + rdev_readat(&rdev, chromeos_acpi->vbt4, + hwid_pointer.hwid_offset, hwid_pointer.hwid_size); + } + const char fwid[] = CONFIG_VBOOT_FWID_MODEL CONFIG_VBOOT_FWID_VERSION; + const uint32_t fwid_size = sizeof(fwid) < sizeof(chromeos_acpi->vbt5) ? sizeof(fwid) : sizeof(chromeos_acpi->vbt5); + memcpy(chromeos_acpi->vbt5, fwid, fwid_size); + memcpy(chromeos_acpi->vbt6, fwid, fwid_size); + chromeos_acpi->vbt7 = 1; /* Firmware type: normal */ + chromeos_acpi->vbt8 = 0; /* Recovery reason: none */ + chromeos_acpi->vbt9 = (u32) cbmem_entry_find(CBMEM_ID_FMAP); + /* Missing: VDAT. */ +#endif } BOOT_STATE_INIT_ENTRY(BS_PRE_DEVICE, BS_ON_EXIT, chromeos_init_chromeos_acpi, NULL); -- To view, visit https://review.coreboot.org/c/coreboot/+/81510?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I20d04ff1d37701dad3fd3ea414f1d5fcd307c3df Gerrit-Change-Number: 81510 Gerrit-PatchSet: 1 Gerrit-Owner: Vladimir Serbinenko <phcoder(a)gmail.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: Clear spurious recovery request
by Vladimir Serbinenko (Code Review) 24 Mar '24

24 Mar '24

Attention is currently required from: Julius Werner, Yu-Ping Wu. Vladimir Serbinenko has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81509?usp=email ) Change subject: Clear spurious recovery request ...................................................................... Clear spurious recovery request Some recovery requests are spurious. Clear them and follow RW path regardless Change-Id: I6ba7c954e6c51ef97abc2ff8e2826a95ff6b0532 Signed-off-by: Vladimir Serbinenko <phcoder(a)gmail.com> --- M src/security/vboot/vboot_logic.c 1 file changed, 7 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/81509/1 diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index 784f1d0..2ca9a81 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -17,6 +17,7 @@ #include <vb2_api.h> #include <boot_device.h> #include <pc80/mc146818rtc.h> +#include "../2lib/include/2nvstorage.h" #include "antirollback.h" @@ -346,6 +347,12 @@ /* Initialize and read nvdata from non-volatile storage. */ vbnv_init(); + /* Spurious. But if we continue we'll get into recovery bootloader. So clear it. */ + if (CONFIG(VBOOT_HYBRID) && vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST) == VB2_RECOVERY_SECDATA_FIRMWARE_INIT) { + vb2_nv_set(ctx, VB2_NV_RECOVERY_REQUEST, 0); + vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, 0); + } + /* Set S3 resume flag if vboot should behave differently when selecting * which slot to boot. This is only relevant to vboot if the platform * does verification of memory init and thus must ensure it resumes with -- To view, visit https://review.coreboot.org/c/coreboot/+/81509?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6ba7c954e6c51ef97abc2ff8e2826a95ff6b0532 Gerrit-Change-Number: 81509 Gerrit-PatchSet: 1 Gerrit-Owner: Vladimir Serbinenko <phcoder(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: Support for creating hybrid vboot images
by Vladimir Serbinenko (Code Review) 24 Mar '24

24 Mar '24

Attention is currently required from: Julius Werner, Nick Vaccaro, Yu-Ping Wu. Vladimir Serbinenko has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81508?usp=email ) Change subject: Support for creating hybrid vboot images ...................................................................... Support for creating hybrid vboot images This allows creating an image where RO is triggered by default with normal RW secrets locked out. On a signal in CMOS, clear the signal and follow normal RW_A/RW_B path. This allows dual-boot between stock ChromeOS and an alternative payload while keeping compatibility with ChromeOS updates. Change-Id: I9b26c332f5bf6befd62b5930b19d1b20e76261e7 Signed-off-by: Vladimir Serbinenko <phcoder(a)gmail.com> --- M src/drivers/mrc_cache/mrc_cache.c M src/mainboard/google/volteer/Kconfig M src/security/vboot/Kconfig M src/security/vboot/misc.h M src/security/vboot/vboot_common.h M src/security/vboot/vboot_loader.c M src/security/vboot/vboot_logic.c M src/soc/intel/common/block/cse/cse_eop.c 8 files changed, 110 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/81508/1 diff --git a/src/drivers/mrc_cache/mrc_cache.c b/src/drivers/mrc_cache/mrc_cache.c index 17f5fee..49f5f7f 100644 --- a/src/drivers/mrc_cache/mrc_cache.c +++ b/src/drivers/mrc_cache/mrc_cache.c @@ -134,7 +134,7 @@ int i; int flags; - if (CONFIG(VBOOT_STARTS_IN_BOOTBLOCK) && vboot_recovery_mode_enabled()) + if (CONFIG(VBOOT_STARTS_IN_BOOTBLOCK) && (CONFIG(VBOOT_HYBRID) || vboot_recovery_mode_enabled())) flags = RECOVERY_FLAG; else flags = NORMAL_FLAG; @@ -296,7 +296,7 @@ * switch is set. */ if (CONFIG(VBOOT_STARTS_IN_BOOTBLOCK) && vboot_recovery_mode_enabled() - && get_recovery_mode_retrain_switch()) + && get_recovery_mode_retrain_switch() && !CONFIG(VBOOT_HYBRID)) return -1; cr = lookup_region(&region, type); @@ -614,6 +614,9 @@ if (!CONFIG(HAS_RECOVERY_MRC_CACHE) && CONFIG(VBOOT_STARTS_IN_ROMSTAGE)) return; + if (CONFIG(VBOOT_HYBRID)) + return; + /* We only invalidate the normal cache in recovery mode. */ if (!vboot_recovery_mode_enabled()) return; diff --git a/src/mainboard/google/volteer/Kconfig b/src/mainboard/google/volteer/Kconfig index bc5ee6c..0ab6ee0 100644 --- a/src/mainboard/google/volteer/Kconfig +++ b/src/mainboard/google/volteer/Kconfig @@ -148,7 +148,7 @@ select GBB_FLAG_FORCE_DEV_SWITCH_ON select GBB_FLAG_FORCE_MANUAL_RECOVERY select HAS_RECOVERY_MRC_CACHE - select VBOOT_EARLY_EC_SYNC + select VBOOT_EARLY_EC_SYNC if !VBOOT_HYBRID select VBOOT_LID_SWITCH config VBOOT_GSCVD diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index d42dc74..7549a83 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -21,6 +21,13 @@ if VBOOT +config VBOOT_HYBRID + bool "Create hybrid vboot image." + default n + help + Enable VBOOT only in bootblock and add code to control it from CMOS. Useful for creating hybrid ROM with RW going vboot path and RO not. + + comment "Anti-Rollback Protection disabled because mocking secdata is enabled." depends on VBOOT_MOCK_SECDATA diff --git a/src/security/vboot/misc.h b/src/security/vboot/misc.h index a7069f3..1ac9df4 100644 --- a/src/security/vboot/misc.h +++ b/src/security/vboot/misc.h @@ -67,6 +67,9 @@ { extern int vboot_executed; /* should not be globally accessible */ + if (CONFIG(VBOOT_HYBRID) && !ENV_BOOTBLOCK) + return false; + /* If we are in the stage that runs verification, or in the stage that both loads the verstage and is returned to from it afterwards, we need to check a global to see if verification has run. */ diff --git a/src/security/vboot/vboot_common.h b/src/security/vboot/vboot_common.h index 2399bf3..fc2d65b 100644 --- a/src/security/vboot/vboot_common.h +++ b/src/security/vboot/vboot_common.h @@ -51,7 +51,7 @@ * Main logic for verified boot. verstage_main() is just the core vboot logic. * If the verstage is a separate stage, it should be entered via main(). */ -void verstage_main(void); +int verstage_main(void); void verstage_mainboard_early_init(void); void verstage_mainboard_init(void); diff --git a/src/security/vboot/vboot_loader.c b/src/security/vboot/vboot_loader.c index 70e6685..4fbde61 100644 --- a/src/security/vboot/vboot_loader.c +++ b/src/security/vboot/vboot_loader.c @@ -60,8 +60,8 @@ { if (verification_should_run()) { /* Note: this path is not used for VBOOT_RETURN_FROM_VERSTAGE */ - verstage_main(); - after_verstage(); + if (verstage_main()) + after_verstage(); } else if (verstage_should_load()) { struct prog verstage = PROG_INIT(PROG_VERSTAGE, diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index f98b083..784f1d0 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -16,6 +16,7 @@ #include <timestamp.h> #include <vb2_api.h> #include <boot_device.h> +#include <pc80/mc146818rtc.h> #include "antirollback.h" @@ -241,12 +242,92 @@ ctx->flags |= VB2_CONTEXT_EC_TRUSTED; } +#define RECOVERY_OVERRIDE_ADDR 0xf7 + +static bool use_vboot(bool is_s3) +{ +#if CONFIG(VBOOT_HYBRID) + uint8_t recovery_override = cmos_read(RECOVERY_OVERRIDE_ADDR); + int counter = recovery_override & 0xf; + + printk(BIOS_INFO, "recovery_override=0x%x, is_s3=%d\n", recovery_override, is_s3); + + if ((recovery_override & 0xf0) != 0xc0) + return 0; + + if (is_s3) + return 1; + + if (counter == 0) { + cmos_write(0xff, RECOVERY_OVERRIDE_ADDR); + return 0; + } + + if (counter != 0xf) + cmos_write(0xc0 | (counter - 1), RECOVERY_OVERRIDE_ADDR); +#endif + + return 1; +} + +static bool ensure_tpm_rw_is_locked(bool is_s3) +{ + static const uint8_t boot_mode_digest[VB2_PCR_DIGEST_RECOMMENDED_SIZE] = { + /* sha256("skipmode") */ + 0x73, 0x17, 0x91, 0x09, 0x91, 0x2a, 0xbd, 0xcc, + 0x23, 0xad, 0x82, 0x2c, 0x2f, 0xd5, 0x81, 0xad, + 0xc5, 0xa6, 0xef, 0xc2, 0xae, 0x73, 0xfd, 0xb3, + 0x7b, 0x56, 0xb7, 0x14, 0xbd, 0xb9, 0x82, 0x99, + }; + static const uint8_t hwid_unknown_digest[VB2_PCR_DIGEST_RECOMMENDED_SIZE] = { + /* sha256("unknown") */ + 0xb2, 0x3a, 0x6a, 0x84, 0x39, 0xc0, 0xdd, 0xe5, + 0x51, 0x58, 0x93, 0xe7, 0xc9, 0x0c, 0x1e, 0x32, + 0x33, 0xb8, 0x61, 0x6e, 0x63, 0x44, 0x70, 0xf2, + 0x0d, 0xc4, 0x92, 0x8b, 0xcf, 0x36, 0x09, 0xbc + }; + uint8_t buffer[VB2_PCR_DIGEST_RECOMMENDED_SIZE]; + + int algo = CONFIG(TPM1) ? VB2_HASH_SHA1 : VB2_HASH_SHA256; + int digest_size = CONFIG(TPM1) ? VB2_SHA1_DIGEST_SIZE : VB2_SHA256_DIGEST_SIZE; + int rv = tpm_setup(is_s3); + + if (rv) { + printk(BIOS_ERR, "TPM setup failed with 0x%x\n", rv); + return 0; + } + + rv = tpm_extend_pcr(CONFIG_PCR_BOOT_MODE, algo, boot_mode_digest, digest_size, "VBOOT: boot mode"); + if (rv) { + printk(BIOS_ERR, "Boot mode extend failed with 0x%x\n", rv); + return 0; + } + + memcpy(buffer, hwid_unknown_digest, sizeof(buffer)); + + struct region_device rdev; + + if (fmap_locate_area_as_rdev("GBB", &rdev)) + rdev_readat(&rdev, buffer, 48, 32); + + rv = tpm_extend_pcr(CONFIG_PCR_HWID, algo, buffer, digest_size, "VBOOT: GBB HWID"); + if (rv) { + printk(BIOS_ERR, "HWID extend failed with 0x%x\n", rv); + return 0; + } + + printk(BIOS_INFO, "Successfully locked-out RW secrets\n"); + + return 1; +} + /* Verify and select the firmware in the RW image */ -void verstage_main(void) +int verstage_main(void) { struct vb2_context *ctx; tpm_result_t tpm_rc; vb2_error_t rv; + bool is_s3 = platform_is_resuming(); timestamp_add_now(TS_VBOOT_START); @@ -254,6 +335,11 @@ if (CONFIG(BOOTMEDIA_LOCK_IN_VERSTAGE)) boot_device_security_lockdown(); + if (!use_vboot(is_s3)) { + if (ensure_tpm_rw_is_locked(is_s3)) + return 0; + } + /* Set up context and work buffer */ ctx = vboot_get_context(); @@ -265,7 +351,7 @@ * does verification of memory init and thus must ensure it resumes with * the same slot that it booted from. */ if (CONFIG(RESUME_PATH_SAME_AS_BOOT) && - platform_is_resuming()) + is_s3) ctx->flags |= VB2_CONTEXT_S3_RESUME; if (!CONFIG(VBOOT_SLOTS_RW_AB)) @@ -420,4 +506,6 @@ verstage_main_exit: timestamp_add_now(TS_VBOOT_END); + + return 1; } diff --git a/src/soc/intel/common/block/cse/cse_eop.c b/src/soc/intel/common/block/cse/cse_eop.c index 265fe04..f883cc9 100644 --- a/src/soc/intel/common/block/cse/cse_eop.c +++ b/src/soc/intel/common/block/cse/cse_eop.c @@ -209,7 +209,7 @@ printk(BIOS_ERR, "Failed to send EOP to CSE, %d\n", result); /* For vboot, trigger recovery mode if applicable, as there is likely something very broken in this case. */ - if (CONFIG(VBOOT) && !vboot_recovery_mode_enabled()) + if (CONFIG(VBOOT) && !CONFIG(VBOOT_HYBRID) && !vboot_recovery_mode_enabled()) cse_trigger_vboot_recovery(CSE_EOP_FAIL); /* In non-vboot builds or recovery mode, follow the BWG in order -- To view, visit https://review.coreboot.org/c/coreboot/+/81508?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9b26c332f5bf6befd62b5930b19d1b20e76261e7 Gerrit-Change-Number: 81508 Gerrit-PatchSet: 1 Gerrit-Owner: Vladimir Serbinenko <phcoder(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: newchange

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2024