coreboot-gerrit August 2020

coreboot-gerrit@coreboot.org

1 participants
2457 discussions

Change in coreboot[master]: [TESTONLY]nb/intel/x4x: force COMMAND 2N mode
by Arthur Heymans (Code Review) 09 Jun '23

09 Jun '23

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36829 ) Change subject: [TESTONLY]nb/intel/x4x: force COMMAND 2N mode ...................................................................... [TESTONLY]nb/intel/x4x: force COMMAND 2N mode Change-Id: I4348ecdcdf07b025dccd68aae2c1d2599cbdd4f2 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/northbridge/intel/x4x/raminit_ddr23.c 1 file changed, 7 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/36829/1 diff --git a/src/northbridge/intel/x4x/raminit_ddr23.c b/src/northbridge/intel/x4x/raminit_ddr23.c index efdcbb6..3626497 100644 --- a/src/northbridge/intel/x4x/raminit_ddr23.c +++ b/src/northbridge/intel/x4x/raminit_ddr23.c @@ -234,13 +234,13 @@ /* Default 2N mode */ s->nmode = 2; - if (s->selected_timings.mem_clk <= MEM_CLOCK_1066MHz) - s->nmode = 1; - /* 2N on DDR3 1066 with with 2 dimms per channel */ - if ((s->selected_timings.mem_clk == MEM_CLOCK_1066MHz) && - (BOTH_DIMMS_ARE_POPULATED(s->dimms, 0) || - BOTH_DIMMS_ARE_POPULATED(s->dimms, 1))) - s->nmode = 2; + /* if (s->selected_timings.mem_clk <= MEM_CLOCK_1066MHz) */ + /* s->nmode = 1; */ + /* /\* 2N on DDR3 1066 with with 2 dimms per channel *\/ */ + /* if ((s->selected_timings.mem_clk == MEM_CLOCK_1066MHz) && */ + /* (BOTH_DIMMS_ARE_POPULATED(s->dimms, 0) || */ + /* BOTH_DIMMS_ARE_POPULATED(s->dimms, 1))) */ + /* s->nmode = 2; */ launch1 = ddr3_launch1_tab[s->nmode - 1] [s->selected_timings.mem_clk - MEM_CLOCK_800MHz]; launch2 = ddr3_launch2_tab[s->nmode - 1] -- To view, visit https://review.coreboot.org/c/coreboot/+/36829 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4348ecdcdf07b025dccd68aae2c1d2599cbdd4f2 Gerrit-Change-Number: 36829 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

2 4

Change in coreboot[master]: TEST ONLY: DO NOT MERGE
by Casper Chang (Code Review) 09 Jun '23

09 Jun '23

Casper Chang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36766 ) Change subject: TEST ONLY: DO NOT MERGE ...................................................................... TEST ONLY: DO NOT MERGE Change-Id: I53ef63261537acdaf74edc13047a3e5e499e749f --- A src/mainboard/google/kukui/TESTONLY 1 file changed, 0 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/36766/1 diff --git a/src/mainboard/google/kukui/TESTONLY b/src/mainboard/google/kukui/TESTONLY new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/src/mainboard/google/kukui/TESTONLY -- To view, visit https://review.coreboot.org/c/coreboot/+/36766 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I53ef63261537acdaf74edc13047a3e5e499e749f Gerrit-Change-Number: 36766 Gerrit-PatchSet: 1 Gerrit-Owner: Casper Chang <casper.chang(a)bitland.corp-partner.google.com> Gerrit-MessageType: newchange

2 2

Change in coreboot[master]: soc/mediatek/mt8183: Always run full calibration for debug DRAM issue...
by huayang duan (Code Review) 09 Jun '23

09 Jun '23

huayang duan has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36906 ) Change subject: soc/mediatek/mt8183: Always run full calibration for debug DRAM issue [DONOT MERGE] Disable fast calibration, that is always run full calibration for debug DRAM bitfilp issue. ...................................................................... soc/mediatek/mt8183: Always run full calibration for debug DRAM issue [DONOT MERGE] Disable fast calibration, that is always run full calibration for debug DRAM bitfilp issue. BUG=b:80501386,b:142358843 BRANCH=kukui TEST=Boots correctly on Kukui Change-Id: I693cfe98ea4317c4b856e9bb1f18046a10fe72f8 --- M src/soc/mediatek/mt8183/memory.c 1 file changed, 6 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/36906/1 diff --git a/src/soc/mediatek/mt8183/memory.c b/src/soc/mediatek/mt8183/memory.c index 13c3d9a..fafe2a6 100644 --- a/src/soc/mediatek/mt8183/memory.c +++ b/src/soc/mediatek/mt8183/memory.c @@ -170,6 +170,11 @@ if (CONFIG(MT8183_DRAM_DVFS) && !recovery_mode) config |= DRAMC_CONFIG_DVFS; + +printk(BIOS_ERR, "DRAM-K: Force disable Fast Calibration for debug DRAM issue\n"); + +// disable fast calibration for debug DRAM issue +if (0) { /* Load calibration params from flash and run fast calibration */ if (recovery_mode) { printk(BIOS_WARNING, "Skip loading cached calibration data\n"); @@ -195,6 +200,7 @@ printk(BIOS_WARNING, "Failed to read calibration data from flash\n"); } +} /* Run full calibration */ printk(BIOS_INFO, "DRAM-K: Full Calibration\n"); -- To view, visit https://review.coreboot.org/c/coreboot/+/36906 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I693cfe98ea4317c4b856e9bb1f18046a10fe72f8 Gerrit-Change-Number: 36906 Gerrit-PatchSet: 1 Gerrit-Owner: huayang duan <huayangduan(a)gmail.com> Gerrit-MessageType: newchange

