Change in coreboot[master]: arch/x86/cbmem.c: Drop API to save cbmem_top across stages - coreboot-gerrit

24 Oct 2019

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36290 )
Change subject: arch/x86/cbmem.c: Drop API to save cbmem_top across stages
......................................................................
arch/x86/cbmem.c: Drop API to save cbmem_top across stages
Cbmem_top is now passed to ramstage via a calling argument, so there
is no need to back it up in a place that persists through stages.
On AMD platforms this uses the common CONFIG_CBMEM_TOP_SAVE API to
save the output of AGESA/PI to implement cbmem_top.
Change-Id: I733a4810b6507752b68f8508a486fbd577289658
Signed-off-by: Arthur Heymans arthur@aheymans.xyz
---
M src/arch/x86/Kconfig
M src/arch/x86/Makefile.inc
D src/arch/x86/cbmem.c
M src/include/cbmem.h
M src/northbridge/amd/agesa/Kconfig
M src/northbridge/amd/agesa/family12/state_machine.c
M src/northbridge/amd/agesa/family14/state_machine.c
M src/northbridge/amd/agesa/family15tn/state_machine.c
M src/northbridge/amd/agesa/family16kb/state_machine.c
M src/northbridge/amd/pi/Kconfig
M src/northbridge/amd/pi/Makefile.inc
M src/northbridge/amd/pi/agesawrapper.c
D src/northbridge/amd/pi/ramtop.c
M src/southbridge/amd/agesa/hudson/ramtop.c
M src/southbridge/amd/cimx/sb800/ramtop.c
M src/southbridge/amd/cimx/sb900/Makefile.inc
D src/southbridge/amd/cimx/sb900/ramtop.c
M src/southbridge/amd/sb700/ramtop.c
M src/southbridge/amd/sb800/ramtop.c
19 files changed, 8 insertions(+), 243 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/36290/1

diff --git a/src/arch/x86/Kconfig b/src/arch/x86/Kconfig
index 8ce5977..9181ff4 100644
--- a/src/arch/x86/Kconfig
+++ b/src/arch/x86/Kconfig
@@ -129,12 +129,6 @@
    int
    default 2
-config CBMEM_TOP_BACKUP
-	def_bool n
-	help
-	  Platform implements non-volatile storage to cache cbmem_top()
-	  over stage transitions and optionally also over S3 suspend.
-
 config PRERAM_CBMEM_CONSOLE_SIZE
    hex
    default 0xc00
diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc
index 612424d..40c50d2 100644
--- a/src/arch/x86/Makefile.inc
+++ b/src/arch/x86/Makefile.inc
@@ -223,7 +223,6 @@
 # gdt_init.S is included by entry32.inc when romstage is the first C
 # environment.
 romstage-$(CONFIG_C_ENVIRONMENT_BOOTBLOCK) += gdt_init.S
-romstage-y += cbmem.c
 romstage-y += cbfs_and_run.c
 romstage-y += cpu_common.c
 romstage-$(CONFIG_EARLY_EBDA_INIT) += ebda.c
@@ -264,7 +263,6 @@
 postcar-y += boot.c
 postcar-y += gdt_init.S
 postcar-y += cbfs_and_run.c
-postcar-y += cbmem.c
 postcar-y += cpu_common.c
 postcar-$(CONFIG_EARLY_EBDA_INIT) += ebda.c
 postcar-$(CONFIG_IDT_IN_EVERY_STAGE) += exception.c
@@ -308,7 +306,6 @@
 ramstage-$(CONFIG_ACPI_BERT) += acpi_bert_storage.c
 ramstage-y += boot.c
 ramstage-y += c_start.S
-ramstage-y += cbmem.c
 ramstage-y += cpu.c
 ramstage-y += cpu_common.c
 ramstage-y += ebda.c
