Change in ...coreboot[master]: drivers/intel: Move FSP stage_cache implementation into common block
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34005
Change subject: drivers/intel: Move FSP stage_cache implementation into common block ......................................................................
drivers/intel: Move FSP stage_cache implementation into common block
Change-Id: Iebb6d698c236a95162b3c7eb07987483a293b50a Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/drivers/intel/fsp1_1/Makefile.inc D src/drivers/intel/fsp1_1/stage_cache.c M src/drivers/intel/fsp2_0/Makefile.inc D src/drivers/intel/fsp2_0/stage_cache.c M src/soc/intel/common/block/smm/smm.c 5 files changed, 11 insertions(+), 63 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/34005/1
diff --git a/src/drivers/intel/fsp1_1/Makefile.inc b/src/drivers/intel/fsp1_1/Makefile.inc index 10877b9..1372e98 100644 --- a/src/drivers/intel/fsp1_1/Makefile.inc +++ b/src/drivers/intel/fsp1_1/Makefile.inc @@ -29,7 +29,6 @@ romstage-y += hob.c romstage-y += raminit.c romstage-y += romstage.c -romstage-y += stage_cache.c romstage-$(CONFIG_MMA) += mma_core.c
ramstage-$(CONFIG_RUN_FSP_GOP) += fsp_gop.c @@ -37,13 +36,11 @@ ramstage-y += fsp_util.c ramstage-y += hob.c ramstage-y += ramstage.c -ramstage-y += stage_cache.c ramstage-$(CONFIG_INTEL_GMA_ADD_VBT) += vbt.c ramstage-$(CONFIG_MMA) += mma_core.c
CPPFLAGS_common += -Isrc/drivers/intel/fsp1_1/include
-postcar-y += stage_cache.c ifneq ($(CONFIG_SKIP_FSP_CAR),y) postcar-y += temp_ram_exit.c postcar-y += exit_car.S diff --git a/src/drivers/intel/fsp1_1/stage_cache.c b/src/drivers/intel/fsp1_1/stage_cache.c deleted file mode 100644 index 2d594e6..0000000 --- a/src/drivers/intel/fsp1_1/stage_cache.c +++ /dev/null @@ -1,28 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2013 Google Inc. - * Copyright (C) 2015 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 <console/console.h> -#include <fsp/memmap.h> -#include <stage_cache.h> - -void stage_cache_external_region(void **base, size_t *size) -{ - if (smm_subregion(SMM_SUBREGION_CACHE, base, size)) { - printk(BIOS_ERR, "ERROR: No cache SMM subregion.\n"); - *base = NULL; - *size = 0; - } -} diff --git a/src/drivers/intel/fsp2_0/Makefile.inc b/src/drivers/intel/fsp2_0/Makefile.inc index 9df28c0..b0a1f2b 100644 --- a/src/drivers/intel/fsp2_0/Makefile.inc +++ b/src/drivers/intel/fsp2_0/Makefile.inc @@ -23,7 +23,6 @@ romstage-$(CONFIG_VERIFY_HOBS) += hob_verify.c romstage-y += util.c romstage-y += memory_init.c -romstage-$(CONFIG_EXTERNAL_STAGE_CACHE) += stage_cache.c romstage-$(CONFIG_MMA) += mma_core.c
ramstage-y += debug.c @@ -34,12 +33,10 @@ ramstage-$(CONFIG_VERIFY_HOBS) += hob_verify.c ramstage-y += notify.c ramstage-y += silicon_init.c -ramstage-$(CONFIG_EXTERNAL_STAGE_CACHE) += stage_cache.c ramstage-$(CONFIG_DISPLAY_UPD_DATA) += upd_display.c ramstage-y += util.c ramstage-$(CONFIG_MMA) += mma_core.c
-postcar-$(CONFIG_EXTERNAL_STAGE_CACHE) += stage_cache.c postcar-$(CONFIG_FSP_CAR) += temp_ram_exit.c postcar-$(CONFIG_FSP_CAR) += util.c postcar-$(CONFIG_DISPLAY_FSP_HEADER) += header_display.c diff --git a/src/drivers/intel/fsp2_0/stage_cache.c b/src/drivers/intel/fsp2_0/stage_cache.c deleted file mode 100644 index a9ec154..0000000 --- a/src/drivers/intel/fsp2_0/stage_cache.c +++ /dev/null @@ -1,29 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2013 Google Inc. - * Copyright (C) 2015 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 <console/console.h> -#include <fsp/memmap.h> -#include <stage_cache.h> -#include <program_loading.h> - -void stage_cache_external_region(void **base, size_t *size) -{ - if (smm_subregion(SMM_SUBREGION_CACHE, base, size)) { - printk(BIOS_ERR, "ERROR: No cache SMM subregion.\n"); - *base = NULL; - *size = 0; - } -} diff --git a/src/soc/intel/common/block/smm/smm.c b/src/soc/intel/common/block/smm/smm.c index 75b933e..38bd447 100644 --- a/src/soc/intel/common/block/smm/smm.c +++ b/src/soc/intel/common/block/smm/smm.c @@ -18,10 +18,21 @@ #include <bootstate.h> #include <console/console.h> #include <cpu/x86/smm.h> +#include <fsp/memmap.h> #include <intelblocks/pmclib.h> #include <intelblocks/smm.h> #include <intelblocks/systemagent.h> #include <soc/pm.h> +#include <stage_cache.h> + +void stage_cache_external_region(void **base, size_t *size) +{ + if (smm_subregion(SMM_SUBREGION_CACHE, base, size)) { + printk(BIOS_ERR, "ERROR: No cache SMM subregion.\n"); + *base = NULL; + *size = 0; + } +}
void smm_southbridge_clear_state(void) {
Hello Aaron Durbin, ron minnich, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34005
to look at the new patch set (#2).
Change subject: drivers/intel: Move FSP stage_cache implementation into common block ......................................................................
drivers/intel: Move FSP stage_cache implementation into common block
Change-Id: Iebb6d698c236a95162b3c7eb07987483a293b50a Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/drivers/intel/fsp2_0/Makefile.inc D src/drivers/intel/fsp2_0/stage_cache.c M src/soc/intel/common/block/smm/smm.c 3 files changed, 11 insertions(+), 32 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/34005/2
Hello Aaron Durbin, ron minnich, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34005
to look at the new patch set (#3).
Change subject: drivers/intel: Move FSP stage_cache implementation into common block ......................................................................
drivers/intel: Move FSP stage_cache implementation into common block
Change-Id: Iebb6d698c236a95162b3c7eb07987483a293b50a Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/drivers/intel/fsp2_0/Makefile.inc D src/drivers/intel/fsp2_0/stage_cache.c M src/soc/intel/common/block/smm/smm.c 3 files changed, 13 insertions(+), 32 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/34005/3
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34005 )
Change subject: drivers/intel: Move FSP stage_cache implementation into common block ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34005/3/src/soc/intel/common/block/... File src/soc/intel/common/block/smm/smm.c:
https://review.coreboot.org/c/coreboot/+/34005/3/src/soc/intel/common/block/... PS3, Line 28: !CONFIG(PLATFORM_USES_FSP1_1) Is this guard really required? Can we get rid of stage_cache_external_region() from drivers/intel/fsp1_1/ too?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34005 )
Change subject: drivers/intel: Move FSP stage_cache implementation into common block ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34005/3/src/soc/intel/common/block/... File src/soc/intel/common/block/smm/smm.c:
https://review.coreboot.org/c/coreboot/+/34005/3/src/soc/intel/common/block/... PS3, Line 28: !CONFIG(PLATFORM_USES_FSP1_1)
Is this guard really required? Can we get rid of stage_cache_external_region() from drivers/intel/fs […]
BSW being only user of FSP1.1 who doesn't support Intel common code hence unable to remove this code from FSP1.1 completely.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34005 )
Change subject: drivers/intel: Move FSP stage_cache implementation into common block ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34005/3/src/soc/intel/common/block/... File src/soc/intel/common/block/smm/smm.c:
https://review.coreboot.org/c/coreboot/+/34005/3/src/soc/intel/common/block/... PS3, Line 28: !CONFIG(PLATFORM_USES_FSP1_1)
BSW being only user of FSP1. […]
In that case, why is a guard needed here at all?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34005 )
Change subject: drivers/intel: Move FSP stage_cache implementation into common block ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34005/3/src/soc/intel/common/block/... File src/soc/intel/common/block/smm/smm.c:
https://review.coreboot.org/c/coreboot/+/34005/3/src/soc/intel/common/block/... PS3, Line 28: !CONFIG(PLATFORM_USES_FSP1_1)
In that case, why is a guard needed here at all?
SKL is still using FSP1.1 and it also usage intel common code
Error here : https://qa.coreboot.org/job/coreboot-gerrit/97623/#showFailuresLink
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34005 )
Change subject: drivers/intel: Move FSP stage_cache implementation into common block ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/34005/3/src/soc/intel/common/block/... File src/soc/intel/common/block/smm/smm.c:
https://review.coreboot.org/c/coreboot/+/34005/3/src/soc/intel/common/block/... PS3, Line 28: !CONFIG(PLATFORM_USES_FSP1_1)
SKL is still using FSP1.1 and it also usage intel common code […]
Aah I see.
Hello Aaron Durbin, ron minnich, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34005
to look at the new patch set (#4).
Change subject: drivers/intel: Move FSP stage_cache implementation into common block ......................................................................
drivers/intel: Move FSP stage_cache implementation into common block
Change-Id: Iebb6d698c236a95162b3c7eb07987483a293b50a Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/drivers/intel/fsp2_0/Makefile.inc D src/drivers/intel/fsp2_0/stage_cache.c M src/soc/intel/common/block/smm/smm.c 3 files changed, 13 insertions(+), 32 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/34005/4
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34005 )
Change subject: drivers/intel: Move FSP stage_cache implementation into common block ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34005/3/src/soc/intel/common/block/... File src/soc/intel/common/block/smm/smm.c:
https://review.coreboot.org/c/coreboot/+/34005/3/src/soc/intel/common/block/... PS3, Line 28: !CONFIG(PLATFORM_USES_FSP1_1)
Aah I see.
Ack
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34005 )
Change subject: drivers/intel: Move FSP stage_cache implementation into common block ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34005/3/src/soc/intel/common/block/... File src/soc/intel/common/block/smm/smm.c:
https://review.coreboot.org/c/coreboot/+/34005/3/src/soc/intel/common/block/... PS3, Line 28: !CONFIG(PLATFORM_USES_FSP1_1)
Ack
+2 lost over rebase :(
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34005 )
Change subject: drivers/intel: Move FSP stage_cache implementation into common block ......................................................................
Patch Set 4: Code-Review+2
Subrata Banik has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34005 )
Change subject: drivers/intel: Move FSP stage_cache implementation into common block ......................................................................
drivers/intel: Move FSP stage_cache implementation into common block
Change-Id: Iebb6d698c236a95162b3c7eb07987483a293b50a Signed-off-by: Subrata Banik subrata.banik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34005 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/drivers/intel/fsp2_0/Makefile.inc D src/drivers/intel/fsp2_0/stage_cache.c M src/soc/intel/common/block/smm/smm.c 3 files changed, 13 insertions(+), 32 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/drivers/intel/fsp2_0/Makefile.inc b/src/drivers/intel/fsp2_0/Makefile.inc index d627a3d..b0a1f2b 100644 --- a/src/drivers/intel/fsp2_0/Makefile.inc +++ b/src/drivers/intel/fsp2_0/Makefile.inc @@ -23,7 +23,6 @@ romstage-$(CONFIG_VERIFY_HOBS) += hob_verify.c romstage-y += util.c romstage-y += memory_init.c -romstage-$(CONFIG_CACHE_RELOCATED_RAMSTAGE_OUTSIDE_CBMEM) += stage_cache.c romstage-$(CONFIG_MMA) += mma_core.c
ramstage-y += debug.c @@ -34,12 +33,10 @@ ramstage-$(CONFIG_VERIFY_HOBS) += hob_verify.c ramstage-y += notify.c ramstage-y += silicon_init.c -ramstage-$(CONFIG_CACHE_RELOCATED_RAMSTAGE_OUTSIDE_CBMEM) += stage_cache.c ramstage-$(CONFIG_DISPLAY_UPD_DATA) += upd_display.c ramstage-y += util.c ramstage-$(CONFIG_MMA) += mma_core.c
-postcar-$(CONFIG_CACHE_RELOCATED_RAMSTAGE_OUTSIDE_CBMEM) += stage_cache.c postcar-$(CONFIG_FSP_CAR) += temp_ram_exit.c postcar-$(CONFIG_FSP_CAR) += util.c postcar-$(CONFIG_DISPLAY_FSP_HEADER) += header_display.c diff --git a/src/drivers/intel/fsp2_0/stage_cache.c b/src/drivers/intel/fsp2_0/stage_cache.c deleted file mode 100644 index a9ec154..0000000 --- a/src/drivers/intel/fsp2_0/stage_cache.c +++ /dev/null @@ -1,29 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2013 Google Inc. - * Copyright (C) 2015 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 <console/console.h> -#include <fsp/memmap.h> -#include <stage_cache.h> -#include <program_loading.h> - -void stage_cache_external_region(void **base, size_t *size) -{ - if (smm_subregion(SMM_SUBREGION_CACHE, base, size)) { - printk(BIOS_ERR, "ERROR: No cache SMM subregion.\n"); - *base = NULL; - *size = 0; - } -} diff --git a/src/soc/intel/common/block/smm/smm.c b/src/soc/intel/common/block/smm/smm.c index 75b933e..a2a7c16 100644 --- a/src/soc/intel/common/block/smm/smm.c +++ b/src/soc/intel/common/block/smm/smm.c @@ -18,10 +18,23 @@ #include <bootstate.h> #include <console/console.h> #include <cpu/x86/smm.h> +#include <fsp/memmap.h> #include <intelblocks/pmclib.h> #include <intelblocks/smm.h> #include <intelblocks/systemagent.h> #include <soc/pm.h> +#include <stage_cache.h> + +#if !CONFIG(PLATFORM_USES_FSP1_1) +void stage_cache_external_region(void **base, size_t *size) +{ + if (smm_subregion(SMM_SUBREGION_CACHE, base, size)) { + printk(BIOS_ERR, "ERROR: No cache SMM subregion.\n"); + *base = NULL; + *size = 0; + } +} +#endif
void smm_southbridge_clear_state(void) {
-
Furquan Shaikh (Code Review) -
Subrata Banik (Code Review)