Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85634?usp=email )
Change subject: drivers/amd/opensil/memmap.c: Factor out common memmap code to driver ......................................................................
drivers/amd/opensil/memmap.c: Factor out common memmap code to driver
Refactor the vendorcode openSIL memory map code and move all common calls that do not require any openSIL headers to the driver. Improve the legibility of the logic to return memory hole type string.
Change-Id: I80b9bdd7fd633c7b12d695ced5d4b9b518570d80 Signed-off-by: Nicolas Kochlowski nickkochlowski@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/85634 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M src/drivers/amd/opensil/Makefile.mk A src/drivers/amd/opensil/memmap.c M src/drivers/amd/opensil/opensil.h M src/soc/amd/genoa_poc/domain.c M src/soc/amd/phoenix/memmap.c M src/vendorcode/amd/opensil/genoa_poc/memmap.c M src/vendorcode/amd/opensil/opensil.h M src/vendorcode/amd/opensil/stub/ramstage.c 8 files changed, 124 insertions(+), 90 deletions(-)
Approvals: Felix Held: Looks good to me, approved build bot (Jenkins): Verified
diff --git a/src/drivers/amd/opensil/Makefile.mk b/src/drivers/amd/opensil/Makefile.mk index 9837718..6ed46d1 100644 --- a/src/drivers/amd/opensil/Makefile.mk +++ b/src/drivers/amd/opensil/Makefile.mk @@ -8,5 +8,6 @@
ramstage-y += acpi.c ramstage-y += ramstage.c +ramstage-y += memmap.c
endif diff --git a/src/drivers/amd/opensil/memmap.c b/src/drivers/amd/opensil/memmap.c new file mode 100644 index 0000000..9b80a88 --- /dev/null +++ b/src/drivers/amd/opensil/memmap.c @@ -0,0 +1,89 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <bootstate.h> +#include <cbmem.h> +#include <cpu/amd/mtrr.h> +#include <string.h> +#include <vendorcode/amd/opensil/opensil.h> + +#include "opensil.h" + +/* + * This structure definition must align exactly with the MEMORY_HOLE_TYPES structure + * defined in openSIL to ensure accurate casting. + */ +typedef struct { + uint64_t base; + uint64_t size; + uint32_t type; + uint32_t reserved; +} HOLE_INFO; + +/* This assumes holes are allocated */ +void amd_opensil_add_memmap(struct device *dev, unsigned long *idx) +{ + uint64_t top_of_mem = 0; + uint32_t n_holes = 0; + void *hole_info = NULL; + + /* Account for UMA and TSEG */ + const uint32_t mem_usable = cbmem_top(); + const uint32_t top_mem = ALIGN_DOWN(get_top_of_mem_below_4gb(), 1 * MiB); + if (mem_usable != top_mem) + reserved_ram_from_to(dev, (*idx)++, mem_usable, top_mem); + + /* Holes in upper DRAM */ + /* This assumes all the holes in upper DRAM are continuous */ + opensil_get_hole_info(&n_holes, &top_of_mem, &hole_info); + if (hole_info == NULL) + return; + + /* Check if we're done */ + if (top_of_mem <= 4ULL * GiB) + return; + + HOLE_INFO *holes = (HOLE_INFO *)hole_info; + + uint64_t lowest_upper_hole_base = top_of_mem; + uint64_t highest_upper_hole_end = 4ULL * GiB; + for (size_t hole = 0; hole < n_holes; hole++) { + if (!strcmp(opensil_get_hole_info_type(holes[hole].type), "MMIO")) + continue; + if (holes[hole].base < 4ULL * GiB) + continue; + lowest_upper_hole_base = MIN(lowest_upper_hole_base, holes[hole].base); + highest_upper_hole_end = MAX(highest_upper_hole_end, holes[hole].base + holes[hole].size); + if (!strcmp(opensil_get_hole_info_type(holes[hole].type), "UMA")) + mmio_range(dev, (*idx)++, holes[hole].base, holes[hole].size); + else + reserved_ram_range(dev, (*idx)++, holes[hole].base, holes[hole].size); + } + + ram_from_to(dev, (*idx)++, 4ULL * GiB, lowest_upper_hole_base); + + if (top_of_mem > highest_upper_hole_end) + ram_from_to(dev, (*idx)++, highest_upper_hole_end, top_of_mem); +} + +static void print_memory_holes(void *unused) +{ + uint64_t top_of_mem = 0; + uint32_t n_holes = 0; + void *hole_info = NULL; + + opensil_get_hole_info(&n_holes, &top_of_mem, &hole_info); + if (hole_info == NULL) + return; + + HOLE_INFO *holes = (HOLE_INFO *)hole_info; + + printk(BIOS_DEBUG, "APOB: top of memory 0x%016llx\n", top_of_mem); + printk(BIOS_DEBUG, "The following holes are reported in APOB\n"); + for (size_t hole = 0; hole < n_holes; hole++) { + printk(BIOS_DEBUG, " Base: 0x%016llx, Size: 0x%016llx, Type: %02d:%s\n", + holes[hole].base, holes[hole].size, holes[hole].type, + opensil_get_hole_info_type(holes[hole].type)); + } +} + +BOOT_STATE_INIT_ENTRY(BS_DEV_RESOURCES, BS_ON_ENTRY, print_memory_holes, NULL); diff --git a/src/drivers/amd/opensil/opensil.h b/src/drivers/amd/opensil/opensil.h index f2bcd6b..e6efbdc 100644 --- a/src/drivers/amd/opensil/opensil.h +++ b/src/drivers/amd/opensil/opensil.h @@ -4,11 +4,14 @@ #define OPENSIL_DRIVER_H
#include <acpi/acpi.h> +#include <device/device.h>
/* Set up openSIL env and call TP1 */ void amd_opensil_silicon_init(void); /* Set global and per-device MPIO configurations */ void configure_mpio(void); +/* Add the memory map to dev, starting at index idx, returns last use idx */ +void amd_opensil_add_memmap(struct device *dev, unsigned long *idx); /* Fill in FADT from openSIL */ void amd_opensil_fill_fadt_io_ports(acpi_fadt_t *fadt);
diff --git a/src/soc/amd/genoa_poc/domain.c b/src/soc/amd/genoa_poc/domain.c index 65b1395..d45d317 100644 --- a/src/soc/amd/genoa_poc/domain.c +++ b/src/soc/amd/genoa_poc/domain.c @@ -9,17 +9,16 @@ #include <arch/ioapic.h> #include <console/console.h> #include <device/device.h> +#include <drivers/amd/opensil/opensil.h> #include <types.h>
-#include <vendorcode/amd/opensil/opensil.h> - #define IOHC_IOAPIC_BASE_ADDR_LO 0x2f0
void read_soc_memmap_resources(struct device *domain, unsigned long *idx) { read_lower_soc_memmap_resources(domain, idx);
- add_opensil_memmap(domain, idx); + amd_opensil_add_memmap(domain, idx); }
static void genoa_domain_set_resources(struct device *domain) diff --git a/src/soc/amd/phoenix/memmap.c b/src/soc/amd/phoenix/memmap.c index d29dcbe..1b45b9c 100644 --- a/src/soc/amd/phoenix/memmap.c +++ b/src/soc/amd/phoenix/memmap.c @@ -4,8 +4,8 @@ #include <amdblocks/memmap.h> #include <amdblocks/root_complex.h> #include <device/device.h> +#include <drivers/amd/opensil/opensil.h> #include <stdint.h> -#include <vendorcode/amd/opensil/opensil.h>
/* * +--------------------------------+ @@ -69,5 +69,5 @@ if (CONFIG(PLATFORM_USES_FSP2_0)) read_fsp_resources(dev, idx); else - add_opensil_memmap(dev, idx); + amd_opensil_add_memmap(dev, idx); } diff --git a/src/vendorcode/amd/opensil/genoa_poc/memmap.c b/src/vendorcode/amd/opensil/genoa_poc/memmap.c index ece9877..eed1be7 100644 --- a/src/vendorcode/amd/opensil/genoa_poc/memmap.c +++ b/src/vendorcode/amd/opensil/genoa_poc/memmap.c @@ -1,20 +1,17 @@ /* SPDX-License-Identifier: GPL-2.0-only */
-#include <bootstate.h> -#include <stdint.h> -#include <stdbool.h> #include <SilCommon.h> #include <Sil-api.h> // needed above ApobCmn.h #include <ApobCmn.h> -#include <device/device.h> #include <xPRF-api.h> -#include <cpu/amd/mtrr.h> -#include <cbmem.h> -#include <amdblocks/memmap.h>
#include "../opensil.h"
-static const char *hole_info_type(MEMORY_HOLE_TYPES type) +_Static_assert(sizeof(uint32_t) == sizeof(((MEMORY_HOLE_DESCRIPTOR){0}).Type), + "Unexpected size of MEMORY_HOLE_TYPES in the MEMORY_HOLE_DESCRIPTOR " + "struct which doesn't match the code in drivers/amd/opensil/memmap.c"); + +const char *opensil_get_hole_info_type(uint32_t type) { const struct hole_type { MEMORY_HOLE_TYPES type; @@ -40,85 +37,21 @@ };
int i; + MEMORY_HOLE_TYPES enum_type = (MEMORY_HOLE_TYPES)type; // Cast int to enum for (i = 0; i < ARRAY_SIZE(types); i++) - if (type == types[i].type) - break; - if (i == ARRAY_SIZE(types)) - return "Unknown type"; - return types[i].string; + if (enum_type == types[i].type) + return types[i].string; + return "Unknown type"; }
-static uint64_t top_of_mem; -static uint32_t n_holes; -static MEMORY_HOLE_DESCRIPTOR *hole_info; - -static void get_hole_info(void) +void opensil_get_hole_info(uint32_t *n_holes, uint64_t *top_of_mem, void **hole_info) { - static bool done; - if (done) - return; - SIL_STATUS status = xPrfGetSystemMemoryMap(&n_holes, &top_of_mem, (void **)&hole_info); + SIL_STATUS status = xPrfGetSystemMemoryMap(n_holes, top_of_mem, hole_info); SIL_STATUS_report("xPrfGetSystemMemoryMap", status); // Make sure hole_info does not get initialized to something odd by xPRF on failure - if (status != SilPass) - hole_info = NULL; - done = true; -} - - -static void print_memory_holes(void *unused) -{ - get_hole_info(); - if (hole_info == NULL) - return; - - printk(BIOS_DEBUG, "APOB: top of memory 0x%016llx\n", top_of_mem); - printk(BIOS_DEBUG, "The following holes are reported in APOB\n"); - for (int hole = 0; hole < n_holes; hole++) { - printk(BIOS_DEBUG, " Base: 0x%016llx, Size: 0x%016llx, Type: %02d:%s\n", - hole_info[hole].Base, hole_info[hole].Size, hole_info[hole].Type, - hole_info_type(hole_info[hole].Type)); + if (status != SilPass) { + *hole_info = NULL; + *n_holes = 0; + *top_of_mem = 0; } } - -BOOT_STATE_INIT_ENTRY(BS_DEV_RESOURCES, BS_ON_ENTRY, print_memory_holes, NULL); - -// This assumes holes are allocated -void add_opensil_memmap(struct device *dev, unsigned long *idx) -{ - // Account for UMA and TSEG - const uint32_t mem_usable = cbmem_top(); - const uint32_t top_mem = ALIGN_DOWN(get_top_of_mem_below_4gb(), 1 * MiB); - if (mem_usable != top_mem) - reserved_ram_from_to(dev, (*idx)++, mem_usable, top_mem); - - // Check if we're done - if (top_of_mem <= 4ULL * GiB) - return; - - // Holes in upper DRAM - // This assumes all the holes in upper DRAM are continuous - get_hole_info(); - if (hole_info == NULL) - return; - uint64_t lowest_upper_hole_base = top_of_mem; - uint64_t highest_upper_hole_end = 4ULL * GiB; - for (int hole = 0; hole < n_holes; hole++) { - if (hole_info[hole].Type == MMIO) - continue; - if (hole_info[hole].Base < 4ULL * GiB) - continue; - lowest_upper_hole_base = MIN(lowest_upper_hole_base, hole_info[hole].Base); - highest_upper_hole_end = MAX(highest_upper_hole_end, hole_info[hole].Base + hole_info[hole].Size); - if (hole_info[hole].Type == UMA) - mmio_range(dev, (*idx)++, hole_info[hole].Base, hole_info[hole].Size); - else - reserved_ram_range(dev, (*idx)++, hole_info[hole].Base, hole_info[hole].Size); - } - - ram_from_to(dev, (*idx)++, 4ULL * GiB, lowest_upper_hole_base); - - // Do we need this? - if (top_of_mem > highest_upper_hole_end) - ram_from_to(dev, (*idx)++, highest_upper_hole_end, top_of_mem); -} diff --git a/src/vendorcode/amd/opensil/opensil.h b/src/vendorcode/amd/opensil/opensil.h index c84a180..3ed8960 100644 --- a/src/vendorcode/amd/opensil/opensil.h +++ b/src/vendorcode/amd/opensil/opensil.h @@ -8,8 +8,9 @@ #include <types.h>
void SIL_STATUS_report(const char *function, const int status); -// Add the memory map to dev, starting at index idx, returns last use idx -void add_opensil_memmap(struct device *dev, unsigned long *idx); + +void opensil_get_hole_info(uint32_t *n_holes, uint64_t *top_of_mem, void **hole_info); +const char *opensil_get_hole_info_type(uint32_t type);
void opensil_fill_fadt(acpi_fadt_t *fadt); unsigned long add_opensil_acpi_table(unsigned long current, acpi_rsdp_t *rsdp); diff --git a/src/vendorcode/amd/opensil/stub/ramstage.c b/src/vendorcode/amd/opensil/stub/ramstage.c index 5da6134..d2e3d6a 100644 --- a/src/vendorcode/amd/opensil/stub/ramstage.c +++ b/src/vendorcode/amd/opensil/stub/ramstage.c @@ -5,9 +5,17 @@
#include "../opensil.h"
-void add_opensil_memmap(struct device *dev, unsigned long *idx) +void opensil_get_hole_info(uint32_t *n_holes, uint64_t *top_of_mem, void **hole_info) +{ + *n_holes = 0; + *top_of_mem = 0xc0000000; + printk(BIOS_NOTICE, "openSIL stub: %s\n", __func__); +} + +const char *opensil_get_hole_info_type(uint32_t type) { printk(BIOS_NOTICE, "openSIL stub: %s\n", __func__); + return ""; }
void opensil_fill_fadt(acpi_fadt_t *fadt)