diff --git a/src/arch/x86/cbmem.c b/src/arch/x86/cbmem.c
deleted file mode 100644
index 16a73c7..0000000
--- a/src/arch/x86/cbmem.c
+++ /dev/null
@@ -1,36 +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.
- *
- * 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 <stdlib.h>
-#include <cbmem.h>
-
-#if CONFIG(CBMEM_TOP_BACKUP)
-
-void *cbmem_top_romstage(void)
-{
-	static void *cbmem_top_backup;
-	void *top_backup;
-
-	if (ENV_RAMSTAGE && cbmem_top_backup != NULL)
-		return cbmem_top_backup;
-
-	/* Top of CBMEM is at highest usable DRAM address below 4GiB. */
-	top_backup = (void *)restore_top_of_low_cacheable();
-
-	if (ENV_RAMSTAGE)
-		cbmem_top_backup = top_backup;
-
-	return top_backup;
-}
-
-#endif /* CBMEM_TOP_BACKUP */
diff --git a/src/include/cbmem.h b/src/include/cbmem.h
index a2326df..2b069cf 100644
--- a/src/include/cbmem.h
+++ b/src/include/cbmem.h
@@ -150,17 +150,6 @@
    static cbmem_init_hook_t init_fn_ ## _unused3_ = init_fn_;
 #endif /* ENV_RAMSTAGE */
-
-/* Any new chipset and board must implement cbmem_top() for both
- * romstage and ramstage to support early features like COLLECT_TIMESTAMPS
- * and CBMEM_CONSOLE. Sometimes it is necessary to have cbmem_top()
- * value stored in nvram to enable early recovery on S3 path.
- */
-#if CONFIG(ARCH_X86)
-void backup_top_of_low_cacheable(uintptr_t ramtop);
-uintptr_t restore_top_of_low_cacheable(void);
-#endif
-
 /* If CONFIG_CBMEM_TOP_MEM is selected cbmem_top will return the value
    of the function argument. */
 void save_top_mem(uintptr_t ramtop);
diff --git a/src/northbridge/amd/agesa/Kconfig b/src/northbridge/amd/agesa/Kconfig
index e1e129a..6790d4e 100644
--- a/src/northbridge/amd/agesa/Kconfig
+++ b/src/northbridge/amd/agesa/Kconfig
@@ -16,7 +16,7 @@
 config NORTHBRIDGE_AMD_AGESA
    bool
    default CPU_AMD_AGESA
-	select CBMEM_TOP_BACKUP
+	select CBMEM_TOP_SAVE
if NORTHBRIDGE_AMD_AGESA
diff --git a/src/northbridge/amd/agesa/family12/state_machine.c b/src/northbridge/amd/agesa/family12/state_machine.c
index 077bf6a..3c448fc 100644
--- a/src/northbridge/amd/agesa/family12/state_machine.c
+++ b/src/northbridge/amd/agesa/family12/state_machine.c
@@ -36,7 +36,7 @@
void platform_AfterInitPost(struct sysinfo *cb, AMD_POST_PARAMS *Post)
 {
-	backup_top_of_low_cacheable(Post->MemConfig.Sub4GCacheTop);
+	save_top_mem(Post->MemConfig.Sub4GCacheTop);
sb_before_pci_init();
 }
diff --git a/src/northbridge/amd/agesa/family14/state_machine.c b/src/northbridge/amd/agesa/family14/state_machine.c
index df55efa..7a1a366 100644
--- a/src/northbridge/amd/agesa/family14/state_machine.c
+++ b/src/northbridge/amd/agesa/family14/state_machine.c
@@ -59,7 +59,7 @@
void platform_AfterInitPost(struct sysinfo *cb, AMD_POST_PARAMS *Post)
 {
-	backup_top_of_low_cacheable(Post->MemConfig.Sub4GCacheTop);
+	save_top_mem(Post->MemConfig.Sub4GCacheTop);
 }
void platform_BeforeInitResume(struct sysinfo *cb, AMD_RESUME_PARAMS *Resume)
diff --git a/src/northbridge/amd/agesa/family15tn/state_machine.c b/src/northbridge/amd/agesa/family15tn/state_machine.c
index 473edfc..bfff00b 100644
--- a/src/northbridge/amd/agesa/family15tn/state_machine.c
+++ b/src/northbridge/amd/agesa/family15tn/state_machine.c
@@ -34,7 +34,7 @@
void platform_AfterInitPost(struct sysinfo *cb, AMD_POST_PARAMS *Post)
 {
-	backup_top_of_low_cacheable(Post->MemConfig.Sub4GCacheTop);
+	save_top_mem(Post->MemConfig.Sub4GCacheTop);
 }
