Change in coreboot[master]: soc/intel/common: Make use of clflush in common platform_segment_loaded
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34992 )
Change subject: soc/intel/common: Make use of clflush in common platform_segment_loaded ......................................................................
soc/intel/common: Make use of clflush in common platform_segment_loaded
This patch clear cache lines based on platform_segment_loaded() supplied start and size values before loading the targeted stage.
This changes is required to fix hang issues appeared due to marking DRAM ranges as WB (CONFIG_MARK_DRAM_CACHE_WB) to speed up next stage loading/ decompression/execution time.
Idea is to run clflush on those ranges just before tearing down the CAR (running invd instruction) and after that postcar frame will set up new MTRR ranges.
Change-Id: I591f21cb4199477af271f0dd6c073e1c11831bfd Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/Makefile.inc A src/soc/intel/common/block/cpu/car/car.c 3 files changed, 71 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/34992/1
diff --git a/src/soc/intel/common/block/cpu/Kconfig b/src/soc/intel/common/block/cpu/Kconfig index 8cc572d..0d1506b 100644 --- a/src/soc/intel/common/block/cpu/Kconfig +++ b/src/soc/intel/common/block/cpu/Kconfig @@ -66,3 +66,15 @@ help This option allows FSP to make use of MP services PPI published by coreboot to perform multiprocessor initialization. + +config MARK_DRAM_CACHE_WB + bool + default n + help + This option allows you to select how DRAM intermediate cache is set up. + Till discovering DRAM ranges, system will make use of CAR and CAR tear + down will handle by postcar/ramstage, that means entire postcar/ramstage + stage will execute from UC range. Intention here is to optimize the boot + flow hence enabling the caching for applicable DRAM ranges before CAR + tear down and setting up new DRAM based MTRR range. MTRR type WB + provides best optimization. diff --git a/src/soc/intel/common/block/cpu/Makefile.inc b/src/soc/intel/common/block/cpu/Makefile.inc index a6c4f37..63a2ac9 100644 --- a/src/soc/intel/common/block/cpu/Makefile.inc +++ b/src/soc/intel/common/block/cpu/Makefile.inc @@ -5,6 +5,7 @@
romstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += car/exit_car.S romstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += cpulib.c +romstage-$(CONFIG_MARK_DRAM_CACHE_WB) += car.c
postcar-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += car/exit_car.S postcar-$(CONFIG_FSP_CAR) += car/exit_car_fsp.S diff --git a/src/soc/intel/common/block/cpu/car/car.c b/src/soc/intel/common/block/cpu/car/car.c new file mode 100644 index 0000000..82d8e9a --- /dev/null +++ b/src/soc/intel/common/block/cpu/car/car.c @@ -0,0 +1,58 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 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; either version 2 of the License, or + * (at your option) any later version. + * + * 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 <assert.h> +#include <cbmem.h> +#include <cpu/x86/cache.h> +#include <program_loading.h> + +static inline int is_usable_dram_addr(uintptr_t addr) +{ + return (addr < (uintptr_t) cbmem_top()); +} + +/* + * CLFLUSH the impacted WB'ed cache lines before loading postcar/ramstage + * in order to avoid getting stuck while tearing down (invd) the CAR. + */ +static void flush_cache(uintptr_t start, size_t size) +{ + uintptr_t end; + uintptr_t addr; + + end = start + (ALIGN_DOWN(size + 4096, 4096)); + for (addr = start; addr < end; addr += 64) + clflush((void *)addr); +} + +void platform_segment_loaded(uintptr_t start, size_t size, int flags) +{ + /* Bail out if this is not the final segment. */ + if (!(flags & SEG_FINAL)) + return; + + char start_dram_check = is_usable_dram_addr(start); + char end_dram_check = is_usable_dram_addr(start + size - 1); + + /* + * Bail out if loaded program segment does not lie in + * usable DRAM region. + */ + if (!start_dram_check && !end_dram_check) + return; + + flush_cache(start, size); +}
Hello Patrick Rudolph, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34992
to look at the new patch set (#2).
Change subject: soc/intel/common: Make use of clflush in common platform_segment_loaded ......................................................................
soc/intel/common: Make use of clflush in common platform_segment_loaded
This patch clear cache lines based on platform_segment_loaded() supplied start and size values before loading the targeted stage.
This changes is required to fix hang issues appeared due to marking DRAM ranges as WB (CONFIG_MARK_DRAM_CACHE_WB) to speed up next stage loading/ decompression/execution time.
Idea is to run clflush on those ranges just before tearing down the CAR (running invd instruction) and after that postcar frame will set up new MTRR ranges.
Change-Id: I591f21cb4199477af271f0dd6c073e1c11831bfd Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/Makefile.inc A src/soc/intel/common/block/cpu/car/car.c 3 files changed, 71 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/34992/2
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34992 )
Change subject: soc/intel/common: Make use of clflush in common platform_segment_loaded ......................................................................
Patch Set 2:
(3 comments)
Move to cpu/x86? I don't think it is Intel specific.
https://review.coreboot.org/c/coreboot/+/34992/2/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/car/car.c:
https://review.coreboot.org/c/coreboot/+/34992/2/src/soc/intel/common/block/... PS2, Line 36: ALIGN_DOWN(size + 4096, 4096) What is this about?
https://review.coreboot.org/c/coreboot/+/34992/2/src/soc/intel/common/block/... PS2, Line 37: 64 you can get cacheline size from cpuid.
https://review.coreboot.org/c/coreboot/+/34992/2/src/soc/intel/common/block/... PS2, Line 38: clflush Check for cflush in cpuid?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34992 )
Change subject: soc/intel/common: Make use of clflush in common platform_segment_loaded ......................................................................
Patch Set 2:
Patch Set 2:
(3 comments)
Move to cpu/x86? I don't think it is Intel specific.
this is to enable WB capability in caching between romstage end and postcar. you can follow the topic CLs
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34992 )
Change subject: soc/intel/common: Make use of clflush in common platform_segment_loaded ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
(3 comments)
Move to cpu/x86? I don't think it is Intel specific.
this is to enable WB capability in caching between romstage end and postcar. you can follow the topic CLs
I understand what these CL are doing and I think it's more generally useable than soc/intel. I have some local patches that lz4 compress postcar which should further speed things up, but for performance reasons decompression needs to happen WB cached.
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/34992?usp=email )
Change subject: soc/intel/common: Make use of clflush in common platform_segment_loaded ......................................................................
Abandoned
This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.
-
Arthur Heymans (Code Review) -
Martin L Roth (Code Review)
-
Subrata Banik (Code Review)