coreboot-gerrit November 2020

coreboot-gerrit@coreboot.org

1 participants
3243 discussions

Change in coreboot[master]: src: Update some incorrect config options in comments
by Martin Roth (Code Review) 16 Nov '20

16 Nov '20

Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47366 ) Change subject: src: Update some incorrect config options in comments ...................................................................... src: Update some incorrect config options in comments This is a trivial patch to fix some comments that were generating notes in the kconfig lint test. Signed-off-by: Martin Roth <martin(a)coreboot.org> Change-Id: I26a95f17e82910f50c62215be5c29780fe98e29a --- M src/drivers/i2c/designware/dw_i2c.h M src/ec/google/chromeec/ec_commands.h M src/include/rules.h M src/lib/romstage_handoff.c M src/soc/intel/braswell/memmap.c 5 files changed, 10 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/47366/1 diff --git a/src/drivers/i2c/designware/dw_i2c.h b/src/drivers/i2c/designware/dw_i2c.h index 45f305e..e20a5d1 100644 --- a/src/drivers/i2c/designware/dw_i2c.h +++ b/src/drivers/i2c/designware/dw_i2c.h @@ -17,7 +17,7 @@ /* * Timing values are in units of clock period, with the clock speed - * provided by the SOC in CONFIG_DRIVERS_I2C_DESIGNWARE_I2C_CLOCK_MHZ + * provided by the SOC in CONFIG_DRIVERS_I2C_DESIGNWARE_CLOCK_MHZ * Automatic configuration is done based on requested speed, but the * values may need tuned depending on the board and the number of * devices present on the bus. diff --git a/src/ec/google/chromeec/ec_commands.h b/src/ec/google/chromeec/ec_commands.h index 241ec39..fb83d60 100644 --- a/src/ec/google/chromeec/ec_commands.h +++ b/src/ec/google/chromeec/ec_commands.h @@ -1388,7 +1388,7 @@ */ EC_FEATURE_REFINED_TABLET_MODE_HYSTERESIS = 37, /* - * Early Firmware Selection ver.2. Enabled by CONFIG_VBOOT_EFS2. + * Early Firmware Selection ver.2. Enabled by VBOOT_EFS2 config option. * Note this is a RO feature. So, a query (EC_CMD_GET_FEATURES) should * be sent to RO to be precise. */ diff --git a/src/include/rules.h b/src/include/rules.h index d30b2b8..6ebb37e 100644 --- a/src/include/rules.h +++ b/src/include/rules.h @@ -63,10 +63,11 @@ /* * NOTE: "verstage" code may either run as a separate stage or linked into the - * bootblock/romstage, depending on the setting of CONFIG_SEPARATE_VERSTAGE. The - * ENV_SEPARATE_VERSTAGE macro will only return true for "verstage" code when - * CONFIG_SEPARATE_VERSTAGE=y, otherwise that code will have ENV_BOOTBLOCK or - * ENV_ROMSTAGE set (depending on the CONFIG_VBOOT_STARTS_IN_... options). + * bootblock/romstage, depending on the setting of the VBOOT_SEPARATE_VERSTAGE + * kconfig option. The ENV_SEPARATE_VERSTAGE macro will only return true for + * "verstage" code when CONFIG(VBOOT_SEPARATE_VERSTAGE) is true, otherwise that + * code will have ENV_BOOTBLOCK or ENV_ROMSTAGE set (depending on the + * "VBOOT_STARTS_IN_"... kconfig options). */ #elif defined(__VERSTAGE__) #define ENV_DECOMPRESSOR 0 diff --git a/src/lib/romstage_handoff.c b/src/lib/romstage_handoff.c index b54619d..0a7a822 100644 --- a/src/lib/romstage_handoff.c +++ b/src/lib/romstage_handoff.c @@ -8,7 +8,7 @@ struct romstage_handoff { /* Indicate if the current boot is an S3 resume. If - * CONFIG_RELOCTABLE_RAMSTAGE is enabled the chipset code is + * CONFIG_RELOCATABLE_RAMSTAGE is enabled the chipset code is * responsible for initializing this variable. Otherwise, ramstage * will be re-loaded from cbfs (which can be slower since it lives * in flash). */ diff --git a/src/soc/intel/braswell/memmap.c b/src/soc/intel/braswell/memmap.c index 6cfce43..4a791ef 100644 --- a/src/soc/intel/braswell/memmap.c +++ b/src/soc/intel/braswell/memmap.c @@ -27,11 +27,11 @@ /* * +-------------------------+ Top of RAM (aligned) * | System Management Mode | - * | code and data | Length: CONFIG_TSEG_SIZE + * | code and data | Length: CONFIG_SMM_TSEG_SIZE * | (TSEG) | * +-------------------------+ SMM base (aligned) * | | - * | Chipset Reserved Memory | Length: Multiple of CONFIG_TSEG_SIZE + * | Chipset Reserved Memory | Length: Multiple of CONFIG_SMM_TSEG_SIZE * | | * +-------------------------+ top_of_ram (aligned) * | | -- To view, visit https://review.coreboot.org/c/coreboot/+/47366 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I26a95f17e82910f50c62215be5c29780fe98e29a Gerrit-Change-Number: 47366 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: newchange

