coreboot-gerrit November 2019

coreboot-gerrit@coreboot.org

1 participants
2418 discussions

Change in coreboot[master]: soc/intel: skl,cnl,icl: consolidate ebda and memmap and move to commo...
by Michael Niewöhner (Code Review) 04 Nov '19

04 Nov '19

Michael Niewöhner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36137 ) Change subject: soc/intel: skl,cnl,icl: consolidate ebda and memmap and move to common code ...................................................................... soc/intel: skl,cnl,icl: consolidate ebda and memmap and move to common code As of CB:36136 ebda and memmap are identical for skl, cnl and icl, thus move them to common code. Change-Id: I9a20c814d2a6874fcb4ff99ef1a7825d891f74e2 Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M src/soc/intel/cannonlake/Makefile.inc D src/soc/intel/cannonlake/memmap.c M src/soc/intel/common/block/ebda/ebda.c M src/soc/intel/common/block/include/intelblocks/ebda.h A src/soc/intel/common/block/memmap/Kconfig A src/soc/intel/common/block/memmap/Makefile.inc R src/soc/intel/common/block/memmap/memmap.c M src/soc/intel/common/pch/Kconfig M src/soc/intel/icelake/Makefile.inc D src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/Makefile.inc 11 files changed, 26 insertions(+), 323 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/36137/1 diff --git a/src/soc/intel/cannonlake/Makefile.inc b/src/soc/intel/cannonlake/Makefile.inc index 724e141..1588bde 100644 --- a/src/soc/intel/cannonlake/Makefile.inc +++ b/src/soc/intel/cannonlake/Makefile.inc @@ -16,7 +16,6 @@ bootblock-y += bootblock/report_platform.c bootblock-y += gspi.c bootblock-y += i2c.c -bootblock-y += memmap.c bootblock-y += spi.c bootblock-y += lpc.c bootblock-y += p2sb.c @@ -26,7 +25,6 @@ romstage-y += gspi.c romstage-y += i2c.c romstage-y += lpc.c -romstage-y += memmap.c romstage-y += pmutil.c romstage-y += reset.c romstage-y += spi.c @@ -44,7 +42,6 @@ ramstage-y += lockdown.c ramstage-y += lpc.c ramstage-y += me.c -ramstage-y += memmap.c ramstage-y += nhlt.c ramstage-y += p2sb.c ramstage-y += pmc.c @@ -66,7 +63,6 @@ smm-y += uart.c smm-y += xhci.c -postcar-y += memmap.c postcar-y += pmutil.c postcar-y += i2c.c postcar-y += gspi.c diff --git a/src/soc/intel/cannonlake/memmap.c b/src/soc/intel/cannonlake/memmap.c deleted file mode 100644 index 4973e8d..0000000 --- a/src/soc/intel/cannonlake/memmap.c +++ /dev/null @@ -1,124 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2014 Google Inc. - * Copyright (C) 2015-2017 Intel Corporation. - * - * 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 <arch/romstage.h> -#include <cbmem.h> -#include <console/console.h> -#include <cpu/x86/mtrr.h> -#include <cpu/x86/smm.h> -#include <fsp/util.h> -#include <intelblocks/ebda.h> -#include <intelblocks/systemagent.h> -#include <stdlib.h> - -void smm_region(uintptr_t *start, size_t *size) -{ - *start = sa_get_tseg_base(); - *size = sa_get_tseg_size(); -} - -/* - * SoC implementation - * - * SoC call to summarize all Intel Reserve MMIO size and report to SA - */ -size_t soc_reserved_mmio_size(void) -{ - struct ebda_config cfg; - - retrieve_ebda_object(&cfg); - - /* Get Intel Reserved Memory Range Size */ - return cfg.reserved_mem_size; -} - -/* Fill up memory layout information */ -void fill_soc_memmap_ebda(struct ebda_config *cfg) -{ - struct range_entry fsp_mem; - struct range_entry tolum; - - /* Lookup the FSP_BOOTLOADER_TOLUM_HOB */ - if (fsp_find_bootloader_tolum(&tolum)) - die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n"); - - /* Locate the FSP reserved memory area */ - if (fsp_find_reserved_memory(&fsp_mem)) - die("9.1: FSP_RESERVED_MEMORY_RESOURCE_HOB missing!\n"); - - cfg->tolum_end = range_entry_end(&tolum); - cfg->reserved_mem_size = range_entry_size(&fsp_mem); -} - -void cbmem_top_init(void) -{ - /* Fill up EBDA area */ - fill_ebda_area(); -} - -/* - * +-------------------------+ Top of RAM (aligned) - * | System Management Mode | - * | code and data | Length: CONFIG_TSEG_SIZE - * | (TSEG) | - * +-------------------------+ SMM base (aligned) - * | | - * | Chipset Reserved Memory | - * | | - * +-------------------------+ top_of_ram (aligned) - * | | - * | CBMEM Root | - * | | - * +-------------------------+ - * | | - * | FSP Reserved Memory | - * | | - * +-------------------------+ - * | | - * | Various CBMEM Entries | - * | | - * +-------------------------+ top_of_stack (8 byte aligned) - * | | - * | stack (CBMEM Entry) | - * | | - * +-------------------------+ - */ -void *cbmem_top(void) -{ - struct ebda_config ebda_cfg; - - retrieve_ebda_object(&ebda_cfg); - - return (void *)(uintptr_t)ebda_cfg.tolum_end; -} - -void fill_postcar_frame(struct postcar_frame *pcf) -{ - uintptr_t top_of_ram; - /* - * We need to make sure ramstage will be run cached. At this - * point exact location of ramstage in cbmem is not known. - * Instruct postcar to cache 16 megs under cbmem top which is - * a safe bet to cover ramstage. - */ - top_of_ram = (uintptr_t) cbmem_top(); - printk(BIOS_DEBUG, "top_of_ram = 0x%lx\n", top_of_ram); - top_of_ram -= 16*MiB; - postcar_frame_add_mtrr(pcf, top_of_ram, 16*MiB, MTRR_TYPE_WRBACK); - - /* Cache the TSEG region */ - postcar_enable_tseg_cache(pcf); -} diff --git a/src/soc/intel/common/block/ebda/ebda.c b/src/soc/intel/common/block/ebda/ebda.c index 6b0bd67..072023c 100644 --- a/src/soc/intel/common/block/ebda/ebda.c +++ b/src/soc/intel/common/block/ebda/ebda.c @@ -17,34 +17,14 @@ #include <intelblocks/ebda.h> #include <string.h> -/* - * Mainboard Override function - * - * Mainboard directory may implement below functionality for romstage. - */ - -/* Fill up EBDA structure inside Mainboard directory */ -__weak void create_mainboard_ebda(struct ebda_config *cfg) -{ - /* no-op */ -} - -static void create_soc_ebda(struct ebda_config *cfg) -{ - /* Create EBDA header */ - cfg->signature = EBDA_SIGNATURE; - /* Fill up memory layout information */ - fill_soc_memmap_ebda(cfg); -} - -void fill_ebda_area(void) +void initialize_ebda_area(void) { struct ebda_config ebda_cfg; /* Initialize EBDA area early during romstage. */ setup_default_ebda(); - create_soc_ebda(&ebda_cfg); - create_mainboard_ebda(&ebda_cfg); + ebda_cfg.signature = EBDA_SIGNATURE; + fill_memmap_ebda(&ebda_cfg); write_ebda_data(&ebda_cfg, sizeof(ebda_cfg)); } diff --git a/src/soc/intel/common/block/include/intelblocks/ebda.h b/src/soc/intel/common/block/include/intelblocks/ebda.h index 16124df..1770e8f 100644 --- a/src/soc/intel/common/block/include/intelblocks/ebda.h +++ b/src/soc/intel/common/block/include/intelblocks/ebda.h @@ -16,52 +16,24 @@ #ifndef SOC_INTEL_COMMON_BLOCK_EBDA_H #define SOC_INTEL_COMMON_BLOCK_EBDA_H -#include <soc/ebda.h> - #define EBDA_SIGNATURE 0xebdaebda -/* - * Mainboard Override function - * - * Mainboard directory may implement below functionality for romstage. - */ +/* EBDA structure */ +struct ebda_config { + uint32_t signature; /* EBDA signature */ + uint32_t tolum_end; /* coreboot memory start */ + uint32_t reserved_mem_size; /* chipset reserved memory size */ +}; -/* Fill up EBDA structure inside Mainboard directory */ -void create_mainboard_ebda(struct ebda_config *cfg); +/* Initialize EBDA and store structure into EBDA area */ +void initialize_ebda_area(void); /* - * SoC overrides - * - * All new SoC must implement below functionality for romstage. - */ -void fill_soc_memmap_ebda(struct ebda_config *cfg); - -/* - * API to perform below operation - * 1. Initialize EBDA area - * 2. Fill up EBDA structure inside SOC directory - * 3. Fill up EBDA structure inside Mainboard directory - * 4. Store EBDA structure into EBDA area - */ -void fill_ebda_area(void); - -/* Fill the ebda object pointed to by cfg. Object will be zero filled + * Fill the ebda object pointed to by cfg. Object will be zero filled * if signature check fails. */ void retrieve_ebda_object(struct ebda_config *cfg); -/* - * EBDA structure - * - * SOC should implement EBDA structure as per need - * as below. - * - * Note: First 4 bytes should be reserved for signature as - * 0xEBDA - * - * struct ebda_config { - * uint32_t signature; - * <Required variables..> - * }; - */ +/* API for filling ebda with data */ +void fill_memmap_ebda(struct ebda_config *cfg); #endif diff --git a/src/soc/intel/common/block/memmap/Kconfig b/src/soc/intel/common/block/memmap/Kconfig new file mode 100644 index 0000000..677400b --- /dev/null +++ b/src/soc/intel/common/block/memmap/Kconfig @@ -0,0 +1,5 @@ +config SOC_INTEL_COMMON_BLOCK_MEMMAP + bool + select SOC_INTEL_COMMON_BLOCK_EBDA + help + Intel Processor common MEMMAP library support diff --git a/src/soc/intel/common/block/memmap/Makefile.inc b/src/soc/intel/common/block/memmap/Makefile.inc new file mode 100644 index 0000000..8da9815 --- /dev/null +++ b/src/soc/intel/common/block/memmap/Makefile.inc @@ -0,0 +1,3 @@ +romstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_MEMMAP) += memmap.c +ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_MEMMAP) += memmap.c +postcar-$(CONFIG_SOC_INTEL_COMMON_BLOCK_MEMMAP) += memmap.c diff --git a/src/soc/intel/skylake/memmap.c b/src/soc/intel/common/block/memmap/memmap.c similarity index 96% rename from src/soc/intel/skylake/memmap.c rename to src/soc/intel/common/block/memmap/memmap.c index e98bf51..03ef3b8 100644 --- a/src/soc/intel/skylake/memmap.c +++ b/src/soc/intel/common/block/memmap/memmap.c @@ -46,7 +46,7 @@ } /* Fill up memory layout information */ -void fill_soc_memmap_ebda(struct ebda_config *cfg) +void fill_memmap_ebda(struct ebda_config *cfg) { struct range_entry fsp_mem; struct range_entry tolum; @@ -65,8 +65,8 @@ void cbmem_top_init(void) { - /* Fill up EBDA area */ - fill_ebda_area(); + /* Initialize EBDA area */ + initialize_ebda_area(); } /* diff --git a/src/soc/intel/common/pch/Kconfig b/src/soc/intel/common/pch/Kconfig index 7ece9549..c8c4c24 100644 --- a/src/soc/intel/common/pch/Kconfig +++ b/src/soc/intel/common/pch/Kconfig @@ -20,6 +20,7 @@ select SOC_INTEL_COMMON_BLOCK_CSE select SOC_INTEL_COMMON_BLOCK_DSP select SOC_INTEL_COMMON_BLOCK_EBDA + select SOC_INTEL_COMMON_BLOCK_MEMMAP select SOC_INTEL_COMMON_BLOCK_FAST_SPI select SOC_INTEL_COMMON_BLOCK_GPIO select SOC_INTEL_COMMON_BLOCK_GPIO_ITSS_POL_CFG diff --git a/src/soc/intel/icelake/Makefile.inc b/src/soc/intel/icelake/Makefile.inc index 15f7030..401b446 100644 --- a/src/soc/intel/icelake/Makefile.inc +++ b/src/soc/intel/icelake/Makefile.inc @@ -17,7 +17,6 @@ bootblock-y += gpio.c bootblock-y += gspi.c bootblock-y += i2c.c -bootblock-y += memmap.c bootblock-y += spi.c bootblock-y += p2sb.c bootblock-y += uart.c @@ -26,7 +25,6 @@ romstage-y += gpio.c romstage-y += gspi.c romstage-y += i2c.c -romstage-y += memmap.c romstage-y += pmutil.c romstage-y += reset.c romstage-y += spi.c @@ -44,7 +42,6 @@ ramstage-y += gspi.c ramstage-y += i2c.c ramstage-y += lockdown.c -ramstage-y += memmap.c ramstage-y += p2sb.c ramstage-y += pmc.c ramstage-y += pmutil.c @@ -62,7 +59,6 @@ smm-y += smihandler.c smm-y += uart.c -postcar-y += memmap.c postcar-y += pmutil.c postcar-y += i2c.c postcar-y += gspi.c diff --git a/src/soc/intel/icelake/memmap.c b/src/soc/intel/icelake/memmap.c deleted file mode 100644 index 9c7f067..0000000 --- a/src/soc/intel/icelake/memmap.c +++ /dev/null @@ -1,123 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2018 Intel Corp. - * - * 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 <arch/romstage.h> -#include <cbmem.h> -#include <console/console.h> -#include <cpu/x86/mtrr.h> -#include <cpu/x86/smm.h> -#include <fsp/util.h> -#include <intelblocks/ebda.h> -#include <intelblocks/systemagent.h> -#include <stdlib.h> - -void smm_region(uintptr_t *start, size_t *size) -{ - *start = sa_get_tseg_base(); - *size = sa_get_tseg_size(); -} - -/* - * SoC implementation - * - * SoC call to summarize all Intel Reserve MMIO size and report to SA - */ -size_t soc_reserved_mmio_size(void) -{ - struct ebda_config cfg; - - retrieve_ebda_object(&cfg); - - /* Get Intel Reserved Memory Range Size */ - return cfg.reserved_mem_size; -} - -/* Fill up memory layout information */ -void fill_soc_memmap_ebda(struct ebda_config *cfg) -{ - struct range_entry fsp_mem; - struct range_entry tolum; - - /* Lookup the FSP_BOOTLOADER_TOLUM_HOB */ - if (fsp_find_bootloader_tolum(&tolum)) - die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n"); - - /* Locate the FSP reserved memory area */ - if (fsp_find_reserved_memory(&fsp_mem)) - die("9.1: FSP_RESERVED_MEMORY_RESOURCE_HOB missing!\n"); - - cfg->tolum_end = range_entry_end(&tolum); - cfg->reserved_mem_size = range_entry_size(&fsp_mem); -} - -void cbmem_top_init(void) -{ - /* Fill up EBDA area */ - fill_ebda_area(); -} - -/* - * +-------------------------+ Top of RAM (aligned) - * | System Management Mode | - * | code and data | Length: CONFIG_TSEG_SIZE - * | (TSEG) | - * +-------------------------+ SMM base (aligned) - * | | - * | Chipset Reserved Memory | - * | | - * +-------------------------+ top_of_ram (aligned) - * | | - * | CBMEM Root | - * | | - * +-------------------------+ - * | | - * | FSP Reserved Memory | - * | | - * +-------------------------+ - * | | - * | Various CBMEM Entries | - * | | - * +-------------------------+ top_of_stack (8 byte aligned) - * | | - * | stack (CBMEM Entry) | - * | | - * +-------------------------+ - */ -void *cbmem_top(void) -{ - struct ebda_config ebda_cfg; - - retrieve_ebda_object(&ebda_cfg); - - return (void *)(uintptr_t)ebda_cfg.tolum_end; -} - -void fill_postcar_frame(struct postcar_frame *pcf) -{ - uintptr_t top_of_ram; - /* - * We need to make sure ramstage will be run cached. At this - * point exact location of ramstage in cbmem is not known. - * Instruct postcar to cache 16 megs under cbmem top which is - * a safe bet to cover ramstage. - */ - top_of_ram = (uintptr_t) cbmem_top(); - printk(BIOS_DEBUG, "top_of_ram = 0x%lx\n", top_of_ram); - top_of_ram -= 16*MiB; - postcar_frame_add_mtrr(pcf, top_of_ram, 16*MiB, MTRR_TYPE_WRBACK); - - /* Cache the TSEG region */ - postcar_enable_tseg_cache(pcf); -} diff --git a/src/soc/intel/skylake/Makefile.inc b/src/soc/intel/skylake/Makefile.inc index ef741f8..dce1d6b 100644 --- a/src/soc/intel/skylake/Makefile.inc +++ b/src/soc/intel/skylake/Makefile.inc @@ -33,7 +33,6 @@ romstage-y += gpio.c romstage-y += gspi.c romstage-y += i2c.c -romstage-y += memmap.c romstage-y += me.c romstage-y += pmc.c romstage-y += pmutil.c @@ -55,7 +54,6 @@ ramstage-y += lockdown.c ramstage-y += lpc.c ramstage-y += me.c -ramstage-y += memmap.c ramstage-y += p2sb.c ramstage-y += pmc.c ramstage-y += pmutil.c @@ -77,7 +75,6 @@ smm-y += uart.c smm-y += xhci.c -postcar-y += memmap.c postcar-y += gspi.c postcar-y += spi.c postcar-y += i2c.c -- To view, visit https://review.coreboot.org/c/coreboot/+/36137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9a20c814d2a6874fcb4ff99ef1a7825d891f74e2 Gerrit-Change-Number: 36137 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

