Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34807 )
Change subject: intel/haswell: Move platform_enter_postcar() ......................................................................
intel/haswell: Move platform_enter_postcar()
For consistency with remaining cpu/intel sources.
Change-Id: I1adde58966eae9205703b87e7aa17c50e5791a85 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/intel/haswell/romstage.c M src/northbridge/intel/haswell/memmap.c 2 files changed, 31 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/34807/1
diff --git a/src/cpu/intel/haswell/romstage.c b/src/cpu/intel/haswell/romstage.c index 47b9976..f5a1650 100644 --- a/src/cpu/intel/haswell/romstage.c +++ b/src/cpu/intel/haswell/romstage.c @@ -38,33 +38,6 @@ #include <cpu/intel/romstage.h> #include "haswell.h"
-/* platform_enter_postcar() determines the stack to use after - * cache-as-ram is torn down as well as the MTRR settings to use, - * and continues execution in postcar stage. */ -void platform_enter_postcar(void) -{ - struct postcar_frame pcf; - uintptr_t top_of_ram; - - if (postcar_frame_init(&pcf, 0)) - die("Unable to initialize postcar frame.\n"); - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(&pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(&pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - - /* Cache at least 8 MiB below the top of ram, and at most 8 MiB - * above top of the ram. This satisfies MTRR alignment requirement - * with different TSEG size configurations. - */ - top_of_ram = ALIGN_DOWN((uintptr_t)cbmem_top(), 8*MiB); - postcar_frame_add_mtrr(&pcf, top_of_ram - 8*MiB, 16*MiB, - MTRR_TYPE_WRBACK); - - run_postcar_phase(&pcf); -} - void romstage_common(const struct romstage_params *params) { int boot_mode; diff --git a/src/northbridge/intel/haswell/memmap.c b/src/northbridge/intel/haswell/memmap.c index 3a63afc..14b66a0 100644 --- a/src/northbridge/intel/haswell/memmap.c +++ b/src/northbridge/intel/haswell/memmap.c @@ -16,8 +16,12 @@ // Use simple device model for this file even in ramstage #define __SIMPLE_DEVICE__
+#include <arch/cpu.h> +#include <console/console.h> +#include <cpu/x86/mtrr.h> #include <device/pci_ops.h> #include <cbmem.h> +#include <cpu/intel/romstage.h> #include <stage_cache.h> #include "haswell.h"
@@ -48,3 +52,30 @@ *size = CONFIG_SMM_RESERVED_SIZE; *base = (void *)((uint32_t)cbmem_top() + RESERVED_SMM_OFFSET); } + +/* platform_enter_postcar() determines the stack to use after + * cache-as-ram is torn down as well as the MTRR settings to use, + * and continues execution in postcar stage. */ +void platform_enter_postcar(void) +{ + struct postcar_frame pcf; + uintptr_t top_of_ram; + + if (postcar_frame_init(&pcf, 0)) + die("Unable to initialize postcar frame.\n"); + /* Cache the ROM as WP just below 4GiB. */ + postcar_frame_add_romcache(&pcf, MTRR_TYPE_WRPROT); + + /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ + postcar_frame_add_mtrr(&pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); + + /* Cache at least 8 MiB below the top of ram, and at most 8 MiB + * above top of the ram. This satisfies MTRR alignment requirement + * with different TSEG size configurations. + */ + top_of_ram = ALIGN_DOWN((uintptr_t)cbmem_top(), 8*MiB); + postcar_frame_add_mtrr(&pcf, top_of_ram - 8*MiB, 16*MiB, + MTRR_TYPE_WRBACK); + + run_postcar_phase(&pcf); +}
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34807 )
Change subject: intel/haswell: Move platform_enter_postcar() ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/34807/1/src/cpu/intel/haswell/romst... File src/cpu/intel/haswell/romstage.c:
https://review.coreboot.org/c/coreboot/+/34807/1/src/cpu/intel/haswell/romst... PS1, Line 16: #include <stdint.h> : #include <console/console.h> : #include <arch/cpu.h> : #include <cf9_reset.h> : #include <cpu/x86/bist.h> : #include <cpu/x86/msr.h> : #include <cpu/x86/mtrr.h> : #include <timestamp.h> : #include <device/pci_def.h> : #include <cpu/x86/lapic.h> : #include <cbmem.h> : #include <commonlib/helpers.h> : #include <program_loading.h> : #include <romstage_handoff.h> : #include <vendorcode/google/chromeos/chromeos.h> : #if CONFIG(EC_GOOGLE_CHROMEEC) : #include <ec/google/chromeec/ec.h> : #endif : #include <northbridge/intel/haswell/haswell.h> : #include <northbridge/intel/haswell/raminit.h> : #include <southbridge/intel/lynxpoint/pch.h> : #include <southbridge/intel/lynxpoint/me.h> : #include <cpu/intel/romstage.h> : #include "haswell.h" It looks like this leaves us with more #includes than statements in the code (I didn't count). Please remove at least those that were only used by platform_enter_postcar(). I wouldn't mind to drop some more, though (stdint? msr? pci? chromeos?).
Hello Patrick Rudolph, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34807
to look at the new patch set (#2).
Change subject: intel/haswell: Move platform_enter_postcar() ......................................................................
intel/haswell: Move platform_enter_postcar()
For consistency with remaining cpu/intel sources.
Change-Id: I1adde58966eae9205703b87e7aa17c50e5791a85 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/intel/haswell/romstage.c M src/northbridge/intel/haswell/memmap.c 2 files changed, 31 insertions(+), 33 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/34807/2
Hello Patrick Rudolph, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34807
to look at the new patch set (#3).
Change subject: intel/haswell: Move platform_enter_postcar() ......................................................................
intel/haswell: Move platform_enter_postcar()
Do this for consistency with remaining cpu/intel sources. Also wipe out some spurious includes.
Change-Id: I1adde58966eae9205703b87e7aa17c50e5791a85 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/intel/haswell/romstage.c M src/northbridge/intel/haswell/memmap.c 2 files changed, 31 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/34807/3
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34807 )
Change subject: intel/haswell: Move platform_enter_postcar() ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34807/1/src/cpu/intel/haswell/romst... File src/cpu/intel/haswell/romstage.c:
https://review.coreboot.org/c/coreboot/+/34807/1/src/cpu/intel/haswell/romst... PS1, Line 16: #include <stdint.h> : #include <console/console.h> : #include <arch/cpu.h> : #include <cf9_reset.h> : #include <cpu/x86/bist.h> : #include <cpu/x86/msr.h> : #include <cpu/x86/mtrr.h> : #include <timestamp.h> : #include <device/pci_def.h> : #include <cpu/x86/lapic.h> : #include <cbmem.h> : #include <commonlib/helpers.h> : #include <program_loading.h> : #include <romstage_handoff.h> : #include <vendorcode/google/chromeos/chromeos.h> : #if CONFIG(EC_GOOGLE_CHROMEEC) : #include <ec/google/chromeec/ec.h> : #endif : #include <northbridge/intel/haswell/haswell.h> : #include <northbridge/intel/haswell/raminit.h> : #include <southbridge/intel/lynxpoint/pch.h> : #include <southbridge/intel/lynxpoint/me.h> : #include <cpu/intel/romstage.h> : #include "haswell.h"
It looks like this leaves us with more #includes than statements […]
Thanks
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34807 )
Change subject: intel/haswell: Move platform_enter_postcar() ......................................................................
Patch Set 3: Code-Review+2
Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34807 )
Change subject: intel/haswell: Move platform_enter_postcar() ......................................................................
intel/haswell: Move platform_enter_postcar()
Do this for consistency with remaining cpu/intel sources. Also wipe out some spurious includes.
Change-Id: I1adde58966eae9205703b87e7aa17c50e5791a85 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34807 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/cpu/intel/haswell/romstage.c M src/northbridge/intel/haswell/memmap.c 2 files changed, 31 insertions(+), 37 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/cpu/intel/haswell/romstage.c b/src/cpu/intel/haswell/romstage.c index 47b9976..544a93f 100644 --- a/src/cpu/intel/haswell/romstage.c +++ b/src/cpu/intel/haswell/romstage.c @@ -15,56 +15,19 @@
#include <stdint.h> #include <console/console.h> -#include <arch/cpu.h> #include <cf9_reset.h> #include <cpu/x86/bist.h> -#include <cpu/x86/msr.h> -#include <cpu/x86/mtrr.h> #include <timestamp.h> -#include <device/pci_def.h> #include <cpu/x86/lapic.h> #include <cbmem.h> #include <commonlib/helpers.h> -#include <program_loading.h> #include <romstage_handoff.h> -#include <vendorcode/google/chromeos/chromeos.h> -#if CONFIG(EC_GOOGLE_CHROMEEC) -#include <ec/google/chromeec/ec.h> -#endif #include <northbridge/intel/haswell/haswell.h> #include <northbridge/intel/haswell/raminit.h> #include <southbridge/intel/lynxpoint/pch.h> #include <southbridge/intel/lynxpoint/me.h> -#include <cpu/intel/romstage.h> #include "haswell.h"
-/* platform_enter_postcar() determines the stack to use after - * cache-as-ram is torn down as well as the MTRR settings to use, - * and continues execution in postcar stage. */ -void platform_enter_postcar(void) -{ - struct postcar_frame pcf; - uintptr_t top_of_ram; - - if (postcar_frame_init(&pcf, 0)) - die("Unable to initialize postcar frame.\n"); - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(&pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(&pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - - /* Cache at least 8 MiB below the top of ram, and at most 8 MiB - * above top of the ram. This satisfies MTRR alignment requirement - * with different TSEG size configurations. - */ - top_of_ram = ALIGN_DOWN((uintptr_t)cbmem_top(), 8*MiB); - postcar_frame_add_mtrr(&pcf, top_of_ram - 8*MiB, 16*MiB, - MTRR_TYPE_WRBACK); - - run_postcar_phase(&pcf); -} - void romstage_common(const struct romstage_params *params) { int boot_mode; diff --git a/src/northbridge/intel/haswell/memmap.c b/src/northbridge/intel/haswell/memmap.c index 3a63afc..14b66a0 100644 --- a/src/northbridge/intel/haswell/memmap.c +++ b/src/northbridge/intel/haswell/memmap.c @@ -16,8 +16,12 @@ // Use simple device model for this file even in ramstage #define __SIMPLE_DEVICE__
+#include <arch/cpu.h> +#include <console/console.h> +#include <cpu/x86/mtrr.h> #include <device/pci_ops.h> #include <cbmem.h> +#include <cpu/intel/romstage.h> #include <stage_cache.h> #include "haswell.h"
@@ -48,3 +52,30 @@ *size = CONFIG_SMM_RESERVED_SIZE; *base = (void *)((uint32_t)cbmem_top() + RESERVED_SMM_OFFSET); } + +/* platform_enter_postcar() determines the stack to use after + * cache-as-ram is torn down as well as the MTRR settings to use, + * and continues execution in postcar stage. */ +void platform_enter_postcar(void) +{ + struct postcar_frame pcf; + uintptr_t top_of_ram; + + if (postcar_frame_init(&pcf, 0)) + die("Unable to initialize postcar frame.\n"); + /* Cache the ROM as WP just below 4GiB. */ + postcar_frame_add_romcache(&pcf, MTRR_TYPE_WRPROT); + + /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ + postcar_frame_add_mtrr(&pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); + + /* Cache at least 8 MiB below the top of ram, and at most 8 MiB + * above top of the ram. This satisfies MTRR alignment requirement + * with different TSEG size configurations. + */ + top_of_ram = ALIGN_DOWN((uintptr_t)cbmem_top(), 8*MiB); + postcar_frame_add_mtrr(&pcf, top_of_ram - 8*MiB, 16*MiB, + MTRR_TYPE_WRBACK); + + run_postcar_phase(&pcf); +}
-
Kyösti Mälkki (Code Review) -
Nico Huber (Code Review)