coreboot-gerrit February 2020

coreboot-gerrit@coreboot.org

1 participants
1797 discussions

Change in coreboot[master]: Doc/mb/asus: Add H110M-E/M.2 Documentation
by Pavel Sayekat (Code Review) 10 Jun '23

10 Jun '23

Pavel Sayekat has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35011 ) Change subject: Doc/mb/asus: Add H110M-E/M.2 Documentation ...................................................................... Doc/mb/asus: Add H110M-E/M.2 Documentation Followed and adapted from the ASRock H110M-DVS Documentation Signed-off-by: Pavel Sayekat <pavelsayekat(a)gmail.com> Change-Id: I6c5aad50b513935eab70b502df1d95fb3413fa6b --- A Documentation/mainboard/asus/h110m-e_m2.md 1 file changed, 136 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/35011/1 diff --git a/Documentation/mainboard/asus/h110m-e_m2.md b/Documentation/mainboard/asus/h110m-e_m2.md new file mode 100644 index 0000000..134dca8 --- /dev/null +++ b/Documentation/mainboard/asus/h110m-e_m2.md @@ -0,0 +1,136 @@ +# ASUS H110M-E/M.2 + +This page describes how to run coreboot on the [ASUS H110M-E/M.2]. + +## Required proprietary blobs + +Mainboard is based on Intel Skylake/Kaby Lake processor and H110 Chipset. +Intel company provides [Firmware Support Package (2.0)](../../soc/intel/fsp/index.md) +(intel FSP 2.0) to initialize this generation silicon. Please see this +[document](../../soc/intel/code_development_model/code_development_model.md). + +FSP Information: + +```eval_rst ++-----------------------------+-------------------+-------------------+ +| FSP Project Name | Directory | Specification | ++-----------------------------+-------------------+-------------------+ +| 7th Generation Intel® Core™ | KabylakeFspBinPkg | 2.0 | +| processors and chipsets | | | +| (formerly Kaby Lake) | | | ++-----------------------------+-------------------+-------------------+ +``` + +## Building coreboot + +The following steps set the default parameters for this board to build a +fully working image: + +```bash +make distclean +touch .config +./util/scripts/config --enable VENDOR_ASUS +./util/scripts/config --enable BOARD_ASUS_H110M_E_M2 +./util/scripts/config --enable CONFIG_ADD_FSP_BINARIES +./util/scripts/config --enable CONFIG_FSP_USE_REPO +./util/scripts/config --set-str REALTEK_8168_MACADDRESS "xx:xx:xx:xx:xx:xx" +make olddefconfig +``` + +However, it is strongly advised to use `make menuconfig` afterwards +(or instead), so that you can see all of the settings. + +Use the following command to disable the serial console if debugging +output is not required: + +```bash +./util/scripts/config --disable CONSOLE_SERIAL +``` + +However, a more flexible method is to change the console log level from +within an OS using `util/nvramtool`, or with the `nvramcui` payload. + +Now, run `make` to build the coreboot image. + +## Flashing coreboot + +### Internal programming + +The main SPI flash can be accessed using [flashrom]. By default, only +the BIOS region of the flash is writable. If you wish to change any +other region, such as the Management Engine or firmware descriptor, then +an external programmer is required (unless you find a clever way around +the flash protection). More information about this [here](../../flash_tutorial/index.md). + +### External programming + +The flash chip is a 16 MiB socketed DIP-8 chip. Specifically, it's a +GIgaDevice GD25B128CPIG, whose datasheet can be found [here][GD25B128CPIG]. +The chip is located to the bottom right-hand side of the board. For +a precise location, refer to section 1.3 (Motherboard Layout) of the +[H110M-E/M.2 manual], where the chip is labelled "128Mb BIOS". Take note of +the chip's orientation, remove it from its socket, and flash it with +an external programmer. For reference, the notch in the chip should be +facing towards the bottom of the board. + +## Known issues + +- The VGA port doesn't work. Discrete graphic card is used as primary + device for display output (if CONFIG_ONBOARD_VGA_IS_PRIMARY is not + set). Dynamic switching between iGPU and PEG is not yet supported. + +- SuperIO GPIO pin is used to reset Realtek chip. However, since the + Logical Device 7 (GPIO7, GPIO8) is not initialized, the network + chip is in a reset state all the time. + +## Untested + +- parallel port +- PS/2 keyboard +- PS/2 mouse +- EHCI debug +- TPM +- infrared module +- chassis intrusion header +- chassis speaker header + +## Working + +- integrated graphics init with libgfxinit (see [Known issues](#known-issues)) +- PCIe x1 +- PEG x16 Gen3 +- SATA +- USB +- serial port +- onboard audio +- using `me_cleaner` +- using `flashrom` + +## TODO + +- NCT5539D GPIOs +- onboard network (see [Known issues](#known-issues)) +- S3 suspend/resume +- Wake-on-LAN +- hardware monitor + +## Technology + +```eval_rst ++------------------+--------------------------------------------------+ +| CPU | Intel Skylake/Kaby Lake (LGA1151) | ++------------------+--------------------------------------------------+ +| PCH | Intel Sunrise Point H110 | ++------------------+--------------------------------------------------+ +| Super I/O | Nuvoton NCT5539D | ++------------------+--------------------------------------------------+ +| EC | None | ++------------------+--------------------------------------------------+ +| Coprocessor | Intel Management Engine | ++------------------+--------------------------------------------------+ +``` + +[ASUS H110M-E/M.2]: https://www.asus.com/Motherboards/H110M-E-M-2/overview/ +[GD25B128CPIG]: https://www.gigadevice.com/datasheet/gd25b127d/ [The closest match found] +[flashrom]: https://flashrom.org/Flashrom +[ASUS H110M-E/M.2 manual]: https://dlcdnets.asus.com/pub/ASUS/mb/LGA1151/H110M-E_M2/E11622_H110M-E_M2_… -- To view, visit https://review.coreboot.org/c/coreboot/+/35011 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6c5aad50b513935eab70b502df1d95fb3413fa6b Gerrit-Change-Number: 35011 Gerrit-PatchSet: 1 Gerrit-Owner: Pavel Sayekat Gerrit-MessageType: newchange