4 64

Change in coreboot[master]: soc/intel/common/ebda: Drop code
by Michael Niewöhner (Code Review) 04 Nov '19

04 Nov '19

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36362 ) Change subject: soc/intel/common/ebda: Drop code ...................................................................... Patch Set 19: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/36362 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I46e2459ff3c785f530cabc5930004ef920ffc89a Gerrit-Change-Number: 36362 Gerrit-PatchSet: 19 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 04 Nov 2019 19:23:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: pmclib: add API to get ETR register address
by Aaron Durbin (Code Review) 04 Nov '19

04 Nov '19

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36565 ) Change subject: soc/intel/common: pmclib: add API to get ETR register address ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/c/coreboot/+/36565/4/src/soc/intel/common/block… File src/soc/intel/common/block/include/intelblocks/pmclib.h: https://review.coreboot.org/c/coreboot/+/36565/4/src/soc/intel/common/block… PS4, Line 173: read_ > I copied the name from soc_read_pmc_base, I'd be fine with soc_pmc_etr_addr, though I see. In that case I don't like 'read' in there either. I can fix that. Your suggestion seems fine. -- To view, visit https://review.coreboot.org/c/coreboot/+/36565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I706f3e220d639a6133625e3cb7267f7009006af2 Gerrit-Change-Number: 36565 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 04 Nov 2019 19:19:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org> Comment-In-Reply-To: Michael Niewöhner Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: pmclib: make use of the new ETR address API
by Aaron Durbin (Code Review) 04 Nov '19

