Change in ...coreboot[master]: soc/amd/picasso: Make romstage a hybrid

[Marshall Dawson (Code Review)]

Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33760

Change subject: soc/amd/picasso: Make romstage a hybrid
......................................................................

soc/amd/picasso: Make romstage a hybrid

Update romstage.c and associated files to create the hybrid romstage.
Make early calls to set up the soc and any mainboard requirements,
similar to what bootblock does in other x86 systems.

Implementation details are in Documentation/soc/amd/family17h.md.

Change-Id: I2c3dc68463d2027d2b1e68e15708cfb6a84f1b39
Signed-off-by: Marshall Dawson 
---
M src/soc/amd/picasso/include/soc/cpu.h
M src/soc/amd/picasso/include/soc/romstage.h
M src/soc/amd/picasso/romstage.c
3 files changed, 119 insertions(+), 25 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/33760/1

diff --git a/src/soc/amd/picasso/include/soc/cpu.h b/src/soc/amd/picasso/include/soc/cpu.h
index 7bc1810..1facf64 100644
--- a/src/soc/amd/picasso/include/soc/cpu.h
+++ b/src/soc/amd/picasso/include/soc/cpu.h
@@ -18,8 +18,8 @@

#include <device/device.h>

-#define SOC_EARLY_VMTRR_FLASH 1
-#define SOC_EARLY_VMTRR_TEMPRAM 2
+#define SOC_EARLY_VMTRR_ROMSTAGE 0
+#define SOC_EARLY_VMTRR_TEMPRAM 1

void picasso_init_cpus(struct device *dev);
 void check_mca(void);
diff --git a/src/soc/amd/picasso/include/soc/romstage.h b/src/soc/amd/picasso/include/soc/romstage.h
index d8b2900..fca8f0c 100644
--- a/src/soc/amd/picasso/include/soc/romstage.h
+++ b/src/soc/amd/picasso/include/soc/romstage.h
@@ -2,6 +2,7 @@
  * This file is part of the coreboot project.
  *
  * Copyright (C) 2018 Google LLC
+ * Copyright (C) 2019 Advanced Micro Devices, Inc.
  *
  * 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
@@ -16,6 +17,19 @@
 #ifndef __PICASSO_ROMSTAGE_H__
 #define __PICASSO_ROMSTAGE_H__

-void mainboard_romstage_entry(int s3_resume);
+#include <stdint.h>
+
+#if (CONFIG_ROMSTAGE_ADDR + CONFIG_ROMSTAGE_MAX_SIZE) & 0xffff
+# error Top of the BIOS binary image must be 64KB aligned
+#endif
+
+struct early_saved_info {
+	uint32_t unused;
+	uint32_t bist;
+	uint64_t early_tsc;
+} __packed;
+
+void romstage_mainboard_early_init(void);
+void romstage_mainboard_init(int s3_resume);

