coreboot-gerrit June 2022

coreboot-gerrit@coreboot.org

1 participants
1232 discussions

Change in coreboot[master]: New PCI ID for FCH SATA Controller (AHCI Mode) for AMD driver.
by Name of user not set (Code Review) 10 Jun '23

10 Jun '23

Name of user not set #1002701 has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37551 ) Change subject: New PCI ID for FCH SATA Controller (AHCI Mode) for AMD driver. ...................................................................... New PCI ID for FCH SATA Controller (AHCI Mode) for AMD driver. The 1022:7904 FCH SATA Controller is found in the AMD Bettong board. Support is added to configure properly the SATA device in initialization. SATA Controller has different device IDs for different drivers. They are renamed accordingly. Change-Id: Icdede188c82bfe8964c32fe81bdf6a3a6a17096c Signed-off-by: Jorge Fernandez <jorgefm(a)cirsa.com> --- M src/include/device/pci_ids.h M src/soc/amd/common/block/sata/sata.c M src/southbridge/amd/pi/hudson/sata.c 3 files changed, 6 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/37551/1 diff --git a/src/include/device/pci_ids.h b/src/include/device/pci_ids.h index 1276994..853e910 100644 --- a/src/include/device/pci_ids.h +++ b/src/include/device/pci_ids.h @@ -447,8 +447,8 @@ #define PCI_DEVICE_ID_AMD_CZ_HDA 0x157A #define PCI_DEVICE_ID_AMD_CZ_LPC 0x790E -#define PCI_DEVICE_ID_AMD_CZ_SATA 0x7900 -#define PCI_DEVICE_ID_AMD_CZ_SATA_AHCI 0x7901 +#define PCI_DEVICE_ID_AMD_CZ_SATA_IDE 0x7900 +#define PCI_DEVICE_ID_AMD_CZ_SATA_AHCI_MS 0x7901 #define PCI_DEVICE_ID_AMD_CZ_SATA_AHCI_AMD 0x7904 #define PCI_DEVICE_ID_AMD_CZ_USB_0 0x7907 #define PCI_DEVICE_ID_AMD_CZ_USB_1 0x7908 diff --git a/src/soc/amd/common/block/sata/sata.c b/src/soc/amd/common/block/sata/sata.c index 1cd342e..b276809 100644 --- a/src/soc/amd/common/block/sata/sata.c +++ b/src/soc/amd/common/block/sata/sata.c @@ -28,8 +28,8 @@ }; static const unsigned short pci_device_ids[] = { - PCI_DEVICE_ID_AMD_CZ_SATA, - PCI_DEVICE_ID_AMD_CZ_SATA_AHCI, + PCI_DEVICE_ID_AMD_CZ_SATA_IDE, + PCI_DEVICE_ID_AMD_CZ_SATA_AHCI_MS, PCI_DEVICE_ID_AMD_CZ_SATA_AHCI_AMD, 0 }; diff --git a/src/southbridge/amd/pi/hudson/sata.c b/src/southbridge/amd/pi/hudson/sata.c index 77365ef..5846a45 100644 --- a/src/southbridge/amd/pi/hudson/sata.c +++ b/src/southbridge/amd/pi/hudson/sata.c @@ -68,8 +68,8 @@ static const unsigned short pci_device_ids[] = { PCI_DEVICE_ID_AMD_SB900_SATA, PCI_DEVICE_ID_AMD_SB900_SATA_AHCI, - PCI_DEVICE_ID_AMD_CZ_SATA, - PCI_DEVICE_ID_AMD_CZ_SATA_AHCI, + PCI_DEVICE_ID_AMD_CZ_SATA_IDE, + PCI_DEVICE_ID_AMD_CZ_SATA_AHCI_MS, PCI_DEVICE_ID_AMD_CZ_SATA_AHCI_AMD, 0 }; -- To view, visit https://review.coreboot.org/c/coreboot/+/37551 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icdede188c82bfe8964c32fe81bdf6a3a6a17096c Gerrit-Change-Number: 37551 Gerrit-PatchSet: 1 Gerrit-Owner: Name of user not set #1002701 Gerrit-MessageType: newchange

5 16

Change in coreboot[4.11_branch]: soc/fsp_broadwell_de: read SPD 3 times and pick the majority element
by David Hendricks (Code Review) 10 Jun '23

10 Jun '23

