Joel Kitching has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38780 )
Change subject: vboot: remove VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT option
......................................................................
vboot: remove VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT option
With CL:1940398, this option is no longer needed. Recovery
requests are not cleared until kernel verification stage is
reached. If the FSP triggers any reboots, recovery requests
will be preserved. In particular:
- Manual requests will be preserved via recovery switch state,
whose behaviour is modified in CB:XXXXXX.
- Other recovery requests will remain in nvdata across reboot.
BUG=b:124141368, b:35576380
TEST=make clean && make test-abuild
BRANCH=none
Change-Id: I52d17a3c6730be5c04c3c0ae020368d11db6ca3c
Signed-off-by: Joel Kitching <kitching(a)google.com>
---
M src/security/vboot/Kconfig
M src/security/vboot/bootmode.c
M src/security/vboot/misc.h
M src/security/vboot/vbnv.c
M src/security/vboot/vbnv.h
M src/security/vboot/vboot_logic.c
M src/soc/amd/stoneyridge/Kconfig
M src/soc/intel/apollolake/Kconfig
M src/soc/intel/cannonlake/Kconfig
M src/soc/intel/icelake/Kconfig
M src/soc/intel/skylake/Kconfig
M src/soc/intel/tigerlake/Kconfig
12 files changed, 2 insertions(+), 87 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/38780/1
diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig
index ea70e65..54e88dd 100644
--- a/src/security/vboot/Kconfig
+++ b/src/security/vboot/Kconfig
@@ -156,14 +156,6 @@
reused by the succeeding stage. This is useful if a RAM space is too
small to fit both the verstage and the succeeding stage.
-config VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT
- bool
- default n
- help
- This option ensures that the recovery request is not lost because of
- reboots caused after vboot verification is run. e.g. reboots caused by
- FSP components on Intel platforms.
-
config VBOOT_MUST_REQUEST_DISPLAY
bool
default y if VGA_ROM_RUN
diff --git a/src/security/vboot/bootmode.c b/src/security/vboot/bootmode.c
index 61b3d76..5f15790 100644
--- a/src/security/vboot/bootmode.c
+++ b/src/security/vboot/bootmode.c
@@ -50,62 +50,21 @@
return sd->recovery_reason;
}
-void vboot_save_recovery_reason_vbnv(void)
-{
- if (!CONFIG(VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT))
- return;
-
- int reason = vboot_get_recovery_reason_shared_data();
- if (!reason)
- return;
-
- set_recovery_mode_into_vbnv(reason);
-}
-
-static void vboot_clear_recovery_reason_vbnv(void *unused)
-{
- if (!CONFIG(VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT))
- return;
-
- set_recovery_mode_into_vbnv(0);
-}
-
-/*
- * Recovery reason stored in VBNV needs to be cleared before the state of VBNV
- * is backed-up anywhere or jumping to the payload (whichever occurs
- * first). Currently, vbnv_cmos.c backs up VBNV on POST_DEVICE. Thus, we need to
- * make sure that the stored recovery reason is cleared off before that
- * happens.
- * IMPORTANT: Any reboot occurring after BS_DEV_INIT state will cause loss of
- * recovery reason on reboot. Until now, we have seen reboots occurring on x86
- * only in FSP stages which run before BS_DEV_INIT.
- */
-BOOT_STATE_INIT_ENTRY(BS_DEV_INIT, BS_ON_EXIT,
- vboot_clear_recovery_reason_vbnv, NULL);
-
/*
* vb2_check_recovery_request looks up different components to identify if there
* is a recovery request and returns appropriate reason code:
* 1. Checks if recovery mode is initiated by EC. If yes, returns
* VB2_RECOVERY_RO_MANUAL.
- * 2. Checks if recovery request is present in VBNV and returns the code read
- * from it.
- * 3. Checks if vboot verification is done. If yes, return the reason code from
+ * 2. Checks if vboot verification is done. If yes, return the reason code from
* shared data.
- * 4. If nothing applies, return 0 indicating no recovery request.
+ * 3. If nothing applies, return 0 indicating no recovery request.
*/
int vboot_check_recovery_request(void)
{
- int reason = 0;
-
/* EC-initiated recovery. */
if (get_recovery_mode_switch())
return VB2_RECOVERY_RO_MANUAL;
- /* Recovery request in VBNV. */
- if ((reason = get_recovery_mode_from_vbnv()) != 0)
- return reason;
-
/* Identify if vboot verification is already complete. */
if (vboot_logic_executed())
return vboot_get_recovery_reason_shared_data();
diff --git a/src/security/vboot/misc.h b/src/security/vboot/misc.h
index 2d5b084..c629ca5 100644
--- a/src/security/vboot/misc.h
+++ b/src/security/vboot/misc.h
@@ -52,11 +52,6 @@
int vboot_locate_firmware(struct vb2_context *ctx, struct region_device *fw);
/*
- * Source: security/vboot/bootmode.c
- */
-void vboot_save_recovery_reason_vbnv(void);
-
-/*
* The stage loading code is compiled and entered from multiple stages. The
* helper functions below attempt to provide more clarity on when certain
* code should be called. They are implemented inline for better compile-time
diff --git a/src/security/vboot/vbnv.c b/src/security/vboot/vbnv.c
index be598ac..a5a7806 100644
--- a/src/security/vboot/vbnv.c
+++ b/src/security/vboot/vbnv.c
@@ -101,26 +101,6 @@
vbnv_initialized = 0;
}
-/* Save a recovery reason into VBNV. */
-void set_recovery_mode_into_vbnv(int recovery_reason)
-{
- uint8_t vbnv_copy[VBOOT_VBNV_BLOCK_SIZE];
-
- read_vbnv(vbnv_copy);
-
- vbnv_copy[RECOVERY_OFFSET] = recovery_reason;
- vbnv_copy[CRC_OFFSET] = crc8_vbnv(vbnv_copy, CRC_OFFSET);
-
- save_vbnv(vbnv_copy);
-}
-
-/* Read the recovery reason from VBNV. */
-int get_recovery_mode_from_vbnv(void)
-{
- vbnv_setup();
- return vbnv[RECOVERY_OFFSET];
-}
-
/* Read the USB Device Controller(UDC) enable flag from VBNV. */
int vbnv_udc_enable_flag(void)
{
diff --git a/src/security/vboot/vbnv.h b/src/security/vboot/vbnv.h
index a2f0b4c..7d288d5 100644
--- a/src/security/vboot/vbnv.h
+++ b/src/security/vboot/vbnv.h
@@ -23,8 +23,6 @@
void save_vbnv(const uint8_t *vbnv_copy);
int verify_vbnv(uint8_t *vbnv_copy);
void regen_vbnv_crc(uint8_t *vbnv_copy);
-int get_recovery_mode_from_vbnv(void);
-void set_recovery_mode_into_vbnv(int recovery_reason);
/* Read the USB Device Controller(UDC) enable flag from VBNV. */
int vbnv_udc_enable_flag(void);
diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c
index eabf8f7..465c85b 100644
--- a/src/security/vboot/vboot_logic.c
+++ b/src/security/vboot/vboot_logic.c
@@ -429,8 +429,5 @@
vboot_is_firmware_slot_a(ctx) ? 'A' : 'B');
verstage_main_exit:
- /* Save recovery reason in case of unexpected reboots on x86. */
- vboot_save_recovery_reason_vbnv();
-
timestamp_add_now(TS_END_VBOOT);
}
diff --git a/src/soc/amd/stoneyridge/Kconfig b/src/soc/amd/stoneyridge/Kconfig
index c3fcad9..7d69a92 100644
--- a/src/soc/amd/stoneyridge/Kconfig
+++ b/src/soc/amd/stoneyridge/Kconfig
@@ -93,7 +93,6 @@
config VBOOT
select VBOOT_SEPARATE_VERSTAGE
select VBOOT_STARTS_IN_BOOTBLOCK
- select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT
select VBOOT_VBNV_CMOS
select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH
diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig
index 0d69da2..6c90294 100644
--- a/src/soc/intel/apollolake/Kconfig
+++ b/src/soc/intel/apollolake/Kconfig
@@ -113,7 +113,6 @@
config VBOOT
select VBOOT_SEPARATE_VERSTAGE
select VBOOT_MUST_REQUEST_DISPLAY
- select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT
select VBOOT_STARTS_IN_BOOTBLOCK
select VBOOT_VBNV_CMOS
select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH
diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig
index d098785..b68e93d 100644
--- a/src/soc/intel/cannonlake/Kconfig
+++ b/src/soc/intel/cannonlake/Kconfig
@@ -260,7 +260,6 @@
config VBOOT
select VBOOT_SEPARATE_VERSTAGE
select VBOOT_MUST_REQUEST_DISPLAY
- select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT
select VBOOT_STARTS_IN_BOOTBLOCK
select VBOOT_VBNV_CMOS
select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH
diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig
index 15e895f..210fd0d 100644
--- a/src/soc/intel/icelake/Kconfig
+++ b/src/soc/intel/icelake/Kconfig
@@ -165,7 +165,6 @@
config VBOOT
select VBOOT_SEPARATE_VERSTAGE
select VBOOT_MUST_REQUEST_DISPLAY
- select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT
select VBOOT_STARTS_IN_BOOTBLOCK
select VBOOT_VBNV_CMOS
select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH
diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig
index 6277cea..7382df0 100644
--- a/src/soc/intel/skylake/Kconfig
+++ b/src/soc/intel/skylake/Kconfig
@@ -93,7 +93,6 @@
config VBOOT
select VBOOT_SEPARATE_VERSTAGE
select VBOOT_MUST_REQUEST_DISPLAY
- select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT
select VBOOT_STARTS_IN_BOOTBLOCK
select VBOOT_VBNV_CMOS
select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH
diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig
index cef1fd0..56a0d74 100644
--- a/src/soc/intel/tigerlake/Kconfig
+++ b/src/soc/intel/tigerlake/Kconfig
@@ -189,7 +189,6 @@
config VBOOT
select VBOOT_SEPARATE_VERSTAGE
select VBOOT_MUST_REQUEST_DISPLAY
- select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT
select VBOOT_STARTS_IN_BOOTBLOCK
select VBOOT_VBNV_CMOS
select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH
--
To view, visit https://review.coreboot.org/c/coreboot/+/38780
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I52d17a3c6730be5c04c3c0ae020368d11db6ca3c
Gerrit-Change-Number: 38780
Gerrit-PatchSet: 1
Gerrit-Owner: Joel Kitching <kitching(a)google.com>
Gerrit-MessageType: newchange
Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37121 )
Change subject: soc/intel/denverton/uart.c: Clean up code
......................................................................
soc/intel/denverton/uart.c: Clean up code
Since there is only one device ID used for UART,
an array is not needed. Therefore, just save the
device ID to the device variable.
Change-Id: Icd325e1102a85cc175f6025519a47a1b64ee5b46
Signed-off-by: Felix Singer <felix.singer(a)9elements.com>
---
M src/soc/intel/denverton_ns/uart.c
1 file changed, 1 insertion(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/37121/1
diff --git a/src/soc/intel/denverton_ns/uart.c b/src/soc/intel/denverton_ns/uart.c
index 28e0e2e..3b851ee 100644
--- a/src/soc/intel/denverton_ns/uart.c
+++ b/src/soc/intel/denverton_ns/uart.c
@@ -57,15 +57,10 @@
.enable = DEVICE_NOOP
};
-static const unsigned short uart_ids[] = {
- PCI_DEVICE_ID_INTEL_DENVERTON_HSUART,
- 0
-};
-
static const struct pci_driver uart_driver __pci_driver = {
.ops = &uart_ops,
.vendor = PCI_VENDOR_ID_INTEL,
- .devices = uart_ids
+ .device = PCI_DEVICE_ID_INTEL_DENVERTON_HSUART
};
static void hide_hsuarts(void)
--
To view, visit https://review.coreboot.org/c/coreboot/+/37121
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icd325e1102a85cc175f6025519a47a1b64ee5b46
Gerrit-Change-Number: 37121
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: newchange
Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38690 )
Change subject: util/amdfwtool: Clarify APOB NV requirements
......................................................................
util/amdfwtool: Clarify APOB NV requirements
Relocate the first size check. This was automatically continuing
and not looking for the caller incorrectly passing a destination.
New information indicates that the APOB_NV should always be present
in the system. Augment the missing size check to inferring whether
a missing size is valid, as in the case of older products, or truly
missing when it's needed.
Signed-off-by: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Change-Id: I51f5333de4392dec1478bd84563c053a508b9e9e
---
M util/amdfwtool/amdfwtool.c
1 file changed, 25 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/38690/1
diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c
index d5c63de..c41da58 100644
--- a/util/amdfwtool/amdfwtool.c
+++ b/util/amdfwtool/amdfwtool.c
@@ -808,6 +808,17 @@
return 0;
}
+static int find_bios_entry(amd_bios_type type)
+{
+ int i;
+
+ for (i = 0; amd_bios_table[i].type != AMD_BIOS_INVALID; i++) {
+ if (amd_bios_table[i].type == type)
+ return i;
+ }
+ return -1;
+}
+
static void integrate_bios_firmwares(context *ctx,
bios_directory_table *biosdir,
bios_directory_table *biosdir2,
@@ -817,6 +828,7 @@
ssize_t bytes;
unsigned int i, count;
int level;
+ int apob_idx;
/* This function can create a primary table, a secondary table, or a
* flattened table which contains all applicable types. These if-else
@@ -843,9 +855,6 @@
fw_table[i].type != AMD_BIOS_L2_PTR &&
fw_table[i].type != AMD_BIOS_BIN))
continue;
- /* APOB_NV needs a size, else no S3 and skip item */
- if (fw_table[i].type == AMD_BIOS_APOB_NV && !fw_table[i].size)
- continue;
/* BIOS Directory items may have additional requirements */
@@ -857,6 +866,19 @@
exit(1);
}
}
+ /* APOB_NV needs a size, else no choice but to skip the item */
+ if (fw_table[i].type == AMD_BIOS_APOB_NV && !fw_table[i].size) {
+ /* Attempt to determine whether this is an error */
+ apob_idx = find_bios_entry(AMD_BIOS_APOB);
+ if (apob_idx < 0 || !fw_table[apob_idx].dest) {
+ /* APOV NV not expected to be used */
+ continue;
+ } else {
+ printf("Error: APOB NV must have a size\n");
+ free(ctx->rom);
+ exit(1);
+ }
+ }
/* APOB_DATA needs destination */
if (fw_table[i].type == AMD_BIOS_APOB && !fw_table[i].dest) {
--
To view, visit https://review.coreboot.org/c/coreboot/+/38690
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I51f5333de4392dec1478bd84563c053a508b9e9e
Gerrit-Change-Number: 38690
Gerrit-PatchSet: 1
Gerrit-Owner: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: newchange
Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38169 )
Change subject: soc/amd/picasso: Add PCI ID for Dali xHCI
......................................................................
soc/amd/picasso: Add PCI ID for Dali xHCI
soc//picasso is intended to be forward-compatible with the Dali APU, a
Family 17h Models 20h-2Fh product. Add the one new device ID it has.
See PPR document #55772 (still NDA only) for more information.
Change-Id: I7e9b90bb00ae6f4a121f10b1467d2ca398ac860c
Signed-off-by: Marshall Dawson <marshalldawson3rd(a)gmail.com>
---
M src/include/device/pci_ids.h
M src/soc/amd/picasso/usb.c
2 files changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/38169/1
diff --git a/src/include/device/pci_ids.h b/src/include/device/pci_ids.h
index bd5b3a5..52c6035 100644
--- a/src/include/device/pci_ids.h
+++ b/src/include/device/pci_ids.h
@@ -459,6 +459,7 @@
#define PCI_DEVICD_ID_AMD_PCO_ACP 0x15e2
#define PCI_DEVICE_ID_AMD_PCO_XHCI0 0x15e0
#define PCI_DEVICE_ID_AMD_PCO_XHCI1 0x15e1
+#define PCI_DEVICE_ID_AMD_DALI_XHCI 0x15e5
#define PCI_VENDOR_ID_VLSI 0x1004
#define PCI_DEVICE_ID_VLSI_82C592 0x0005
diff --git a/src/soc/amd/picasso/usb.c b/src/soc/amd/picasso/usb.c
index 80e960c..faea3c3 100644
--- a/src/soc/amd/picasso/usb.c
+++ b/src/soc/amd/picasso/usb.c
@@ -48,6 +48,7 @@
static const unsigned short pci_device_ids[] = {
PCI_DEVICE_ID_AMD_PCO_XHCI0,
PCI_DEVICE_ID_AMD_PCO_XHCI1,
+ PCI_DEVICE_ID_AMD_DALI_XHCI,
0
};
--
To view, visit https://review.coreboot.org/c/coreboot/+/38169
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7e9b90bb00ae6f4a121f10b1467d2ca398ac860c
Gerrit-Change-Number: 38169
Gerrit-PatchSet: 1
Gerrit-Owner: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: newchange
Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30806
Change subject: x86/acpi_s3: Remove trailing dots from debug message
......................................................................
x86/acpi_s3: Remove trailing dots from debug message
The dot is not needed, as it is no sentence and followed by a line
break.
Change-Id: I3905853eb7039f9c6d2486a77da47a4460276624
Signed-off-by: Paul Menzel <pmenzel(a)molgen.mpg.de>
---
M src/arch/x86/acpi_s3.c
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/30806/1
diff --git a/src/arch/x86/acpi_s3.c b/src/arch/x86/acpi_s3.c
index ad9fe00..07c0332 100644
--- a/src/arch/x86/acpi_s3.c
+++ b/src/arch/x86/acpi_s3.c
@@ -34,10 +34,10 @@
{
if (acpi_slp_type < 0) {
if (romstage_handoff_is_resume()) {
- printk(BIOS_DEBUG, "S3 Resume.\n");
+ printk(BIOS_DEBUG, "S3 Resume\n");
acpi_slp_type = ACPI_S3;
} else {
- printk(BIOS_DEBUG, "Normal boot.\n");
+ printk(BIOS_DEBUG, "Normal boot\n");
acpi_slp_type = ACPI_S0;
}
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/30806
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3905853eb7039f9c6d2486a77da47a4460276624
Gerrit-Change-Number: 30806
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newchange