2 2

Change in coreboot[master]: amd/bettong: Enable SATA controller
by Jorge Fernandez (Code Review) 09 Jun '23

09 Jun '23

Jorge Fernandez has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37618 ) Change subject: amd/bettong: Enable SATA controller ...................................................................... amd/bettong: Enable SATA controller A minor typo fixed and the SATA controller is enabled on FCH reset. Change-Id: Ib5a66a99379f53b7376dd69e58bb4c1cb082f20b Signed-off-by: Jorge Fernandez <jorgefm(a)cirsa.com> --- M src/mainboard/amd/bettong/BiosCallOuts.c 1 file changed, 5 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/37618/1 diff --git a/src/mainboard/amd/bettong/BiosCallOuts.c b/src/mainboard/amd/bettong/BiosCallOuts.c index e983690..3d2aa48 100644 --- a/src/mainboard/amd/bettong/BiosCallOuts.c +++ b/src/mainboard/amd/bettong/BiosCallOuts.c @@ -60,7 +60,7 @@ */ static void oem_fan_control(FCH_DATA_BLOCK *FchParams) { - /* Enable IMC fan control. the recommand way */ + /* Enable IMC fan control. the recommended way */ imc_reg_init(); FchParams->Imc.ImcEnable = TRUE; @@ -87,6 +87,10 @@ FchParams_reset->FchReset.Xhci0Enable = CONFIG(HUDSON_XHCI_ENABLE); FchParams_reset->FchReset.Xhci1Enable = FALSE; FchParams_reset->EarlyOemGpioTable = oem_bettong_gpio; + + /* SATA configuration */ + FchParams_reset->FchReset.SataEnable = TRUE; + } else if (StdHeader->Func == AMD_INIT_ENV) { FCH_DATA_BLOCK *FchParams_env = (FCH_DATA_BLOCK *)FchData; printk(BIOS_DEBUG, "Fch OEM config in INIT ENV "); @@ -103,7 +107,6 @@ FchParams_env->Usb.Xhci1Enable = FALSE; FchParams_env->Usb.USB30PortInit = 8; /* 8: If USB3 port is unremoveable. */ - /* sata configuration */ /* SD configuration */ /* Rev F has an on-board eMMC, which only supports SD 2.0 */ if (board_id() == 'F') { -- To view, visit https://review.coreboot.org/c/coreboot/+/37618 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib5a66a99379f53b7376dd69e58bb4c1cb082f20b Gerrit-Change-Number: 37618 Gerrit-PatchSet: 1 Gerrit-Owner: Jorge Fernandez Gerrit-MessageType: newchange

3 4

Change in coreboot[master]: [TEST-ONLY] Enable compliance mode for test
by Peichao Li (Code Review) 09 Jun '23

09 Jun '23

Peichao Li has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37323 ) Change subject: [TEST-ONLY] Enable compliance mode for test ...................................................................... [TEST-ONLY] Enable compliance mode for test BUG=None TEST=compliance mode test Signed-off-by: Peichao Wang <peichao.wang(a)bitland.corp-partner.google.com> Change-Id: Ieb65c2062352ae12b3355381a14d8ab66677febe --- M src/soc/intel/cannonlake/fsp_params.c 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/37323/1 diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c index 0713ef4..f533d04 100644 --- a/src/soc/intel/cannonlake/fsp_params.c +++ b/src/soc/intel/cannonlake/fsp_params.c @@ -218,6 +218,9 @@ ignore_gbe_ltr(); } } + + /* enable compliance mode */ + params->PchEnableComplianceMode = 1; /* Audio */ params->PchHdaDspEnable = config->PchHdaDspEnable; -- To view, visit https://review.coreboot.org/c/coreboot/+/37323 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieb65c2062352ae12b3355381a14d8ab66677febe Gerrit-Change-Number: 37323 Gerrit-PatchSet: 1 Gerrit-Owner: Peichao Li <peichao.wang(a)bitland.corp-partner.google.com> Gerrit-MessageType: newchange

3 5

Change in coreboot[master]: [TEST]cpu/x86/mp_init: Park APs in C7
by Nico Huber (Code Review) 09 Jun '23

09 Jun '23

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37007 ) Change subject: [TEST]cpu/x86/mp_init: Park APs in C7 ...................................................................... [TEST]cpu/x86/mp_init: Park APs in C7 Change-Id: I960871450407ade6e8f18e69921ce74aa7fbca24 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M src/cpu/x86/mp_init.c 1 file changed, 16 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/37007/1 diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 29ae3de..16435fc 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -184,7 +184,22 @@ static void park_this_cpu(void *unused) { - stop_this_cpu(); + /* need some address of write-back memory, use AP's stack for now */ + const struct cpu_info *const info = cpu_info(); + const uintptr_t stack_bottom = ALIGN_DOWN((uintptr_t)&_estack, CONFIG_STACK_SIZE) + - (info->index + 1) * CONFIG_STACK_SIZE; + asm volatile( + "xor %%ecx, %%ecx\n\t" + "xor %%edx, %%edx\n" + "__monitor:\n\t" + "mov %0, %%eax\n\t" + "monitor\n\t" + "mov $0x60, %%eax\n\t" + "mwait\n\t" + "jmp __monitor\n\t" + : + : "r" (stack_bottom) + : "eax", "ecx", "edx"); } /* By the time APs call ap_init() caching has been setup, and microcode has -- To view, visit https://review.coreboot.org/c/coreboot/+/37007 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I960871450407ade6e8f18e69921ce74aa7fbca24 Gerrit-Change-Number: 37007 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

5 7

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

09 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) 09 Jun '23

09 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) 09 Jun '23

09 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) 09 Jun '23

09 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

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit August 2020