#endif /* __PICASSO_ROMSTAGE_H__ */
diff --git a/src/soc/amd/picasso/romstage.c b/src/soc/amd/picasso/romstage.c
index 950b41f..da4ed8d 100644
--- a/src/soc/amd/picasso/romstage.c
+++ b/src/soc/amd/picasso/romstage.c
@@ -1,7 +1,7 @@
 /*
  * This file is part of the coreboot project.
  *
- * Copyright (C) 2015-2016 Advanced Micro Devices, Inc.
+ * Copyright (C) 2019 Advanced Micro Devices, Inc.
  * Copyright (C) 2015 Intel Corp.
  *
  * This program is free software; you can redistribute it and/or modify
@@ -17,25 +17,67 @@
 #include <device/pci_ops.h>
 #include <arch/cpu.h>
 #include <arch/acpi.h>
+#include <arch/exception.h>
+#include <arch/symbols.h>
+#include <delay.h>
+#include <pc80/mc146818rtc.h>
 #include <cpu/x86/msr.h>
 #include <cpu/x86/mtrr.h>
+#include <cpu/x86/tsc.h>
+#include <cpu/x86/bist.h>
 #include <cpu/amd/mtrr.h>
+#include <cpu/amd/msr.h>
 #include <cbmem.h>
-#include <commonlib/helpers.h>
 #include <console/console.h>
-#include <device/device.h>
+#include <timestamp.h>
 #include <program_loading.h>
 #include <romstage_handoff.h>
 #include <elog.h>
+#include <soc/cpu.h>
 #include <soc/northbridge.h>
-#include <soc/romstage.h>
 #include <soc/southbridge.h>
+#include <soc/romstage.h>

-#include "chip.h"
+__weak void romstage_mainboard_early_init(void) {}
+__weak void romstage_mainboard_init(int s3_resume) {}

-void __weak mainboard_romstage_entry(int s3_resume)
+static void romstage_soc_early_init(void)
 {
-	/* By default, don't do anything */
+	msr_t mmconf;
+
+	mmconf.hi = 0;
+	mmconf.lo = CONFIG_MMCONF_BASE_ADDRESS | MMIO_RANGE_EN
+			| fms(CONFIG_MMCONF_BUS_NUMBER) << MMIO_BUS_RANGE_SHIFT;
+	wrmsr(MMIO_CONF_BASE, mmconf);
+
+	fch_pre_init();
+}
+
+static void romstage_soc_init(int s3_resume)
+{
+	fch_early_init();
+}
+
+static uint32_t find_bist(void)
+{
+	volatile struct early_saved_info *early;
+
+	early = (volatile struct early_saved_info *)_car_stack_end - 1;
+	return early->bist;
+}
+
+static void init_timestamps(tsc_t tsc)
+{
+	volatile struct early_saved_info *early;
+	uint64_t stage_start;
+
+	/* don't rely on tsc_t ordering */
+	stage_start = (uint64_t)tsc.hi << 32;
+	stage_start += tsc.lo;
+
+	early = (volatile struct early_saved_info *)_car_stack_end - 1;
+	timestamp_init(early->early_tsc);
+	timestamp_add(TS_START_ROMSTAGE, stage_start);
 }

asmlinkage void car_stage_entry(void)
@@ -45,29 +87,67 @@
    void *smm_base;
    size_t smm_size;
    uintptr_t tseg_base;
-	int s3_resume = acpi_s3_resume_allowed() && acpi_is_wakeup_s3();
+	int s3_resume;
+	tsc_t tsc = rdtsc();

+	post_code(0x40);
+	/*
+	 * Many of these tasks typically happen in bootblock, but execution
+	 * begins in romstage for this device.
+	 */
+
+	/* cache this code and the memory used to mimic TEMPRAM */
+	set_var_mtrr(SOC_EARLY_VMTRR_ROMSTAGE, CONFIG_ROMSTAGE_ADDR,
+			CONFIG_ROMSTAGE_MAX_SIZE, MTRR_TYPE_WRBACK);
+	set_var_mtrr(SOC_EARLY_VMTRR_TEMPRAM, CONFIG_DCACHE_RAM_BASE,
+			CONFIG_DCACHE_RAM_SIZE, MTRR_TYPE_WRBACK);
+
+	romstage_soc_early_init();
+	romstage_mainboard_early_init();
+
+	post_code(0x41);
+	init_timer();
+	sanitize_cmos();
+	cmos_post_init();
+
+	post_code(0x42);
    console_init();

-	mainboard_romstage_entry(s3_resume);
-
-	if (!s3_resume) {
-		post_code(0x40);
-
-		if (CONFIG(ELOG_BOOT_COUNT))
-			boot_count_increment();
-	} else {
-		printk(BIOS_INFO, "S3 detected\n");
-		post_code(0x60);
-	}
-
    post_code(0x43);