5 4

Change in coreboot[master]: soc/intel/denverton_ns: Generate ACPI DMAR Table
by Patrick Georgi (Code Review) 16 Nov '20

16 Nov '20

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/25446 ) Change subject: soc/intel/denverton_ns: Generate ACPI DMAR Table ...................................................................... soc/intel/denverton_ns: Generate ACPI DMAR Table - Write ACPI DMAR Table if VT-d is enabled. - The entries are defined to follow FSP settings. Change-Id: I263b03b96280599266d4c5e193583ecdfe9697b7 Signed-off-by: Julien Viard de Galbert <jviarddegalbert(a)online.net> Reviewed-on: https://review.coreboot.org/c/coreboot/+/25446 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/intel/denverton_ns/acpi.c M src/soc/intel/denverton_ns/include/soc/acpi.h M src/soc/intel/denverton_ns/include/soc/iomap.h M src/soc/intel/denverton_ns/include/soc/pci_devs.h M src/soc/intel/denverton_ns/include/soc/systemagent.h M src/soc/intel/denverton_ns/systemagent.c 6 files changed, 78 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/src/soc/intel/denverton_ns/acpi.c b/src/soc/intel/denverton_ns/acpi.c index 944e8eb..e7d29ff 100644 --- a/src/soc/intel/denverton_ns/acpi.c +++ b/src/soc/intel/denverton_ns/acpi.c @@ -16,6 +16,7 @@ #include <soc/soc_util.h> #include <soc/pmc.h> #include <soc/systemagent.h> +#include <soc/pci_devs.h> #define MWAIT_RES(state, sub_state) \ { \ @@ -268,3 +269,54 @@ } __weak void acpi_create_serialio_ssdt(acpi_header_t *ssdt) {} + +static unsigned long acpi_fill_dmar(unsigned long current) +{ + uint64_t vtbar; + unsigned long tmp = current; + + vtbar = read64((void *)(DEFAULT_MCHBAR + MCH_VTBAR_OFFSET)) & MCH_VTBAR_MASK; + printk(BIOS_DEBUG, "DEFVTBAR:0x%llx\n", vtbar); + if (!vtbar) + return current; + + current += acpi_create_dmar_drhd(current, + DRHD_INCLUDE_PCI_ALL, 0, vtbar); + + current += acpi_create_dmar_ds_ioapic(current, + 2, PCH_IOAPIC_PCI_BUS, PCH_IOAPIC_PCI_SLOT, 0); + current += acpi_create_dmar_ds_msi_hpet(current, + 0, PCH_HPET_PCI_BUS, PCH_HPET_PCI_SLOT, 0); + + acpi_dmar_drhd_fixup(tmp, current); + + /* Create RMRR; see "VTD PLATFORM CONFIGURATION" in FSP log */ + tmp = current; + current += acpi_create_dmar_rmrr(current, 0, + RMRR_USB_BASE_ADDRESS, + RMRR_USB_LIMIT_ADDRESS); + current += acpi_create_dmar_ds_pci(current, + 0, XHCI_DEV, XHCI_FUNC); + acpi_dmar_rmrr_fixup(tmp, current); + + return current; +} + +unsigned long systemagent_write_acpi_tables(const struct device *dev, + unsigned long current, + struct acpi_rsdp *const rsdp) +{ + /* Create DMAR table only if we have VT-d capability. */ + const u32 capid0_a = pci_read_config32(dev, CAPID0_A); + if (capid0_a & VTD_DISABLE) + return current; + + acpi_dmar_t *const dmar = (acpi_dmar_t *)current; + printk(BIOS_DEBUG, "ACPI: * DMAR\n"); + acpi_create_dmar(dmar, DMAR_INTR_REMAP, acpi_fill_dmar); + current += dmar->header.length; + current = acpi_align_current(current); + acpi_add_table(rsdp, dmar); + + return current; +} diff --git a/src/soc/intel/denverton_ns/include/soc/acpi.h b/src/soc/intel/denverton_ns/include/soc/acpi.h index 9bc5ed0..86bed00 100644 --- a/src/soc/intel/denverton_ns/include/soc/acpi.h +++ b/src/soc/intel/denverton_ns/include/soc/acpi.h @@ -12,5 +12,8 @@ unsigned long current, struct acpi_rsdp *rsdp); void southcluster_inject_dsdt(const struct device *device); +unsigned long systemagent_write_acpi_tables(const struct device *dev, + unsigned long start, + struct acpi_rsdp *const rsdp); #endif /* _DENVERTON_NS_ACPI_H_ */ diff --git a/src/soc/intel/denverton_ns/include/soc/iomap.h b/src/soc/intel/denverton_ns/include/soc/iomap.h index 6e5f313..fb5aafd 100644 --- a/src/soc/intel/denverton_ns/include/soc/iomap.h +++ b/src/soc/intel/denverton_ns/include/soc/iomap.h @@ -24,4 +24,8 @@ #define DEFAULT_HPET_ADDR CONFIG_HPET_ADDRESS #define DEFAULT_SPI_BASE 0xfed01000 +/* "VTD PLATFORM CONFIGURATION" (Set to match FSP settings) */ +#define RMRR_USB_BASE_ADDRESS 0x3e2e0000 +#define RMRR_USB_LIMIT_ADDRESS 0x3e2fffff + #endif /* _DENVERTON_NS_IOMAP_H_ */ diff --git a/src/soc/intel/denverton_ns/include/soc/pci_devs.h b/src/soc/intel/denverton_ns/include/soc/pci_devs.h index b6bac0b..5eac5bd 100644 --- a/src/soc/intel/denverton_ns/include/soc/pci_devs.h +++ b/src/soc/intel/denverton_ns/include/soc/pci_devs.h @@ -143,4 +143,12 @@ #define PCH_DEV_LPC _PCH_DEV(LPC, 0) #define PCH_DEV_SPI _PCH_DEV(LPC, 5) +/* VT-d support value to match FSP settings */ +/* "PCH IOAPIC Config" */ +#define PCH_IOAPIC_PCI_BUS 0xf0 +#define PCH_IOAPIC_PCI_SLOT 0x1f +/* "PCH HPET Config" */ +#define PCH_HPET_PCI_BUS 0 +#define PCH_HPET_PCI_SLOT 0 + #endif /* _DENVERTON_NS_PCI_DEVS_H_ */ diff --git a/src/soc/intel/denverton_ns/include/soc/systemagent.h b/src/soc/intel/denverton_ns/include/soc/systemagent.h index 561f482..0606a3e 100644 --- a/src/soc/intel/denverton_ns/include/soc/systemagent.h +++ b/src/soc/intel/denverton_ns/include/soc/systemagent.h @@ -31,6 +31,9 @@ #define TOLUD 0xbc /* Top of Low Used Memory */ #define MASK_TOLUD 0xFFF00000 +#define CAPID0_A 0xe4 +#define VTD_DISABLE (1 << 23) + /* SideBand B-UNIT */ #define B_UNIT 3 @@ -57,6 +60,10 @@ #define MCH_BMISC_RESDRAM \ 0x01 /* Bit 0: 1 - reads targeting E-segment are routed to DRAM. */ +#define MCH_VTBAR_OFFSET 0x6c80 +#define MCH_VTBAR_ENABLE_MASK 0x1 +#define MCH_VTBAR_MASK 0x7ffffff000 + #define MCH_BAR_BIOS_RESET_CPL 0x7078 #define RST_CPL_BIT (1 << 0) #define PCODE_INIT_DONE (1 << 8) diff --git a/src/soc/intel/denverton_ns/systemagent.c b/src/soc/intel/denverton_ns/systemagent.c index daac3eb..114ee48 100644 --- a/src/soc/intel/denverton_ns/systemagent.c +++ b/src/soc/intel/denverton_ns/systemagent.c @@ -15,6 +15,7 @@ #include <soc/pci_devs.h> #include <soc/ramstage.h> #include <soc/systemagent.h> +#include <soc/acpi.h> #define _1ms 1 #define WAITING_STEP 100 @@ -325,6 +326,9 @@ .enable_resources = pci_dev_enable_resources, .init = systemagent_init, .ops_pci = &soc_pci_ops, +#if CONFIG(HAVE_ACPI_TABLES) + .write_acpi_tables = systemagent_write_acpi_tables, +#endif }; /* IDs for System Agent device of Intel Denverton SoC */ -- To view, visit https://review.coreboot.org/c/coreboot/+/25446 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I263b03b96280599266d4c5e193583ecdfe9697b7 Gerrit-Change-Number: 25446 Gerrit-PatchSet: 24 Gerrit-Owner: Julien Viard de Galbert <coreboot-review-ju(a)vdg.name> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com> Gerrit-Reviewer: Julien Viard de Galbert <coreboot-review-ju(a)vdg.name> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jay Talbott <JayTalbott(a)sysproconsulting.com> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: cpu/x86/smm/smm_module_loaderv2: Properly print stack_end
by Arthur Heymans (Code Review) 16 Nov '20