Hello Andrey Petrov, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/37858 to review the following change. Change subject: soc/fsp_broadwell_de: read SPD 3 times and pick the majority element ...................................................................... soc/fsp_broadwell_de: read SPD 3 times and pick the majority element On the platform we consistently observe errneous reads of SPD data over IMC's SMBus. Since DDR4 SPD does not include CRC checksum for "Manufacturing Information" we can not determine serial/part numbers reliable. The root cause for SMBus errors is unknown. The corruption happens in 1 byte only with 3-4 bits flipped. Interestingly this happens after fixed amount of time, in case of tight-loop reads about every 3 seconds with almost exactly same number of CPU cycles, or multiples of that number. The stop-gap solution is to read data 3 times and pick out the prevalent member TEST=tested on OCP monolake Signed-off-by: Andrey Petrov <anpetrov(a)fb.com> Change-Id: If145cec2766f03412e33c98befb1d98b87306615 --- M src/soc/intel/fsp_broadwell_de/smbus-imc.c 1 file changed, 44 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/37858/1 diff --git a/src/soc/intel/fsp_broadwell_de/smbus-imc.c b/src/soc/intel/fsp_broadwell_de/smbus-imc.c index 61dc080..f483aae 100644 --- a/src/soc/intel/fsp_broadwell_de/smbus-imc.c +++ b/src/soc/intel/fsp_broadwell_de/smbus-imc.c @@ -13,6 +13,7 @@ * GNU General Public License for more details. */ +#include <console/console.h> #include <stddef.h> #include <device/pci_def.h> #include <device/early_smbus.h> @@ -20,6 +21,33 @@ #include <soc/pci_devs.h> #include <spd.h> +#define VOTE_ELEMENTS 3 + +static bool vote(uint16_t *arr, uint8_t n, uint8_t *res_idx) +{ + int maxcount = 0; + int index = -1; + + for(int i = 0; i < n; i++) { + int count = 0; + for(int j = 0; j < n; j++) { + if(arr[i] == arr[j]) + count++; + } + if(count > maxcount) { + maxcount = count; + index = i; + } + } + + if (maxcount > n/2) { + *res_idx = index; + return true; + } + + return false; +} + /* read word, return value on success */ uint16_t smbus_read_word(u32 smbus_dev, u8 addr, u8 offset) { @@ -36,13 +64,24 @@ /* read byte, return value on success */ uint8_t smbus_read_byte(u32 smbus_dev, u8 addr, u8 offset) { - uint16_t res = 0; + uint16_t res[VOTE_ELEMENTS] = {0}; + uint8_t idx = 0; - if (imc_smbus_spd_xfer(IMC_DEV, addr, offset, IMC_DEVICE_EEPROM, IMC_DATA_BYTE, - IMC_CONTROLLER_ID0, IMC_READ, &res) - == 0) { - return res; + for (int i = 0; i < ARRAY_SIZE(res); i++) { + if (imc_smbus_spd_xfer(IMC_DEV, + addr, offset, IMC_DEVICE_EEPROM, IMC_DATA_BYTE, + IMC_CONTROLLER_ID0, IMC_READ, &res[i]) != 0) { + return 0; + } } + + /* vote for majority element, pick 1 out of 3 */ + if (vote(res, ARRAY_SIZE(res), &idx)) { + return res[idx]; + } + + printk(BIOS_ALERT, "IMC: read failure, couldn't vote for correct byte\n"); + return 0; } -- To view, visit https://review.coreboot.org/c/coreboot/+/37858 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: 4.11_branch Gerrit-Change-Id: If145cec2766f03412e33c98befb1d98b87306615 Gerrit-Change-Number: 37858 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Andrey Petrov <anpetrov(a)fb.com> Gerrit-MessageType: newchange

3 3

Change in coreboot[master]: coreboot_table: Measure time of `write_coreboot_table()`
by Paul Menzel (Code Review) 10 Jun '23