+	report_bist_failure(find_bist());
+
+	post_code(0x44);
+	if (CONFIG(COLLECT_TIMESTAMPS))
+		init_timestamps(tsc);
+
+	post_code(0x45);
+	exception_init();
+
+	post_code(0x46);
+	s3_resume = acpi_s3_resume_allowed() && acpi_is_wakeup_s3();
+	romstage_soc_init(s3_resume);
+	romstage_mainboard_init(s3_resume);
+
+	post_code(0x47);
+	u32 val = cpuid_eax(1);
+	printk(BIOS_DEBUG, "Family_Model: %08x\n", val);
+
+	post_code(0x48);
+	if (!s3_resume && CONFIG(ELOG_BOOT_COUNT))
+		boot_count_increment();
+
+	post_code(0x49);
+	msr_t tom = rdmsr(TOP_MEM);
+	tom.lo &= ~0xffffff;
+	backup_top_of_low_cacheable(tom.lo);
+
+	post_code(0x4a);
    if (cbmem_recovery(s3_resume))
    	printk(BIOS_CRIT, "Failed to recover cbmem\n");
    if (romstage_handoff_init(s3_resume))
    	printk(BIOS_ERR, "Failed to set romstage handoff data\n");

-	post_code(0x44);
+	post_code(0x4b);
    if (postcar_frame_init(&pcf, 1 * KiB))
    	die("Unable to initialize postcar frame.\n");

@@ -94,7 +174,7 @@
    tseg_base = (uintptr_t)smm_base;
    postcar_frame_add_mtrr(&pcf, tseg_base, smm_size, MTRR_TYPE_WRBACK);

-	post_code(0x45);
+	post_code(0x4c);
    run_postcar_phase(&pcf);

post_code(0x50);  /* Should never see this post code. */

[Richard Spiegel (Code Review)]

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33760 )

Change subject: soc/amd/picasso: Make romstage a hybrid
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/33760/1/src/soc/amd/picasso/include/soc/roms... 
File src/soc/amd/picasso/include/soc/romstage.h:

https://review.coreboot.org/#/c/33760/1/src/soc/amd/picasso/include/soc/roms... 
PS1, Line 26: truct early_saved_info {
            : 	uint32_t unused;
            : 	uint32_t bist;
            : 	uint64_t early_tsc;
            : } __packed;
            : 
Nice trick pushing it into stack and landing in the structure. Maybe add a comment explaining how it happens? Not obvious if I had not read the code that does the push before reading this patch.

[Marshall Dawson (Code Review)]

Hello Richard Spiegel, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33760

to look at the new patch set (#2).

Change subject: soc/amd/picasso: Make romstage a hybrid
......................................................................

soc/amd/picasso: Make romstage a hybrid

Update romstage.c and associated files to create the hybrid romstage.
Make early calls to set up the soc and any mainboard requirements,
similar to what bootblock does in other x86 systems.

Implementation details are in Documentation/soc/amd/family17h.md.

Change-Id: I2c3dc68463d2027d2b1e68e15708cfb6a84f1b39
Signed-off-by: Marshall Dawson 
---
M src/soc/amd/picasso/include/soc/cpu.h
M src/soc/amd/picasso/include/soc/romstage.h
M src/soc/amd/picasso/reset_vector.S
M src/soc/amd/picasso/romstage.c
4 files changed, 123 insertions(+), 26 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/33760/2

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33760 )

Change subject: soc/amd/picasso: Make romstage a hybrid
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/33760/1/src/soc/amd/picasso/include/soc/roms... 
File src/soc/amd/picasso/include/soc/romstage.h:

https://review.coreboot.org/#/c/33760/1/src/soc/amd/picasso/include/soc/roms... 
PS1, Line 26: truct early_saved_info {
            : 	uint32_t unused;
            : 	uint32_t bist;
            : 	uint64_t early_tsc;
            : } __packed;
            :
> Nice trick pushing it into stack and landing in the structure. […]

Done

Change in coreboot[master]: soc/amd/picasso: Make romstage a hybrid

[Marshall Dawson (Code Review)]

Marshall Dawson has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/33760 )

Change subject: soc/amd/picasso: Make romstage a hybrid
......................................................................

Abandoned

Squashed into https://review.coreboot.org/c/coreboot/+/33759/5