From 1e33255408ed376945591b8c93caf2142f07be56 Mon Sep 17 00:00:00 2001 From: Adam Joseph Date: Wed, 3 May 2023 15:17:14 -0700 Subject: [PATCH] src/coreboot/patches: add !CONFIG_USE_OPTION_TABLE patches This commit imports four patches which fix bugs in coreboot's behavior when !CONFIG_USE_OPTION_TABLE on KGPE-D16. --- src/coreboot/default.nix | 5 +++ ...mc146818rtc.c-mute-erroneous-warning.patch | 35 +++++++++++++++ ....c-always-report-cmos_chksum_valid-i.patch | 39 ++++++++++++++++ ...ck.c-use-RTC_BOOT_BYTE-even-when-CON.patch | 44 +++++++++++++++++++ ...w83667hg-a-superio.c-do-not-use-get_.patch | 38 ++++++++++++++++ 5 files changed, 161 insertions(+) create mode 100644 src/coreboot/patches/0001-mc146818rtc.c-mute-erroneous-warning.patch create mode 100644 src/coreboot/patches/0002-mc146818rtc_boot.c-always-report-cmos_chksum_valid-i.patch create mode 100644 src/coreboot/patches/0003-kgpe-d16-bootblock.c-use-RTC_BOOT_BYTE-even-when-CON.patch create mode 100644 src/coreboot/patches/0004-superio-winbond-w83667hg-a-superio.c-do-not-use-get_.patch diff --git a/src/coreboot/default.nix b/src/coreboot/default.nix index 07306ce..469f7b8 100644 --- a/src/coreboot/default.nix +++ b/src/coreboot/default.nix @@ -97,6 +97,11 @@ stdenv.mkDerivation { hash = "sha256-sjw6X6UjOV00S3uRfnTLEPjAvvMxSn1RdNdNtwRXYjE="; includes = [ "util/rockchip/make_idb.py" ]; }) + + ./patches/0001-mc146818rtc.c-mute-erroneous-warning.patch + ./patches/0002-mc146818rtc_boot.c-always-report-cmos_chksum_valid-i.patch + ./patches/0003-kgpe-d16-bootblock.c-use-RTC_BOOT_BYTE-even-when-CON.patch + ./patches/0004-superio-winbond-w83667hg-a-superio.c-do-not-use-get_.patch ]; postPatch = '' diff --git a/src/coreboot/patches/0001-mc146818rtc.c-mute-erroneous-warning.patch b/src/coreboot/patches/0001-mc146818rtc.c-mute-erroneous-warning.patch new file mode 100644 index 0000000..9a09c38 --- /dev/null +++ b/src/coreboot/patches/0001-mc146818rtc.c-mute-erroneous-warning.patch @@ -0,0 +1,35 @@ +From b675046088b682b05742758ac4bf6ee3c70a166d Mon Sep 17 00:00:00 2001 +From: Your Name +Date: Wed, 3 May 2023 15:07:04 -0700 +Subject: [PATCH 1/4] mc146818rtc.c: mute erroneous warning + +mc146818rtc.c includes a BIOS_NOTICE warning message about accessing +the CMOS from non-ROMCC code, but the accsess is guarded by +IS_ENABLED(CONFIG_USE_OPTION_TABLE) whereas the warning message +isn't. So the warning shouldn't be emitted if +!IS_ENABLED(CONFIG_USE_OPTION_TABLE). + +This commit fixes the warning by moving the warning inside the +if-block where like the access takes place. +--- + src/drivers/pc80/rtc/mc146818rtc.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/drivers/pc80/rtc/mc146818rtc.c b/src/drivers/pc80/rtc/mc146818rtc.c +index eaa71c908f..ebc5774b16 100644 +--- a/src/drivers/pc80/rtc/mc146818rtc.c ++++ b/src/drivers/pc80/rtc/mc146818rtc.c +@@ -385,9 +385,9 @@ static enum cb_err set_cmos_value(unsigned long bit, unsigned long length, + unsigned int read_option_lowlevel(unsigned int start, unsigned int size, + unsigned int def) + { ++ if (IS_ENABLED(CONFIG_USE_OPTION_TABLE)) { + printk(BIOS_NOTICE, "NOTICE: read_option() used to access CMOS " + "from non-ROMCC code, please use get_option() instead.\n"); +- if (IS_ENABLED(CONFIG_USE_OPTION_TABLE)) { + const unsigned char byte = cmos_read(start / 8); + return (byte >> (start & 7U)) & ((1U << size) - 1U); + } +-- +2.39.1 + diff --git a/src/coreboot/patches/0002-mc146818rtc_boot.c-always-report-cmos_chksum_valid-i.patch b/src/coreboot/patches/0002-mc146818rtc_boot.c-always-report-cmos_chksum_valid-i.patch new file mode 100644 index 0000000..7eb1a0a --- /dev/null +++ b/src/coreboot/patches/0002-mc146818rtc_boot.c-always-report-cmos_chksum_valid-i.patch @@ -0,0 +1,39 @@ +From 063f922b034161b269af460bd4b02965f9c9ebaa Mon Sep 17 00:00:00 2001 +From: Your Name +Date: Wed, 3 May 2023 15:09:58 -0700 +Subject: [PATCH 2/4] mc146818rtc_boot.c: always report cmos_chksum_valid() if + !CONFIG_USE_OPTION_TABLE + +If !CONFIG_USE_OPTION_TABLE then coreboot should not use the CMOS +NVRAM *except* for the RTC_BOOT_BYTE. + +Unfortunately when !CONFIG_USE_OPTION_TABLE, coreboot will still +calculate the CMOS checksum, and erase the RTC_BOOT_BYTE if it finds +an invalid checksum. Since !CONFIG_USE_OPTION_TABLE the checksum +will never be fixed, meaning that RTC_BOOT_BYTE becomes effectively +useless. + +This commit causes coreboot to assume that the cmos checksum is +always valid when !CONFIG_USE_OPTION_TABLE, since under that +condition coreboot shouldn't expect to be able to use any part of +the CMOS NVRAM other than the RTC_BOOT_BYTE. +--- + src/drivers/pc80/rtc/mc146818rtc_boot.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/drivers/pc80/rtc/mc146818rtc_boot.c b/src/drivers/pc80/rtc/mc146818rtc_boot.c +index e5164d11ea..dd23f52a8a 100644 +--- a/src/drivers/pc80/rtc/mc146818rtc_boot.c ++++ b/src/drivers/pc80/rtc/mc146818rtc_boot.c +@@ -50,7 +50,7 @@ int cmos_chksum_valid(void) + + return sum == old_sum; + #else +- return 0; ++ return 1; + #endif + } + +-- +2.39.1 + diff --git a/src/coreboot/patches/0003-kgpe-d16-bootblock.c-use-RTC_BOOT_BYTE-even-when-CON.patch b/src/coreboot/patches/0003-kgpe-d16-bootblock.c-use-RTC_BOOT_BYTE-even-when-CON.patch new file mode 100644 index 0000000..1c2d226 --- /dev/null +++ b/src/coreboot/patches/0003-kgpe-d16-bootblock.c-use-RTC_BOOT_BYTE-even-when-CON.patch @@ -0,0 +1,44 @@ +From 72884c3492c623c5239a2ac664a3f1249d234dd6 Mon Sep 17 00:00:00 2001 +From: Your Name +Date: Wed, 3 May 2023 15:12:42 -0700 +Subject: [PATCH 3/4] kgpe-d16/bootblock.c: use RTC_BOOT_BYTE even when + !CONFIG_USE_OPTION_TABLE + +My initial implementation of the fallback mechanism assumed +CONFIG_USE_OPTION_TABLE. This commit expands it to function +properly even when !CONFIG_USE_OPTION_TABLE. +--- + src/mainboard/asus/kgpe-d16/bootblock.c | 9 ++------- + 1 file changed, 2 insertions(+), 7 deletions(-) + +diff --git a/src/mainboard/asus/kgpe-d16/bootblock.c b/src/mainboard/asus/kgpe-d16/bootblock.c +index 8a149737e8..9687d20a3d 100644 +--- a/src/mainboard/asus/kgpe-d16/bootblock.c ++++ b/src/mainboard/asus/kgpe-d16/bootblock.c +@@ -33,21 +33,16 @@ void bootblock_mainboard_init(void) + if (recovery_enabled) { + #if IS_ENABLED(CONFIG_USE_OPTION_TABLE) + /* Clear NVRAM checksum */ +-/* ++ + for (addr = LB_CKS_RANGE_START; addr <= LB_CKS_RANGE_END; addr++) { + cmos_write(0x0, addr); + } +-*/ + rewrite_cmos(); ++#endif /* IS_ENABLED(CONFIG_USE_OPTION_TABLE) */ + + /* Set fallback boot */ + byte = cmos_read(RTC_BOOT_BYTE); + byte &= 0xfc; + cmos_write(byte, RTC_BOOT_BYTE); +-#else +- /* FIXME +- * Figure out how to recover if the option table is not available +- */ +-#endif + } + } +-- +2.39.1 + diff --git a/src/coreboot/patches/0004-superio-winbond-w83667hg-a-superio.c-do-not-use-get_.patch b/src/coreboot/patches/0004-superio-winbond-w83667hg-a-superio.c-do-not-use-get_.patch new file mode 100644 index 0000000..7d1bf38 --- /dev/null +++ b/src/coreboot/patches/0004-superio-winbond-w83667hg-a-superio.c-do-not-use-get_.patch @@ -0,0 +1,38 @@ +From 1b8c0f721ccd6fd43bd6a636fc3439f41e98e552 Mon Sep 17 00:00:00 2001 +From: Your Name +Date: Wed, 3 May 2023 15:13:42 -0700 +Subject: [PATCH 4/4] superio/winbond/w83667hg-a/superio.c: do not use + get_option() if CONFIG_USE_OPTION_TABLE + +Prior to this commit, w83667hg-a/superio.c would attempt to read the +"power on after power loss" byte from CMOS NVRAM even if +!CONFIG_USE_OPTION_TABLE. + +This commit removes that read, since when !CONFIG_USE_OPTION_TABLE +coreboot should not expect to be able to get useful information from +any part of the CMOS NVRAM other than RTC_BOOT_BYTE. + +When !CONFIG_USE_OPTION_TABLE coreboot will use the safest possible +option: the system is always configured to power on after a power +loss, every time coreboot starts up. +--- + src/superio/winbond/w83667hg-a/superio.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/src/superio/winbond/w83667hg-a/superio.c b/src/superio/winbond/w83667hg-a/superio.c +index 09859cf2c7..ec1de699d0 100644 +--- a/src/superio/winbond/w83667hg-a/superio.c ++++ b/src/superio/winbond/w83667hg-a/superio.c +@@ -69,7 +69,9 @@ static void w83667hg_a_init(struct device *dev) + case W83667HG_A_ACPI: + /* Set power state after power fail */ + power_status = CONFIG_MAINBOARD_POWER_ON_AFTER_POWER_FAIL; ++#if IS_ENABLED(CONFIG_USE_OPTION_TABLE) + get_option(&power_status, "power_on_after_fail"); ++#endif + pnp_enter_conf_mode_8787(dev); + pnp_set_logical_device(dev); + byte = pnp_read_config(dev, 0xe4); +-- +2.39.1 +