5 12

Change in coreboot[master]: Documentation/soc/intel: Add documentation for MCU update
by Varshit B Pandya (Code Review) 10 Jun '23

10 Jun '23

Varshit B Pandya has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34657 ) Change subject: Documentation/soc/intel: Add documentation for MCU update ...................................................................... Documentation/soc/intel: Add documentation for MCU update Change-Id: I3deb24550c1f1cd4c7b8082af75115ca174c295a Signed-off-by: Pandya, Varshit B <varshit.b.pandya(a)intel.com> --- A Documentation/soc/intel/ucode_update/Flash_Layout.PNG A Documentation/soc/intel/ucode_update/microcode_update_model.md 2 files changed, 43 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/34657/1 diff --git a/Documentation/soc/intel/ucode_update/Flash_Layout.PNG b/Documentation/soc/intel/ucode_update/Flash_Layout.PNG new file mode 100644 index 0000000..ac18977 --- /dev/null +++ b/Documentation/soc/intel/ucode_update/Flash_Layout.PNG Binary files differ diff --git a/Documentation/soc/intel/ucode_update/microcode_update_model.md b/Documentation/soc/intel/ucode_update/microcode_update_model.md new file mode 100644 index 0000000..e2d7f35 --- /dev/null +++ b/Documentation/soc/intel/ucode_update/microcode_update_model.md @@ -0,0 +1,43 @@ +# Microcode update mechanism for devices in field + +## Introduction + +Processor microcode (MCU) is akin to processor firmware. Processors may need updates to their microcode to operate correctly. These updates fix bugs/errata that can cause anything from incorrect processing, to code and data corruption, and system lockups. + +##Problem Statement + +As per chrome design, any field updatable FW has to be first verified by vboot (chrome root of trust - CROT) before it is allowed to run on the system. CROT executes on the CPU, hence FW such as MCU loaded before the CPU reset cannot be verified before it actually gets loaded. As a result today Chromebooks are not able to apply MCU in the field. This proposal tries to address the issue + +Prior to CPU reset, the CPU HW locates the FIT table from the storage, walks over the FIT table and picks-up the MCU that matches the CPU Family, Model, Stepping and PLATFORM ID with the highest revision ID. CPU verifies that MCU is signed by Intel and once verification is successful, it attempts to load the MCU. + +However FIT does not contain any fall back mechanism. If one MCU loading fails for whatever reason, it drops the further loading process and moves on to CPU reset. While it is fairly a remote chance, in case of any failure to load MCU which cause hang like situations before CPU reset, a platform based mechanism can be put in place to enter recovery mode. As detailed below, if we are using BUC register based top swap implementation, then an RTC reset may be required to clear top swap bit. + +## Design Proposal + +The proposal relies on the following Intel SoC features: + +1. Top swap, which enables the BIOS to have 2 bootblocks and bot from either using a RTC backed Top swap control bit in BUC register. +2. Firmware Interface Table (FIT) based MCU loading. + +The idea is to have 2 bootblocks, each having its own FIT table. The FIT in one of them will be configured to point to an MCU in RO firmware, and the FIT in other one will point to a fixed flash region (aka MCU staging area), which will contain an MCU from RW firmware. + +Top-swap feature of Intel CPUs, will be used to switch between 2 bootblocks, based on the bootmode i.e., recovery or normal/developer mode. + +Coreboot will keep the MCU staging area updated with the MCU from the currently selected RW firmware. + +## Implementation Details + +1. Create 2 BBs (bb_ro_ucode and bb_staged_ucode) each containing their own FIT tables. Both bootblocks will be in the RO section. + a. bb_ro_ucode contains a FIT which has pointer to MCU (microcode_blob.bin) which resides in RO section. This is will be used in the recovery scenario. + b. bb_staged_ucode is identical to bb_ro_ucode but contains a FIT which has pointer to a MCU that will reside in a staging area. +2. Create a MCU staging area + a. Reserve a region in the FMAP which is equal to or greater than the MCU size specified in the BWG for that SoC. This is a RW region just lke the RW_MRC_CACHE. + b. MCU from RW-A/RW-B will be copied to this region during boot. + c. A Flash Protected Range (FPR) should be configured to write protect this MCU Staging area. + d. bb_staged_ucode FIT points to the MCU in this staging area. +3. The resulting flash layout is shown below, +**Flash Layout** +![Flash Layout][flash_layout] +[flash_layout]: Flash_Layout.PNG + +4. The proposed design uses the PCR register based control for top-swap -- To view, visit https://review.coreboot.org/c/coreboot/+/34657 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3deb24550c1f1cd4c7b8082af75115ca174c295a Gerrit-Change-Number: 34657 Gerrit-PatchSet: 1 Gerrit-Owner: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-MessageType: newchange