16 Nov '20

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47071 ) Change subject: cpu/x86/smm/smm_module_loaderv2: Properly print stack_end ...................................................................... cpu/x86/smm/smm_module_loaderv2: Properly print stack_end Change-Id: I2b8c54fd3851d1c2a9f4c3c36828922067bec79f Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/x86/smm/smm_module_loaderv2.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/47071/1 diff --git a/src/cpu/x86/smm/smm_module_loaderv2.c b/src/cpu/x86/smm/smm_module_loaderv2.c index 821aa3f..7a72c10 100644 --- a/src/cpu/x86/smm/smm_module_loaderv2.c +++ b/src/cpu/x86/smm/smm_module_loaderv2.c @@ -447,8 +447,8 @@ stub_params->runtime.save_state_size = params->per_cpu_save_state_size; stub_params->runtime.num_cpus = params->num_concurrent_stacks; - printk(BIOS_DEBUG, "%s: stack_end = 0x%x\n", - __func__, stub_params->runtime.smbase); + printk(BIOS_DEBUG, "%s: stack_end = 0x%lx\n", + __func__, stub_params->stack_top - total_stack_size); printk(BIOS_DEBUG, "%s: stack_top = 0x%x\n", __func__, stub_params->stack_top); printk(BIOS_DEBUG, "%s: stack_size = 0x%x\n", -- To view, visit https://review.coreboot.org/c/coreboot/+/47071 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2b8c54fd3851d1c2a9f4c3c36828922067bec79f Gerrit-Change-Number: 47071 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

4 4

Change in coreboot[master]: cpu/x86/smm/smm_module_loaderv2.c: Use more variables
by Arthur Heymans (Code Review) 16 Nov '20

16 Nov '20

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47070 ) Change subject: cpu/x86/smm/smm_module_loaderv2.c: Use more variables ...................................................................... cpu/x86/smm/smm_module_loaderv2.c: Use more variables Reusing the size variable for a different purpose later on in the function makes the code harder to read. Change-Id: Iceb10aa40ad473b41b7da0310554725585e3c2c2 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/x86/smm/smm_module_loaderv2.c 1 file changed, 4 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/47070/1 diff --git a/src/cpu/x86/smm/smm_module_loaderv2.c b/src/cpu/x86/smm/smm_module_loaderv2.c index 3fa5871..821aa3f 100644 --- a/src/cpu/x86/smm/smm_module_loaderv2.c +++ b/src/cpu/x86/smm/smm_module_loaderv2.c @@ -413,12 +413,13 @@ * for default handler, but for relocated handler it lives at the beginning * of SMRAM which is TSEG base */ - size = params->num_concurrent_stacks * params->per_cpu_stack_size; - stacks_top = smm_stub_place_stacks((char *)params->smram_start, size, params); + const total_stack_size = params->num_concurrent_stacks * params->per_cpu_stack_size; + stacks_top = smm_stub_place_stacks((char *)params->smram_start, total_stack_size, + params); if (stacks_top == NULL) { printk(BIOS_ERR, "%s: not enough space for stacks\n", __func__); printk(BIOS_ERR, "%s: ....need -> %p : available -> %zx\n", __func__, - base, size); + base, total_stack_size); return -1; } params->stack_top = stacks_top; -- To view, visit https://review.coreboot.org/c/coreboot/+/47070 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iceb10aa40ad473b41b7da0310554725585e3c2c2 Gerrit-Change-Number: 47070 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

