Change in coreboot[master]: soc/amd: Use common functions to pass UMA information to ramstage
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36292 )
Change subject: soc/amd: Use common functions to pass UMA information to ramstage ......................................................................
soc/amd: Use common functions to pass UMA information to ramstage
This removes the custom implementation using the BIOSRAM scratchpad space.
Change-Id: I97a14044a93d1188cacc488006abe50a112d0eb8 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/amd/common/block/acpimmio/mmio_util.c M src/soc/amd/common/block/include/amdblocks/acpimmio.h M src/soc/amd/common/block/pi/Makefile.inc M src/soc/amd/common/block/pi/agesawrapper.c A src/soc/amd/common/block/pi/uma.c M src/soc/amd/picasso/northbridge.c M src/soc/amd/picasso/southbridge.c M src/soc/amd/stoneyridge/include/soc/southbridge.h M src/soc/amd/stoneyridge/northbridge.c M src/soc/amd/stoneyridge/southbridge.c 10 files changed, 65 insertions(+), 122 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/36292/1
diff --git a/src/soc/amd/common/block/acpimmio/mmio_util.c b/src/soc/amd/common/block/acpimmio/mmio_util.c index edb3882..03cea2e 100644 --- a/src/soc/amd/common/block/acpimmio/mmio_util.c +++ b/src/soc/amd/common/block/acpimmio/mmio_util.c @@ -139,45 +139,6 @@ /* pm2 read/write - access registers at 0xfed80400 - currently unused by any soc */ #endif
-#if SUPPORTS_ACPIMMIO_BIOSRAM_BASE -/* biosram read/write - access registers at 0xfed80500 */ - -uint8_t biosram_read8(uint8_t reg) -{ - return read8((void *)(ACPIMMIO_BIOSRAM_BASE + reg)); -} - -uint16_t biosram_read16(uint8_t reg) /* Must be 1 byte at a time */ -{ - return (biosram_read8(reg + sizeof(uint8_t)) << 8 | biosram_read8(reg)); -} - -uint32_t biosram_read32(uint8_t reg) -{ - uint32_t value = biosram_read16(reg + sizeof(uint16_t)) << 16; - return value | biosram_read16(reg); -} - -void biosram_write8(uint8_t reg, uint8_t value) -{ - write8((void *)(ACPIMMIO_BIOSRAM_BASE + reg), value); -} - -void biosram_write16(uint8_t reg, uint16_t value) -{ - biosram_write8(reg, value & 0xff); - value >>= 8; - biosram_write8(reg + sizeof(uint8_t), value & 0xff); -} - -void biosram_write32(uint8_t reg, uint32_t value) -{ - biosram_write16(reg, value & 0xffff); - value >>= 16; - biosram_write16(reg + sizeof(uint16_t), value & 0xffff); -} -#endif /* SUPPORTS_ACPIMMIO_BIOSRAM_BASE */ - #if SUPPORTS_ACPIMMIO_CMOSRAM_BASE /* cmosram read/write - access registers at 0xfed80600 - currently unused by any soc */ #endif diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio.h b/src/soc/amd/common/block/include/amdblocks/acpimmio.h index 59ab9e5..879b9b9 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpimmio.h +++ b/src/soc/amd/common/block/include/amdblocks/acpimmio.h @@ -194,14 +194,6 @@ void pm2_write16(uint8_t reg, uint16_t value); void pm2_write32(uint8_t reg, uint32_t value);
-/* Access BIOS RAM storage at 0xfed80500 */ -uint8_t biosram_read8(uint8_t reg); -uint16_t biosram_read16(uint8_t reg); -uint32_t biosram_read32(uint8_t reg); -void biosram_write8(uint8_t reg, uint8_t value); -void biosram_write16(uint8_t reg, uint16_t value); -void biosram_write32(uint8_t reg, uint32_t value); - /* Access ACPI registers at 0xfed80800 */ uint8_t acpi_read8(uint8_t reg); uint16_t acpi_read16(uint8_t reg); diff --git a/src/soc/amd/common/block/pi/Makefile.inc b/src/soc/amd/common/block/pi/Makefile.inc index 5afbb3e..41995f6 100644 --- a/src/soc/amd/common/block/pi/Makefile.inc +++ b/src/soc/amd/common/block/pi/Makefile.inc @@ -1,11 +1,13 @@ ifeq ($(CONFIG_SOC_AMD_COMMON_BLOCK_PI),y)
+romstage-y += uma.c romstage-y += agesawrapper.c romstage-y += def_callouts.c romstage-y += heapmanager.c romstage-y += image.c romstage-y += refcode_loader.c
+ramstage-y += uma.c ramstage-y += agesawrapper.c ramstage-y += amd_late_init.c ramstage-$(CONFIG_HAVE_ACPI_RESUME) += amd_resume_final.c diff --git a/src/soc/amd/common/block/pi/agesawrapper.c b/src/soc/amd/common/block/pi/agesawrapper.c index 022a8bc..77afeab 100644 --- a/src/soc/amd/common/block/pi/agesawrapper.c +++ b/src/soc/amd/common/block/pi/agesawrapper.c @@ -205,8 +205,8 @@ top = PostParams->MemConfig.Sub4GCacheTop; save_ram_top(top);
- save_uma_size(PostParams->MemConfig.UmaSize * 64 * KiB); - save_uma_base((u64)PostParams->MemConfig.UmaBase * 64 * KiB); + save_uma((u64)PostParams->MemConfig.UmaBase * 64 * KiB, + PostParams->MemConfig.UmaSize * 64 * KiB);
print_init_post_settings(PostParams);
diff --git a/src/soc/amd/common/block/pi/uma.c b/src/soc/amd/common/block/pi/uma.c new file mode 100644 index 0000000..5d46855 --- /dev/null +++ b/src/soc/amd/common/block/pi/uma.c @@ -0,0 +1,47 @@ +/* + * 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 <stdint.h> +#include <stdlib.h> +#include <romstage_handoff.h> +#include <soc/southbridge.h> +#include <console/console.h> + + +struct save_uma { + uint64_t base; + uint32_t size; +}; + +void save_uma(uint64_t uma_base, uint32_t uma_size) +{ + struct save_uma save = { + .base = uma_base, + .size = uma_size + }; + if (save_romstage_info(&save, sizeof(save))) { + printk(BIOS_DEBUG, "Saving uma information failed!\n"); + return; + } +} + +void get_uma(uint64_t *uma_base, uint32_t *uma_size) +{ + struct save_uma save; + if (get_romstage_info(&save, sizeof(save))) { + printk(BIOS_ERR, "Getting saved uma information failed!\n"); + return; + } + *uma_base = save.base; + *uma_size = save.size; +} diff --git a/src/soc/amd/picasso/northbridge.c b/src/soc/amd/picasso/northbridge.c index 36135f9..f52c622 100644 --- a/src/soc/amd/picasso/northbridge.c +++ b/src/soc/amd/picasso/northbridge.c @@ -261,14 +261,16 @@
void domain_set_resources(struct device *dev) { - uint64_t uma_base = get_uma_base(); - uint32_t uma_size = get_uma_size(); + uint64_t uma_base; + uint32_t uma_size; uint32_t mem_useable = (uintptr_t)cbmem_top(); msr_t tom = rdmsr(TOP_MEM); msr_t high_tom = rdmsr(TOP_MEM2); uint64_t high_mem_useable; int idx = 0x10;
+ get_uma(&uma_base, &uma_size); + /* 0x0 -> 0x9ffff */ ram_resource(dev, idx++, 0, 0xa0000 / KiB);
diff --git a/src/soc/amd/picasso/southbridge.c b/src/soc/amd/picasso/southbridge.c index 00e7b0f..1562dc7 100644 --- a/src/soc/amd/picasso/southbridge.c +++ b/src/soc/amd/picasso/southbridge.c @@ -576,27 +576,3 @@ * on entry into BS_DEV_ENABLE. */ BOOT_STATE_INIT_ENTRY(BS_DEV_ENABLE, BS_ON_ENTRY, set_pci_irqs, NULL); - -void save_uma_size(uint32_t size) -{ - biosram_write32(BIOSRAM_UMA_SIZE, size); -} - -void save_uma_base(uint64_t base) -{ - biosram_write32(BIOSRAM_UMA_BASE, (uint32_t) base); - biosram_write32(BIOSRAM_UMA_BASE + 4, (uint32_t) (base >> 32)); -} - -uint32_t get_uma_size(void) -{ - return biosram_read32(BIOSRAM_UMA_SIZE); -} - -uint64_t get_uma_base(void) -{ - uint64_t base; - base = biosram_read32(BIOSRAM_UMA_BASE); - base |= ((uint64_t)(biosram_read32(BIOSRAM_UMA_BASE + 4)) << 32); - return base; -} diff --git a/src/soc/amd/stoneyridge/include/soc/southbridge.h b/src/soc/amd/stoneyridge/include/soc/southbridge.h index dd514ab..4504621 100644 --- a/src/soc/amd/stoneyridge/include/soc/southbridge.h +++ b/src/soc/amd/stoneyridge/include/soc/southbridge.h @@ -353,35 +353,19 @@ /** * @brief Save the UMA bize returned by AGESA * - * @param size = in bytes + * @param base, size = in bytes * * @return none */ -void save_uma_size(uint32_t size); +void save_uma(uint64_t uma_base, uint32_t uma_size); /** - * @brief Save the UMA base address returned by AGESA + * @brief Get the UMA bize returned by AGESA * - * @param base = 64bit base address + * @param Pointer base, size = in bytes * * @return none */ -void save_uma_base(uint64_t base); -/** - * @brief Get the saved UMA size - * - * @param none - * - * @return size in bytes - */ -uint32_t get_uma_size(void); -/** - * @brief Get the saved UMA base - * - * @param none - * - * @return 64bit base address - */ -uint64_t get_uma_base(void); +void get_uma(uint64_t *uma_base, uint32_t *uma_size); /* * Call the mainboard to get the USB Over Current Map. The mainboard * returns the map and 0 on Success or -1 on error or no map. There is diff --git a/src/soc/amd/stoneyridge/northbridge.c b/src/soc/amd/stoneyridge/northbridge.c index 044a1b0..c6f7058 100644 --- a/src/soc/amd/stoneyridge/northbridge.c +++ b/src/soc/amd/stoneyridge/northbridge.c @@ -413,14 +413,16 @@
void domain_set_resources(struct device *dev) { - uint64_t uma_base = get_uma_base(); - uint32_t uma_size = get_uma_size(); + uint64_t uma_base; + uint32_t uma_size; uint32_t mem_useable = (uintptr_t)cbmem_top(); msr_t tom = rdmsr(TOP_MEM); msr_t high_tom = rdmsr(TOP_MEM2); uint64_t high_mem_useable; int idx = 0x10;
+ get_uma(&uma_base, &uma_size); + /* 0x0 -> 0x9ffff */ ram_resource(dev, idx++, 0, 0xa0000 / KiB);
diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c index d7a09aa..96e587d 100644 --- a/src/soc/amd/stoneyridge/southbridge.c +++ b/src/soc/amd/stoneyridge/southbridge.c @@ -36,6 +36,7 @@ #include <agesa_headers.h> #include <soc/nvs.h> #include <types.h> +#include <romstage_handoff.h>
/* * Table of devices that need their AOAC registers enabled and waited @@ -648,27 +649,3 @@ * on entry into BS_DEV_ENABLE. */ BOOT_STATE_INIT_ENTRY(BS_DEV_ENABLE, BS_ON_ENTRY, set_pci_irqs, NULL); - -void save_uma_size(uint32_t size) -{ - biosram_write32(BIOSRAM_UMA_SIZE, size); -} - -void save_uma_base(uint64_t base) -{ - biosram_write32(BIOSRAM_UMA_BASE, (uint32_t) base); - biosram_write32(BIOSRAM_UMA_BASE + 4, (uint32_t) (base >> 32)); -} - -uint32_t get_uma_size(void) -{ - return biosram_read32(BIOSRAM_UMA_SIZE); -} - -uint64_t get_uma_base(void) -{ - uint64_t base; - base = biosram_read32(BIOSRAM_UMA_BASE); - base |= ((uint64_t)(biosram_read32(BIOSRAM_UMA_BASE + 4)) << 32); - return base; -}
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36292 )
Change subject: soc/amd: Use common functions to pass UMA information to ramstage ......................................................................
Patch Set 2: Code-Review-1
I am anticipating this is -2 like related cbmem_top() changes on AGESAv5.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36292 )
Change subject: soc/amd: Use common functions to pass UMA information to ramstage ......................................................................
Patch Set 2:
Patch Set 2: Code-Review-1
I am anticipating this is -2 like related cbmem_top() changes on AGESAv5.
Hmm if uma_base/size is needed before cbmem init this won't work. otherwise it will. Even if the latter is the case it deserves more comments.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36292 )
Change subject: soc/amd: Use common functions to pass UMA information to ramstage ......................................................................
Patch Set 2:
(1 comment)
Still -1 as biosram is required for cbmem_top(), UMA base/size change is probably fine.
https://review.coreboot.org/c/coreboot/+/36292/2/src/soc/amd/common/block/pi... File src/soc/amd/common/block/pi/agesawrapper.c:
https://review.coreboot.org/c/coreboot/+/36292/2/src/soc/amd/common/block/pi... PS2, Line 209: PostParams->MemConfig.UmaSize * 64 * KiB); Ah... I did not review preceding work and did not realise this does not depend of cbmem_initialize() having being called.
Hello Kyösti Mälkki, Marshall Dawson, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36292
to look at the new patch set (#6).
Change subject: soc/amd: Use common functions to pass UMA information to ramstage ......................................................................
soc/amd: Use common functions to pass UMA information to ramstage
Change-Id: I97a14044a93d1188cacc488006abe50a112d0eb8 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/amd/common/block/pi/Kconfig M src/soc/amd/common/block/pi/Makefile.inc M src/soc/amd/common/block/pi/agesawrapper.c A src/soc/amd/common/block/pi/uma.c M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/picasso/northbridge.c M src/soc/amd/picasso/southbridge.c M src/soc/amd/stoneyridge/include/soc/iomap.h M src/soc/amd/stoneyridge/include/soc/southbridge.h M src/soc/amd/stoneyridge/northbridge.c M src/soc/amd/stoneyridge/southbridge.c 11 files changed, 69 insertions(+), 79 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/36292/6
Hello Kyösti Mälkki, Marshall Dawson, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36292
to look at the new patch set (#7).
Change subject: soc/amd: Use common functions to pass UMA information to ramstage ......................................................................
soc/amd: Use common functions to pass UMA information to ramstage
Change-Id: I97a14044a93d1188cacc488006abe50a112d0eb8 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/amd/common/block/pi/Kconfig M src/soc/amd/common/block/pi/Makefile.inc M src/soc/amd/common/block/pi/agesawrapper.c A src/soc/amd/common/block/pi/uma.c M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/picasso/northbridge.c M src/soc/amd/picasso/southbridge.c M src/soc/amd/stoneyridge/include/soc/iomap.h M src/soc/amd/stoneyridge/include/soc/southbridge.h M src/soc/amd/stoneyridge/northbridge.c M src/soc/amd/stoneyridge/southbridge.c 11 files changed, 69 insertions(+), 79 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/36292/7
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36292 )
Change subject: soc/amd: Use common functions to pass UMA information to ramstage ......................................................................
Patch Set 13:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36292/13/src/soc/amd/common/block/p... File src/soc/amd/common/block/pi/agesawrapper.c:
https://review.coreboot.org/c/coreboot/+/36292/13/src/soc/amd/common/block/p... PS13, Line 171: static AGESA_STATUS amd_init_post(AMD_POST_PARAMS *PostParams) You may notice that drivers/amd/agesa/state_machine.c passes 'struct sysinfo *cb' here with the intention to pass necessary fields from AMD_POST_PARAMS to top-level romstage_main().
https://review.coreboot.org/c/coreboot/+/36292/13/src/soc/amd/common/block/p... PS13, Line 209: PostParams->MemConfig.UmaSize * 64 * KiB); You could just stash base and size to .bss here, and delay save_uma() call to be done after cbmem_recovery() on normal boot path. But if you have other cases in mind where you need to save romstage information before CBMEM is ready, this is fine I guess.
It might get a bit messy if you need to call save_romstage_info() multiple times.
Hello Kyösti Mälkki, Marshall Dawson, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36292
to look at the new patch set (#32).
Change subject: soc/amd: Use common functions to pass UMA information to ramstage ......................................................................
soc/amd: Use common functions to pass UMA information to ramstage
Change-Id: I97a14044a93d1188cacc488006abe50a112d0eb8 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/amd/common/block/pi/Kconfig M src/soc/amd/common/block/pi/Makefile.inc M src/soc/amd/common/block/pi/agesawrapper.c A src/soc/amd/common/block/pi/uma.c M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/picasso/northbridge.c M src/soc/amd/picasso/southbridge.c M src/soc/amd/stoneyridge/include/soc/iomap.h M src/soc/amd/stoneyridge/include/soc/southbridge.h M src/soc/amd/stoneyridge/northbridge.c M src/soc/amd/stoneyridge/southbridge.c 11 files changed, 69 insertions(+), 79 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/36292/32
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36292 )
Change subject: soc/amd: Use common functions to pass UMA information to ramstage ......................................................................
Patch Set 32:
Arthur you may reuse something from CB:37402 or even base on it. The patch has already some cleanup made.
Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/36292 )
Change subject: soc/amd: Use common functions to pass UMA information to ramstage ......................................................................
Abandoned
-
Arthur Heymans (Code Review) -
Kyösti Mälkki (Code Review)
-
Michał Żygowski (Code Review)