6 15

Change in coreboot[master]: emulation/qemu-i440fx: Add \_SB.PCI0.MBRS
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/+/38304 ) Change subject: emulation/qemu-i440fx: Add \_SB.PCI0.MBRS ...................................................................... emulation/qemu-i440fx: Add \_SB.PCI0.MBRS Linux 5.4.8 shows the ACPI error below. [ 0.732324] ACPI BIOS Error (bug): Could not resolve symbol [\_SB.PCI0.MBRS], AE_NOT_FOUND (20190816/dswload2-159) [ 0.732784] ACPI Error: AE_NOT_FOUND, During name lookup/catalog (20190816/psobject-220) So, add that symbol. Copied from (removed) `src/mainboard/hp/dl145_g1/dsdt.asl`. Change-Id: I0243f6a469f5addbf2f09f3aed52d1a067b22cc9 Signed-off-by: Paul Menzel <pmenzel(a)molgen.mpg.de> --- M src/mainboard/emulation/qemu-i440fx/dsdt.asl 1 file changed, 7 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/38304/1 diff --git a/src/mainboard/emulation/qemu-i440fx/dsdt.asl b/src/mainboard/emulation/qemu-i440fx/dsdt.asl index 9bef7d9..95b4b42 100644 --- a/src/mainboard/emulation/qemu-i440fx/dsdt.asl +++ b/src/mainboard/emulation/qemu-i440fx/dsdt.asl @@ -33,6 +33,13 @@ Scope(\_SB) { Device(PCI0) { + /* Dummy device to hold auto generated reserved resources */ + Device(MBRS) { + Name (_HID, EisaId ("PNP0C02")) + Name (_UID, 0x01) + External(_CRS) /* Resource Template in SSDT */ + } + Name(_HID, EisaId("PNP0A03")) Name(_UID, 1) } -- To view, visit https://review.coreboot.org/c/coreboot/+/38304 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0243f6a469f5addbf2f09f3aed52d1a067b22cc9 Gerrit-Change-Number: 38304 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newchange

3 4

Change in coreboot[master]: [UNTESTED] intel/stm: Introduce stm_update_smm_info()
by Nico Huber (Code Review) 10 Jun '23

10 Jun '23

3 6

Change in coreboot[master]: [UNTESTED] cpu/x86/smm: Drop now unused `num_cpus` param
by Nico Huber (Code Review) 10 Jun '23

10 Jun '23

3 2

Change in coreboot[master]: [UNTESTED] intel/stm: Move SMRAM allocations into stm_update_smm_info()
by Nico Huber (Code Review) 10 Jun '23

10 Jun '23

Hello Kyösti Mälkki, Aaron Durbin, Arthur Heymans, cedarhouse1(a)comcast.net, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/38769 to review the following change. Change subject: [UNTESTED] intel/stm: Move SMRAM allocations into stm_update_smm_info() ...................................................................... [UNTESTED] intel/stm: Move SMRAM allocations into stm_update_smm_info() Also keep related calculations local to STM code. Change-Id: I3aa5915ddeffdabe792b9c30cd72cc39a8f54ca6 --- M src/cpu/x86/mp_init.c M src/cpu/x86/smm/smm_module_loader.c M src/security/intel/stm/SmmStm.h M src/security/intel/stm/StmPlatformSmm.c 4 files changed, 39 insertions(+), 33 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/38769/1 diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 5b34d4b..45f7337 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -746,22 +746,8 @@ /* Setup code checks this callback for validity. */ mp_state.ops.relocation_handler(cpu, curr_smbase, perm_smbase); - if (CONFIG(STM)) { - if (is_smm_enabled()) { - uintptr_t mseg; - - mseg = mp_state.perm_smbase + - (mp_state.perm_smsize - CONFIG_MSEG_SIZE); - - stm_setup(mseg, p->cpu, - perm_smbase, - mp_state.perm_smbase, - runtime->start32_offset); - } else { - printk(BIOS_DEBUG, - "STM not loaded because SMM is not enabled!\n"); - } - } + stm_setup(cpu, mp_state.perm_smbase, mp_state.perm_smsize, + perm_smbase, runtime->start32_offset); } static void adjust_smm_apic_id_map(struct smm_loader_params *smm_params) diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index d4a384a..6482c35 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -17,7 +17,6 @@ #include <cpu/x86/cache.h> #include <commonlib/helpers.h> #include <console/console.h> -#include <security/intel/stm/SmmStm.h> #define FXSAVE_SIZE 512 @@ -314,13 +313,10 @@ /* The SMM module is placed within the provided region in the following * manner: * +-----------------+ <- smram + size - * | BIOS resource | - * | list (STM) | - * +-----------------+ <- smram + size - CONFIG_BIOS_RESOURCE_LIST_SIZE * | stacks | - * +-----------------+ <- .. - total_stack_size + * +-----------------+ <- smram + size - total_stack_size * | fxsave area | - * +-----------------+ <- .. - total_stack_size - fxsave_size + * +-----------------+ <- smram + size - total_stack_size - fxsave_size * | ... | * +-----------------+ <- smram + handler_size + SMM_DEFAULT_SIZE * | handler | @@ -362,10 +358,6 @@ /* Stacks start at the top of the region. */ base = smram; base += size; - - if (CONFIG(STM)) - base -= CONFIG_MSEG_SIZE + CONFIG_BIOS_RESOURCE_LIST_SIZE; - params->stack_top = base; /* SMM module starts at offset SMM_DEFAULT_SIZE with the load alignment diff --git a/src/security/intel/stm/SmmStm.h b/src/security/intel/stm/SmmStm.h index f6ab0bf..58d74c0 100644 --- a/src/security/intel/stm/SmmStm.h +++ b/src/security/intel/stm/SmmStm.h @@ -30,13 +30,14 @@ #if CONFIG(STM) void stm_update_smm_info( uintptr_t *perm_smbase, size_t *perm_smsize, size_t *smm_save_state_size); +void stm_setup( + int cpu, uintptr_t perm_smbase, size_t perm_smsize, + uintptr_t staggered_smbase, size_t offset32); #else static inline void stm_update_smm_info(uintptr_t *, size_t *, size_t *) {} +static inline void stm_setup(int, uintptr_t, size_t, uintptr_t, size_t) {} #endif -void stm_setup( - uintptr_t mseg, int cpu, uintptr_t smbase, - uintptr_t smbase_base, uint32_t offset32); /* * Add resources in list to database. Allocate new memory areas as needed. diff --git a/src/security/intel/stm/StmPlatformSmm.c b/src/security/intel/stm/StmPlatformSmm.c index c389afa..c3c3ee8 100644 --- a/src/security/intel/stm/StmPlatformSmm.c +++ b/src/security/intel/stm/StmPlatformSmm.c @@ -154,10 +154,34 @@ static int stm_load_status = 0; +/* + * We cut off some space from the top of SMRAM and also increase + * the per CPU save-state for the TXT SMM descriptor: + * +--------------------+ <- smram + size + * | MSEG | + * +--------------------+ <- smram + size - MSEG size + * | BIOS Resource List | + * +--------------------+ <- smram + size - MSEG size - BIOS Resource list size + * | SMM module | + * | ... | + * | +-------------+ | + * | + + | + * | + ... + | + * | +-------------+ | + * | + TXT Desc. + | + * | + CPU 1 save + | + * | +-------------+ | + * | + TXT Desc. + | + * | + CPU 0 save + | + * | +-------------+ | + * +--------------------+ <- smram + */ void stm_update_smm_info( uintptr_t *const perm_smbase, size_t *const perm_smsize, size_t *const smm_save_state_size) { + *perm_smsize -= CONFIG_MSEG_SIZE + CONFIG_BIOS_RESOURCE_LIST_SIZE; + *smm_save_state_size += sizeof(TXT_PROCESSOR_SMM_DESCRIPTOR); /* @@ -169,19 +193,22 @@ *smm_save_state_size = ALIGN_UP(*smm_save_state_size, 0x1000); } -void stm_setup(uintptr_t mseg, int cpu, uintptr_t smbase, - uintptr_t base_smbase, uint32_t offset32) +void stm_setup( + const int cpu, const uintptr_t perm_smbase, const size_t perm_smsize, + const uintptr_t staggered_smbase, const size_t offset32) { msr_t InitMseg; msr_t MsegChk; uintptr_t addr_calc; // used to calculate the stm resource heap area + const uintptr_t mseg = perm_smbase + perm_smsize + CONFIG_BIOS_RESOURCE_LIST_SIZE; + printk(BIOS_DEBUG, "STM: set up for cpu %d\n", cpu); if (cpu == 0) { // need to create the BIOS resource list once // first calculate the location in SMRAM - addr_calc = mseg - CONFIG_BIOS_RESOURCE_LIST_SIZE; + addr_calc = perm_smbase + perm_smsize; stm_resource_heap = (uint8_t *) addr_calc; printk(BIOS_DEBUG, "STM: stm_resource_heap located at %p\n", stm_resource_heap); @@ -204,7 +231,7 @@ cpu, MsegChk.hi, MsegChk.lo); // setup the descriptor for this cpu - setup_smm_descriptor((void *)smbase, (void *) base_smbase, + setup_smm_descriptor((void *)staggered_smbase, (void *)perm_smbase, cpu, offset32); } else { printk(BIOS_DEBUG, -- To view, visit https://review.coreboot.org/c/coreboot/+/38769 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3aa5915ddeffdabe792b9c30cd72cc39a8f54ca6 Gerrit-Change-Number: 38769 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: cedarhouse1(a)comcast.net Gerrit-MessageType: newchange