10 Jun '23

Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38315 ) Change subject: coreboot_table: Measure time of `write_coreboot_table()` ...................................................................... coreboot_table: Measure time of `write_coreboot_table()` Change-Id: Ibd3e1595854ba15253d86cd3a09671a17f10c993 Signed-off-by: Paul Menzel <paulepanter(a)users.sourceforge.net> --- M src/lib/coreboot_table.c 1 file changed, 5 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/38315/1 diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c index e42cb3b..c081df0 100644 --- a/src/lib/coreboot_table.c +++ b/src/lib/coreboot_table.c @@ -35,6 +35,7 @@ #include <spi_flash.h> #include <security/vboot/misc.h> #include <security/vboot/vbnv_layout.h> +#include <timer.h> #if CONFIG(USE_OPTION_TABLE) #include <option_table.h> #endif @@ -581,6 +582,7 @@ uintptr_t cbtable_end; size_t cbtable_size; const size_t max_table_size = COREBOOT_TABLE_SIZE; + struct stopwatch sw; cbtable_start = (uintptr_t)cbmem_add(CBMEM_ID_CBTABLE, max_table_size); @@ -593,6 +595,7 @@ arch_write_tables(cbtable_start); /* Write the coreboot table. */ + stopwatch_init_msecs(&sw); cbtable_end = write_coreboot_table(cbtable_start); cbtable_size = cbtable_end - cbtable_start; @@ -601,7 +604,8 @@ __func__, cbtable_size, max_table_size); } - printk(BIOS_DEBUG, "coreboot table: %zd bytes.\n", cbtable_size); + printk(BIOS_DEBUG, "coreboot table: %zd bytes (took %lu ms).\n", + cbtable_size, stopwatch_duration_msecs(&sw)); /* Print CBMEM sections */ cbmem_list(); -- To view, visit https://review.coreboot.org/c/coreboot/+/38315 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibd3e1595854ba15253d86cd3a09671a17f10c993 Gerrit-Change-Number: 38315 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newchange

2 2

Change in coreboot[master]: vc/amd/agesa/[...]/Proc/Fch/Sata: Avoid Coverity Warnings
by Name of user not set (Code Review) 10 Jun '23

10 Jun '23