void platform_BeforeInitResume(struct sysinfo *cb, AMD_RESUME_PARAMS *Resume)
diff --git a/src/northbridge/amd/agesa/family16kb/state_machine.c b/src/northbridge/amd/agesa/family16kb/state_machine.c
index fea097f..c14c817 100644
--- a/src/northbridge/amd/agesa/family16kb/state_machine.c
+++ b/src/northbridge/amd/agesa/family16kb/state_machine.c
@@ -41,7 +41,7 @@
void platform_AfterInitPost(struct sysinfo *cb, AMD_POST_PARAMS *Post)
 {
-	backup_top_of_low_cacheable(Post->MemConfig.Sub4GCacheTop);
+	save_top_mem(Post->MemConfig.Sub4GCacheTop);
 }
void platform_BeforeInitResume(struct sysinfo *cb, AMD_RESUME_PARAMS *Resume)
diff --git a/src/northbridge/amd/pi/Kconfig b/src/northbridge/amd/pi/Kconfig
index 65c6556..010f775 100644
--- a/src/northbridge/amd/pi/Kconfig
+++ b/src/northbridge/amd/pi/Kconfig
@@ -17,7 +17,7 @@
    bool
    default y if CPU_AMD_PI
    default n
-	select CBMEM_TOP_BACKUP
+	select CBMEM_TOP_SAVE
if NORTHBRIDGE_AMD_PI
diff --git a/src/northbridge/amd/pi/Makefile.inc b/src/northbridge/amd/pi/Makefile.inc
index c2c8d88..1e98da1 100644
--- a/src/northbridge/amd/pi/Makefile.inc
+++ b/src/northbridge/amd/pi/Makefile.inc
@@ -24,7 +24,4 @@
 ramstage-y += agesawrapper.c
 endif
-romstage-y += ramtop.c
-postcar-y += ramtop.c
-ramstage-y += ramtop.c
 endif
diff --git a/src/northbridge/amd/pi/agesawrapper.c b/src/northbridge/amd/pi/agesawrapper.c
index e3bfd90..3a0156ee 100644
--- a/src/northbridge/amd/pi/agesawrapper.c
+++ b/src/northbridge/amd/pi/agesawrapper.c
@@ -139,9 +139,9 @@
     * UMA may or may not be cacheable, so Sub4GCacheTop could be
     * higher than UmaBase. With UMA_NONE we see UmaBase==0. */
    if (PostParams->MemConfig.UmaBase)
-		backup_top_of_low_cacheable(PostParams->MemConfig.UmaBase << 16);
+		save_top_mem(PostParams->MemConfig.UmaBase << 16);
    else
-		backup_top_of_low_cacheable(PostParams->MemConfig.Sub4GCacheTop);
+		save_top_mem(PostParams->MemConfig.Sub4GCacheTop);
printk(
    		BIOS_SPEW,
diff --git a/src/northbridge/amd/pi/ramtop.c b/src/northbridge/amd/pi/ramtop.c
deleted file mode 100644
index 823a15c..0000000
--- a/src/northbridge/amd/pi/ramtop.c
+++ /dev/null
@@ -1,33 +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.
- *
- * 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.
- */
-
-#define __SIMPLE_DEVICE__
-
-#include <stdint.h>
-#include <device/pci_ops.h>
-#include <cbmem.h>
-
-#define CBMEM_TOP_SCRATCHPAD 0x78
-
-void backup_top_of_low_cacheable(uintptr_t ramtop)
-{
-	uint16_t top_cache = ramtop >> 16;
-	pci_write_config16(PCI_DEV(0,0,0), CBMEM_TOP_SCRATCHPAD, top_cache);
-}
-
-uintptr_t restore_top_of_low_cacheable(void)
-{
-	uint16_t top_cache;
-	top_cache = pci_read_config16(PCI_DEV(0,0,0), CBMEM_TOP_SCRATCHPAD);
-	return (top_cache << 16);
-}
diff --git a/src/southbridge/amd/agesa/hudson/ramtop.c b/src/southbridge/amd/agesa/hudson/ramtop.c
index 22b291d..894711e 100644
--- a/src/southbridge/amd/agesa/hudson/ramtop.c
+++ b/src/southbridge/amd/agesa/hudson/ramtop.c
@@ -25,27 +25,3 @@
    tmp = ((tmp & (7 << 10)) >> 10);
    return (int)tmp;
 }