04 Nov '19

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36570 ) Change subject: soc/intel/common: pmclib: make use of the new ETR address API ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/c/coreboot/+/36570/4/src/soc/intel/common/block… File src/soc/intel/common/block/pmc/pmclib.c: https://review.coreboot.org/c/coreboot/+/36570/4/src/soc/intel/common/block… PS4, Line 432: void pmc_global_reset_lock(void) : { : uint32_t *etr = soc_read_pmc_etr_addr(); : uint32_t reg; : : reg = read32(etr); : reg = (reg & ~CF9_GLB_RST) | CF9_LOCK; : write32(etr, reg); : } > Well, I just wanted to continue your idea of moving this back to soc/ (which I definitely don't pref […] I was responding to Arthur, but gerrit won't let me quote old comments. Sorry for not being clearer. I was explaining (and your other CL brings it to attention) the dependencies and the combinations needed to support as well as open coding everything which Arthur suggested. That's what triggered this CL to begin with because of noisiness and implicit dependencies. -- To view, visit https://review.coreboot.org/c/coreboot/+/36570 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I49f59efb4a7c7d3d629ac54a7922bbcc8a87714d Gerrit-Change-Number: 36570 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 04 Nov 2019 19:16:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Michael Niewöhner Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel: skl,cnl,icl: rely on TOLUM and reserved memory returned by...
by Michael Niewöhner (Code Review) 04 Nov '19