awokd(a)danwin1210.me has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38238 ) Change subject: vc/amd/agesa/[...]/Proc/Fch/Sata: Avoid Coverity Warnings ...................................................................... vc/amd/agesa/[...]/Proc/Fch/Sata: Avoid Coverity Warnings Although the original code worked, it caused Coverity to warn about pointer arithmetic potentially allowing out-of-bounds access. Simplify code using SATA initialization from SB900 as an example. Tested on Lenovo G505s. Change-Id: I6829a55d6c029abbf2da36e255e499c158c00e0d Signed-off-by: Joe Moore <awokd(a)danwin1210.me> Found-by: Coverity CID 1241886 --- M src/vendorcode/amd/agesa/f15tn/Proc/Fch/Sata/Family/Hudson2/Hudson2SataEnvService.c M src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataEnvService.c 2 files changed, 74 insertions(+), 112 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/38238/1 diff --git a/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Sata/Family/Hudson2/Hudson2SataEnvService.c b/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Sata/Family/Hudson2/Hudson2SataEnvService.c index 26ff51d..cfb3df9 100644 --- a/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Sata/Family/Hudson2/Hudson2SataEnvService.c +++ b/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Sata/Family/Hudson2/Hudson2SataEnvService.c @@ -59,9 +59,6 @@ // // Local Routine // -VOID FchSataCombineControlDataByte (IN UINT8 *ControlReg); -VOID FchSataCombineControlDataWord (IN UINT16 *ControlReg); - SATA_PHY_SETTING SataPhyTable[] = { //Gen3 @@ -94,8 +91,6 @@ IN VOID *FchDataPtr ) { - UINT8 *PortRegByte; - UINT16 *PortRegWord; FCH_DATA_BLOCK *LocalCfgPtr; AMD_CONFIG_PARAMS *StdHeader; @@ -104,14 +99,42 @@ // // Caculate SataPortReg for SATA_ESP_PORT // - PortRegByte = &(LocalCfgPtr->Sata.SataEspPort.SataPortReg); - FchSataCombineControlDataByte (PortRegByte); - PortRegByte = &(LocalCfgPtr->Sata.SataPortPower.SataPortReg); - FchSataCombineControlDataByte (PortRegByte); - PortRegWord = &(LocalCfgPtr->Sata.SataPortMd.SataPortMode); - FchSataCombineControlDataWord (PortRegWord); - PortRegByte = &(LocalCfgPtr->Sata.SataHotRemovalEnhPort.SataPortReg); - FchSataCombineControlDataByte (PortRegByte); + LocalCfgPtr->Sata.SataEspPort.SataPortReg = \ + + LocalCfgPtr->Sata.SataEspPort.Port0 \ + + (LocalCfgPtr->Sata.SataEspPort.Port1 << 1) \ + + (LocalCfgPtr->Sata.SataEspPort.Port2 << 2) \ + + (LocalCfgPtr->Sata.SataEspPort.Port3 << 3) \ + + (LocalCfgPtr->Sata.SataEspPort.Port4 << 4) \ + + (LocalCfgPtr->Sata.SataEspPort.Port5 << 5) \ + + (LocalCfgPtr->Sata.SataEspPort.Port6 << 6) \ + + (LocalCfgPtr->Sata.SataEspPort.Port7 << 7); + LocalCfgPtr->Sata.SataPortPower.SataPortReg = \ + + LocalCfgPtr->Sata.SataPortPower.Port0 \ + + (LocalCfgPtr->Sata.SataPortPower.Port1 << 1) \ + + (LocalCfgPtr->Sata.SataPortPower.Port2 << 2) \ + + (LocalCfgPtr->Sata.SataPortPower.Port3 << 3) \ + + (LocalCfgPtr->Sata.SataPortPower.Port4 << 4) \ + + (LocalCfgPtr->Sata.SataPortPower.Port5 << 5) \ + + (LocalCfgPtr->Sata.SataPortPower.Port6 << 6) \ + + (LocalCfgPtr->Sata.SataPortPower.Port7 << 7); + LocalCfgPtr->Sata.SataPortMd.SataPortMode = \ + + LocalCfgPtr->Sata.SataPortMd.Port0 \ + + (LocalCfgPtr->Sata.SataPortMd.Port1 << 2) \ + + (LocalCfgPtr->Sata.SataPortMd.Port2 << 4) \ + + (LocalCfgPtr->Sata.SataPortMd.Port3 << 6) \ + + (LocalCfgPtr->Sata.SataPortMd.Port4 << 8) \ + + (LocalCfgPtr->Sata.SataPortMd.Port5 << 10) \ + + (LocalCfgPtr->Sata.SataPortMd.Port6 << 12) \ + + (LocalCfgPtr->Sata.SataPortMd.Port7 << 14); + LocalCfgPtr->Sata.SataHotRemovalEnhPort.SataPortReg = \ + + LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port0 \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port1 << 1) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port2 << 2) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port3 << 3) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port4 << 4) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port5 << 5) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port6 << 6) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port7 << 7); // // Set Sata PCI Configuration Space Write enable @@ -156,52 +179,12 @@ } /** - * FchSataCombineControlDataByte - Combine port control options - * to one control byte. + * FchProgramSataPhy - Program Sata PHY registers * - * - * @param[in] *ControlReg - Data pointer for control byte. + * @param[in] StdHeader * */ VOID -FchSataCombineControlDataByte ( - IN UINT8 *ControlReg - ) -{ - UINT8 Index; - UINT8 PortControl; - - *ControlReg = 0; - for ( Index = 0; Index < 8; Index++ ) { - PortControl = *( ControlReg + 1 + Index ); - *ControlReg |= PortControl << Index; - } -} -/** - * FchSataCombineControlDataWord - Combine port control options - * to one control Word. - * - * - * @param[in] *ControlReg - Data pointer for control byte. - * - */ -VOID -FchSataCombineControlDataWord ( - IN UINT16 *ControlReg - ) -{ - UINT8 Index; - UINT8 PortControl; - - *ControlReg = 0; - for ( Index = 0; Index < 8; Index++ ) { - PortControl = *( (UINT8 *)ControlReg + 2 + Index ); - *ControlReg |= PortControl << (Index * 2); - } -} - - -VOID FchProgramSataPhy ( IN AMD_CONFIG_PARAMS *StdHeader ) diff --git a/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataEnvService.c b/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataEnvService.c index 0ae0993..161c758 100644 --- a/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataEnvService.c +++ b/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataEnvService.c @@ -72,8 +72,6 @@ // // Local Routine // -VOID FchSataCombineControlDataByte (IN UINT8 *ControlReg); -VOID FchSataCombineControlDataWord (IN UINT16 *ControlReg); /** @@ -89,8 +87,6 @@ IN VOID *FchDataPtr ) { - UINT8 *PortRegByte; - UINT16 *PortRegWord; FCH_DATA_BLOCK *LocalCfgPtr; AMD_CONFIG_PARAMS *StdHeader; @@ -99,14 +95,42 @@ // //Caculate SataPortReg for SATA_ESP_PORT // - PortRegByte = &(LocalCfgPtr->Sata.SataEspPort.SataPortReg); - FchSataCombineControlDataByte (PortRegByte); - PortRegByte = &(LocalCfgPtr->Sata.SataPortPower.SataPortReg); - FchSataCombineControlDataByte (PortRegByte); - PortRegWord = &(LocalCfgPtr->Sata.SataPortMd.SataPortMode); - FchSataCombineControlDataWord (PortRegWord); - PortRegByte = &(LocalCfgPtr->Sata.SataHotRemovalEnhPort.SataPortReg); - FchSataCombineControlDataByte (PortRegByte); + LocalCfgPtr->Sata.SataEspPort.SataPortReg = \ + + LocalCfgPtr->Sata.SataEspPort.Port0 \ + + (LocalCfgPtr->Sata.SataEspPort.Port1 << 1) \ + + (LocalCfgPtr->Sata.SataEspPort.Port2 << 2) \ + + (LocalCfgPtr->Sata.SataEspPort.Port3 << 3) \ + + (LocalCfgPtr->Sata.SataEspPort.Port4 << 4) \ + + (LocalCfgPtr->Sata.SataEspPort.Port5 << 5) \ + + (LocalCfgPtr->Sata.SataEspPort.Port6 << 6) \ + + (LocalCfgPtr->Sata.SataEspPort.Port7 << 7); + LocalCfgPtr->Sata.SataPortPower.SataPortReg = \ + + LocalCfgPtr->Sata.SataPortPower.Port0 \ + + (LocalCfgPtr->Sata.SataPortPower.Port1 << 1) \ + + (LocalCfgPtr->Sata.SataPortPower.Port2 << 2) \ + + (LocalCfgPtr->Sata.SataPortPower.Port3 << 3) \ + + (LocalCfgPtr->Sata.SataPortPower.Port4 << 4) \ + + (LocalCfgPtr->Sata.SataPortPower.Port5 << 5) \ + + (LocalCfgPtr->Sata.SataPortPower.Port6 << 6) \ + + (LocalCfgPtr->Sata.SataPortPower.Port7 << 7); + LocalCfgPtr->Sata.SataPortMd.SataPortMode = \ + + LocalCfgPtr->Sata.SataPortMd.Port0 \ + + (LocalCfgPtr->Sata.SataPortMd.Port1 << 2) \ + + (LocalCfgPtr->Sata.SataPortMd.Port2 << 4) \ + + (LocalCfgPtr->Sata.SataPortMd.Port3 << 6) \ + + (LocalCfgPtr->Sata.SataPortMd.Port4 << 8) \ + + (LocalCfgPtr->Sata.SataPortMd.Port5 << 10) \ + + (LocalCfgPtr->Sata.SataPortMd.Port6 << 12) \ + + (LocalCfgPtr->Sata.SataPortMd.Port7 << 14); + LocalCfgPtr->Sata.SataHotRemovalEnhPort.SataPortReg = \ + + LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port0 \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port1 << 1) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port2 << 2) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port3 << 3) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port4 << 4) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port5 << 5) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port6 << 6) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port7 << 7); // // Set Sata PCI Configuration Space Write enable @@ -140,51 +164,6 @@ } /** - * FchSataCombineControlDataByte - Combine port control options - * to one control byte. - * - * - * @param[in] *ControlReg - Data pointer for control byte. - * - */ -VOID -FchSataCombineControlDataByte ( - IN UINT8 *ControlReg - ) -{ - UINT8 Index; - UINT8 PortControl; - - *ControlReg = 0; - for ( Index = 0; Index < 8; Index++ ) { - PortControl = *( ControlReg + 1 + Index ); - *ControlReg |= PortControl << Index; - } -} -/** - * FchSataCombineControlDataWord - Combine port control options - * to one control Word. - * - * - * @param[in] *ControlReg - Data pointer for control byte. - * - */ -VOID -FchSataCombineControlDataWord ( - IN UINT16 *ControlReg - ) -{ - UINT8 Index; - UINT8 PortControl; - - *ControlReg = 0; - for ( Index = 0; Index < 8; Index++ ) { - PortControl = *( (UINT8 *)ControlReg + 2 + Index ); - *ControlReg |= PortControl << (Index * 2); - } -} - -/** * FchProgramSataPhy - Program Sata PHY registers * * @param[in] StdHeader -- To view, visit https://review.coreboot.org/c/coreboot/+/38238 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6829a55d6c029abbf2da36e255e499c158c00e0d Gerrit-Change-Number: 38238 Gerrit-PatchSet: 1 Gerrit-Owner: awokd(a)danwin1210.me Gerrit-MessageType: newchange