-
-void backup_top_of_low_cacheable(uintptr_t ramtop)
-{
-	u32 dword = ramtop;
-	int nvram_pos = 0xf8, i; /* temp */
-	for (i = 0; i < 4; i++) {
-		outb(nvram_pos, BIOSRAM_INDEX);
-		outb((dword >> (8 * i)) & 0xff, BIOSRAM_DATA);
-		nvram_pos++;
-	}
-}
-
-uintptr_t restore_top_of_low_cacheable(void)
-{
-	uint32_t xdata = 0;
-	int xnvram_pos = 0xf8, xi;
-	for (xi = 0; xi < 4; xi++) {
-		outb(xnvram_pos, BIOSRAM_INDEX);
-		xdata &= ~(0xff << (xi * 8));
-		xdata |= inb(BIOSRAM_DATA) << (xi *8);
-		xnvram_pos++;
-	}
-	return xdata;
-}
diff --git a/src/southbridge/amd/cimx/sb800/ramtop.c b/src/southbridge/amd/cimx/sb800/ramtop.c
index b9fc00d..98d12c7 100644
--- a/src/southbridge/amd/cimx/sb800/ramtop.c
+++ b/src/southbridge/amd/cimx/sb800/ramtop.c
@@ -25,27 +25,3 @@
    tmp = ((tmp & (7 << 10)) >> 10);
    return (int)tmp;
 }
-
-void backup_top_of_low_cacheable(uintptr_t ramtop)
-{
-	u32 dword = ramtop;
-	int nvram_pos = 0xf8, i;	/* temp */
-	for (i = 0; i < 4; i++) {
-		outb(nvram_pos, BIOSRAM_INDEX);
-		outb((dword >> (8 * i)) & 0xff, BIOSRAM_DATA);
-		nvram_pos++;
-	}
-}
-
-uintptr_t restore_top_of_low_cacheable(void)
-{
-	u32 xdata = 0;
-	int xnvram_pos = 0xf8, xi;
-	for (xi = 0; xi < 4; xi++) {
-		outb(xnvram_pos, BIOSRAM_INDEX);
-		xdata &= ~(0xff << (xi * 8));
-		xdata |= inb(BIOSRAM_DATA) << (xi * 8);
-		xnvram_pos++;
-	}
-	return xdata;
-}
diff --git a/src/southbridge/amd/cimx/sb900/Makefile.inc b/src/southbridge/amd/cimx/sb900/Makefile.inc
index ff9ada6..05ebaea 100644
--- a/src/southbridge/amd/cimx/sb900/Makefile.inc
+++ b/src/southbridge/amd/cimx/sb900/Makefile.inc
@@ -20,15 +20,11 @@
 romstage-y += early.c
 romstage-y += smbus.c smbus_spd.c
 romstage-y += reset.c
-romstage-y += ramtop.c
-
-postcar-y += ramtop.c
ramstage-y += cfg.c
 ramstage-y += early.c
 ramstage-y += late.c
 ramstage-y += reset.c
-ramstage-y += ramtop.c
ramstage-y   += smbus.c
 ramstage-y   += lpc.c