04 Nov '19

Michael Niewöhner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36136 ) Change subject: soc/intel: skl,cnl,icl: rely on TOLUM and reserved memory returned by FSP ...................................................................... soc/intel: skl,cnl,icl: rely on TOLUM and reserved memory returned by FSP Instead of doing our own calculations, rely on the TOLUM and reserved memory size returned by FSP. This (hopefully) saves us from making mistakes in calculations of offsets and aligments. Further this makes it easier to implement e.g. SGX PRMRR size selection via Kconfig as we do not have to know anything and do not have to make any assumptions about aligning but simply pass (valid) values to FSP. Change-Id: If66a00d1320917bc68afb32c19db0e24c6732812 Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M src/soc/intel/cannonlake/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/include/soc/ebda.h M src/soc/intel/skylake/memmap.c 4 files changed, 43 insertions(+), 533 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/36136/1 diff --git a/src/soc/intel/cannonlake/memmap.c b/src/soc/intel/cannonlake/memmap.c index f3286cc..b2fcad0 100644 --- a/src/soc/intel/cannonlake/memmap.c +++ b/src/soc/intel/cannonlake/memmap.c @@ -28,8 +28,7 @@ #include <soc/pci_devs.h> #include <soc/systemagent.h> #include <stdlib.h> - -#include "chip.h" +#include <fsp/util.h> void smm_region(uintptr_t *start, size_t *size) { @@ -37,161 +36,6 @@ *size = sa_get_tseg_size(); } -/* Calculate ME Stolen size */ -static size_t get_imr_size(void) -{ - size_t imr_size; - - /* ME stolen memory */ - imr_size = MCHBAR32(IMRLIMIT) - MCHBAR32(IMRBASE); - - return imr_size; -} - -/* Calculate PRMRR size based on user input PRMRR size and alignment */ -static size_t get_prmrr_size(uintptr_t dram_base, - const struct soc_intel_cannonlake_config *config) -{ - uintptr_t prmrr_base = dram_base; - size_t prmrr_size; - - prmrr_size = config->PrmrrSize; - - /* Allocate PRMRR memory for C6DRAM */ - if (!prmrr_size) { - if (config->enable_c6dram) - prmrr_size = 1*MiB; - else - return 0; - } - - /* - * PRMRR Sizes that are > 1MB and < 32MB are - * not supported and will fail out. - */ - if ((prmrr_size > 1*MiB) && (prmrr_size < 32*MiB)) - die("PRMRR Sizes that are > 1MB and < 32MB are not" - "supported!\n"); - - prmrr_base -= prmrr_size; - if (prmrr_size >= 32*MiB) - prmrr_base = ALIGN_DOWN(prmrr_base, 128*MiB); - else - prmrr_base = ALIGN_DOWN(prmrr_base, 16*MiB); - /* PRMRR Area Size */ - prmrr_size = dram_base - prmrr_base; - - return prmrr_size; -} - -/* Calculate Intel Traditional Memory size based on GSM, DSM, TSEG and DPR. */ -static size_t calculate_traditional_mem_size(uintptr_t dram_base, - const struct device *dev) -{ - uintptr_t traditional_mem_base = dram_base; - size_t traditional_mem_size; - - if (dev->enabled) { - /* Read BDSM from Host Bridge */ - traditional_mem_base -= sa_get_dsm_size(); - - /* Read BGSM from Host Bridge */ - traditional_mem_base -= sa_get_gsm_size(); - } - /* Get TSEG size */ - traditional_mem_base -= sa_get_tseg_size(); - - /* Get DPR size */ - if (CONFIG(SA_ENABLE_DPR)) - traditional_mem_base -= sa_get_dpr_size(); - - /* Traditional Area Size */ - traditional_mem_size = dram_base - traditional_mem_base; - - return traditional_mem_size; -} - -/* - * Calculate Intel Reserved Memory size based on - * PRMRR size, Me stolen memory and PTT selection. - */ -static size_t calculate_reserved_mem_size(uintptr_t dram_base, - const struct device *dev) -{ - uintptr_t reserve_mem_base = dram_base; - size_t reserve_mem_size; - const struct soc_intel_cannonlake_config *config; - - config = config_of(dev); - - /* Get PRMRR size */ - reserve_mem_base -= get_prmrr_size(reserve_mem_base, config); - - /* Get Tracehub size */ - reserve_mem_base -= get_imr_size(); - - /* Traditional Area Size */ - reserve_mem_size = dram_base - reserve_mem_base; - - return reserve_mem_size; -} - -/* - * Host Memory Map: - * - * +--------------------------+ TOUUD - * | | - * +--------------------------+ 4GiB - * | PCI Address Space | - * +--------------------------+ TOLUD (also maps into MC address space) - * | iGD | - * +--------------------------+ BDSM - * | GTT | - * +--------------------------+ BGSM - * | TSEG | - * +--------------------------+ TSEGMB - * | DMA Protected Region | - * +--------------------------+ DPR - * | PRM (C6DRAM/SGX) | - * +--------------------------+ PRMRR - * | ME Stolen Memory | - * +--------------------------+ ME Stolen - * | PTT | - * +--------------------------+ top_of_ram - * | Reserved - FSP/CBMEM | - * +--------------------------+ TOLUM - * | Usage DRAM | - * +--------------------------+ 0 - * - * Some of the base registers above can be equal making the size of those - * regions 0. The reason is because the memory controller internally subtracts - * the base registers from each other to determine sizes of the regions. In - * other words, the memory map is in a fixed order no matter what. - */ -static uintptr_t calculate_dram_base(size_t *reserved_mem_size) -{ - uintptr_t dram_base; - const struct device *dev; - - dev = pcidev_on_root(SA_DEV_SLOT_IGD, 0); - if (!dev) - die_with_post_code(POST_HW_INIT_FAILURE, - "ERROR - IGD device not found!"); - - /* Read TOLUD from Host Bridge offset */ - dram_base = sa_get_tolud_base(); - - /* Get Intel Traditional Memory Range Size */ - dram_base -= calculate_traditional_mem_size(dram_base, dev); - - /* Get Intel Reserved Memory Range Size */ - *reserved_mem_size = calculate_reserved_mem_size(dram_base, dev); - - dram_base -= *reserved_mem_size; - - return dram_base; -} - /* * SoC implementation * @@ -210,10 +54,19 @@ /* Fill up memory layout information */ void fill_soc_memmap_ebda(struct ebda_config *cfg) { - size_t chipset_mem_size; + struct range_entry fsp_mem; + struct range_entry tolum; - cfg->tolum_base = calculate_dram_base(&chipset_mem_size); - cfg->reserved_mem_size = chipset_mem_size; + /* Lookup the FSP_BOOTLOADER_TOLUM_HOB */ + if (fsp_find_bootloader_tolum(&tolum)) + die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n"); + + /* Locate the FSP reserved memory area */ + if (fsp_find_reserved_memory(&fsp_mem)) + die("9.1: FSP_RESERVED_MEMORY_RESOURCE_HOB missing!\n"); + + cfg->tolum_end = range_entry_end(&tolum); + cfg->reserved_mem_size = range_entry_size(&fsp_mem); } void cbmem_top_init(void) @@ -253,15 +106,6 @@ { struct ebda_config ebda_cfg; - /* - * Check if Tseg has been initialized, we will use this as a flag - * to check if the MRC is done, and only then continue to read the - * PRMMR_BASE MSR. The system hangs if PRMRR_BASE MSR is read before - * PRMRR_MASK MSR lock bit is set. - */ - if (sa_get_tseg_base() == 0) - return NULL; - retrieve_ebda_object(&ebda_cfg); return (void *)(uintptr_t)ebda_cfg.tolum_base; diff --git a/src/soc/intel/icelake/memmap.c b/src/soc/intel/icelake/memmap.c index 20c4e6f..9df1065 100644 --- a/src/soc/intel/icelake/memmap.c +++ b/src/soc/intel/icelake/memmap.c @@ -28,6 +28,7 @@ #include <soc/soc_chip.h> #include <soc/systemagent.h> #include <stdlib.h> +#include <fsp/util.h> void smm_region(uintptr_t *start, size_t *size) { @@ -35,161 +36,6 @@ *size = sa_get_tseg_size(); } -/* Calculate ME Stolen size */ -static size_t get_imr_size(void) -{ - size_t imr_size; - - /* ME stolen memory */ - imr_size = MCHBAR32(IMRLIMIT) - MCHBAR32(IMRBASE); - - return imr_size; -} - -/* Calculate PRMRR size based on user input PRMRR size and alignment */ -static size_t get_prmrr_size(uintptr_t dram_base, - const struct soc_intel_icelake_config *config) -{ - uintptr_t prmrr_base = dram_base; - size_t prmrr_size; - - prmrr_size = config->PrmrrSize; - - /* Allocate PRMRR memory for C6DRAM */ - if (!prmrr_size) { - if (config->enable_c6dram) - prmrr_size = 1*MiB; - else - return 0; - } - - /* - * PRMRR Sizes that are > 1MB and < 32MB are - * not supported and will fail out. - */ - if ((prmrr_size > 1*MiB) && (prmrr_size < 32*MiB)) - die("PRMRR Sizes that are > 1MB and < 32MB are not" - "supported!\n"); - - prmrr_base -= prmrr_size; - if (prmrr_size >= 32*MiB) - prmrr_base = ALIGN_DOWN(prmrr_base, 128*MiB); - else - prmrr_base = ALIGN_DOWN(prmrr_base, 16*MiB); - /* PRMRR Area Size */ - prmrr_size = dram_base - prmrr_base; - - return prmrr_size; -} - -/* Calculate Intel Traditional Memory size based on GSM, DSM, TSEG and DPR. */ -static size_t calculate_traditional_mem_size(uintptr_t dram_base, - const struct device *dev) -{ - uintptr_t traditional_mem_base = dram_base; - size_t traditional_mem_size; - - if (dev->enabled) { - /* Read BDSM from Host Bridge */ - traditional_mem_base -= sa_get_dsm_size(); - - /* Read BGSM from Host Bridge */ - traditional_mem_base -= sa_get_gsm_size(); - } - /* Get TSEG size */ - traditional_mem_base -= sa_get_tseg_size(); - - /* Get DPR size */ - if (CONFIG(SA_ENABLE_DPR)) - traditional_mem_base -= sa_get_dpr_size(); - - /* Traditional Area Size */ - traditional_mem_size = dram_base - traditional_mem_base; - - return traditional_mem_size; -} - -/* - * Calculate Intel Reserved Memory size based on - * PRMRR size, Me stolen memory and PTT selection. - */ -static size_t calculate_reserved_mem_size(uintptr_t dram_base, - const struct device *dev) -{ - uintptr_t reserve_mem_base = dram_base; - size_t reserve_mem_size; - const struct soc_intel_icelake_config *config; - - config = config_of(dev); - - /* Get PRMRR size */ - reserve_mem_base -= get_prmrr_size(reserve_mem_base, config); - - /* Get Tracehub size */ - reserve_mem_base -= get_imr_size(); - - /* Traditional Area Size */ - reserve_mem_size = dram_base - reserve_mem_base; - - return reserve_mem_size; -} - -/* - * Host Memory Map: - * - * +--------------------------+ TOUUD - * | | - * +--------------------------+ 4GiB - * | PCI Address Space | - * +--------------------------+ TOLUD (also maps into MC address space) - * | iGD | - * +--------------------------+ BDSM - * | GTT | - * +--------------------------+ BGSM - * | TSEG | - * +--------------------------+ TSEGMB - * | DMA Protected Region | - * +--------------------------+ DPR - * | PRM (C6DRAM/SGX) | - * +--------------------------+ PRMRR - * | ME Stolen Memory | - * +--------------------------+ ME Stolen - * | PTT | - * +--------------------------+ top_of_ram - * | Reserved - FSP/CBMEM | - * +--------------------------+ TOLUM - * | Usage DRAM | - * +--------------------------+ 0 - * - * Some of the base registers above can be equal making the size of those - * regions 0. The reason is because the memory controller internally subtracts - * the base registers from each other to determine sizes of the regions. In - * other words, the memory map is in a fixed order no matter what. - */ -static uintptr_t calculate_dram_base(size_t *reserved_mem_size) -{ - uintptr_t dram_base; - const struct device *dev; - - dev = pcidev_on_root(SA_DEV_SLOT_IGD, 0); - if (!dev) - die_with_post_code(POST_HW_INIT_FAILURE, - "ERROR - IGD device not found!"); - - /* Read TOLUD from Host Bridge offset */ - dram_base = sa_get_tolud_base(); - - /* Get Intel Traditional Memory Range Size */ - dram_base -= calculate_traditional_mem_size(dram_base, dev); - - /* Get Intel Reserved Memory Range Size */ - *reserved_mem_size = calculate_reserved_mem_size(dram_base, dev); - - dram_base -= *reserved_mem_size; - - return dram_base; -} - /* * SoC implementation * @@ -208,10 +54,19 @@ /* Fill up memory layout information */ void fill_soc_memmap_ebda(struct ebda_config *cfg) { - size_t chipset_mem_size; + struct range_entry fsp_mem; + struct range_entry tolum; - cfg->tolum_base = calculate_dram_base(&chipset_mem_size); - cfg->reserved_mem_size = chipset_mem_size; + /* Lookup the FSP_BOOTLOADER_TOLUM_HOB */ + if (fsp_find_bootloader_tolum(&tolum)) + die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n"); + + /* Locate the FSP reserved memory area */ + if (fsp_find_reserved_memory(&fsp_mem)) + die("9.1: FSP_RESERVED_MEMORY_RESOURCE_HOB missing!\n"); + + cfg->tolum_end = range_entry_end(&tolum); + cfg->reserved_mem_size = range_entry_size(&fsp_mem); } void cbmem_top_init(void) @@ -251,18 +106,9 @@ { struct ebda_config ebda_cfg; - /* - * Check if Tseg has been initialized, we will use this as a flag - * to check if the MRC is done, and only then continue to read the - * PRMMR_BASE MSR. The system hangs if PRMRR_BASE MSR is read before - * PRMRR_MASK MSR lock bit is set. - */ - if (sa_get_tseg_base() == 0) - return NULL; - retrieve_ebda_object(&ebda_cfg); - return (void *)(uintptr_t)ebda_cfg.tolum_base; + return (void *)(uintptr_t)ebda_cfg.tolum_end; } void fill_postcar_frame(struct postcar_frame *pcf) diff --git a/src/soc/intel/skylake/include/soc/ebda.h b/src/soc/intel/skylake/include/soc/ebda.h index ad62394..d588c18 100644 --- a/src/soc/intel/skylake/include/soc/ebda.h +++ b/src/soc/intel/skylake/include/soc/ebda.h @@ -20,8 +20,8 @@ struct ebda_config { uint32_t signature; /* 0x00 - EBDA signature */ - uint32_t tolum_base; /* 0x04 - coreboot memory start */ - uint32_t reserved_mem_size; /* 0x08 - chipset reserved memory size */ + uint32_t tolum_end; /* 0x04 - coreboot memory start */ + uint32_t reserved_mem_size; /* 0x0C - chipset reserved memory size */ }; #endif diff --git a/src/soc/intel/skylake/memmap.c b/src/soc/intel/skylake/memmap.c index 29f2517..5c4cc3a 100644 --- a/src/soc/intel/skylake/memmap.c +++ b/src/soc/intel/skylake/memmap.c @@ -29,8 +29,7 @@ #include <soc/pci_devs.h> #include <soc/systemagent.h> #include <stdlib.h> - -#include "chip.h" +#include <fsp/util.h> void smm_region(uintptr_t *start, size_t *size) { @@ -38,185 +37,6 @@ *size = sa_get_tseg_size(); } -static bool is_ptt_enable(void) -{ - if ((read32((void *)PTT_TXT_BASE_ADDRESS) & PTT_PRESENT) == - PTT_PRESENT) - return true; - - return false; -} - -/* Calculate PTT size */ -static size_t get_ptt_size(void) -{ - /* Allocate 4KB for PTT if enabled */ - return is_ptt_enable() ? 4*KiB : 0; -} - -/* Calculate Trace Hub size */ -static size_t get_tracehub_size(uintptr_t dram_base, - const struct soc_intel_skylake_config *config) -{ - uintptr_t tracehub_base = dram_base; - size_t tracehub_size = 0; - - if (!config->ProbelessTrace) - return 0; - - /* GDXC MOT */ - tracehub_base -= GDXC_MOT_MEMORY_SIZE; - /* Round down to natural boundary according to PSMI size */ - tracehub_base = ALIGN_DOWN(tracehub_base, PSMI_BUFFER_AREA_SIZE); - /* GDXC IOT */ - tracehub_base -= GDXC_IOT_MEMORY_SIZE; - /* PSMI buffer area */ - tracehub_base -= PSMI_BUFFER_AREA_SIZE; - - /* Tracehub Area Size */ - tracehub_size = dram_base - tracehub_base; - - return tracehub_size; -} - -/* Calculate PRMRR size based on user input PRMRR size and alignment */ -static size_t get_prmrr_size(uintptr_t dram_base, - const struct soc_intel_skylake_config *config) -{ - uintptr_t prmrr_base = dram_base; - size_t prmrr_size; - - if (CONFIG(PLATFORM_USES_FSP1_1)) - prmrr_size = 1*MiB; - else - prmrr_size = config->PrmrrSize; - - if (!prmrr_size) - return 0; - - /* - * PRMRR Sizes that are > 1MB and < 32MB are - * not supported and will fail out. - */ - if ((prmrr_size > 1*MiB) && (prmrr_size < 32*MiB)) - die("PRMRR Sizes that are > 1MB and < 32MB are not" - "supported!\n"); - - prmrr_base -= prmrr_size; - if (prmrr_size >= 32*MiB) - prmrr_base = ALIGN_DOWN(prmrr_base, 128*MiB); - - /* PRMRR Area Size */ - prmrr_size = dram_base - prmrr_base; - - return prmrr_size; -} - -/* Calculate Intel Traditional Memory size based on GSM, DSM, TSEG and DPR. */ -static size_t calculate_traditional_mem_size(uintptr_t dram_base) -{ - const struct device *igd_dev = pcidev_path_on_root(SA_DEVFN_IGD); - uintptr_t traditional_mem_base = dram_base; - size_t traditional_mem_size; - - if (igd_dev && igd_dev->enabled) { - /* Read BDSM from Host Bridge */ - traditional_mem_base -= sa_get_dsm_size(); - - /* Read BGSM from Host Bridge */ - traditional_mem_base -= sa_get_gsm_size(); - } - /* Get TSEG size */ - traditional_mem_base -= sa_get_tseg_size(); - - /* Get DPR size */ - if (CONFIG(SA_ENABLE_DPR)) - traditional_mem_base -= sa_get_dpr_size(); - - /* Traditional Area Size */ - traditional_mem_size = dram_base - traditional_mem_base; - - return traditional_mem_size; -} - -/* - * Calculate Intel Reserved Memory size based on - * PRMRR size, Trace Hub config and PTT selection. - */ -static size_t calculate_reserved_mem_size(uintptr_t dram_base) -{ - const struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT); - uintptr_t reserve_mem_base = dram_base; - size_t reserve_mem_size; - const struct soc_intel_skylake_config *config; - - config = config_of(dev); - - /* Get PRMRR size */ - reserve_mem_base -= get_prmrr_size(reserve_mem_base, config); - - /* Get Tracehub size */ - reserve_mem_base -= get_tracehub_size(reserve_mem_base, config); - - /* Get PTT size */ - reserve_mem_base -= get_ptt_size(); - - /* Traditional Area Size */ - reserve_mem_size = dram_base - reserve_mem_base; - - return reserve_mem_size; -} - -/* - * Host Memory Map: - * - * +--------------------------+ TOUUD - * | | - * +--------------------------+ 4GiB - * | PCI Address Space | - * +--------------------------+ TOLUD (also maps into MC address space) - * | iGD | - * +--------------------------+ BDSM - * | GTT | - * +--------------------------+ BGSM - * | TSEG | - * +--------------------------+ TSEGMB - * | DMA Protected Region | - * +--------------------------+ DPR - * | PRM (C6DRAM/SGX) | - * +--------------------------+ PRMRR - * | Trace Memory | - * +--------------------------+ Probless Trace - * | PTT | - * +--------------------------+ top_of_ram - * | Reserved - FSP/CBMEM | - * +--------------------------+ TOLUM - * | Usage DRAM | - * +--------------------------+ 0 - * - * Some of the base registers above can be equal making the size of those - * regions 0. The reason is because the memory controller internally subtracts - * the base registers from each other to determine sizes of the regions. In - * other words, the memory map is in a fixed order no matter what. - */ -static uintptr_t calculate_dram_base(size_t *reserved_mem_size) -{ - uintptr_t dram_base; - - /* Read TOLUD from Host Bridge offset */ - dram_base = sa_get_tolud_base(); - - /* Get Intel Traditional Memory Range Size */ - dram_base -= calculate_traditional_mem_size(dram_base); - - /* Get Intel Reserved Memory Range Size */ - *reserved_mem_size = calculate_reserved_mem_size(dram_base); - - dram_base -= *reserved_mem_size; - - return dram_base; -} - /* * SoC implementation * @@ -235,10 +55,19 @@ /* Fill up memory layout information */ void fill_soc_memmap_ebda(struct ebda_config *cfg) { - size_t chipset_mem_size; + struct range_entry fsp_mem; + struct range_entry tolum; - cfg->tolum_base = calculate_dram_base(&chipset_mem_size); - cfg->reserved_mem_size = chipset_mem_size; + /* Lookup the FSP_BOOTLOADER_TOLUM_HOB */ + if (fsp_find_bootloader_tolum(&tolum)) + die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n"); + + /* Locate the FSP reserved memory area */ + if (fsp_find_reserved_memory(&fsp_mem)) + die("9.1: FSP_RESERVED_MEMORY_RESOURCE_HOB missing!\n"); + + cfg->tolum_end = range_entry_end(&tolum); + cfg->reserved_mem_size = range_entry_size(&fsp_mem); } void cbmem_top_init(void) @@ -278,18 +107,9 @@ { struct ebda_config ebda_cfg; - /* - * Check if Tseg has been initialized, we will use this as a flag - * to check if the MRC is done, and only then continue to read the - * PRMMR_BASE MSR. The system hangs if PRMRR_BASE MSR is read before - * PRMRR_MASK MSR lock bit is set. - */ - if (sa_get_tseg_base() == 0) - return NULL; - retrieve_ebda_object(&ebda_cfg); - return (void *)(uintptr_t)ebda_cfg.tolum_base; + return (void *)(uintptr_t)ebda_cfg.tolum_end; } #if CONFIG(PLATFORM_USES_FSP2_0) -- To view, visit https://review.coreboot.org/c/coreboot/+/36136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If66a00d1320917bc68afb32c19db0e24c6732812 Gerrit-Change-Number: 36136 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Niewöhner Gerrit-MessageType: newchange

