Hello Marshall Dawson,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/31361
to review the following change.
Change subject: [WIP] AGESA: Fix CAR_GLOBAL use for AP CPUs
......................................................................
[WIP] AGESA: Fix CAR_GLOBAL use for AP CPUs
The memory between _car_region_start .. _car_region_end has to
be set up as WB in MTRRs for all the cores executing through
bootblock, verstage and romstage. Otherwise CAR_GLOBALs may
fail on AP CPUs.
Fixes combination of CBMEM_CONSOLE=y with SQUELCH_EARLY_SMP=n,
which previously did not boot at all.
Does not fix family14.
Change-Id: I3a8a389ca0d7a88c779f27f4ead0d9581465edfd
Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Signed-off-by: Marshall Dawson <marshalldawson3rd(a)gmail.com>
---
M src/vendorcode/amd/agesa/f12/gcccar.inc
M src/vendorcode/amd/agesa/f14/gcccar.inc
M src/vendorcode/amd/agesa/f15tn/gcccar.inc
M src/vendorcode/amd/agesa/f16kb/gcccar.inc
4 files changed, 28 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/31361/1
diff --git a/src/vendorcode/amd/agesa/f12/gcccar.inc b/src/vendorcode/amd/agesa/f12/gcccar.inc
index 95dd74d..e6dba6a 100644
--- a/src/vendorcode/amd/agesa/f12/gcccar.inc
+++ b/src/vendorcode/amd/agesa/f12/gcccar.inc
@@ -1451,6 +1451,13 @@
0:
_WRMSR #
+ # All cores must see BSP stack region that is also used to
+ # communicate global variables before DRAM is up.
+ mov $AMD_MTRR_FIX64k_00000, %ecx # MSR:0000_0250
+ _RDMSR
+ or $0x1e000000, %eax
+ _WRMSR
+
# Enable MTRR defaults as UC type
mov $AMD_MTRR_DEFTYPE, %ecx # MSR:0000_02FF
_RDMSR # Read-modify-write the MSR
diff --git a/src/vendorcode/amd/agesa/f14/gcccar.inc b/src/vendorcode/amd/agesa/f14/gcccar.inc
index 95dd74d..e6dba6a 100644
--- a/src/vendorcode/amd/agesa/f14/gcccar.inc
+++ b/src/vendorcode/amd/agesa/f14/gcccar.inc
@@ -1451,6 +1451,13 @@
0:
_WRMSR #
+ # All cores must see BSP stack region that is also used to
+ # communicate global variables before DRAM is up.
+ mov $AMD_MTRR_FIX64k_00000, %ecx # MSR:0000_0250
+ _RDMSR
+ or $0x1e000000, %eax
+ _WRMSR
+
# Enable MTRR defaults as UC type
mov $AMD_MTRR_DEFTYPE, %ecx # MSR:0000_02FF
_RDMSR # Read-modify-write the MSR
diff --git a/src/vendorcode/amd/agesa/f15tn/gcccar.inc b/src/vendorcode/amd/agesa/f15tn/gcccar.inc
index b13e02a..e96c7f7 100644
--- a/src/vendorcode/amd/agesa/f15tn/gcccar.inc
+++ b/src/vendorcode/amd/agesa/f15tn/gcccar.inc
@@ -1767,6 +1767,13 @@
0:
_WRMSR #
+ # All cores must see BSP stack region that is also used to
+ # communicate global variables before DRAM is up.
+ mov $AMD_MTRR_FIX64k_00000, %ecx # MSR:0000_0250
+ _RDMSR
+ or $0x1e000000, %eax
+ _WRMSR
+
# Enable MTRR defaults as UC type
mov $AMD_MTRR_DEFTYPE, %ecx # MSR:0000_02FF
_RDMSR # Read-modify-write the MSR
diff --git a/src/vendorcode/amd/agesa/f16kb/gcccar.inc b/src/vendorcode/amd/agesa/f16kb/gcccar.inc
index c818d97..26745c9 100644
--- a/src/vendorcode/amd/agesa/f16kb/gcccar.inc
+++ b/src/vendorcode/amd/agesa/f16kb/gcccar.inc
@@ -1131,6 +1131,13 @@
0:
_WRMSR #
+ # All cores must see BSP stack region that is also used to
+ # communicate global variables before DRAM is up.
+ mov $AMD_MTRR_FIX64k_00000, %ecx # MSR:0000_0250
+ _RDMSR
+ or $0x1e000000, %eax
+ _WRMSR
+
# Enable MTRR defaults as UC type
mov $AMD_MTRR_DEFTYPE, %ecx # MSR:0000_02FF
_RDMSR # Read-modify-write the MSR
--
To view, visit https://review.coreboot.org/c/coreboot/+/31361
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3a8a389ca0d7a88c779f27f4ead0d9581465edfd
Gerrit-Change-Number: 31361
Gerrit-PatchSet: 1
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: newchange
Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36530 )
Change subject: [WIP] cpu/x86/lapic: Use udelay() from LAPIC_MONOTONIC_TIMER
......................................................................
[WIP] cpu/x86/lapic: Use udelay() from LAPIC_MONOTONIC_TIMER
TBD: PI/AGESA reconfigures LAPIC breaking udelay().
TBD: Use of init_timer() is wrong for smm-y
Change-Id: I3c982e4121586e932856f390a12ce125f964b2e7
Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
---
M src/cpu/amd/agesa/Kconfig
M src/cpu/amd/agesa/family15tn/Makefile.inc
D src/cpu/amd/agesa/family15tn/udelay.c
M src/cpu/amd/family_10h-family_15h/Kconfig
M src/cpu/amd/pi/00630F01/Makefile.inc
M src/cpu/amd/pi/00630F01/fixme.c
D src/cpu/amd/pi/00630F01/udelay.c
M src/cpu/amd/pi/00660F01/fixme.c
M src/cpu/amd/pi/00730F01/fixme.c
M src/cpu/amd/pi/Kconfig
M src/cpu/x86/Kconfig
M src/cpu/x86/lapic/Makefile.inc
M src/cpu/x86/lapic/apic_timer.c
13 files changed, 7 insertions(+), 160 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/36530/1
diff --git a/src/cpu/amd/agesa/Kconfig b/src/cpu/amd/agesa/Kconfig
index b1fde2d..96bf043 100644
--- a/src/cpu/amd/agesa/Kconfig
+++ b/src/cpu/amd/agesa/Kconfig
@@ -24,7 +24,6 @@
select ARCH_RAMSTAGE_X86_32
select DRIVERS_AMD_PI
select TSC_SYNC_LFENCE
- select UDELAY_LAPIC
select LAPIC_MONOTONIC_TIMER
select SPI_FLASH if HAVE_ACPI_RESUME
select SMM_ASEG
diff --git a/src/cpu/amd/agesa/family15tn/Makefile.inc b/src/cpu/amd/agesa/family15tn/Makefile.inc
index 8522eb2..3554196 100644
--- a/src/cpu/amd/agesa/family15tn/Makefile.inc
+++ b/src/cpu/amd/agesa/family15tn/Makefile.inc
@@ -17,8 +17,6 @@
ramstage-y += chip_name.c
ramstage-y += model_15_init.c
-smm-y += udelay.c
-
subdirs-y += ../../mtrr
subdirs-y += ../../smm
subdirs-y += ../../../x86/tsc
diff --git a/src/cpu/amd/agesa/family15tn/udelay.c b/src/cpu/amd/agesa/family15tn/udelay.c
deleted file mode 100644
index 898f6c1..0000000
--- a/src/cpu/amd/agesa/family15tn/udelay.c
+++ /dev/null
@@ -1,57 +0,0 @@
-/*
- * This file is part of the coreboot project.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-/*
- * udelay() implementation for SMI handlers
- * This is neat in that it never writes to hardware registers, and thus does
- * not modify the state of the hardware while servicing SMIs.
- */
-
-#include <cpu/x86/msr.h>
-#include <cpu/amd/msr.h>
-#include <cpu/x86/tsc.h>
-#include <delay.h>
-#include <stdint.h>
-
-void udelay(uint32_t us)
-{
- uint8_t fid, did, pstate_idx;
- uint64_t tsc_clock, tsc_start, tsc_now, tsc_wait_ticks;
- msr_t msr;
- const uint64_t tsc_base = 100000000;
-
- /* Get initial timestamp before we do the math */
- tsc_start = rdtscll();
-
- /* Get the P-state. This determines which MSR to read */
- msr = rdmsr(PS_STS_REG);
- pstate_idx = msr.lo & 0x07;
-
- /* Get FID and VID for current P-State */
- msr = rdmsr(PSTATE_0_MSR + pstate_idx);
-
- /* Extract the FID and VID values */
- fid = msr.lo & 0x3f;
- did = (msr.lo >> 6) & 0x7;
-
- /* Calculate the CPU clock (from base freq of 100MHz) */
- tsc_clock = tsc_base * (fid + 0x10) / (1 << did);
-
- /* Now go on and wait */
- tsc_wait_ticks = (tsc_clock / 1000000) * us;
-
- do {
- tsc_now = rdtscll();
- } while (tsc_now - tsc_wait_ticks < tsc_start);
-}
diff --git a/src/cpu/amd/family_10h-family_15h/Kconfig b/src/cpu/amd/family_10h-family_15h/Kconfig
index ad4f5f4..d81628b 100644
--- a/src/cpu/amd/family_10h-family_15h/Kconfig
+++ b/src/cpu/amd/family_10h-family_15h/Kconfig
@@ -6,7 +6,6 @@
select ARCH_RAMSTAGE_X86_32
select SSE2
select TSC_SYNC_LFENCE
- select UDELAY_LAPIC
select SUPPORT_CPU_UCODE_IN_CBFS
select CPU_MICROCODE_MULTIPLE_FILES
select CAR_GLOBAL_MIGRATION
diff --git a/src/cpu/amd/pi/00630F01/Makefile.inc b/src/cpu/amd/pi/00630F01/Makefile.inc
index 8522eb2..3554196 100644
--- a/src/cpu/amd/pi/00630F01/Makefile.inc
+++ b/src/cpu/amd/pi/00630F01/Makefile.inc
@@ -17,8 +17,6 @@
ramstage-y += chip_name.c
ramstage-y += model_15_init.c
-smm-y += udelay.c
-
subdirs-y += ../../mtrr
subdirs-y += ../../smm
subdirs-y += ../../../x86/tsc
diff --git a/src/cpu/amd/pi/00630F01/fixme.c b/src/cpu/amd/pi/00630F01/fixme.c
index 12f8062..6c88bb6 100644
--- a/src/cpu/amd/pi/00630F01/fixme.c
+++ b/src/cpu/amd/pi/00630F01/fixme.c
@@ -86,7 +86,7 @@
MsrReg = ((1ULL << CONFIG_CPU_ADDR_BITS) - CACHE_ROM_SIZE) | 0x800ull;
LibAmdMsrWrite(MTRR_PHYS_MASK(6), &MsrReg, &StdHeader);
- if (CONFIG(UDELAY_LAPIC)){
+ if (CONFIG(LAPIC_MONOTONIC_TIMER)) {
LibAmdMsrRead(0x1B, &MsrReg, &StdHeader);
MsrReg |= 1 << 11;
LibAmdMsrWrite(0x1B, &MsrReg, &StdHeader);
diff --git a/src/cpu/amd/pi/00630F01/udelay.c b/src/cpu/amd/pi/00630F01/udelay.c
deleted file mode 100644
index d4bf45f..0000000
--- a/src/cpu/amd/pi/00630F01/udelay.c
+++ /dev/null
@@ -1,57 +0,0 @@
-/*
- * This file is part of the coreboot project.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; version 2 of the License or (at your option)
- * any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-/*
- * udelay() implementation for SMI handlers
- * This is neat in that it never writes to hardware registers, and thus does not
- * modify the state of the hardware while servicing SMIs.
- */
-
-#include <cpu/x86/msr.h>
-#include <cpu/amd/msr.h>
-#include <cpu/x86/tsc.h>
-#include <delay.h>
-#include <stdint.h>
-
-void udelay(uint32_t us)
-{
- uint8_t fid, did, pstate_idx;
- uint64_t tsc_clock, tsc_start, tsc_now, tsc_wait_ticks;
- msr_t msr;
- const uint64_t tsc_base = 100000000;
-
- /* Get initial timestamp before we do the math */
- tsc_start = rdtscll();
-
- /* Get the P-state. This determines which MSR to read */
- msr = rdmsr(PS_STS_REG);
- pstate_idx = msr.lo & 0x07;
-
- /* Get FID and VID for current P-State */
- msr = rdmsr(PSTATE_0_MSR + pstate_idx);
-
- /* Extract the FID and VID values */
- fid = msr.lo & 0x3f;
- did = (msr.lo >> 6) & 0x7;
-
- /* Calculate the CPU clock (from base freq of 100MHz) */
- tsc_clock = tsc_base * (fid + 0x10) / (1 << did);
-
- /* Now go on and wait */
- tsc_wait_ticks = (tsc_clock / 1000000) * us;
-
- do {
- tsc_now = rdtscll();
- } while (tsc_now - tsc_wait_ticks < tsc_start);
-}
diff --git a/src/cpu/amd/pi/00660F01/fixme.c b/src/cpu/amd/pi/00660F01/fixme.c
index 237d52b..1e52864 100644
--- a/src/cpu/amd/pi/00660F01/fixme.c
+++ b/src/cpu/amd/pi/00660F01/fixme.c
@@ -92,7 +92,7 @@
MsrReg = ((1ULL << CONFIG_CPU_ADDR_BITS) - CACHE_ROM_SIZE) | 0x800ull;
LibAmdMsrWrite(MTRR_PHYS_MASK(6), &MsrReg, &StdHeader);
- if (CONFIG(UDELAY_LAPIC)) {
+ if (CONFIG(LAPIC_MONOTONIC_TIMER)) {
LibAmdMsrRead(0x1B, &MsrReg, &StdHeader);
MsrReg |= 1 << 11;
LibAmdMsrWrite(0x1B, &MsrReg, &StdHeader);
diff --git a/src/cpu/amd/pi/00730F01/fixme.c b/src/cpu/amd/pi/00730F01/fixme.c
index a0621cb..40c8a40 100644
--- a/src/cpu/amd/pi/00730F01/fixme.c
+++ b/src/cpu/amd/pi/00730F01/fixme.c
@@ -97,7 +97,7 @@
MsrReg = ((1ULL << CONFIG_CPU_ADDR_BITS) - CACHE_ROM_SIZE) | 0x800ull;
LibAmdMsrWrite(MTRR_PHYS_MASK(6), &MsrReg, &StdHeader);
- if (CONFIG(UDELAY_LAPIC)) {
+ if (CONFIG(LAPIC_MONOTONIC_TIMER)) {
LibAmdMsrRead(0x1B, &MsrReg, &StdHeader);
MsrReg |= 1 << 11;
LibAmdMsrWrite(0x1B, &MsrReg, &StdHeader);
diff --git a/src/cpu/amd/pi/Kconfig b/src/cpu/amd/pi/Kconfig
index b33302e..8addecb 100644
--- a/src/cpu/amd/pi/Kconfig
+++ b/src/cpu/amd/pi/Kconfig
@@ -23,7 +23,6 @@
select ARCH_RAMSTAGE_X86_32
select DRIVERS_AMD_PI
select TSC_SYNC_LFENCE
- select UDELAY_LAPIC
select LAPIC_MONOTONIC_TIMER
select SPI_FLASH if HAVE_ACPI_RESUME
select CAR_GLOBAL_MIGRATION if BINARYPI_LEGACY_WRAPPER
diff --git a/src/cpu/x86/Kconfig b/src/cpu/x86/Kconfig
index 01700f2..556a54d 100644
--- a/src/cpu/x86/Kconfig
+++ b/src/cpu/x86/Kconfig
@@ -17,13 +17,8 @@
Allow APs to do other work after initialization instead of going
to sleep.
-config UDELAY_LAPIC
- bool
- default n
-
config LAPIC_MONOTONIC_TIMER
def_bool n
- depends on UDELAY_LAPIC
help
Expose monotonic time using the local APIC.
diff --git a/src/cpu/x86/lapic/Makefile.inc b/src/cpu/x86/lapic/Makefile.inc
index 9454f8f..058e25b 100644
--- a/src/cpu/x86/lapic/Makefile.inc
+++ b/src/cpu/x86/lapic/Makefile.inc
@@ -1,9 +1,9 @@
ramstage-y += lapic.c
ramstage-y += lapic_cpu_init.c
ramstage-$(CONFIG_SMP) += secondary.S
-romstage-$(CONFIG_UDELAY_LAPIC) += apic_timer.c
-ramstage-$(CONFIG_UDELAY_LAPIC) += apic_timer.c
-postcar-$(CONFIG_UDELAY_LAPIC) += apic_timer.c
+romstage-$(CONFIG_LAPIC_MONOTONIC_TIMER) += apic_timer.c
+ramstage-$(CONFIG_LAPIC_MONOTONIC_TIMER) += apic_timer.c
+postcar-$(CONFIG_LAPIC_MONOTONIC_TIMER) += apic_timer.c
bootblock-y += boot_cpu.c
verstage-y += boot_cpu.c
romstage-y += boot_cpu.c
diff --git a/src/cpu/x86/lapic/apic_timer.c b/src/cpu/x86/lapic/apic_timer.c
index 58836b5..a2c4c4a 100644
--- a/src/cpu/x86/lapic/apic_timer.c
+++ b/src/cpu/x86/lapic/apic_timer.c
@@ -19,6 +19,7 @@
#include <arch/early_variables.h>
#include <cpu/x86/msr.h>
#include <cpu/x86/lapic.h>
+#include <timer.h>
void init_timer(void)
{
@@ -32,33 +33,6 @@
lapic_write(LAPIC_TMICT, 0xffffffff);
}
-void udelay(u32 usecs)
-{
- u32 start, value, ticks, timer_fsb;
-
- if (!thread_yield_microseconds(usecs))
- return;
-
- timer_fsb = get_timer_fsb();
- if (!timer_fsb || (lapic_read(LAPIC_LVTT) &
- (LAPIC_LVT_TIMER_PERIODIC | LAPIC_LVT_MASKED)) !=
- (LAPIC_LVT_TIMER_PERIODIC | LAPIC_LVT_MASKED)) {
- init_timer();
- timer_fsb = get_timer_fsb();
- }
-
- /* Calculate the number of ticks to run, our FSB runs at timer_fsb Mhz
- */
- ticks = usecs * timer_fsb;
- start = lapic_read(LAPIC_TMCCT);
- do {
- value = lapic_read(LAPIC_TMCCT);
- } while ((start - value) < ticks);
-}
-
-#if CONFIG(LAPIC_MONOTONIC_TIMER)
-#include <timer.h>
-
static struct monotonic_counter {
int initialized;
struct mono_time time;
@@ -102,4 +76,3 @@
/* Save result. */
*mt = mono_counter->time;
}
-#endif
--
To view, visit https://review.coreboot.org/c/coreboot/+/36530
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3c982e4121586e932856f390a12ce125f964b2e7
Gerrit-Change-Number: 36530
Gerrit-PatchSet: 1
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-MessageType: newchange