4 6

Change in coreboot[master]: cpu/x86/smm: Check that the stub size is < save state size
by Arthur Heymans (Code Review) 16 Nov '20

16 Nov '20

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47069 ) Change subject: cpu/x86/smm: Check that the stub size is < save state size ...................................................................... cpu/x86/smm: Check that the stub size is < save state size If the stub size would be larger than the save state size. The check is placed in the stub placement code. The stub placement code is called twice. Once for the initial SMM relocatation and for the permanent handler in TSEG. So the check is done twice, which is not really needed. Change-Id: I253e1a7112cd8f7496cb1a826311f4dd5ccfc73a Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/x86/smm/smm_module_loader.c M src/cpu/x86/smm/smm_module_loaderv2.c 2 files changed, 12 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/47069/1 diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index fc1e1b3..876fde6 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -209,6 +209,12 @@ smm_stub_size = rmodule_memory_size(&smm_stub); stub_entry_offset = rmodule_entry_offset(&smm_stub); + if (smm_stub_size > params->per_cpu_save_state_size) { + printk(BIOS_ERR, "SMM Module: SMM stub size larger than save state size\n"); + printk(BIOS_ERR, "SMM Module: Staggered entry points will overlap stub\n"); + return -1; + } + /* Assume the stub is always small enough to live within upper half of * SMRAM region after the save state space has been allocated. */ smm_stub_loc = &base[SMM_ENTRY_OFFSET]; diff --git a/src/cpu/x86/smm/smm_module_loaderv2.c b/src/cpu/x86/smm/smm_module_loaderv2.c index 22f336e..3fa5871 100644 --- a/src/cpu/x86/smm/smm_module_loaderv2.c +++ b/src/cpu/x86/smm/smm_module_loaderv2.c @@ -134,6 +134,12 @@ return 0; } + if (stub_size > ss_size) { + printk(BIOS_ERR, "%s: Save state larger than SMM stub size\n", __func__); + printk(BIOS_ERR, " Decrease stub size or increase the size allocated for the save state\n"); + return 0; + } + for (i = 0; i < num_cpus; i++) { cpus[i].smbase = base; cpus[i].entry = base + smm_entry_offset; -- To view, visit https://review.coreboot.org/c/coreboot/+/47069 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I253e1a7112cd8f7496cb1a826311f4dd5ccfc73a Gerrit-Change-Number: 47069 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

4 5

Change in coreboot[master]: nb/intel/sandybridge: Clarify some parts of raminit
by Angel Pons (Code Review) 16 Nov '20

16 Nov '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47489 ) Change subject: nb/intel/sandybridge: Clarify some parts of raminit ...................................................................... nb/intel/sandybridge: Clarify some parts of raminit Put names and expand comments for some parts of the code. Change-Id: If1f83bf113ef08469768a9e4dd13819f76633f18 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/northbridge/intel/sandybridge/raminit_common.c 1 file changed, 18 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/47489/1 diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index 57c28fc..20c048f 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -2321,17 +2321,17 @@ { int i; - /* good */ + /* DQS is good enough */ if (val == 0xffffffffffffffffLL) return 0; if (val >= 0xf000000000000000LL) { - /* needs negative adjustment */ + /* DQS is late, needs negative adjustment */ for (i = 0; i < 8; i++) if (val << (8 * (7 - i) + 4)) return -i; } else { - /* needs positive adjustment */ + /* DQS is early, needs positive adjustment */ for (i = 0; i < 8; i++) if (val >> (8 * (7 - i) + 4)) return i; @@ -2339,7 +2339,7 @@ return 8; } -static void adjust_high_timB(ramctr_timing *ctrl) +static void train_write_flyby(ramctr_timing *ctrl) { int channel, slotrank, lane, old; MCHBAR32(GDCRTRAININGMOD) = 0x200; @@ -2715,7 +2715,7 @@ program_timings(ctrl, channel); /* measure and adjust timB timings */ - adjust_high_timB(ctrl); + train_write_flyby(ctrl); FOR_ALL_POPULATED_CHANNELS program_timings(ctrl, channel); @@ -2726,7 +2726,7 @@ return 0; } -static int test_320c(ramctr_timing *ctrl, int channel, int slotrank) +static int test_command_training(ramctr_timing *ctrl, int channel, int slotrank) { struct ram_rank_timings saved_rt = ctrl->timings[channel][slotrank]; int timC_delta; @@ -3006,14 +3006,18 @@ toggle_io_reset(); } +#define CT_MIN_PI -127 +#define CT_MAX_PI 128 +#define CT_PI_LENGTH (CT_MAX_PI - CT_MIN_PI + 1) + #define MIN_C320C_LEN 13 static int try_cmd_stretch(ramctr_timing *ctrl, int channel, int cmd_stretch) { struct ram_rank_timings saved_timings[NUM_CHANNELS][NUM_SLOTRANKS]; int slotrank; - int c320c; - int stat[NUM_SLOTRANKS][256]; + int command_pi; + int stat[NUM_SLOTRANKS][CT_PI_LENGTH]; int delta = 0; printram("Trying cmd_stretch %d on channel %d\n", cmd_stretch, channel); @@ -3042,20 +3046,21 @@ ctrl->timings[channel][slotrank].roundtrip_latency -= delta; } - for (c320c = -127; c320c <= 127; c320c++) { + for (command_pi = CT_MIN_PI; command_pi < CT_MAX_PI; command_pi++) { FOR_ALL_POPULATED_RANKS { - ctrl->timings[channel][slotrank].pi_coding = c320c; + ctrl->timings[channel][slotrank].pi_coding = command_pi; } program_timings(ctrl, channel); reprogram_320c(ctrl); FOR_ALL_POPULATED_RANKS { - stat[slotrank][c320c + 127] = test_320c(ctrl, channel, slotrank); + stat[slotrank][command_pi - CT_MIN_PI] = + test_command_training(ctrl, channel, slotrank); } } FOR_ALL_POPULATED_RANKS { - struct run rn = get_longest_zero_run(stat[slotrank], 255); + struct run rn = get_longest_zero_run(stat[slotrank], CT_PI_LENGTH - 1); - ctrl->timings[channel][slotrank].pi_coding = rn.middle - 127; + ctrl->timings[channel][slotrank].pi_coding = rn.middle + CT_MIN_PI; printram("cmd_stretch: %d, %d: 0x%02x-0x%02x-0x%02x\n", channel, slotrank, rn.start, rn.middle, rn.end); -- To view, visit https://review.coreboot.org/c/coreboot/+/47489 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If1f83bf113ef08469768a9e4dd13819f76633f18 Gerrit-Change-Number: 47489 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

4 3

Change in coreboot[master]: nb/intel/sandybridge: Fix typo in comment
by Angel Pons (Code Review) 16 Nov '20

16 Nov '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47488 ) Change subject: nb/intel/sandybridge: Fix typo in comment ...................................................................... nb/intel/sandybridge: Fix typo in comment Change-Id: I8271911695f41ef7cac1bb228309af0568e5bb0c Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/northbridge/intel/sandybridge/raminit_common.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/47488/1 diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index e23753a..57c28fc 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -4238,7 +4238,7 @@ * The following loops writes to every DRAM address, setting the ECC bits to the * correct value. A read from this location will no longer return a CRC error, * except when a bit has toggled due to external events. - * The same could be accieved by writing to the physical memory map, but it's + * The same could be achieved by writing to the physical memory map, but it's * much more difficult due to SMM remapping, ME stolen memory, GFX stolen memory, * and firmware running in x86_32. */ -- To view, visit https://review.coreboot.org/c/coreboot/+/47488 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8271911695f41ef7cac1bb228309af0568e5bb0c Gerrit-Change-Number: 47488 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

3 2

Change in coreboot[master]: nb/intel/sandybridge: Retype constant
by Angel Pons (Code Review) 16 Nov '20

16 Nov '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47487 ) Change subject: nb/intel/sandybridge: Retype constant ...................................................................... nb/intel/sandybridge: Retype constant There's no need to use size_t to store a boolean. Change-Id: I0069fa8d75583dc34b402004d753220943406a04 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/northbridge/intel/sandybridge/raminit_common.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/47487/1 diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index 3fd8cb0..e23753a 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -4433,7 +4433,7 @@ /* FIXME: values in this function should be hardware revision-dependent */ void final_registers(ramctr_timing *ctrl) { - const size_t is_mobile = get_platform_type() == PLATFORM_MOBILE; + const bool is_mobile = get_platform_type() == PLATFORM_MOBILE; int channel; int t1_cycles = 0, t1_ns = 0, t2_ns; -- To view, visit https://review.coreboot.org/c/coreboot/+/47487 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0069fa8d75583dc34b402004d753220943406a04 Gerrit-Change-Number: 47487 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

3 2

Change in coreboot[master]: nb/intel/sandybridge: Drop write_controller_mr() function
by Angel Pons (Code Review) 16 Nov '20

16 Nov '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47486 ) Change subject: nb/intel/sandybridge: Drop write_controller_mr() function ...................................................................... nb/intel/sandybridge: Drop write_controller_mr() function The only reason to write the MR values to the training result registers is for EV (Electrical Validation) usage. The hardware doesn't need it. Change-Id: I808174494729453f4ebcaa13258d735faae68d72 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/northbridge/intel/sandybridge/raminit_common.c M src/northbridge/intel/sandybridge/raminit_common.h M src/northbridge/intel/sandybridge/raminit_native.c 3 files changed, 0 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/47486/1 diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index 60a218b..3fd8cb0 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -4078,18 +4078,6 @@ } } -void write_controller_mr(ramctr_timing *ctrl) -{ - int channel, slotrank; - - FOR_ALL_CHANNELS FOR_ALL_POPULATED_RANKS { - MCHBAR32(lane_base[slotrank] + GDCRTRAININGRESULT1(channel)) = - make_mr0(ctrl, slotrank); - MCHBAR32(lane_base[slotrank] + GDCRTRAININGRESULT2(channel)) = - make_mr1(ctrl, slotrank, channel); - } -} - int channel_test(ramctr_timing *ctrl) { int channel, slotrank, lane; diff --git a/src/northbridge/intel/sandybridge/raminit_common.h b/src/northbridge/intel/sandybridge/raminit_common.h index 5b08ce5..a6f4e0b 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.h +++ b/src/northbridge/intel/sandybridge/raminit_common.h @@ -244,7 +244,6 @@ int discover_edges_write(ramctr_timing *ctrl); int discover_timC_write(ramctr_timing *ctrl); void normalize_training(ramctr_timing *ctrl); -void write_controller_mr(ramctr_timing *ctrl); int channel_test(ramctr_timing *ctrl); void set_scrambling_seed(ramctr_timing *ctrl); void set_wmm_behavior(const u32 cpu); diff --git a/src/northbridge/intel/sandybridge/raminit_native.c b/src/northbridge/intel/sandybridge/raminit_native.c index 844e69a..53017eb 100644 --- a/src/northbridge/intel/sandybridge/raminit_native.c +++ b/src/northbridge/intel/sandybridge/raminit_native.c @@ -715,8 +715,6 @@ set_read_write_timings(ctrl); - write_controller_mr(ctrl); - if (!s3resume) { err = channel_test(ctrl); if (err) -- To view, visit https://review.coreboot.org/c/coreboot/+/47486 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I808174494729453f4ebcaa13258d735faae68d72 Gerrit-Change-Number: 47486 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

3 3

Change in coreboot[master]: nb/intel/sandybridge: Reduce the scope of get_CWL()
by Angel Pons (Code Review) 16 Nov '20

16 Nov '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47485 ) Change subject: nb/intel/sandybridge: Reduce the scope of get_CWL() ...................................................................... nb/intel/sandybridge: Reduce the scope of get_CWL() It is only used once, and can thus be moved to the same file. Change-Id: I4ee0621449da7fa1970a475d5a2f6e66546357ea Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/northbridge/intel/sandybridge/raminit_common.c M src/northbridge/intel/sandybridge/raminit_common.h M src/northbridge/intel/sandybridge/raminit_native.c 3 files changed, 35 insertions(+), 36 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/47485/1 diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index 3ab8df2..60a218b 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -80,41 +80,6 @@ return reg; } -/* CAS write latency. To be programmed in MR2. See DDR3 SPEC for MR2 documentation. */ -u8 get_CWL(u32 tCK) -{ - /* Get CWL based on tCK using the following rule */ - switch (tCK) { - case TCK_1333MHZ: - return 12; - - case TCK_1200MHZ: - case TCK_1100MHZ: - return 11; - - case TCK_1066MHZ: - case TCK_1000MHZ: - return 10; - - case TCK_933MHZ: - case TCK_900MHZ: - return 9; - - case TCK_800MHZ: - case TCK_700MHZ: - return 8; - - case TCK_666MHZ: - return 7; - - case TCK_533MHZ: - return 6; - - default: - return 5; - } -} - void dram_find_common_params(ramctr_timing *ctrl) { size_t valid_dimms; diff --git a/src/northbridge/intel/sandybridge/raminit_common.h b/src/northbridge/intel/sandybridge/raminit_common.h index 32f2b44..5b08ce5 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.h +++ b/src/northbridge/intel/sandybridge/raminit_common.h @@ -227,7 +227,6 @@ #define MAKE_ERR ((channel << 16) | (slotrank << 8) | 1) #define GET_ERR_CHANNEL(x) (x >> 16) -u8 get_CWL(u32 tCK); void dram_mrscommands(ramctr_timing *ctrl); void program_timings(ramctr_timing *ctrl, int channel); void dram_find_common_params(ramctr_timing *ctrl); diff --git a/src/northbridge/intel/sandybridge/raminit_native.c b/src/northbridge/intel/sandybridge/raminit_native.c index 326197b..844e69a 100644 --- a/src/northbridge/intel/sandybridge/raminit_native.c +++ b/src/northbridge/intel/sandybridge/raminit_native.c @@ -42,6 +42,41 @@ die("Unsupported CPU or base frequency."); } +/* CAS write latency. To be programmed in MR2. See DDR3 SPEC for MR2 documentation. */ +static u8 get_CWL(u32 tCK) +{ + /* Get CWL based on tCK using the following rule */ + switch (tCK) { + case TCK_1333MHZ: + return 12; + + case TCK_1200MHZ: + case TCK_1100MHZ: + return 11; + + case TCK_1066MHZ: + case TCK_1000MHZ: + return 10; + + case TCK_933MHZ: + case TCK_900MHZ: + return 9; + + case TCK_800MHZ: + case TCK_700MHZ: + return 8; + + case TCK_666MHZ: + return 7; + + case TCK_533MHZ: + return 6; + + default: + return 5; + } +} + /* Get REFI based on frequency index, tREFI = 7.8usec */ static u32 get_REFI(u32 FRQ, u8 base_freq) { -- To view, visit https://review.coreboot.org/c/coreboot/+/47485 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4ee0621449da7fa1970a475d5a2f6e66546357ea Gerrit-Change-Number: 47485 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

3 2

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2020