Change in coreboot[master]: soc/amd/common/biosram: Add functions to get/set bootram used

[Marshall Dawson (Code Review)]

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

Change subject: soc/amd/common/biosram: Add functions to get/set bootram used
......................................................................

soc/amd/common/biosram: Add functions to get/set bootram used

Designs that use RESET_VECTOR_IN_RAM (typ. AMD Family 17h and later)
do not run as XIP from the boot flash device.  Stage and other images
are loaded to DRAM to improve performance.  These regions that are
consumed must be reserved, otherwise resuming from S3 will corrupt
the OS's memory.

Define offsets where the base and size of early stages can be saved
and add get/set functions to access them.

Signed-off-by: Marshall Dawson 
Change-Id: I14f5be795803b4cda1a4f0fcb2ee151adb9980a9
---
M src/soc/amd/common/block/acpimmio/biosram.c
M src/soc/amd/common/block/include/amdblocks/biosram.h
2 files changed, 83 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/38701/1

diff --git a/src/soc/amd/common/block/acpimmio/biosram.c b/src/soc/amd/common/block/acpimmio/biosram.c
index 814fdf3..8450759 100644
--- a/src/soc/amd/common/block/acpimmio/biosram.c
+++ b/src/soc/amd/common/block/acpimmio/biosram.c
@@ -18,6 +18,14 @@
 #include <stdint.h>

/* BiosRam Ranges at 0xfed80500 or I/O 0xcd4/0xcd5 */
+#define BIOSRAM_RAM_BOOTBLOCK_SIZE	0xb8 /* 4 bytes */
+#define BIOSRAM_RAM_VERSTAGE_SIZE	0xbc /* 4 bytes */
+#define BIOSRAM_RAM_ROMSTAGE_SIZE	0xc0 /* 4 bytes */
+#define BIOSRAM_RAM_FSPM_SIZE		0xc4 /* 4 bytes */
+#define BIOSRAM_RAM_BOOTBLOCK_BASE	0xc8 /* 8 bytes */
+#define BIOSRAM_RAM_VERSTAGE_BASE	0xd0 /* 8 bytes */
+#define BIOSRAM_RAM_ROMSTAGE_BASE	0xd8 /* 8 bytes */
+#define BIOSRAM_RAM_FSPM_BASE		0xe0 /* 8 bytes */
 #define BIOSRAM_AP_ENTRY		0xe8 /* 8 bytes */
 #define BIOSRAM_CBMEM_TOP		0xf0 /* 4 bytes */
 #define BIOSRAM_UMA_SIZE		0xf4 /* 4 bytes */
@@ -105,3 +113,59 @@
    base |= ((uint64_t)(biosram_read32(BIOSRAM_UMA_BASE + 4)) << 32);
    return base;
 }
+
+void set_bootram_bootblock(uint64_t base, uint32_t size)
+{
+	biosram_write32(BIOSRAM_RAM_BOOTBLOCK_BASE, (uint32_t)base);
+	biosram_write32(BIOSRAM_RAM_BOOTBLOCK_BASE + 4, (uint32_t)(base >> 32));
+	biosram_write32(BIOSRAM_RAM_BOOTBLOCK_SIZE, size);
+}
+
+void get_bootram_bootblock(uint64_t *base, uint32_t *size)
+{
+	*base = biosram_read32(BIOSRAM_RAM_BOOTBLOCK_BASE);
+	*base |= ((uint64_t)(biosram_read32(BIOSRAM_RAM_BOOTBLOCK_BASE + 4)) << 32);
+	*size = biosram_read32(BIOSRAM_RAM_BOOTBLOCK_SIZE);
+}
+
+void set_bootram_verstage(uint64_t base, uint32_t size)
+{
+	biosram_write32(BIOSRAM_RAM_VERSTAGE_BASE, (uint32_t)base);
+	biosram_write32(BIOSRAM_RAM_VERSTAGE_BASE + 4, (uint32_t)(base >> 32));
+	biosram_write32(BIOSRAM_RAM_VERSTAGE_SIZE, size);
+}
+
+void get_bootram_verstage(uint64_t *base, uint32_t *size)
+{
+	*base = biosram_read32(BIOSRAM_RAM_VERSTAGE_BASE);
+	*base |= ((uint64_t)(biosram_read32(BIOSRAM_RAM_VERSTAGE_BASE + 4)) << 32);
+	*size = biosram_read32(BIOSRAM_RAM_VERSTAGE_SIZE);
+}
+
+void set_bootram_romstage(uint64_t base, uint32_t size)
+{
+	biosram_write32(BIOSRAM_RAM_ROMSTAGE_BASE, (uint32_t)base);
+	biosram_write32(BIOSRAM_RAM_ROMSTAGE_BASE + 4, (uint32_t)(base >> 32));
+	biosram_write32(BIOSRAM_RAM_ROMSTAGE_SIZE, size);
+}
+
+void get_bootram_romstage(uint64_t *base, uint32_t *size)
+{
+	*base = biosram_read32(BIOSRAM_RAM_ROMSTAGE_BASE);
+	*base |= ((uint64_t)(biosram_read32(BIOSRAM_RAM_ROMSTAGE_BASE + 4)) << 32);
+	*size = biosram_read32(BIOSRAM_RAM_ROMSTAGE_SIZE);
+}
+
+void set_bootram_fspm(uint64_t base, uint32_t size)
+{
+	biosram_write32(BIOSRAM_RAM_FSPM_BASE, (uint32_t)base);
+	biosram_write32(BIOSRAM_RAM_FSPM_BASE + 4, (uint32_t)(base >> 32));
+	biosram_write32(BIOSRAM_RAM_FSPM_SIZE, size);
+}
+
+void get_bootram_fspm(uint64_t *base, uint32_t *size)
+{
+	*base = biosram_read32(BIOSRAM_RAM_FSPM_BASE);
+	*base |= ((uint64_t)(biosram_read32(BIOSRAM_RAM_FSPM_BASE + 4)) << 32);
+	*size = biosram_read32(BIOSRAM_RAM_FSPM_SIZE);
+}
diff --git a/src/soc/amd/common/block/include/amdblocks/biosram.h b/src/soc/amd/common/block/include/amdblocks/biosram.h
index 4bfd629..f3d95df 100644
--- a/src/soc/amd/common/block/include/amdblocks/biosram.h
+++ b/src/soc/amd/common/block/include/amdblocks/biosram.h
@@ -29,4 +29,23 @@
 /* Returns the saved UMA base */
 uint64_t get_uma_base(void);

