coreboot-gerrit June 2020

coreboot-gerrit@coreboot.org

1 participants
2826 discussions

Change in coreboot[master]: superio/nuvoton/*: Standardise config state entry/exit functions
by Keith Hui (Code Review) 10 Jun '23

10 Jun '23

Keith Hui has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40963 ) Change subject: superio/nuvoton/*: Standardise config state entry/exit functions ...................................................................... superio/nuvoton/*: Standardise config state entry/exit functions Rename nuvoton_pnp_{enter,exit}_conf_state() to pnp_{enter,exit}_conf_state() to match other Super I/Os. Part of an attempt to standardise the pre-RAM API across all Super I/Os. Change-Id: I238808b36545f3595a1b1a007ffe5dd41b448231 Signed-off-by: Keith Hui <buurin(a)gmail.com> --- M src/mainboard/asrock/b75pro3-m/early_init.c M src/mainboard/asrock/b85m_pro4/bootblock.c M src/mainboard/asrock/g41c-gs/early_init.c M src/mainboard/asrock/h81m-hds/bootblock.c M src/mainboard/asus/h61m-cs/early_init.c M src/mainboard/asus/p8h61-m_pro/early_init.c M src/mainboard/asus/p8z77-m_pro/early_init.c M src/mainboard/asus/p8z77-v_lx2/early_init.c M src/mainboard/supermicro/x10slm-f/bootblock.c M src/superio/nuvoton/common/early_serial.c M src/superio/nuvoton/common/nuvoton.h 11 files changed, 22 insertions(+), 24 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/40963/1 diff --git a/src/mainboard/asrock/b75pro3-m/early_init.c b/src/mainboard/asrock/b75pro3-m/early_init.c index 552e664..7a4a30c 100644 --- a/src/mainboard/asrock/b75pro3-m/early_init.c +++ b/src/mainboard/asrock/b75pro3-m/early_init.c @@ -30,14 +30,14 @@ void bootblock_mainboard_early_init(void) { /* Set GPIOs on superio, enable UART */ - nuvoton_pnp_enter_conf_state(SERIAL_DEV); + pnp_enter_conf_state(SERIAL_DEV); pnp_set_logical_device(SERIAL_DEV); pnp_write_config(SERIAL_DEV, 0x1c, 0x80); pnp_write_config(SERIAL_DEV, 0x27, 0x80); pnp_write_config(SERIAL_DEV, 0x2a, 0x60); - nuvoton_pnp_exit_conf_state(SERIAL_DEV); + pnp_exit_conf_state(SERIAL_DEV); nuvoton_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE); } diff --git a/src/mainboard/asrock/b85m_pro4/bootblock.c b/src/mainboard/asrock/b85m_pro4/bootblock.c index 5f91b29..2e72b22 100644 --- a/src/mainboard/asrock/b85m_pro4/bootblock.c +++ b/src/mainboard/asrock/b85m_pro4/bootblock.c @@ -12,7 +12,7 @@ void mainboard_config_superio(void) { - nuvoton_pnp_enter_conf_state(GLOBAL_DEV); + pnp_enter_conf_state(GLOBAL_DEV); /* Select HWM/LED functions instead of floppy functions */ pnp_write_config(GLOBAL_DEV, 0x1c, 0x03); @@ -22,7 +22,7 @@ pnp_set_logical_device(ACPI_DEV); pnp_write_config(ACPI_DEV, 0xe4, 0x70); - nuvoton_pnp_exit_conf_state(GLOBAL_DEV); + pnp_exit_conf_state(GLOBAL_DEV); /* Enable UART */ nuvoton_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE); diff --git a/src/mainboard/asrock/g41c-gs/early_init.c b/src/mainboard/asrock/g41c-gs/early_init.c index d35bb5d..209885a 100644 --- a/src/mainboard/asrock/g41c-gs/early_init.c +++ b/src/mainboard/asrock/g41c-gs/early_init.c @@ -29,14 +29,14 @@ { /* Set GPIOs on superio, enable UART */ if (CONFIG(SUPERIO_NUVOTON_NCT6776)) { - nuvoton_pnp_enter_conf_state(SERIAL_DEV_R2); + pnp_enter_conf_state(SERIAL_DEV_R2); pnp_set_logical_device(SERIAL_DEV_R2); pnp_write_config(SERIAL_DEV_R2, 0x1c, 0x80); pnp_write_config(SERIAL_DEV_R2, 0x27, 0x80); pnp_write_config(SERIAL_DEV_R2, 0x2a, 0x60); - nuvoton_pnp_exit_conf_state(SERIAL_DEV_R2); + pnp_exit_conf_state(SERIAL_DEV_R2); nuvoton_enable_serial(SERIAL_DEV_R2, CONFIG_TTYS0_BASE); } else { winbond_enable_serial(SERIAL_DEV_R1, CONFIG_TTYS0_BASE); diff --git a/src/mainboard/asrock/h81m-hds/bootblock.c b/src/mainboard/asrock/h81m-hds/bootblock.c index f3fb2c2..f5bbf6f 100644 --- a/src/mainboard/asrock/h81m-hds/bootblock.c +++ b/src/mainboard/asrock/h81m-hds/bootblock.c @@ -15,7 +15,7 @@ nuvoton_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE); - nuvoton_pnp_enter_conf_state(GLOBAL_PSEUDO_DEV); + pnp_enter_conf_state(GLOBAL_PSEUDO_DEV); /* Select HWM/LED functions instead of floppy functions. */ pnp_write_config(GLOBAL_PSEUDO_DEV, 0x1c, 0x03); @@ -32,5 +32,5 @@ pnp_set_logical_device(IR_DEV); pnp_write_config(IR_DEV, 0xf1, 0x5c); - nuvoton_pnp_exit_conf_state(GLOBAL_PSEUDO_DEV); + pnp_exit_conf_state(GLOBAL_PSEUDO_DEV); } diff --git a/src/mainboard/asus/h61m-cs/early_init.c b/src/mainboard/asus/h61m-cs/early_init.c index 1b9adb4..45459a4 100644 --- a/src/mainboard/asus/h61m-cs/early_init.c +++ b/src/mainboard/asus/h61m-cs/early_init.c @@ -32,10 +32,10 @@ void bootblock_mainboard_early_init(void) { - nuvoton_pnp_enter_conf_state(SIO_DEV); + pnp_enter_conf_state(SIO_DEV); pnp_set_logical_device(ACPI_DEV); pnp_write_config(ACPI_DEV, 0xe4, 0x10); - nuvoton_pnp_exit_conf_state(SIO_DEV); + pnp_exit_conf_state(SIO_DEV); } void mainboard_get_spd(spd_raw_data *spd, bool id_only) diff --git a/src/mainboard/asus/p8h61-m_pro/early_init.c b/src/mainboard/asus/p8h61-m_pro/early_init.c index 582366d..f6eb812 100644 --- a/src/mainboard/asus/p8h61-m_pro/early_init.c +++ b/src/mainboard/asus/p8h61-m_pro/early_init.c @@ -33,7 +33,7 @@ void bootblock_mainboard_early_init(void) { /* Enable UART */ - nuvoton_pnp_enter_conf_state(GLOBAL_DEV); + pnp_enter_conf_state(GLOBAL_DEV); /* Select SIO pin states. */ pnp_write_config(GLOBAL_DEV, 0x1c, 0x83); @@ -47,7 +47,7 @@ pnp_set_logical_device(SERIAL_DEV); - nuvoton_pnp_exit_conf_state(GLOBAL_DEV); + pnp_exit_conf_state(GLOBAL_DEV); nuvoton_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE); } diff --git a/src/mainboard/asus/p8z77-m_pro/early_init.c b/src/mainboard/asus/p8z77-m_pro/early_init.c index 687cbfd..eb0df1c 100644 --- a/src/mainboard/asus/p8z77-m_pro/early_init.c +++ b/src/mainboard/asus/p8z77-m_pro/early_init.c @@ -38,12 +38,12 @@ void bootblock_mainboard_early_init(void) { /* Setup COM/UART */ - nuvoton_pnp_enter_conf_state(GLOBAL_DEV); + pnp_enter_conf_state(GLOBAL_DEV); /* TODO / FIXME: Setup Multifuncion/SIO pins for COM */ pnp_set_logical_device(SERIAL_DEV); - nuvoton_pnp_exit_conf_state(GLOBAL_DEV); + pnp_exit_conf_state(GLOBAL_DEV); nuvoton_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE); } diff --git a/src/mainboard/asus/p8z77-v_lx2/early_init.c b/src/mainboard/asus/p8z77-v_lx2/early_init.c index cbbf9dc..5a7b4cb 100644 --- a/src/mainboard/asus/p8z77-v_lx2/early_init.c +++ b/src/mainboard/asus/p8z77-v_lx2/early_init.c @@ -32,7 +32,7 @@ void bootblock_mainboard_early_init(void) { - nuvoton_pnp_enter_conf_state(GLOBAL_DEV); + pnp_enter_conf_state(GLOBAL_DEV); /* Select SIO pin states */ pnp_write_config(GLOBAL_DEV, 0x1a, 0x02); @@ -47,7 +47,7 @@ pnp_set_logical_device(ACPI_DEV); pnp_write_config(ACPI_DEV, 0xe4, 0x10); - nuvoton_pnp_exit_conf_state(GLOBAL_DEV); + pnp_exit_conf_state(GLOBAL_DEV); /* Enable UART */ nuvoton_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE); diff --git a/src/mainboard/supermicro/x10slm-f/bootblock.c b/src/mainboard/supermicro/x10slm-f/bootblock.c index c0d4502..64b5e6d 100644 --- a/src/mainboard/supermicro/x10slm-f/bootblock.c +++ b/src/mainboard/supermicro/x10slm-f/bootblock.c @@ -26,7 +26,7 @@ nuvoton_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE); - nuvoton_pnp_enter_conf_state(GLOBAL_PSEUDO_DEV); + pnp_enter_conf_state(GLOBAL_PSEUDO_DEV); /* Select HWM/LED functions instead of floppy functions. */ pnp_write_config(GLOBAL_PSEUDO_DEV, 0x1c, 0x03); @@ -36,5 +36,5 @@ pnp_set_logical_device(ACPI_DEV); pnp_write_config(ACPI_DEV, 0xe4, 0x70); - nuvoton_pnp_exit_conf_state(GLOBAL_PSEUDO_DEV); + pnp_exit_conf_state(GLOBAL_PSEUDO_DEV); } diff --git a/src/superio/nuvoton/common/early_serial.c b/src/superio/nuvoton/common/early_serial.c index ed31b72..b709969 100644 --- a/src/superio/nuvoton/common/early_serial.c +++ b/src/superio/nuvoton/common/early_serial.c @@ -31,7 +31,7 @@ /* Enable configuration: pass entry key '0x87' into index port dev * two times. */ -void nuvoton_pnp_enter_conf_state(pnp_devfn_t dev) +void pnp_enter_conf_state(pnp_devfn_t dev) { u16 port = dev >> 8; outb(NUVOTON_ENTRY_KEY, port); @@ -39,7 +39,7 @@ } /* Disable configuration: pass exit key '0xAA' into index port dev. */ -void nuvoton_pnp_exit_conf_state(pnp_devfn_t dev) +void pnp_exit_conf_state(pnp_devfn_t dev) { u16 port = dev >> 8; outb(NUVOTON_EXIT_KEY, port); @@ -48,7 +48,7 @@ /* Bring up early serial debugging output before the RAM is initialized. */ void nuvoton_enable_serial(pnp_devfn_t dev, u16 iobase) { - nuvoton_pnp_enter_conf_state(dev); + pnp_enter_conf_state(dev); if (CONFIG(SUPERIO_NUVOTON_NCT5539D_COM_A)) /* Route COM A to GPIO8 pin group */ @@ -66,5 +66,5 @@ pnp_set_enable(dev, 0); pnp_set_iobase(dev, PNP_IDX_IO0, iobase); pnp_set_enable(dev, 1); - nuvoton_pnp_exit_conf_state(dev); + pnp_exit_conf_state(dev); } diff --git a/src/superio/nuvoton/common/nuvoton.h b/src/superio/nuvoton/common/nuvoton.h index f5b3176..d69548e 100644 --- a/src/superio/nuvoton/common/nuvoton.h +++ b/src/superio/nuvoton/common/nuvoton.h @@ -7,8 +7,6 @@ #include <device/pnp_type.h> #include <stdint.h> -void nuvoton_pnp_enter_conf_state(pnp_devfn_t dev); -void nuvoton_pnp_exit_conf_state(pnp_devfn_t dev); void nuvoton_enable_serial(pnp_devfn_t dev, u16 iobase); #endif /* SUPERIO_NUVOTON_COMMON_PRE_RAM_H */ -- To view, visit https://review.coreboot.org/c/coreboot/+/40963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I238808b36545f3595a1b1a007ffe5dd41b448231 Gerrit-Change-Number: 40963 Gerrit-PatchSet: 1 Gerrit-Owner: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Tristan Corrick <tristan(a)corrick.kiwi> Gerrit-MessageType: newchange

2 3

Change in coreboot[master]: Doc/mb/lenovo/t420s.md: Add documentation for Lenovo T420s
by David Gebski (Code Review) 10 Jun '23

10 Jun '23

David Gebski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41490 ) Change subject: Doc/mb/lenovo/t420s.md: Add documentation for Lenovo T420s ...................................................................... Doc/mb/lenovo/t420s.md: Add documentation for Lenovo T420s Borrowed text mostly from documentation for Lenovo T420. Added two images showing the pinout. Signed-off-by: Zalckos <mail(a)davidgebski.nl> Change-Id: Ia2c6701bc8269e73e4af5a9414b7ded77a4d3571 --- A Documentation/mainboard/lenovo/t420s.md A Documentation/mainboard/lenovo/t420s_board.jpg A Documentation/mainboard/lenovo/t420s_rpi.jpg 3 files changed, 22 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41490/1 diff --git a/Documentation/mainboard/lenovo/t420s.md b/Documentation/mainboard/lenovo/t420s.md new file mode 100644 index 0000000..82be80c --- /dev/null +++ b/Documentation/mainboard/lenovo/t420s.md @@ -0,0 +1,22 @@ +# Lenovo T420s + +## Flashing instructions +The flash IC is located at the bottom center of the mainboard. Sadly, access +to the IC is blocked by the magnesium frame, so you need to disassemble the +entire laptop and remove the mainboard. + +Below is a picture of IC on the mainboard with clip attached, and a picture of +the Raspberry Pi with cables attached. In this case it's a Winbond flash chip +"W25Q64.V". + +![t420s_board](t420s_board.jpg) +![t420s_rpi](t420s_rpi.jpg) + +For more details have a look at [T420 / T520 / X220 / T420s / W520 common] and +the general [flashing tutorial]. + +Steps to access the flash IC are described here [T4xx series]. + +[T4xx series]: t4xx_series.md +[flashing tutorial]: ../../flash_tutorial/ext_power.md +[T420 / T520 / X220 / T420s / W520 common]: Sandy_Bridge_series.md diff --git a/Documentation/mainboard/lenovo/t420s_board.jpg b/Documentation/mainboard/lenovo/t420s_board.jpg new file mode 100644 index 0000000..3ec4201 --- /dev/null +++ b/Documentation/mainboard/lenovo/t420s_board.jpg Binary files differ diff --git a/Documentation/mainboard/lenovo/t420s_rpi.jpg b/Documentation/mainboard/lenovo/t420s_rpi.jpg new file mode 100644 index 0000000..6fe204c --- /dev/null +++ b/Documentation/mainboard/lenovo/t420s_rpi.jpg Binary files differ -- To view, visit https://review.coreboot.org/c/coreboot/+/41490 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia2c6701bc8269e73e4af5a9414b7ded77a4d3571 Gerrit-Change-Number: 41490 Gerrit-PatchSet: 1 Gerrit-Owner: David Gebski Gerrit-MessageType: newchange