3 6

Change in coreboot[master]: [UNTESTED] intel/stm: Drop now unneeded `num_cpus` param
by Nico Huber (Code Review) 10 Jun '23

10 Jun '23

3 3

Change in coreboot[master]: [UNTESTED] intel/stm: Place resource list right below MSEG
by Nico Huber (Code Review) 10 Jun '23

10 Jun '23

4 5

Change in coreboot[master]: sb/amd/{pi,agesa}/hudson/smbus_spd.c: use coreboot code style and calls
by Michał Żygowski (Code Review) 10 Jun '23

10 Jun '23

Michał Żygowski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38163 ) Change subject: sb/amd/{pi,agesa}/hudson/smbus_spd.c: use coreboot code style and calls ...................................................................... sb/amd/{pi,agesa}/hudson/smbus_spd.c: use coreboot code style and calls Use code comments in the coreboot style and replace outb/inb.rdtsc calls to use coreboot provided functions. Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> Change-Id: I40919df7d6dfbadd09800c8e689382483ffaca14 --- M src/southbridge/amd/agesa/hudson/smbus_spd.c M src/southbridge/amd/pi/hudson/smbus_spd.c 2 files changed, 118 insertions(+), 99 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/38163/1 diff --git a/src/southbridge/amd/agesa/hudson/smbus_spd.c b/src/southbridge/amd/agesa/hudson/smbus_spd.c index 9ddae38..ac52887 100644 --- a/src/southbridge/amd/agesa/hudson/smbus_spd.c +++ b/src/southbridge/amd/agesa/hudson/smbus_spd.c @@ -14,89 +14,98 @@ */ #include <amdblocks/acpimmio.h> +#include <arch/io.h> +#include <cpu/x86/tsc.h> #include <device/pci_def.h> #include <device/device.h> #include <console/console.h> -/* warning: Porting.h includes an open #pragma pack(1) */ -#include <Porting.h> #include <AGESA.h> -#include <amdlib.h> #include <northbridge/amd/agesa/dimmSpd.h> -/*----------------------------------------------------------------------------- - * - * readSmbusByteData - read a single SPD byte from any offset - */ - -static int readSmbusByteData (int iobase, int address, char *buffer, int offset) +/* readSmbusByteData - read a single SPD byte from any offset */ +static int readSmbusByteData(uint16_t iobase, int address, char *buffer, + int offset) { unsigned int status; - UINT64 limit; + unsigned long long limit; - address |= 1; // set read bit + address |= 1; /* set read bit */ + outb(0xff, iobase); /* clear error status */ + outb(0x1f, iobase + 1); /* clear error status */ + outb(offset, iobase + 3); /* offset in eeprom */ + outb(address, iobase + 4); /* slave address and read bit */ + outb(0x48, iobase + 2); /* read byte command */ - __outbyte (iobase + 0, 0xFF); // clear error status - __outbyte (iobase + 1, 0x1F); // clear error status - __outbyte (iobase + 3, offset); // offset in eeprom - __outbyte (iobase + 4, address); // slave address and read bit - __outbyte (iobase + 2, 0x48); // read byte command - - // time limit to avoid hanging for unexpected error status (should never happen) - limit = __rdtsc () + 2000000000 / 10; + /* + * time limit to avoid hanging for unexpected error status + * (should never happen) + */ + limit = rdtscll() + 2000000000 / 10; for (;;) { - status = __inbyte (iobase); - if (__rdtsc () > limit) break; - if ((status & 2) == 0) continue; // SMBusInterrupt not set, keep waiting - if ((status & 1) == 1) continue; // HostBusy set, keep waiting + status = inb(iobase); + if (rdtscll() > limit) + break; + /* SMBusInterrupt not set, keep waiting */ + if ((status & 2) == 0) + continue; + /* HostBusy set, keep waiting */ + if ((status & 1) == 1) + continue; break; } - buffer [0] = __inbyte (iobase + 5); - if (status == 2) status = 0; // check for done with no errors + buffer[0] = inb(iobase + 5); + /* check for done with no errors */ + if (status == 2) + status = 0; return status; } -/*----------------------------------------------------------------------------- - * +/* * readSmbusByte - read a single SPD byte from the default offset * this function is faster function readSmbusByteData */ - -static int readSmbusByte (int iobase, int address, char *buffer) +static int readSmbusByte(uint16_t iobase, int address, char *buffer) { unsigned int status; - UINT64 limit; + unsigned long long limit; - __outbyte (iobase + 0, 0xFF); // clear error status - __outbyte (iobase + 2, 0x44); // read command + outb(0xff, iobase + 0); /* clear error status */ + outb(0x44, iobase + 2); /* read command */ - // time limit to avoid hanging for unexpected error status - limit = __rdtsc () + 2000000000 / 10; + /* time limit to avoid hanging for unexpected error status*/ + limit = rdtscll() + 2000000000 / 10; for (;;) { - status = __inbyte (iobase); - if (__rdtsc () > limit) break; - if ((status & 2) == 0) continue; // SMBusInterrupt not set, keep waiting - if ((status & 1) == 1) continue; // HostBusy set, keep waiting + status = inb(iobase); + if (rdtscll() > limit) + break; + /* SMBusInterrupt not set, keep waiting */ + if ((status & 2) == 0) + continue; + /* HostBusy set, keep waiting */ + if ((status & 1) == 1) + continue; break; } - buffer [0] = __inbyte (iobase + 5); - if (status == 2) status = 0; // check for done with no errors + buffer[0] = inb(iobase + 5); + /* check for done with no errors */ + if (status == 2) + status = 0; return status; } -/*--------------------------------------------------------------------------- - * +/* * readspd - Read one or more SPD bytes from a DIMM. * Start with offset zero and read sequentially. * Optimization relies on autoincrement to avoid * sending offset for every byte. - * Reads 128 bytes in 7-8 ms at 400 KHz. + * Reads 128 bytes in 7-8 ms at 400 KHz. */ - -static int readspd (int iobase, int SmbusSlaveAddress, char *buffer, int count) +static int readspd(uint16_t iobase, int SmbusSlaveAddress, char *buffer, + int count) { int index, error; @@ -127,15 +136,16 @@ return 0; } -static void setupFch (int ioBase) +static void setupFch(uint16_t ioBase) { pm_write16(0x2c, ioBase | 1); - __outbyte (ioBase + 0x0E, 66000000 / 400000 / 4); // set SMBus clock to 400 KHz + /* set SMBus clock to 400 KHz */ + outb(66000000 / 400000 / 4, ioBase + 0x0e); } int hudson_readSpd(int spdAddress, char *buf, size_t len) { - int ioBase = 0xB00; + uint16_t ioBase = 0xb00; setupFch (ioBase); return readspd (ioBase, spdAddress, buf, len); } diff --git a/src/southbridge/amd/pi/hudson/smbus_spd.c b/src/southbridge/amd/pi/hudson/smbus_spd.c index 8523db5..ac52887 100644 --- a/src/southbridge/amd/pi/hudson/smbus_spd.c +++ b/src/southbridge/amd/pi/hudson/smbus_spd.c @@ -14,90 +14,98 @@ */ #include <amdblocks/acpimmio.h> -#include <console/console.h> +#include <arch/io.h> +#include <cpu/x86/tsc.h> #include <device/pci_def.h> #include <device/device.h> - -/* warning: Porting.h includes an open #pragma pack(1) */ -#include <Porting.h> +#include <console/console.h> #include <AGESA.h> -#include <amdlib.h> -#include <northbridge/amd/pi/dimmSpd.h> +#include <northbridge/amd/agesa/dimmSpd.h> -/*----------------------------------------------------------------------------- - * - * readSmbusByteData - read a single SPD byte from any offset - */ - -static int readSmbusByteData (int iobase, int address, char *buffer, int offset) +/* readSmbusByteData - read a single SPD byte from any offset */ +static int readSmbusByteData(uint16_t iobase, int address, char *buffer, + int offset) { unsigned int status; - UINT64 limit; + unsigned long long limit; - address |= 1; // set read bit + address |= 1; /* set read bit */ + outb(0xff, iobase); /* clear error status */ + outb(0x1f, iobase + 1); /* clear error status */ + outb(offset, iobase + 3); /* offset in eeprom */ + outb(address, iobase + 4); /* slave address and read bit */ + outb(0x48, iobase + 2); /* read byte command */ - __outbyte (iobase + 0, 0xFF); // clear error status - __outbyte (iobase + 1, 0x1F); // clear error status - __outbyte (iobase + 3, offset); // offset in eeprom - __outbyte (iobase + 4, address); // slave address and read bit - __outbyte (iobase + 2, 0x48); // read byte command - - // time limit to avoid hanging for unexpected error status (should never happen) - limit = __rdtsc () + 2000000000 / 10; + /* + * time limit to avoid hanging for unexpected error status + * (should never happen) + */ + limit = rdtscll() + 2000000000 / 10; for (;;) { - status = __inbyte (iobase); - if (__rdtsc () > limit) break; - if ((status & 2) == 0) continue; // SMBusInterrupt not set, keep waiting - if ((status & 1) == 1) continue; // HostBusy set, keep waiting + status = inb(iobase); + if (rdtscll() > limit) + break; + /* SMBusInterrupt not set, keep waiting */ + if ((status & 2) == 0) + continue; + /* HostBusy set, keep waiting */ + if ((status & 1) == 1) + continue; break; } - buffer [0] = __inbyte (iobase + 5); - if (status == 2) status = 0; // check for done with no errors + buffer[0] = inb(iobase + 5); + /* check for done with no errors */ + if (status == 2) + status = 0; return status; } -/*----------------------------------------------------------------------------- - * +/* * readSmbusByte - read a single SPD byte from the default offset * this function is faster function readSmbusByteData */ - -static int readSmbusByte (int iobase, int address, char *buffer) +static int readSmbusByte(uint16_t iobase, int address, char *buffer) { unsigned int status; - UINT64 limit; + unsigned long long limit; - __outbyte (iobase + 0, 0xFF); // clear error status - __outbyte (iobase + 2, 0x44); // read command + outb(0xff, iobase + 0); /* clear error status */ + outb(0x44, iobase + 2); /* read command */ - // time limit to avoid hanging for unexpected error status - limit = __rdtsc () + 2000000000 / 10; + /* time limit to avoid hanging for unexpected error status*/ + limit = rdtscll() + 2000000000 / 10; for (;;) { - status = __inbyte (iobase); - if (__rdtsc () > limit) break; - if ((status & 2) == 0) continue; // SMBusInterrupt not set, keep waiting - if ((status & 1) == 1) continue; // HostBusy set, keep waiting + status = inb(iobase); + if (rdtscll() > limit) + break; + /* SMBusInterrupt not set, keep waiting */ + if ((status & 2) == 0) + continue; + /* HostBusy set, keep waiting */ + if ((status & 1) == 1) + continue; break; } - buffer [0] = __inbyte (iobase + 5); - if (status == 2) status = 0; // check for done with no errors + buffer[0] = inb(iobase + 5); + /* check for done with no errors */ + if (status == 2) + status = 0; return status; } -/*--------------------------------------------------------------------------- - * +/* * readspd - Read one or more SPD bytes from a DIMM. * Start with offset zero and read sequentially. * Optimization relies on autoincrement to avoid * sending offset for every byte. - * Reads 128 bytes in 7-8 ms at 400 KHz. + * Reads 128 bytes in 7-8 ms at 400 KHz. */ - -static int readspd (int iobase, int SmbusSlaveAddress, char *buffer, int count) +static int readspd(uint16_t iobase, int SmbusSlaveAddress, char *buffer, + int count) { int index, error; @@ -128,15 +136,16 @@ return 0; } -static void setupFch (int ioBase) +static void setupFch(uint16_t ioBase) { pm_write16(0x2c, ioBase | 1); - __outbyte (ioBase + 0x0E, 66000000 / 400000 / 4); // set SMBus clock to 400 KHz + /* set SMBus clock to 400 KHz */ + outb(66000000 / 400000 / 4, ioBase + 0x0e); } int hudson_readSpd(int spdAddress, char *buf, size_t len) { - int ioBase = 0xB00; + uint16_t ioBase = 0xb00; setupFch (ioBase); return readspd (ioBase, spdAddress, buf, len); } -- To view, visit https://review.coreboot.org/c/coreboot/+/38163 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I40919df7d6dfbadd09800c8e689382483ffaca14 Gerrit-Change-Number: 38163 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-MessageType: newchange

