Hello John Zhao,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/34249
to review the following change.
Change subject: cpu/x86: Initialize the argument value msr_index
......................................................................
cpu/x86: Initialize the argument value msr_index
Clang Static Analyzer version 8.0.0 treats the uninitialized argument
value msr_index as logic error in a function call. Just set msr_index
with 0 initially.
Change-Id: I051b290998207a1129ada06bb455f44c2d291975
Signed-off-by: John Zhao <john.zhao(a)intel.com>
---
M src/cpu/x86/mtrr/mtrr.c
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/34249/1
diff --git a/src/cpu/x86/mtrr/mtrr.c b/src/cpu/x86/mtrr/mtrr.c
index 98449d5..4dc71d6 100644
--- a/src/cpu/x86/mtrr/mtrr.c
+++ b/src/cpu/x86/mtrr/mtrr.c
@@ -321,6 +321,7 @@
fixed_mtrrs_expose_amd_rwdram();
+ memset(&msr_index, 0, sizeof(msr_index));
memset(&fixed_msrs, 0, sizeof(fixed_msrs));
msr_num = 0;
--
To view, visit https://review.coreboot.org/c/coreboot/+/34249
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I051b290998207a1129ada06bb455f44c2d291975
Gerrit-Change-Number: 34249
Gerrit-PatchSet: 1
Gerrit-Owner: John Zhao <john.zhao(a)intel.corp-partner.google.com>
Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com>
Gerrit-MessageType: newchange
Name of user not set #1002476 has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34630 )
Change subject: mb/intel/minnowmax: Adding config for FSP_PACKAGE_DEFAULT
......................................................................
mb/intel/minnowmax: Adding config for FSP_PACKAGE_DEFAULT
Config VGA_BIOS is using FSP_PACKAGE_DEFAULT, so added config for the same.
Also updated the default path for config FSP_FILE according to the current coreboot directory structure.
Change-Id: Ifb1a92287ad5beea6559db0cd4cf66eeb0f46f95
Signed-off-by: Himanshu Sahdev <Coreboot(a)hcl.com>
---
M src/mainboard/intel/minnowmax/Kconfig
1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/34630/1
diff --git a/src/mainboard/intel/minnowmax/Kconfig b/src/mainboard/intel/minnowmax/Kconfig
index ca24c92..0e3de50 100644
--- a/src/mainboard/intel/minnowmax/Kconfig
+++ b/src/mainboard/intel/minnowmax/Kconfig
@@ -37,7 +37,7 @@
config FSP_FILE
string
- default "../intel/fsp/baytrail/BAYTRAIL_FSP.fd"
+ default "3rdparty/fsp/BayTrailFspBinPkg/FspBin/BAYTRAIL_FSP.fd"
config CBFS_SIZE
hex
@@ -57,6 +57,10 @@
bool
default n
+config FSP_PACKAGE_DEFAULT
+ bool "Configure defaults for the Intel FSP package"
+ default n
+
config VGA_BIOS
bool
default y if FSP_PACKAGE_DEFAULT
--
To view, visit https://review.coreboot.org/c/coreboot/+/34630
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifb1a92287ad5beea6559db0cd4cf66eeb0f46f95
Gerrit-Change-Number: 34630
Gerrit-PatchSet: 1
Gerrit-Owner: Name of user not set #1002476
Gerrit-MessageType: newchange
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34752 )
Change subject: arch/x86: Attempt to boot without postcar stage/phase
......................................................................
arch/x86: Attempt to boot without postcar stage/phase
This patch provides option for soc to exclude postcar
(!HAVE_POSTCAR) stage to avoid an additional stage loading
and executing time. This effort has 2 benefits:
1. Save boot time by ~4ms
2. Avoid generation of postcar.elf which saves (25kB *3copies = 75kB)
of SPI footprint.
By current design the postcar stage/phase handles the cache-as-ram
tear down and as cache-as-ram is volatile and tearing it down leads
to its contents disappearing. Therefore provide a shim layer in ramstage
(similar to postcar) to perform below operations:
1. Tears down cache-as-ram with a chipset helper function.
2. Loads and runs ramstage.
Because those 2 things are executed out of ram there's no issue
of the code's backing store while executing the code that
tears down cache-as-ram. The current implementation makes no
assumption regarding cacheability of the DRAM itself. If the
chipset code wishes to cache DRAM for loading of the ramstage
stage/phase then it's also up to the chipset to handle any
coherency issues pertaining to cache-as-ram destruction.
Change-Id: Ibc88e6727b2fa692585dffe576e69f6d4d0b349d
Credit-to: Aaron Durbin <adurbin(a)chromium.org>
Signed-off-by: Subrata Banik <subrata.banik(a)intel.com>
---
M src/arch/x86/Makefile.inc
M src/arch/x86/c_start.S
M src/arch/x86/exit_car.S
M src/arch/x86/include/arch/cpu.h
A src/arch/x86/ramstage_loader.c
M src/lib/program.ld
M src/soc/intel/common/block/cpu/Makefile.inc
7 files changed, 118 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/34752/1
diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc
index 2fad75e..f8d8c5f 100644
--- a/src/arch/x86/Makefile.inc
+++ b/src/arch/x86/Makefile.inc
@@ -239,6 +239,9 @@
romstage-y += memset.c
romstage-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c
romstage-y += postcar_loader.c
+ifneq ($(CONFIG_HAVE_POSTCAR),y)
+romstage-y += ramstage_loader.c
+endif
romstage-y += stage_loader.c
romstage-$(CONFIG_COLLECT_TIMESTAMPS_TSC) += timestamp.c
romstage-$(CONFIG_ARCH_ROMSTAGE_X86_32) += walkcbfs.S
@@ -305,6 +308,10 @@
ifeq ($(CONFIG_ARCH_RAMSTAGE_X86_32)$(CONFIG_ARCH_RAMSTAGE_X86_64),y)
+ifneq ($(CONFIG_HAVE_POSTCAR),y)
+ramstage-y += exit_car.S
+ramstage-y += gdt_init.S
+endif
ramstage-$(CONFIG_HAVE_ACPI_TABLES) += acpi.c
ramstage-$(CONFIG_HAVE_ACPI_TABLES) += acpigen.c
ramstage-$(CONFIG_HAVE_ACPI_TABLES) += acpigen_dsm.c
diff --git a/src/arch/x86/c_start.S b/src/arch/x86/c_start.S
index 32b848d..463eea9 100644
--- a/src/arch/x86/c_start.S
+++ b/src/arch/x86/c_start.S
@@ -37,8 +37,13 @@
#else
.code32
#endif
+#if CONFIG(HAVE_POSTCAR)
.globl _start
_start:
+#else
+ .globl ramstage_start
+ramstage_start:
+#endif
cli
lgdt %cs:gdtaddr
#ifndef __x86_64__
diff --git a/src/arch/x86/exit_car.S b/src/arch/x86/exit_car.S
index 769a758..662b199 100644
--- a/src/arch/x86/exit_car.S
+++ b/src/arch/x86/exit_car.S
@@ -127,7 +127,11 @@
/* Align stack to 16 bytes at call instruction. */
andl $0xfffffff0, %esp
/* Call into main for postcar. */
+#if CONFIG(HAVE_POSTCAR)
call main
+#else
+ call ramstage_start
+#endif
/* Should never return. */
1:
jmp 1b
diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h
index 293ca02..0f8838b 100644
--- a/src/arch/x86/include/arch/cpu.h
+++ b/src/arch/x86/include/arch/cpu.h
@@ -345,6 +345,8 @@
*/
void run_postcar_phase(struct postcar_frame *pcf);
+void run_ramstage_phase(struct postcar_frame *pcf);
+
/*
* Systems without a native coreboot cache-as-ram teardown may implement
* this to use an alternate method.
diff --git a/src/arch/x86/ramstage_loader.c b/src/arch/x86/ramstage_loader.c
new file mode 100644
index 0000000..bf35ef5
--- /dev/null
+++ b/src/arch/x86/ramstage_loader.c
@@ -0,0 +1,90 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2019 Intel Corp.
+ *
+ * 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.
+ *
+ * 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.
+ */
+
+#include <arch/cpu.h>
+#include <arch/stage_loader.h>
+#include <cbmem.h>
+#include <cbfs.h>
+#include <console/console.h>
+#include <cpu/cpu.h>
+#include <cpu/x86/msr.h>
+#include <cpu/x86/mtrr.h>
+#include <program_loading.h>
+#include <rmodule.h>
+#include <romstage_handoff.h>
+#include <stage_cache.h>
+#include <timestamp.h>
+
+static void *ramstage_commit_mtrrs(struct postcar_frame *pcf)
+{
+ /*
+ * Place the number of used variable MTRRs on stack then max number
+ * of variable MTRRs supported in the system.
+ */
+ stack_push(pcf, pcf->num_var_mtrrs);
+ stack_push(pcf, pcf->max_var_mtrrs);
+ return (void *) pcf->stack;
+}
+
+static void load_ramstage(struct prog *ramstage, struct postcar_frame *pcf)
+{
+ struct rmod_stage_load rmod_ram = {
+ .cbmem_id = CBMEM_ID_RAMSTAGE,
+ .prog = ramstage,
+ };
+
+ if (prog_locate(ramstage))
+ die_with_post_code(POST_INVALID_ROM,
+ "Failed to locate after CAR program.\n");
+
+ if (rmodule_stage_load(&rmod_ram))
+ die_with_post_code(POST_INVALID_ROM,
+ "Failed to load after CAR program.\n");
+
+ /* Set the stack pointer within parameters of the program loaded. */
+ if (rmod_ram.params == NULL)
+ die_with_post_code(POST_INVALID_ROM,
+ "No parameters found in after CAR program.\n");
+
+ finalize_load(rmod_ram.params, pcf->stack);
+
+ stage_cache_add(STAGE_RAMSTAGE, ramstage);
+}
+
+void run_ramstage_phase(struct postcar_frame *pcf)
+{
+ struct prog ramstage =
+ PROG_INIT(PROG_RAMSTAGE, CONFIG_CBFS_PREFIX "/ramstage");
+
+ timestamp_add_now(TS_END_ROMSTAGE);
+
+ ramstage_commit_mtrrs(pcf);
+
+ timestamp_add_now(TS_START_COPYRAM);
+ if (!CONFIG(NO_STAGE_CACHE) &&
+ romstage_handoff_is_resume()) {
+ stage_cache_load_stage(STAGE_RAMSTAGE, &ramstage);
+ /* This is here to allow platforms to pass different stack
+ parameters between S3 resume and normal boot. On the
+ platforms where the values are the same it's a nop. */
+ finalize_load(ramstage.arg, pcf->stack);
+ } else
+ load_ramstage(&ramstage, pcf);
+
+ /* As postcar exist, it's end of romstage here */
+ timestamp_add_now(TS_END_COPYRAM);
+
+ prog_run(&ramstage);
+}
diff --git a/src/lib/program.ld b/src/lib/program.ld
index 851aa75..f6a6421 100644
--- a/src/lib/program.ld
+++ b/src/lib/program.ld
@@ -91,13 +91,13 @@
_data = .;
/*
- * The postcar phase uses a stack value that is located in the relocatable
- * module section. While the postcar stage could be linked like smm and
- * other rmodules the postcar stage needs similar semantics of the more
- * traditional stages in the coreboot infrastructure. Therefore it's easier
- * to specialize this case.
+ * The postcar or ramstage (incase postcar not enable) phase uses a stack value
+ * that is located in the relocatable module section. While the postcar/
+ * ramstage stage could be linked like smm and other rmodules the postcar/
+ * ramstage stage needs similar semantics of the more traditional stages in the
+ * coreboot infrastructure. Therefore it's easier to specialize this case.
*/
-#if ENV_RMODULE || ENV_POSTCAR
+#if ENV_RMODULE || ENV_POSTCAR || !CONFIG(HAVE_POSTCAR)
_rmodule_params = .;
KEEP(*(.module_parameters));
_ermodule_params = .;
diff --git a/src/soc/intel/common/block/cpu/Makefile.inc b/src/soc/intel/common/block/cpu/Makefile.inc
index a6c4f37..1f57598 100644
--- a/src/soc/intel/common/block/cpu/Makefile.inc
+++ b/src/soc/intel/common/block/cpu/Makefile.inc
@@ -9,5 +9,9 @@
postcar-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += car/exit_car.S
postcar-$(CONFIG_FSP_CAR) += car/exit_car_fsp.S
+ifneq ($(CONFIG_HAVE_POSTCAR),y)
+ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += car/exit_car.S
+endif
+
ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += cpulib.c
ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU_MPINIT) += mp_init.c
--
To view, visit https://review.coreboot.org/c/coreboot/+/34752
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibc88e6727b2fa692585dffe576e69f6d4d0b349d
Gerrit-Change-Number: 34752
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-MessageType: newchange