3 2

Change in coreboot[master]: vc/amd/agesa/f14/Proc/GNB: Avoid potential out-of-bounds read
by Name of user not set (Code Review) 10 Jun '23

10 Jun '23

awokd(a)danwin1210.me has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38172 ) Change subject: vc/amd/agesa/f14/Proc/GNB: Avoid potential out-of-bounds read ...................................................................... vc/amd/agesa/f14/Proc/GNB: Avoid potential out-of-bounds read In F14NbServices.c's NbFmFuseAdjustFuseTablePatch function, if the PpFuseArray->PolicyLabel[] array does not contain an element == POLICY_LABEL_PERFORMANCE, SwSatateIndex will be left at 6 after the loop ends, resulting in an out-of-bounds read in the following ASSERT. Move code inside loop so it only executes when expected. Change-Id: Iad83865faa6084476c6b399d7360b6af26db6d6d Signed-off-by: Joe Moore <awokd(a)danwin1210.me> Found-by: Coverity CID 1241910 --- M src/vendorcode/amd/agesa/f14/Proc/GNB/Nb/Family/0x14/F14NbServices.c 1 file changed, 11 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/38172/1 diff --git a/src/vendorcode/amd/agesa/f14/Proc/GNB/Nb/Family/0x14/F14NbServices.c b/src/vendorcode/amd/agesa/f14/Proc/GNB/Nb/Family/0x14/F14NbServices.c index d067875..db20c5b 100644 --- a/src/vendorcode/amd/agesa/f14/Proc/GNB/Nb/Family/0x14/F14NbServices.c +++ b/src/vendorcode/amd/agesa/f14/Proc/GNB/Nb/Family/0x14/F14NbServices.c @@ -208,20 +208,20 @@ PpFuseArray->LclkDpmVid[2] = PpFuseArray->PcieGen2Vid; if (GfxLibIsControllerPresent (StdHeader)) { //VID index = VID index associated with highest SCLK DPM state in the Powerplay state where Label_Performance=1 // This would ignore the UVD case (where Label_Performance would be 0). - for (SwSatateIndex = 0 ; SwSatateIndex < PP_FUSE_MAX_NUM_SW_STATE; SwSatateIndex++) { - if (PpFuseArray->PolicyLabel[SwSatateIndex] == POLICY_LABEL_PERFORMANCE) { - break; - } - } MaxSclkIndex = 0; CurrentSclkDpmDid = 0xff; - ASSERT (PpFuseArray->SclkDpmValid[SwSatateIndex] != 0); - for (DpmStateIndex = 0; DpmStateIndex < PP_FUSE_MAX_NUM_DPM_STATE; DpmStateIndex++) { - if ((PpFuseArray->SclkDpmValid[SwSatateIndex] & (1 << DpmStateIndex)) != 0) { - if (PpFuseArray->SclkDpmDid[DpmStateIndex] < CurrentSclkDpmDid) { - CurrentSclkDpmDid = PpFuseArray->SclkDpmDid[DpmStateIndex]; - MaxSclkIndex = DpmStateIndex; + for (SwSatateIndex = 0 ; SwSatateIndex < PP_FUSE_MAX_NUM_SW_STATE; SwSatateIndex++) { + if (PpFuseArray->PolicyLabel[SwSatateIndex] == POLICY_LABEL_PERFORMANCE) { + ASSERT (PpFuseArray->SclkDpmValid[SwSatateIndex] != 0); + for (DpmStateIndex = 0; DpmStateIndex < PP_FUSE_MAX_NUM_DPM_STATE; DpmStateIndex++) { + if ((PpFuseArray->SclkDpmValid[SwSatateIndex] & (1 << DpmStateIndex)) != 0) { + if (PpFuseArray->SclkDpmDid[DpmStateIndex] < CurrentSclkDpmDid) { + CurrentSclkDpmDid = PpFuseArray->SclkDpmDid[DpmStateIndex]; + MaxSclkIndex = DpmStateIndex; + } + } } + break; } } PpFuseArray->LclkDpmVid[1] = PpFuseArray->SclkDpmVid[MaxSclkIndex]; -- To view, visit https://review.coreboot.org/c/coreboot/+/38172 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iad83865faa6084476c6b399d7360b6af26db6d6d Gerrit-Change-Number: 38172 Gerrit-PatchSet: 1 Gerrit-Owner: awokd(a)danwin1210.me Gerrit-MessageType: newchange