+/* set/get_bootram typically only used for RESET_VECTOR_IN_RAM */
+
+/* Saves RAM consumed by bootblock */
+void set_bootram_bootblock(uint64_t base, uint32_t size);
+/* Gets RAM consumed by bootblock */
+void get_bootram_bootblock(uint64_t *base, uint32_t *size);
+/* Saves RAM consumed by verstage */
+void set_bootram_verstage(uint64_t base, uint32_t size);
+/* Gets RAM consumed by verstage */
+void get_bootram_verstage(uint64_t *base, uint32_t *size);
+/* Saves RAM consumed by romstage */
+void set_bootram_romstage(uint64_t base, uint32_t size);
+/* Gets RAM consumed by romstage */
+void get_bootram_romstage(uint64_t *base, uint32_t *size);
+/* Saves RAM consumed by FSP-M */
+void set_bootram_fspm(uint64_t base, uint32_t size);
+/* Gets RAM consumed by FSP-M */
+void get_bootram_fspm(uint64_t *base, uint32_t *size);
+
 #endif

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38701 )

Change subject: soc/amd/common/biosram: Add functions to get/set bootram used
......................................................................

Patch Set 4:

What is the motivation to consume non-volatile 8-bit accessible biosram registers here instead of DRAM and migrate these to CBMEM? You need persistent .earlyram.data anyways implemented across early stages.

[Marshall Dawson (Code Review)]

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

Change subject: soc/amd/common/biosram: Add functions to get/set bootram used
......................................................................

Patch Set 4:
> Patch Set 4:
>
> What is the motivation to consume non-volatile 8-bit accessible biosram registers here instead of DRAM and migrate these to CBMEM? You need persistent .earlyram.data anyways implemented across early stages.

Adding a small section in .earlyram.data seems acceptable.  We'll need to copy that to cbmem in romstage, then use the info stored in cbmem during ramstage.

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38701 )

Change subject: soc/amd/common/biosram: Add functions to get/set bootram used
......................................................................

Patch Set 4:
> Patch Set 4:
> > Patch Set 4:
> >
> > What is the motivation to consume non-volatile 8-bit accessible biosram registers here instead of DRAM and migrate these to CBMEM? You need persistent .earlyram.data anyways implemented across early stages.
>
> Adding a small section in .earlyram.data seems acceptable.  We'll need to copy that to cbmem in romstage, then use the info stored in cbmem during ramstage.

Yes. That's the design used for pretty much everything else that has to migrate from bootblock to ramstage.

That biosram is comparable to CMOS nvram and/or PCI config scratchpad. You should avoid use of it unless absolutely required, and from what I remember we find arguments during amd/stoneyridge for doing this with CBMEM top location and possibly UMA allocation size.

I can't say how it will look like with picasso, but previously it was necessary to lock the layout of .car.data with the first shipped bootblock build. This has to do with having read-only bootblock and verstage builds as root-of-trust. Just saying, moving things to .earlyram.data later on may get tricky.

About 25% of biosram is already consumed? Previous AGESA had some of it's own (undocumented) reservations in that memory too. I think that's why coreboot started to fill that region from the end instead?

[Marshall Dawson (Code Review)]

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

Change subject: soc/amd/common/biosram: Add functions to get/set bootram used
......................................................................

Abandoned

No longer necessary CB:42264