6 66

Change in coreboot[master]: soc/intel/common: pmclib: make use of the new ETR address API
by Michael Niewöhner (Code Review) 04 Nov '19

04 Nov '19

1 0

Change in coreboot[master]: soc/intel/common: pmclib: add API to get ETR register address
by Michael Niewöhner (Code Review) 04 Nov '19

04 Nov '19

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36565 ) Change subject: soc/intel/common: pmclib: add API to get ETR register address ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/c/coreboot/+/36565/4/src/soc/intel/common/block… File src/soc/intel/common/block/include/intelblocks/pmclib.h: https://review.coreboot.org/c/coreboot/+/36565/4/src/soc/intel/common/block… PS4, Line 173: read_ > Why is there a 'read' in this function? I copied the name from soc_read_pmc_base, I'd be fine with soc_pmc_etr_addr, though -- To view, visit https://review.coreboot.org/c/coreboot/+/36565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I706f3e220d639a6133625e3cb7267f7009006af2 Gerrit-Change-Number: 36565 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 04 Nov 2019 18:53:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: pmclib: make use of the new ETR address API
by Michael Niewöhner (Code Review) 04 Nov '19

04 Nov '19

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36570 ) Change subject: soc/intel/common: pmclib: make use of the new ETR address API ...................................................................... Patch Set 4: (4 comments) https://review.coreboot.org/c/coreboot/+/36570/4/src/soc/intel/common/block… File src/soc/intel/common/block/pmc/pmclib.c: https://review.coreboot.org/c/coreboot/+/36570/4/src/soc/intel/common/block… PS4, Line 432: void pmc_global_reset_lock(void) > Header file should be changed to reflect its actual implementation. Comment current indicates: […] Done https://review.coreboot.org/c/coreboot/+/36570/4/src/soc/intel/common/block… PS4, Line 434: soc_read_pmc_etr_addr(); > I guess when this was null, we had a bigger problem and wouldn't even reach that code Done https://review.coreboot.org/c/coreboot/+/36570/4/src/soc/intel/common/block… PS4, Line 438: reg = (reg & ~CF9_GLB_RST) | CF9_LOCK; > This is still changing the semantics of the previous implementation. […] Yup, this is to save one additional read-modify-write cycle; will rename it https://review.coreboot.org/c/coreboot/+/36570/4/src/soc/intel/common/block… PS4, Line 432: void pmc_global_reset_lock(void) : { : uint32_t *etr = soc_read_pmc_etr_addr(); : uint32_t reg; : : reg = read32(etr); : reg = (reg & ~CF9_GLB_RST) | CF9_LOCK; : write32(etr, reg); : } > How was one going to convey the difference of the location of the register? Add another Kconfig? It […] Well, I just wanted to continue your idea of moving this back to soc/ (which I definitely don't prefer, though). Then no Kconfig would be needed and ETR is already in soc/pm.h -- To view, visit https://review.coreboot.org/c/coreboot/+/36570 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I49f59efb4a7c7d3d629ac54a7922bbcc8a87714d Gerrit-Change-Number: 36570 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 04 Nov 2019 18:42:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Michael Niewöhner Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: include: introduce update* for mmio operations
by Nico Huber (Code Review) 04 Nov '19