3 2

Change in coreboot[master]: drivers/i2c/at24rf08c: Log number of failed writes
by Paul Menzel (Code Review) 10 Jun '23

10 Jun '23

Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38393 ) Change subject: drivers/i2c/at24rf08c: Log number of failed writes ...................................................................... drivers/i2c/at24rf08c: Log number of failed writes Change-Id: I1f786043557b11824d4efbde0689d1feb73de6e1 Signed-off-by: Paul Menzel <paulepanter(a)users.sourceforge.net> --- M src/drivers/i2c/at24rf08c/at24rf08c.c 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/38393/1 diff --git a/src/drivers/i2c/at24rf08c/at24rf08c.c b/src/drivers/i2c/at24rf08c/at24rf08c.c index 11a6fd2..50501ba 100644 --- a/src/drivers/i2c/at24rf08c/at24rf08c.c +++ b/src/drivers/i2c/at24rf08c/at24rf08c.c @@ -35,8 +35,10 @@ /* After a register write AT24RF08C sometimes stops responding. Retry several times in case of failure. */ for (j = 0; j < 100; j++) - if (smbus_write_byte(dev, i, 0x0f) >= 0) + if (smbus_write_byte(dev, i, 0x0f) >= 0) { + printk(BIOS_DEBUG, "j = %d", j); break; + } } printk(BIOS_DEBUG, "init EEPROM done\n"); -- To view, visit https://review.coreboot.org/c/coreboot/+/38393 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1f786043557b11824d4efbde0689d1feb73de6e1 Gerrit-Change-Number: 38393 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newchange

