Change in coreboot[master]: Revert "{drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLO...
Hello Patrick Rudolph, Huang Jin, Arthur Heymans, York Yang, Frans Hendriks, Lee Leahy, Matt DeVillier, build bot (Jenkins), Hannah Williams, Michał Żygowski, Patrick Georgi, Martin Roth,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/34304
to review the following change.
Change subject: Revert "{drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLOCK support" ......................................................................
Revert "{drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLOCK support"
This reverts commit 4e0ec592553fd94e14a239eeb05ba9ccb668b814.
Reason for revert: Causes boot failures and no fix was ever issued. Revert it before a release gets tagged with a broken platform.
Change-Id: I67b68f0d6154249ab2633997f29c0a1699ef0c76 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/drivers/intel/fsp1_1/Makefile.inc R src/drivers/intel/fsp1_1/cache_as_ram.inc M src/drivers/intel/fsp1_1/car.c M src/drivers/intel/fsp1_1/include/fsp/car.h M src/mainboard/google/cyan/Makefile.inc M src/mainboard/google/cyan/com_init.c M src/mainboard/intel/strago/Makefile.inc M src/mainboard/intel/strago/com_init.c M src/soc/intel/braswell/Kconfig M src/soc/intel/braswell/Makefile.inc M src/soc/intel/braswell/bootblock/bootblock.c M src/soc/intel/braswell/include/soc/romstage.h M src/soc/intel/braswell/romstage/Makefile.inc R src/soc/intel/braswell/romstage/pmc.c M src/soc/intel/braswell/romstage/romstage.c 15 files changed, 195 insertions(+), 138 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/34304/1
diff --git a/src/drivers/intel/fsp1_1/Makefile.inc b/src/drivers/intel/fsp1_1/Makefile.inc index 10877b9..93f3b59 100644 --- a/src/drivers/intel/fsp1_1/Makefile.inc +++ b/src/drivers/intel/fsp1_1/Makefile.inc @@ -21,7 +21,6 @@ verstage-$(CONFIG_SEPARATE_VERSTAGE) += verstage.c
bootblock-y += bootblock.c -bootblock-$(CONFIG_USE_GENERIC_FSP_CAR_INC) += cache_as_ram.S bootblock-y += fsp_util.c
romstage-y += car.c @@ -43,6 +42,8 @@
CPPFLAGS_common += -Isrc/drivers/intel/fsp1_1/include
+cpu_incs-$(CONFIG_USE_GENERIC_FSP_CAR_INC) += $(src)/drivers/intel/fsp1_1/cache_as_ram.inc + postcar-y += stage_cache.c ifneq ($(CONFIG_SKIP_FSP_CAR),y) postcar-y += temp_ram_exit.c diff --git a/src/drivers/intel/fsp1_1/cache_as_ram.S b/src/drivers/intel/fsp1_1/cache_as_ram.inc similarity index 93% rename from src/drivers/intel/fsp1_1/cache_as_ram.S rename to src/drivers/intel/fsp1_1/cache_as_ram.inc index 3460b9d..493dbc8 100644 --- a/src/drivers/intel/fsp1_1/cache_as_ram.S +++ b/src/drivers/intel/fsp1_1/cache_as_ram.inc @@ -5,7 +5,6 @@ * Copyright (C) 2007-2008 coresystems GmbH * Copyright (C) 2013-2014 Sage Electronic Engineering, LLC. * Copyright (C) 2015 Intel Corp. - * Copyright (C) 2018-2019 Eltan B.V. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -17,8 +16,6 @@ * GNU General Public License for more details. */
-#include <cpu/x86/post_code.h> - /* * Replacement for cache_as_ram.inc when using the FSP binary. This code * locates the FSP binary, initializes the cache as RAM and performs the @@ -27,10 +24,8 @@ * performs the final stage of initialization. */
-#define LHLT_DELAY 0x50000 /* I/O delay between post codes on failure */ - -.global bootblock_pre_c_entry -bootblock_pre_c_entry: +/* I/O delay between post codes on failure */ +#define LHLT_DELAY 0x50000 /* * Per FSP1.1 specs, following registers are preserved: * EBX, EDI, ESI, EBP, MM0, MM1 @@ -134,9 +129,10 @@ /* Need to align stack to 16 bytes at call instruction. Account for the pushes below. */ andl $0xfffffff0, %esp - subl $8, %esp + subl $4, %esp
- /* Push initial timestamp on the stack */ + /* Push BIST and initial timestamp on the stack */ + pushl %ebx /* bist */ movd %mm1, %eax pushl %eax /* tsc[63:32] */ movd %mm0, %eax @@ -145,10 +141,12 @@ before_romstage: post_code(0x2A)
- /* Call bootblock_c_entry(uint64_t base_timestamp) */ - call bootblock_c_entry + /* Call bootblock_c_entry_bist(uint64_t base_timestamp, uint32_t bist) + in cpu/intel/car/romstage.c */ + call bootblock_c_entry_bist
- /* Never reached */ + movb $0x69, %ah + jmp .Lhlt
halt1: /* diff --git a/src/drivers/intel/fsp1_1/car.c b/src/drivers/intel/fsp1_1/car.c index 67ed099..dd1766403 100644 --- a/src/drivers/intel/fsp1_1/car.c +++ b/src/drivers/intel/fsp1_1/car.c @@ -101,6 +101,18 @@ * is still enabled. We can directly access work buffer here. */ struct prog fsp = PROG_INIT(PROG_REFCODE, "fsp.bin");
+ if (!CONFIG(C_ENVIRONMENT_BOOTBLOCK)) { + /* Call into pre-console init code then initialize console. */ + car_soc_pre_console_init(); + car_mainboard_pre_console_init(); + console_init(); + + display_mtrrs(); + + car_soc_post_console_init(); + car_mainboard_post_console_init(); + } + if (prog_locate(&fsp)) die_with_post_code(POST_INVALID_CBFS, "Unable to locate fsp.bin");
@@ -113,3 +125,19 @@
cache_as_ram_stage_main(fih); } + +void __weak car_mainboard_pre_console_init(void) +{ +} + +void __weak car_soc_pre_console_init(void) +{ +} + +void __weak car_mainboard_post_console_init(void) +{ +} + +void __weak car_soc_post_console_init(void) +{ +} diff --git a/src/drivers/intel/fsp1_1/include/fsp/car.h b/src/drivers/intel/fsp1_1/include/fsp/car.h index 3d99fa6..8d7a683 100644 --- a/src/drivers/intel/fsp1_1/include/fsp/car.h +++ b/src/drivers/intel/fsp1_1/include/fsp/car.h @@ -24,4 +24,12 @@ * cache_as_ram_stage_main() is the stack pointer to use in RAM after * exiting cache-as-ram mode. */ void cache_as_ram_stage_main(FSP_INFO_HEADER *fih); + +/* Mainboard and SoC initialization prior to console. */ +void car_mainboard_pre_console_init(void); +void car_soc_pre_console_init(void); +/* Mainboard and SoC initialization post console initialization. */ +void car_mainboard_post_console_init(void); +void car_soc_post_console_init(void); + #endif diff --git a/src/mainboard/google/cyan/Makefile.inc b/src/mainboard/google/cyan/Makefile.inc index 027c49c..92b0422 100644 --- a/src/mainboard/google/cyan/Makefile.inc +++ b/src/mainboard/google/cyan/Makefile.inc @@ -14,9 +14,8 @@ ## GNU General Public License for more details. ##
-bootblock-$(CONFIG_ENABLE_BUILTIN_COM1) += com_init.c - romstage-$(CONFIG_CHROMEOS) += chromeos.c +romstage-$(CONFIG_ENABLE_BUILTIN_COM1) += com_init.c romstage-y += spd/spd.c
ramstage-$(CONFIG_CHROMEOS) += chromeos.c diff --git a/src/mainboard/google/cyan/com_init.c b/src/mainboard/google/cyan/com_init.c index b08dbce..44260bb 100644 --- a/src/mainboard/google/cyan/com_init.c +++ b/src/mainboard/google/cyan/com_init.c @@ -14,14 +14,14 @@ * GNU General Public License for more details. */
-#include <bootblock_common.h> #include <device/mmio.h> #include <device/pci_ops.h> #include <soc/gpio.h> #include <soc/lpc.h> #include <soc/pci_devs.h> +#include <soc/romstage.h>
-void bootblock_mainboard_early_init(void) +void car_mainboard_pre_console_init(void) { uint32_t reg; uint32_t *pad_config_reg; diff --git a/src/mainboard/intel/strago/Makefile.inc b/src/mainboard/intel/strago/Makefile.inc index e6f0c9e..bbef8b9 100644 --- a/src/mainboard/intel/strago/Makefile.inc +++ b/src/mainboard/intel/strago/Makefile.inc @@ -14,9 +14,8 @@ ## GNU General Public License for more details. ##
-bootblock-$(CONFIG_ENABLE_BUILTIN_COM1) += com_init.c - romstage-$(CONFIG_MAINBOARD_HAS_CHROMEOS) += chromeos.c +romstage-$(CONFIG_ENABLE_BUILTIN_COM1) += com_init.c
ramstage-$(CONFIG_MAINBOARD_HAS_CHROMEOS) += chromeos.c ramstage-$(CONFIG_MAINBOARD_HAS_CHROMEOS) += ec.c diff --git a/src/mainboard/intel/strago/com_init.c b/src/mainboard/intel/strago/com_init.c index 695ea98..b89d655 100644 --- a/src/mainboard/intel/strago/com_init.c +++ b/src/mainboard/intel/strago/com_init.c @@ -14,12 +14,12 @@ * GNU General Public License for more details. */
-#include <bootblock_common.h> #include <device/mmio.h> #include <device/pci_ops.h> #include <soc/gpio.h> #include <soc/lpc.h> #include <soc/pci_devs.h> +#include <soc/romstage.h>
/* * return family number and internal pad number in that community @@ -30,7 +30,7 @@
/* family number in high byte and inner pad number in lowest byte */
-void bootblock_mainboard_early_init(void) +void car_mainboard_pre_console_init(void) { uint32_t reg; uint32_t *pad_config_reg; diff --git a/src/soc/intel/braswell/Kconfig b/src/soc/intel/braswell/Kconfig index 920179f83..ed5c972 100644 --- a/src/soc/intel/braswell/Kconfig +++ b/src/soc/intel/braswell/Kconfig @@ -51,23 +51,15 @@ select INTEL_GMA_SWSMISCI select CPU_INTEL_COMMON select SOUTHBRIDGE_INTEL_COMMON_SMBUS - select C_ENVIRONMENT_BOOTBLOCK - -config DCACHE_BSP_STACK_SIZE - hex - default 0x2000 - help - The amount of anticipated stack usage in CAR by bootblock and - other stages. - -config C_ENV_BOOTBLOCK_SIZE - hex - default 0x8000
config VBOOT select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_ROMSTAGE
+config BOOTBLOCK_CPU_INIT + string + default "soc/intel/braswell/bootblock/bootblock.c" + config MMCONF_BASE_ADDRESS hex default 0xe0000000 diff --git a/src/soc/intel/braswell/Makefile.inc b/src/soc/intel/braswell/Makefile.inc index 1017d80..e479a3c 100644 --- a/src/soc/intel/braswell/Makefile.inc +++ b/src/soc/intel/braswell/Makefile.inc @@ -9,14 +9,9 @@ subdirs-y += ../../../cpu/intel/turbo subdirs-y += ../../../cpu/intel/common
-bootblock-y += gpio_support.c -bootblock-y += bootblock/bootblock.c -bootblock-y += lpc_init.c -bootblock-y += pmutil.c -bootblock-y += tsc_freq.c - romstage-y += gpio_support.c romstage-y += iosf.c +romstage-y += lpc_init.c romstage-y += memmap.c romstage-y += pmutil.c romstage-y += smbus.c diff --git a/src/soc/intel/braswell/bootblock/bootblock.c b/src/soc/intel/braswell/bootblock/bootblock.c index 2d1a3e8..457b8b8 100644 --- a/src/soc/intel/braswell/bootblock/bootblock.c +++ b/src/soc/intel/braswell/bootblock/bootblock.c @@ -3,7 +3,6 @@ * * Copyright (C) 2013 Google, Inc. * Copyright (C) 2015 Intel Corp. - * Copyright (C) 2018 Eltan B.V. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -15,93 +14,37 @@ * GNU General Public License for more details. */
-#include <bootblock_common.h> -#include <build.h> -#include <console/console.h> #include <device/pci_ops.h> -#include <pc80/mc146818rtc.h> -#include <soc/bootblock.h> -#include <soc/gpio.h> -#include <soc/iomap.h> +#include <cpu/x86/cache.h> +#include <cpu/x86/msr.h> +#include <cpu/x86/mtrr.h> #include <soc/iosf.h> -#include <soc/lpc.h> -#include <soc/pm.h> -#include <soc/spi.h> +#include <cpu/intel/microcode/microcode.c>
-asmlinkage void bootblock_c_entry(uint64_t base_timestamp) +static void set_var_mtrr(int reg, uint32_t base, uint32_t size, int type) { - /* Call lib/bootblock.c main */ - bootblock_main_with_timestamp(base_timestamp, NULL, 0); + msr_t basem, maskm; + basem.lo = base | type; + basem.hi = 0; + wrmsr(MTRR_PHYS_BASE(reg), basem); + maskm.lo = ~(size - 1) | MTRR_PHYS_MASK_VALID; + maskm.hi = (1 << (CONFIG_CPU_ADDR_BITS - 32)) - 1; + wrmsr(MTRR_PHYS_MASK(reg), maskm); }
-static void program_base_addresses(void) +static void enable_rom_caching(void) { - uint32_t reg; - const uint32_t lpc_dev = PCI_DEV(0, LPC_DEV, LPC_FUNC); + msr_t msr;
- /* Memory Mapped IO registers. */ - reg = PMC_BASE_ADDRESS | 2; - pci_write_config32(lpc_dev, PBASE, reg); - reg = IO_BASE_ADDRESS | 2; - pci_write_config32(lpc_dev, IOBASE, reg); - reg = ILB_BASE_ADDRESS | 2; - pci_write_config32(lpc_dev, IBASE, reg); - reg = SPI_BASE_ADDRESS | 2; - pci_write_config32(lpc_dev, SBASE, reg); - reg = MPHY_BASE_ADDRESS | 2; - pci_write_config32(lpc_dev, MPBASE, reg); - reg = PUNIT_BASE_ADDRESS | 2; - pci_write_config32(lpc_dev, PUBASE, reg); - reg = RCBA_BASE_ADDRESS | 1; - pci_write_config32(lpc_dev, RCBA, reg); + disable_cache(); + /* Why only top 4MiB ? */ + set_var_mtrr(1, 0xffc00000, 4*1024*1024, MTRR_TYPE_WRPROT); + enable_cache();
- /* IO Port Registers. */ - reg = ACPI_BASE_ADDRESS | 2; - pci_write_config32(lpc_dev, ABASE, reg); - reg = GPIO_BASE_ADDRESS | 2; - pci_write_config32(lpc_dev, GBASE, reg); -} - -static void tco_disable(void) -{ - uint32_t reg; - - reg = inl(ACPI_BASE_ADDRESS + TCO1_CNT); - reg |= TCO_TMR_HALT; - outl(reg, ACPI_BASE_ADDRESS + TCO1_CNT); -} - -static void spi_init(void) -{ - void *scs = (void *)(SPI_BASE_ADDRESS + SCS); - void *bcr = (void *)(SPI_BASE_ADDRESS + BCR); - uint32_t reg; - - /* Disable generating SMI when setting WPD bit. */ - write32(scs, read32(scs) & ~SMIWPEN); - /* - * Enable caching and prefetching in the SPI controller. Disable - * the SMM-only BIOS write and set WPD bit. - */ - reg = (read32(bcr) & ~SRC_MASK) | SRC_CACHE_PREFETCH | BCR_WPD; - reg &= ~EISS; - write32(bcr, reg); -} - -static void soc_rtc_init(void) -{ - int rtc_failed = rtc_failure(); - - if (rtc_failed) { - printk(BIOS_ERR, - "RTC Failure detected. Resetting date to %x/%x/%x%x\n", - COREBOOT_BUILD_MONTH_BCD, - COREBOOT_BUILD_DAY_BCD, - 0x20, - COREBOOT_BUILD_YEAR_BCD); - } - - cmos_init(rtc_failed); + /* Enable Variable MTRRs */ + msr.hi = 0x00000000; + msr.lo = 0x00000800; + wrmsr(MTRR_DEF_TYPE_MSR, msr); }
static void setup_mmconfig(void) @@ -124,22 +67,12 @@ pci_io_write_config32(IOSF_PCI_DEV, MCR_REG, reg); }
- -void bootblock_soc_early_init(void) +static void bootblock_cpu_init(void) { /* Allow memory-mapped PCI config access. */ setup_mmconfig();
- /* Early chipset initialization */ - program_base_addresses(); - tco_disable(); -} -void bootblock_soc_init(void) -{ - /* Continue chipset initialization */ - soc_rtc_init(); - set_max_freq(); - spi_init(); - - lpc_init(); + /* Load microcode before any caching. */ + intel_update_microcode_from_cbfs(); + enable_rom_caching(); } diff --git a/src/soc/intel/braswell/include/soc/romstage.h b/src/soc/intel/braswell/include/soc/romstage.h index 9fad9bc..4ecbd2c 100644 --- a/src/soc/intel/braswell/include/soc/romstage.h +++ b/src/soc/intel/braswell/include/soc/romstage.h @@ -23,10 +23,12 @@ #include <soc/pm.h>
void gfx_init(void); +void tco_disable(void); void punit_init(void); void set_max_freq(void);
-/* romstage.c functions */ +/* romstage_common.c functions */ +void program_base_addresses(void); int chipset_prev_sleep_state(struct chipset_power_state *ps);
/* Values for FSP's PcdMemoryTypeEnable */ diff --git a/src/soc/intel/braswell/romstage/Makefile.inc b/src/soc/intel/braswell/romstage/Makefile.inc index d405133..3d3e407 100644 --- a/src/soc/intel/braswell/romstage/Makefile.inc +++ b/src/soc/intel/braswell/romstage/Makefile.inc @@ -1,2 +1,3 @@ romstage-y += ../../../../cpu/intel/car/romstage.c +romstage-y += pmc.c romstage-y += romstage.c diff --git a/src/soc/intel/braswell/include/soc/bootblock.h b/src/soc/intel/braswell/romstage/pmc.c similarity index 67% rename from src/soc/intel/braswell/include/soc/bootblock.h rename to src/soc/intel/braswell/romstage/pmc.c index e6e25cc..127458e 100644 --- a/src/soc/intel/braswell/include/soc/bootblock.h +++ b/src/soc/intel/braswell/romstage/pmc.c @@ -2,7 +2,7 @@ * This file is part of the coreboot project. * * Copyright (C) 2013 Google Inc. - * Copyright (C) 2015-2016 Intel Corp. + * Copyright (C) 2015 Intel Corp. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -14,9 +14,15 @@ * GNU General Public License for more details. */
-#ifndef _SOC_BOOTBLOCK_H_ -#define _SOC_BOOTBLOCK_H_ +#include <arch/io.h> +#include <soc/iomap.h> +#include <soc/romstage.h>
-void set_max_freq(void); +void tco_disable(void) +{ + uint32_t reg;
-#endif /* _SOC_BOOTBLOCK_H_ */ + reg = inl(ACPI_BASE_ADDRESS + TCO1_CNT); + reg |= TCO_TMR_HALT; + outl(reg, ACPI_BASE_ADDRESS + TCO1_CNT); +} diff --git a/src/soc/intel/braswell/romstage/romstage.c b/src/soc/intel/braswell/romstage/romstage.c index e0e22f2..8dfc291 100644 --- a/src/soc/intel/braswell/romstage/romstage.c +++ b/src/soc/intel/braswell/romstage/romstage.c @@ -19,13 +19,90 @@ #include <stddef.h> #include <arch/io.h> #include <device/mmio.h> +#include <device/pci_ops.h> +#include <arch/cbfs.h> +#include <cpu/x86/mtrr.h> #include <console/console.h> +#include <device/device.h> +#include <device/pci_def.h> +#include <elog.h> +#include <mrc_cache.h> +#include <string.h> +#include <vendorcode/google/chromeos/chromeos.h> +#include <fsp/util.h> +#include <soc/gpio.h> #include <soc/iomap.h> #include <soc/iosf.h> +#include <soc/lpc.h> +#include <soc/pci_devs.h> #include <soc/romstage.h> +#include <soc/smm.h> +#include <soc/spi.h> +#include <build.h> +#include <pc80/mc146818rtc.h>
#include "../chip.h"
+void program_base_addresses(void) +{ + uint32_t reg; + const uint32_t lpc_dev = PCI_DEV(0, LPC_DEV, LPC_FUNC); + + /* Memory Mapped IO registers. */ + reg = PMC_BASE_ADDRESS | 2; + pci_write_config32(lpc_dev, PBASE, reg); + reg = IO_BASE_ADDRESS | 2; + pci_write_config32(lpc_dev, IOBASE, reg); + reg = ILB_BASE_ADDRESS | 2; + pci_write_config32(lpc_dev, IBASE, reg); + reg = SPI_BASE_ADDRESS | 2; + pci_write_config32(lpc_dev, SBASE, reg); + reg = MPHY_BASE_ADDRESS | 2; + pci_write_config32(lpc_dev, MPBASE, reg); + reg = PUNIT_BASE_ADDRESS | 2; + pci_write_config32(lpc_dev, PUBASE, reg); + reg = RCBA_BASE_ADDRESS | 1; + pci_write_config32(lpc_dev, RCBA, reg); + + /* IO Port Registers. */ + reg = ACPI_BASE_ADDRESS | 2; + pci_write_config32(lpc_dev, ABASE, reg); + reg = GPIO_BASE_ADDRESS | 2; + pci_write_config32(lpc_dev, GBASE, reg); +} + +static void spi_init(void) +{ + void *scs = (void *)(SPI_BASE_ADDRESS + SCS); + void *bcr = (void *)(SPI_BASE_ADDRESS + BCR); + uint32_t reg; + + /* Disable generating SMI when setting WPD bit. */ + write32(scs, read32(scs) & ~SMIWPEN); + /* + * Enable caching and prefetching in the SPI controller. Disable + * the SMM-only BIOS write and set WPD bit. + */ + reg = (read32(bcr) & ~SRC_MASK) | SRC_CACHE_PREFETCH | BCR_WPD; + reg &= ~EISS; + write32(bcr, reg); +} + +static void soc_rtc_init(void) +{ + int rtc_failed = rtc_failure(); + + if (rtc_failed) { + printk(BIOS_ERR, + "RTC Failure detected. Resetting date to %x/%x/%x%x\n", + COREBOOT_BUILD_MONTH_BCD, + COREBOOT_BUILD_DAY_BCD, + 0x20, + COREBOOT_BUILD_YEAR_BCD); + } + + cmos_init(rtc_failed); +}
static struct chipset_power_state power_state;
@@ -94,6 +171,24 @@ return prev_sleep_state; }
+/* SOC initialization before the console is enabled */ +void car_soc_pre_console_init(void) +{ + /* Early chipset initialization */ + program_base_addresses(); + tco_disable(); +} + +/* SOC initialization after console is enabled */ +void car_soc_post_console_init(void) +{ + /* Continue chipset initialization */ + soc_rtc_init(); + set_max_freq(); + spi_init(); + + lpc_init(); +}
/* SOC initialization after RAM is enabled */ void soc_after_ram_init(struct romstage_params *params)
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34304 )
Change subject: Revert "{drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLOCK support" ......................................................................
Patch Set 1: Code-Review+1
fixes boot on google/cyan (all variants)
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34304 )
Change subject: Revert "{drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLOCK support" ......................................................................
Patch Set 1: Code-Review-1
Facebook FBG1701 is based on this commit. Can I assist solving this issue on Google/Cyan?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34304 )
Change subject: Revert "{drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLOCK support" ......................................................................
Patch Set 1:
Patch Set 1: Code-Review-1
Facebook FBG1701 is based on this commit. Can I assist solving this issue on Google/Cyan?
It has been a month since the issue with google/cyan was known. I would prefer if you could test that FBG1701 boots properly with this change, so that the coreboot 4.10 release is working on google/cyan and FBG1701. Then we could try to get C_ENVIRONMENT_BOOTBLOCK to work without master being broken.
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34304 )
Change subject: Revert "{drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLOCK support" ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1: Code-Review-1
Facebook FBG1701 is based on this commit. Can I assist solving this issue on Google/Cyan?
It has been a month since the issue with google/cyan was known. I would prefer if you could test that FBG1701 boots properly with this change, so that the coreboot 4.10 release is working on google/cyan and FBG1701. Then we could try to get C_ENVIRONMENT_BOOTBLOCK to work without master being broken.
I will test.
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34304 )
Change subject: Revert "{drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLOCK support" ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1: Code-Review-1
Facebook FBG1701 is based on this commit. Can I assist solving this issue on Google/Cyan?
It has been a month since the issue with google/cyan was known. I would prefer if you could test that FBG1701 boots properly with this change, so that the coreboot 4.10 release is working on google/cyan and FBG1701. Then we could try to get C_ENVIRONMENT_BOOTBLOCK to work without master being broken.
Did a test today, but system did not boot till the end. Need some more investigation.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34304 )
Change subject: Revert "{drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLOCK support" ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
Patch Set 1: Code-Review-1
Facebook FBG1701 is based on this commit. Can I assist solving this issue on Google/Cyan?
It has been a month since the issue with google/cyan was known. I would prefer if you could test that FBG1701 boots properly with this change, so that the coreboot 4.10 release is working on google/cyan and FBG1701. Then we could try to get C_ENVIRONMENT_BOOTBLOCK to work without master being broken.
Did a test today, but system did not boot till the end. Need some more investigation.
Got any logs?
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34304 )
Change subject: Revert "{drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLOCK support" ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
Patch Set 1:
Patch Set 1: Code-Review-1
Facebook FBG1701 is based on this commit. Can I assist solving this issue on Google/Cyan?
It has been a month since the issue with google/cyan was known. I would prefer if you could test that FBG1701 boots properly with this change, so that the coreboot 4.10 release is working on google/cyan and FBG1701. Then we could try to get C_ENVIRONMENT_BOOTBLOCK to work without master being broken.
Did a test today, but system did not boot till the end. Need some more investigation.
Got any logs?
I rebuild today and the system is booting. However not having bootblock in C provides weak verified boot and measured boot solution for Facebook FBG1701.
First patch sets of https://review.coreboot.org/c/coreboot/+/29662 provide solution where C_ENVIRONMENT_BOOTBLOCK was configurable. This would mean actual boards are not upgrades and new boards can use C_ENVIRONMENT_BOOTBLOCK. The comment was that all boards should be upgraded to C_ENVIRONMENT_BOOTBLOCK.
I suggest using (and fix Google/Cyan) C_ENVIRONMENT_BOOTBLOCK or make C_ENVIRONMENT_BOOTBLOCK optional.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34304 )
Change subject: Revert "{drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLOCK support" ......................................................................
Patch Set 1: -Code-Review
I suggest using (and fix Google/Cyan) C_ENVIRONMENT_BOOTBLOCK or make C_ENVIRONMENT_BOOTBLOCK optional.
we've tried fixing Cyan without much luck, and in the interest of not having it broken for the 4.10 release, I'd suggest we make C_ENVIRONMENT_BOOTBLOCK the default (since all new boards should be able to use it) and add a Kconfig for the original implementation which Cyan (and Strago) will select (USE_DEPRECATED_ROMCC_BOOTBLOCK ?)
Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/34304 )
Change subject: Revert "{drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLOCK support" ......................................................................
Abandoned
Murdering FSP with a completely undocumented board sounds much easier than making it work
-
Angel Pons (Code Review) -
Frans Hendriks (Code Review)
-
Matt DeVillier (Code Review)