7 12

Change in ...coreboot[master]: vendorcode/google: load sar config from CBFS first then VPD
by Marco Chen (Code Review) 10 Jun '23

10 Jun '23

Hello Marco Chen, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/33661 to review the following change. Change subject: vendorcode/google: load sar config from CBFS first then VPD ...................................................................... vendorcode/google: load sar config from CBFS first then VPD SAR config provisioned in RO VPD can be done in the factory only. Once it is wrong, we can override the SAR config by updating FW RW which can carry new SAR config in CBFS. As a result, we should check CBFS first then VPD. Change-Id: I5aa6235fb7a6d0b2ed52893a42f7bd57806af6c1 Signed-off-by: Marco Chen <marcochen(a)chromium.org> --- M src/vendorcode/google/chromeos/sar.c 1 file changed, 23 insertions(+), 22 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/33661/1 diff --git a/src/vendorcode/google/chromeos/sar.c b/src/vendorcode/google/chromeos/sar.c index bbcb211..a5a2c3b 100644 --- a/src/vendorcode/google/chromeos/sar.c +++ b/src/vendorcode/google/chromeos/sar.c @@ -82,36 +82,37 @@ sizeof(struct wifi_sar_delta_table); } - /* Try to read the SAR limit entry from VPD */ - if (!vpd_gets(wifi_sar_limit_key, wifi_sar_limit_str, - buffer_size, VPD_ANY)) { - printk(BIOS_ERR, "Error: Could not locate '%s' in VPD.\n", - wifi_sar_limit_key); - - if (!CONFIG(WIFI_SAR_CBFS)) - return -1; - + if (CONFIG(WIFI_SAR_CBFS)) { printk(BIOS_DEBUG, "Checking CBFS for default SAR values\n"); sar_cbfs_len = load_sar_file_from_cbfs( (void *) wifi_sar_limit_str, sar_expected_len); - if (sar_cbfs_len != sar_expected_len) { - printk(BIOS_ERR, "%s has bad len in CBFS\n", - WIFI_SAR_CBFS_FILENAME); - return -1; - } - } else { - /* VPD key "wifi_sar" found. strlen is checked with addition of - * 1 as we have created buffer size 1 char larger for the reason - * mentioned at start of this function itself */ - if (strlen(wifi_sar_limit_str) + 1 != sar_expected_len) { - printk(BIOS_ERR, "WIFI SAR key has bad len in VPD\n"); - return -1; - } + if (sar_cbfs_len == sar_expected_len) + goto done; + + printk(BIOS_ERR, "%s has bad len in CBFS\n", + WIFI_SAR_CBFS_FILENAME); } + /* Try to read the SAR limit entry from VPD */ + if (!vpd_gets(wifi_sar_limit_key, wifi_sar_limit_str, + buffer_size, VPD_ANY)) { + printk(BIOS_ERR, "Error: Could not locate '%s' in VPD.\n", + wifi_sar_limit_key); + return -1; + } + + /* VPD key "wifi_sar" found. strlen is checked with addition of + * 1 as we have created buffer size 1 char larger for the reason + * mentioned at start of this function itself */ + if (strlen(wifi_sar_limit_str) + 1 != sar_expected_len) { + printk(BIOS_ERR, "WIFI SAR key has bad len in VPD\n"); + return -1; + } + +done: /* Decode the heximal encoded string to binary values */ if (hexstrtobin(wifi_sar_limit_str, bin_buffer, bin_buff_adjusted_size) < bin_buff_adjusted_size) { -- To view, visit https://review.coreboot.org/c/coreboot/+/33661 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5aa6235fb7a6d0b2ed52893a42f7bd57806af6c1 Gerrit-Change-Number: 33661 Gerrit-PatchSet: 1 Gerrit-Owner: Marco Chen <marcochen(a)google.com> Gerrit-Reviewer: Marco Chen <marcochen(a)chromium.org> Gerrit-MessageType: newchange

7 17

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2020