2 2

Change in coreboot[master]: arch/x86/smbios: Measure execution time of `get_smbios_strings()`
by Paul Menzel (Code Review) 10 Jun '23

10 Jun '23

Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38314 ) Change subject: arch/x86/smbios: Measure execution time of `get_smbios_strings()` ...................................................................... arch/x86/smbios: Measure execution time of `get_smbios_strings()` Change-Id: I97cfcd0849665b039920083bccfad8349d5de2c1 Signed-off-by: Paul Menzel <paulepanter(a)users.sourceforge.net> --- M src/arch/x86/smbios.c 1 file changed, 7 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/38314/1 diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c index 8cd4518..7de9acb 100644 --- a/src/arch/x86/smbios.c +++ b/src/arch/x86/smbios.c @@ -28,6 +28,7 @@ #include <device/pci_ids.h> #include <device/pci_def.h> #include <device/pci.h> +#include <timer.h> #if CONFIG(CHROMEOS) #include <vendorcode/google/chromeos/gnvs.h> #endif @@ -942,6 +943,7 @@ struct smbios_type11 *t = (struct smbios_type11 *)*current; int len; struct device *dev; + struct stopwatch sw; memset(t, 0, sizeof(*t)); t->type = SMBIOS_OEM_STRINGS; @@ -949,8 +951,12 @@ t->length = len = sizeof(*t) - 2; for (dev = all_devices; dev; dev = dev->next) { - if (dev->ops && dev->ops->get_smbios_strings) + if (dev->ops && dev->ops->get_smbios_strings) { + stopwatch_init_msecs(&sw); dev->ops->get_smbios_strings(dev, t); + printk(BIOS_SPEW, "%s get_smbios_strings took %lu ms\n", + dev_path(dev), stopwatch_duration_msecs(&sw)); + } } if (t->count == 0) { -- To view, visit https://review.coreboot.org/c/coreboot/+/38314 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I97cfcd0849665b039920083bccfad8349d5de2c1 Gerrit-Change-Number: 38314 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newchange

3 3

Change in coreboot[master]: drivers/i2c/at24rf08c: Tighten count variable type to unsigned integers
by Paul Menzel (Code Review) 10 Jun '23

10 Jun '23

Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38394 ) Change subject: drivers/i2c/at24rf08c: Tighten count variable type to unsigned integers ...................................................................... drivers/i2c/at24rf08c: Tighten count variable type to unsigned integers Change-Id: I225d0177b78984783d4f2506873ed220863b0467 Signed-off-by: Paul Menzel <paulepanter(a)users.sourceforge.net> --- M src/drivers/i2c/at24rf08c/at24rf08c.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/38394/1 diff --git a/src/drivers/i2c/at24rf08c/at24rf08c.c b/src/drivers/i2c/at24rf08c/at24rf08c.c index 50501ba..624df7d 100644 --- a/src/drivers/i2c/at24rf08c/at24rf08c.c +++ b/src/drivers/i2c/at24rf08c/at24rf08c.c @@ -19,7 +19,7 @@ static void at24rf08c_init(struct device *dev) { - int i, j; + unsigned int i, j; if (!dev->enabled) return; @@ -36,7 +36,7 @@ Retry several times in case of failure. */ for (j = 0; j < 100; j++) if (smbus_write_byte(dev, i, 0x0f) >= 0) { - printk(BIOS_DEBUG, "j = %d", j); + printk(BIOS_DEBUG, "i= = %u, j = %u\n", j); break; } } -- To view, visit https://review.coreboot.org/c/coreboot/+/38394 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I225d0177b78984783d4f2506873ed220863b0467 Gerrit-Change-Number: 38394 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newchange

3 4

Change in coreboot[master]: [under test]mb/google/hatch: Program gpio clk power gating settings i...
by Kane Chen (Code Review) 10 Jun '23