04 Nov '19

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36596 ) Change subject: include: introduce update* for mmio operations ...................................................................... Patch Set 2: (2 comments) https://review.coreboot.org/c/coreboot/+/36596/2/src/include/mmio.h File src/include/mmio.h: https://review.coreboot.org/c/coreboot/+/36596/2/src/include/mmio.h@13 PS2, Line 13: Should we allow users to expect the indirect inclusion of `arch/mmio.h`? e.g. only include `mmio.h` and then use read32() and update32(). If so, please leave a comment here that `mmio.h` provides read*() and write*() as well. https://review.coreboot.org/c/coreboot/+/36596/1/src/include/mmio.h File src/include/mmio.h: https://review.coreboot.org/c/coreboot/+/36596/1/src/include/mmio.h@19 PS1, Line 19: static __always_inline void update8(const volatile void *addr, uint8_t mask, uint8_t or) > s/yell/well/ For the record, I'm ok with the short names. It seems obvious what these functions do. Clutters the namespace a little more, though. -- To view, visit https://review.coreboot.org/c/coreboot/+/36596 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f188d586c09ee9f1e17290563f68970603204fb Gerrit-Change-Number: 36596 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 04 Nov 2019 18:17:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org> Comment-In-Reply-To: Michael Niewöhner Gerrit-MessageType: comment

1 0

Change in coreboot[master]: include: introduce update* for mmio operations
by Michael Niewöhner (Code Review) 04 Nov '19

04 Nov '19

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36596 ) Change subject: include: introduce update* for mmio operations ...................................................................... Patch Set 2: (2 comments) https://review.coreboot.org/c/coreboot/+/36596/1/src/include/mmio.h File src/include/mmio.h: https://review.coreboot.org/c/coreboot/+/36596/1/src/include/mmio.h@19 PS1, Line 19: static __always_inline void update8(const volatile void *addr, uint8_t mask, uint8_t or) > yell, read* and write* don't have mmio in it's name. […] s/yell/well/ https://review.coreboot.org/c/coreboot/+/36596/1/src/include/mmio.h@21 PS1, Line 21: read8 > oops, ofc. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/36596 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f188d586c09ee9f1e17290563f68970603204fb Gerrit-Change-Number: 36596 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 04 Nov 2019 17:50:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org> Comment-In-Reply-To: Michael Niewöhner Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2019