4 3

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]: amdfwtool: Unpack struct `_bios_directory_entry`
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/+/41154 ) Change subject: amdfwtool: Unpack struct `_bios_directory_entry` ...................................................................... amdfwtool: Unpack struct `_bios_directory_entry` gcc (Debian 10-20200502-1) 10.0.1 20200502 (prerelease) [gcc-10 revision d128356d460:a9ed47dd155:0118d0397f94c307b76aa14abec99347a93da621] shows the warning below. /coreboot/util/amdfwtool/amdfwtool.c: In function 'integrate_bios_firmwares': /dev/shm/coreboot/util/amdfwtool/amdfwtool.c:964:7: warning: taking address of packed member of 'struct _bios_directory_entry' may result in an unaligned pointer value [-Waddress-of-packed-member] 964 | &biosdir->entries[count].source, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /coreboot/util/amdfwtool/amdfwtool.c:965:7: warning: taking address of packed member of 'struct _bios_directory_entry' may result in an unaligned pointer value [-Waddress-of-packed-member] 965 | &biosdir->entries[count].size)) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Change-Id: I375ffec589cca8fe9d07dbe908c79348e863aa13 Signed-off-by: Paul Menzel <pmenzel(a)molgen.mpg.de> --- M util/amdfwtool/amdfwtool.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/41154/1 diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 1ff86e6..c4acf81 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -484,7 +484,7 @@ uint32_t size; uint64_t source; uint64_t dest; -} __attribute__((packed)) bios_directory_entry; +} bios_directory_entry; typedef struct _bios_directory_table { bios_directory_hdr header; -- To view, visit https://review.coreboot.org/c/coreboot/+/41154 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I375ffec589cca8fe9d07dbe908c79348e863aa13 Gerrit-Change-Number: 41154 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newchange

5 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:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2020