[coreboot-gerrit] [S] Change in coreboot[main]: soc/amd: Select Bank 0 in vbnv_platform_init_cmos()

[Yu-Ping Wu (Code Review)]

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Jérémy Compostella, Matt DeVillier.

Yu-Ping Wu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83495?usp=email )

Change subject: soc/amd: Select Bank 0 in vbnv_platform_init_cmos()
......................................................................

soc/amd: Select Bank 0 in vbnv_platform_init_cmos()

In AMD platforms, the bit 4 of CMOS's Register A (0x0a) is DV0 bank
selection (0 for Bank 0; 1 for Bank 1) [1]. Since the MC146818 driver
accesses VBNV via Bank 0, the bit must be cleared before we can save
VBNV to CMOS in verstage.

Usually there's no problem with that, because the Register A is
configured in cmos_init() in ramstage. However, if CMOS has lost power,
then in the first boot after that, the bit may contain arbitrary data in
verstage. If that bit happens to be 1, then CMOS writes in verstage will
fail.

To fix the problem, define vbnv_platform_init_cmos() to configure the
Register A (RTC_FREQ_SELECT). Static assertions are also added to make
sure the whole VBNV is accessible via Bank 0 and that the written byte
doesn't contain RTC_AMD_BANK_SELECT (bit 4). Note that the kernel driver
also ensures RTC_AMD_BANK_SELECT isn't set for AMD [2].

[1] 48751_16h_bkdg.pdf
[2] lore.kernel.org/lkml/[e-mail]

BUG=b:346716300
TEST=CMOS writes succeeded in verstage after battery cutoff
BRANCH=skyrim

Change-Id: Idf167387b403be1977ebc08daa1f40646dd8c83f
Signed-off-by: Yu-Ping Wu 
---
M src/include/pc80/mc146818rtc.h
M src/soc/amd/common/vboot/vbnv_cmos.c
2 files changed, 14 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/83495/1

diff --git a/src/include/pc80/mc146818rtc.h b/src/include/pc80/mc146818rtc.h
index 7818421..701cc73 100644
--- a/src/include/pc80/mc146818rtc.h
+++ b/src/include/pc80/mc146818rtc.h
@@ -33,6 +33,8 @@
 #  define RTC_REF_CLCK_4MHZ	0x00
 #  define RTC_REF_CLCK_1MHZ	0x10
 #  define RTC_REF_CLCK_32KHZ	0x20
+   /* In AMD BKDG, bit 4 is DV0 bank selection. Bits 5 and 6 are reserved. */
+#  define RTC_AMD_BANK_SELECT	0x10
    /* 2 values for divider stage reset, others for "testing purposes only" */
 #  define RTC_DIV_RESET1	0x60
 #  define RTC_DIV_RESET2	0x70
diff --git a/src/soc/amd/common/vboot/vbnv_cmos.c b/src/soc/amd/common/vboot/vbnv_cmos.c
index 5baf923..5b61653 100644
--- a/src/soc/amd/common/vboot/vbnv_cmos.c
+++ b/src/soc/amd/common/vboot/vbnv_cmos.c
@@ -3,6 +3,18 @@
 #include <security/vboot/vbnv.h>
 #include <pc80/mc146818rtc.h>

+#define RTC_FREQ_SELECT_AMD (RTC_REF_CLCK_32KHZ | RTC_RATE_1024HZ)
+
+void vbnv_platform_init_cmos(void)
+{
+	/* Select Bank 0 to allow writing to VBNV. */
+	_Static_assert(CONFIG_VBOOT_VBNV_OFFSET + 14 + VBOOT_VBNV_BLOCK_SIZE <= 128,
+		       "VBNV exceeds Bank 0 (CMOS address 0-127)");
+	_Static_assert(!(RTC_FREQ_SELECT_AMD & RTC_AMD_BANK_SELECT),
+		       "Bank 1 should not be selected for AMD");
+	cmos_write(RTC_FREQ_SELECT_AMD, RTC_FREQ_SELECT);
+}
+
 int vbnv_cmos_failed(void)
 {
    /* If CMOS power has failed, the century will be set to 0xff */