diff --git a/src/southbridge/amd/cimx/sb900/ramtop.c b/src/southbridge/amd/cimx/sb900/ramtop.c
deleted file mode 100644
index 26e930b..0000000
--- a/src/southbridge/amd/cimx/sb900/ramtop.c
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- * This file is part of the coreboot project.
- *
- * Copyright (C) 2010 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
- * 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 <stdint.h>
-#include <arch/io.h>
-#include <cbmem.h>
-#include <southbridge/amd/cimx/cimx_util.h>
-
-void backup_top_of_low_cacheable(uintptr_t ramtop)
-{
-	u32 dword = ramtop;
-	int nvram_pos = 0xf8, i; /* temp */
-	for (i = 0; i < 4; i++) {
-		outb(nvram_pos, BIOSRAM_INDEX);
-		outb((dword >> (8 * i)) & 0xff, BIOSRAM_DATA);
-		nvram_pos++;
-	}
-}
-
-uintptr_t restore_top_of_low_cacheable(void)
-{
-	u32 xdata = 0;
-	int xnvram_pos = 0xf8, xi;
-	for (xi = 0; xi < 4; xi++) {
-		outb(xnvram_pos, BIOSRAM_INDEX);
-		xdata &= ~(0xff << (xi * 8));
-		xdata |= inb(BIOSRAM_DATA) << (xi *8);
-		xnvram_pos++;
-	}
-	return xdata;
-}
diff --git a/src/southbridge/amd/sb700/ramtop.c b/src/southbridge/amd/sb700/ramtop.c
index 4d26121..063f63e 100644
--- a/src/southbridge/amd/sb700/ramtop.c
+++ b/src/southbridge/amd/sb700/ramtop.c
@@ -25,27 +25,3 @@
    tmp = inw(ACPI_PM1_CNT_BLK);
    return ((tmp & (7 << 10)) >> 10);
 }
-
-void backup_top_of_low_cacheable(uintptr_t ramtop)
-{
-	u32 dword = (u32) ramtop;
-	int nvram_pos = 0xfc, i;
-	for (i = 0; i < 4; i++) {
-		outb(nvram_pos, BIOSRAM_INDEX);
-		outb((dword >> (8 * i)) & 0xff, BIOSRAM_DATA);
-		nvram_pos++;
-	}
-}
-
-uintptr_t restore_top_of_low_cacheable(void)
-{
-	uint32_t xdata = 0;
-	int xnvram_pos = 0xfc, xi;
-	for (xi = 0; xi < 4; xi++) {
-		outb(xnvram_pos, BIOSRAM_INDEX);
-		xdata &= ~(0xff << (xi * 8));
-		xdata |= inb(BIOSRAM_DATA) << (xi *8);
-		xnvram_pos++;
-	}
-	return xdata;
-}
diff --git a/src/southbridge/amd/sb800/ramtop.c b/src/southbridge/amd/sb800/ramtop.c
index 889c699..b483f8d 100644
--- a/src/southbridge/amd/sb800/ramtop.c
+++ b/src/southbridge/amd/sb800/ramtop.c
@@ -25,27 +25,3 @@
    tmp = inw(ACPI_PM1_CNT_BLK);
    return ((tmp & (7 << 10)) >> 10);
 }
-
-void backup_top_of_low_cacheable(uintptr_t ramtop)
-{
-	u32 dword = ramtop;
-	int nvram_pos = 0xfc, i;
-	for (i = 0; i < 4; i++) {
-		outb(nvram_pos, BIOSRAM_INDEX);
-		outb((dword >> (8 * i)) & 0xff, BIOSRAM_DATA);
-		nvram_pos++;
-	}
-}
-
-uintptr_t restore_top_of_low_cacheable(void)
-{
-	uint32_t xdata = 0;
-	int xnvram_pos = 0xfc, xi;
-	for (xi = 0; xi < 4; xi++) {
-		outb(xnvram_pos, BIOSRAM_INDEX);
-		xdata &= ~(0xff << (xi * 8));
-		xdata |= inb(BIOSRAM_DATA) << (xi *8);
-		xnvram_pos++;
-	}
-	return xdata;
-}
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/36290
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I733a4810b6507752b68f8508a486fbd577289658
Gerrit-Change-Number: 36290
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz
Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz
Gerrit-Reviewer: Martin Roth martinroth@google.com
Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com
Gerrit-MessageType: newchange



    