10 Jun '23

Kane Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37865 ) Change subject: [under test]mb/google/hatch: Program gpio clk power gating settings in SPI0 PS3/PS0 ...................................................................... [under test]mb/google/hatch: Program gpio clk power gating settings in SPI0 PS3/PS0 With PchPmSlpS0Vm075VSupport FSP UPD set, SoC requires gpio clk to be power gated. But when gpio clk is power gated, it requires longer interrupt assertion from device. This commit provides a way to set gpio clk power gating settings in SPIO PS0/PS3 so that cr50 doesn't need longer interrupt assertion and SoC can still enter runtime s0ix with PchPmSlpS0Vm075VSupport set. BUG=:141831197 Change-Id: I33a3d5897ec40afee29759160963363c322d5ad0 Signed-off-by: Kane Chen <kane.chen(a)intel.com> --- M src/mainboard/google/hatch/mainboard.asl 1 file changed, 14 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/37865/1 diff --git a/src/mainboard/google/hatch/mainboard.asl b/src/mainboard/google/hatch/mainboard.asl index dff1a75..db3c7ff 100644 --- a/src/mainboard/google/hatch/mainboard.asl +++ b/src/mainboard/google/hatch/mainboard.asl @@ -55,3 +55,17 @@ LOCL (0) } } + +Scope (\_SB.PCI0.SPI0) +{ + Method (_PS0, 0) + { + LOCL (0) + } + + Method(_PS3, 0) + { + LOCL (MISCCFG_ENABLE_GPIO_PM_CONFIG) + } + +} -- To view, visit https://review.coreboot.org/c/coreboot/+/37865 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I33a3d5897ec40afee29759160963363c322d5ad0 Gerrit-Change-Number: 37865 Gerrit-PatchSet: 1 Gerrit-Owner: Kane Chen <kane.chen(a)intel.com> Gerrit-MessageType: newchange

6 28

Change in coreboot[master]: security: Move TS_{START,END}_TPMINIT out of vboot
by Paul Menzel (Code Review) 10 Jun '23

10 Jun '23

Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38057 ) Change subject: security: Move TS_{START,END}_TPMINIT out of vboot ...................................................................... security: Move TS_{START,END}_TPMINIT out of vboot These are generic timestamps, and not vboot specific. Therefore, move them to `tpm_setup()`, so that these timestamps are added in all cases. (vboot timestamps should be namespaced with VBOOT anyway.) Change-Id: Ib1048f7b7a5903d186cdd750822b4bc8ea7dc665 Signed-off-by: Paul Menzel <pmenzel(a)molgen.mpg.de> --- M src/security/tpm/tspi/tspi.c M src/security/vboot/vboot_logic.c 2 files changed, 5 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/38057/1 diff --git a/src/security/tpm/tspi/tspi.c b/src/security/tpm/tspi/tspi.c index 5fcf92d..9ca9f98 100644 --- a/src/security/tpm/tspi/tspi.c +++ b/src/security/tpm/tspi/tspi.c @@ -19,6 +19,7 @@ #include <console/console.h> #include <security/tpm/tspi.h> #include <security/tpm/tss.h> +#include <timestamp.h> #if CONFIG(VBOOT) #include <vb2_api.h> #include <vb2_sha.h> @@ -127,6 +128,8 @@ { uint32_t result; + timestamp_add_now(TS_START_TPMINIT); + result = tlcl_lib_init(); if (result != TPM_SUCCESS) { printk(BIOS_ERR, "TPM: Can't initialize.\n"); @@ -174,6 +177,8 @@ result = tpm1_invoke_state_machine(); #endif + timestamp_add_now(TS_END_TPMINIT); + return tpm_setup_epilogue(result); } diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index 1d17a17..c65ea3a 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -320,10 +320,8 @@ /* Read secdata from TPM. Initialize TPM if secdata not found. We don't * check the return value here because vb2api_fw_phase1 will catch * invalid secdata and tell us what to do (=reboot). */ - timestamp_add_now(TS_START_TPMINIT); if (vboot_setup_tpm(ctx) == TPM_SUCCESS) antirollback_read_space_firmware(ctx); - timestamp_add_now(TS_END_TPMINIT); /* Enable measured boot mode */ if (CONFIG(VBOOT_MEASURED_BOOT) && -- To view, visit https://review.coreboot.org/c/coreboot/+/38057 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib1048f7b7a5903d186cdd750822b4bc8ea7dc665 Gerrit-Change-Number: 38057 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newchange

4 6

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2022