Vladimir Serbinenko (phcoder@gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/5146
-gerrit
commit b7b0f8f4be4002841e13be8cee4ca0ad7dbab9ef Author: Vladimir Serbinenko phcoder@gmail.com Date: Wed Feb 5 19:46:45 2014 +0100
Remove CACHE_ROM.
With the recent improvement 3d6ffe76f8a505c2dff5d5c6146da3d63dad6e82, speedup by CACHE_ROM is reduced a lot. On the other hand this makes coreboot run out of MTRRs depending on system configuration, hence screwing up I/O access and cache coherencyin worst cases.
CACHE_ROM requires the user to sanity check their boot output because the feature is brittle. The working configuration is dependent on I/O hole size, ram size, and chipset. Because of this the current implementation can leave a system configured in an inconsistent state leading to unexpected results such as poor performance and/or inconsistent cache-coherency
Remove this as a buggy feature until we figure out how to do it properly if necessary.
Change-Id: I858d78a907bf042fcc21fdf7a2bf899e9f6b591d Signed-off-by: Vladimir Serbinenko phcoder@gmail.com --- src/cpu/intel/haswell/haswell_init.c | 3 - src/cpu/intel/haswell/mp_init.c | 15 ----- src/cpu/x86/Kconfig | 7 --- src/cpu/x86/mtrr/mtrr.c | 76 ------------------------- src/include/cpu/x86/mtrr.h | 23 -------- src/lib/coreboot_table.c | 20 ------- src/mainboard/google/Kconfig | 5 -- src/mainboard/google/bolt/Kconfig | 1 - src/mainboard/google/falco/Kconfig | 1 - src/mainboard/google/peppy/Kconfig | 1 - src/mainboard/google/slippy/Kconfig | 1 - src/mainboard/intel/wtm2/Kconfig | 1 - src/northbridge/intel/fsp_sandybridge/Kconfig | 4 -- src/northbridge/intel/nehalem/northbridge.c | 2 - src/northbridge/intel/sandybridge/northbridge.c | 2 - src/soc/intel/baytrail/Kconfig | 1 - 16 files changed, 163 deletions(-)
diff --git a/src/cpu/intel/haswell/haswell_init.c b/src/cpu/intel/haswell/haswell_init.c index dc6012b..b7bea20 100644 --- a/src/cpu/intel/haswell/haswell_init.c +++ b/src/cpu/intel/haswell/haswell_init.c @@ -789,9 +789,6 @@ void bsp_init_and_start_aps(struct bus *cpu_bus)
/* Restore the default SMM region. */ restore_default_smm_area(smm_save_area); - - /* Enable ROM caching if option was selected. */ - x86_mtrr_enable_rom_caching(); }
static struct device_operations cpu_dev_ops = { diff --git a/src/cpu/intel/haswell/mp_init.c b/src/cpu/intel/haswell/mp_init.c index 51130a5..0398360 100644 --- a/src/cpu/intel/haswell/mp_init.c +++ b/src/cpu/intel/haswell/mp_init.c @@ -143,14 +143,6 @@ void release_aps_for_smm_relocation(int do_parallel) printk(BIOS_DEBUG, "Timed out waiting for AP SMM relocation\n"); }
-/* The mtrr code sets up ROM caching on the BSP, but not the others. However, - * the boot loader payload disables this. In order for Linux not to complain - * ensure the caching is disabled for the APs before going to sleep. */ -static void cleanup_rom_caching(void) -{ - x86_mtrr_disable_rom_caching(); -} - /* By the time APs call ap_init() caching has been setup, and microcode has * been loaded. */ static void asmlinkage ap_init(unsigned int cpu, void *microcode_ptr) @@ -184,13 +176,6 @@ static void asmlinkage ap_init(unsigned int cpu, void *microcode_ptr) /* After SMM relocation a 2nd microcode load is required. */ intel_microcode_load_unlocked(microcode_ptr);
- /* The MTRR resources are core scoped. Therefore, there is no need - * to do the same work twice. Additionally, this check keeps the - * ROM cache enabled on the BSP since its hyperthread sibling won't - * call cleanup_rom_caching(). */ - if ((lapicid() & 1) == 0) - cleanup_rom_caching(); - /* FIXME(adurbin): park CPUs properly -- preferably somewhere in a * reserved part of memory that the OS cannot get to. */ stop_this_cpu(); diff --git a/src/cpu/x86/Kconfig b/src/cpu/x86/Kconfig index 98238d8..19fa246 100644 --- a/src/cpu/x86/Kconfig +++ b/src/cpu/x86/Kconfig @@ -76,13 +76,6 @@ config LOGICAL_CPUS bool default y
-config CACHE_ROM - bool "Allow for caching system ROM." - default n - help - When selected a variable range MTRR is allocated for coreboot and - the bootloader enables caching of the system ROM for faster access. - config SMM_TSEG bool default n diff --git a/src/cpu/x86/mtrr/mtrr.c b/src/cpu/x86/mtrr/mtrr.c index d85a869..86e31d0 100644 --- a/src/cpu/x86/mtrr/mtrr.c +++ b/src/cpu/x86/mtrr/mtrr.c @@ -199,16 +199,6 @@ static struct memranges *get_physical_address_space(void) memranges_add_resources_filter(addr_space, mask, match, MTRR_TYPE_WRCOMB, filter_vga_wrcomb);
-#if CONFIG_CACHE_ROM - /* Add a write-protect region covering the ROM size - * when CONFIG_CACHE_ROM is enabled. The ROM is assumed - * to be located at 4GiB - rom size. */ - resource_t rom_base = RANGE_TO_PHYS_ADDR( - RANGE_4GB - PHYS_TO_RANGE_ADDR(CACHE_ROM_SIZE)); - memranges_insert(addr_space, rom_base, CACHE_ROM_SIZE, - MTRR_TYPE_WRPROT); -#endif - /* The address space below 4GiB is special. It needs to be * covered entirly by range entries so that MTRR calculations * can be properly done for the full 32-bit address space. @@ -380,61 +370,6 @@ void x86_setup_fixed_mtrrs(void) enable_fixed_mtrr(); }
-/* Keep track of the MTRR that covers the ROM for caching purposes. */ -#if CONFIG_CACHE_ROM -static long rom_cache_mtrr = -1; - -long x86_mtrr_rom_cache_var_index(void) -{ - return rom_cache_mtrr; -} - -void x86_mtrr_enable_rom_caching(void) -{ - msr_t msr_val; - unsigned long index; - - if (rom_cache_mtrr < 0) - return; - - index = rom_cache_mtrr; - disable_cache(); - msr_val = rdmsr(MTRRphysBase_MSR(index)); - msr_val.lo &= ~0xff; - msr_val.lo |= MTRR_TYPE_WRPROT; - wrmsr(MTRRphysBase_MSR(index), msr_val); - enable_cache(); -} - -void x86_mtrr_disable_rom_caching(void) -{ - msr_t msr_val; - unsigned long index; - - if (rom_cache_mtrr < 0) - return; - - index = rom_cache_mtrr; - disable_cache(); - msr_val = rdmsr(MTRRphysBase_MSR(index)); - msr_val.lo &= ~0xff; - wrmsr(MTRRphysBase_MSR(index), msr_val); - enable_cache(); -} - -static void disable_cache_rom(void *unused) -{ - x86_mtrr_disable_rom_caching(); -} - -BOOT_STATE_INIT_ENTRIES(disable_rom_cache_bscb) = { - BOOT_STATE_INIT_ENTRY(BS_OS_RESUME, BS_ON_ENTRY, - disable_cache_rom, NULL), - BOOT_STATE_INIT_ENTRY(BS_PAYLOAD_LOAD, BS_ON_EXIT, - disable_cache_rom, NULL), -}; -#endif - struct var_mtrr_state { struct memranges *addr_space; int above4gb; @@ -482,17 +417,6 @@ static void write_var_mtrr(struct var_mtrr_state *var_state, mask = (1ULL << var_state->address_bits) - 1; rsize = rsize & mask;
-#if CONFIG_CACHE_ROM - /* CONFIG_CACHE_ROM allocates an MTRR specifically for allowing - * one to turn on caching for faster ROM access. However, it is - * left to the MTRR callers to enable it. */ - if (mtrr_type == MTRR_TYPE_WRPROT) { - mtrr_type = MTRR_TYPE_UNCACHEABLE; - if (rom_cache_mtrr < 0) - rom_cache_mtrr = var_state->mtrr_index; - } -#endif - printk(BIOS_DEBUG, "MTRR: %d base 0x%016llx mask 0x%016llx type %d\n", var_state->mtrr_index, rbase, rsize, mtrr_type);
diff --git a/src/include/cpu/x86/mtrr.h b/src/include/cpu/x86/mtrr.h index 9414687..2a44072 100644 --- a/src/include/cpu/x86/mtrr.h +++ b/src/include/cpu/x86/mtrr.h @@ -52,10 +52,6 @@ * of the nature of the global MTRR enable flag. Therefore, all direct * or indirect callers of enable_fixed_mtrr() should ensure that the * variable MTRR MSRs do not contain bad ranges. - * 3. If CONFIG_CACHE_ROM is selected an MTRR is allocated for enabling - * the caching of the ROM. However, it is set to uncacheable (UC). It - * is the responsibility of the caller to enable it by calling - * x86_mtrr_enable_rom_caching(). */ void x86_setup_mtrrs(void); /* @@ -71,25 +67,6 @@ void x86_setup_fixed_mtrrs(void); /* Set up fixed MTRRs but do not enable them. */ void x86_setup_fixed_mtrrs_no_enable(void); int x86_mtrr_check(void); -/* ROM caching can be used after variable MTRRs are set up. Beware that - * enabling CONFIG_CACHE_ROM will eat through quite a few MTRRs based on - * one's IO hole size and WRCOMB resources. Be sure to check the console - * log when enabling CONFIG_CACHE_ROM or adding WRCOMB resources. Beware that - * on CPUs with core-scoped MTRR registers such as hyperthreaded CPUs the - * rom caching will be disabled if all threads run the MTRR code. Therefore, - * one needs to call x86_mtrr_enable_rom_caching() after all threads of the - * same core have run the MTRR code. */ -#if CONFIG_CACHE_ROM -void x86_mtrr_enable_rom_caching(void); -void x86_mtrr_disable_rom_caching(void); -/* Return the variable range MTRR index of the ROM cache. */ -long x86_mtrr_rom_cache_var_index(void); -#else -static inline void x86_mtrr_enable_rom_caching(void) {} -static inline void x86_mtrr_disable_rom_caching(void) {} -static inline long x86_mtrr_rom_cache_var_index(void) { return -1; } -#endif /* CONFIG_CACHE_ROM */ - #endif
#if !defined(__ASSEMBLER__) && defined(__PRE_RAM__) && !defined(__ROMCC__) diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c index d5dc14c..904185a 100644 --- a/src/lib/coreboot_table.c +++ b/src/lib/coreboot_table.c @@ -235,24 +235,6 @@ static inline void lb_vboot_handoff(struct lb_header *header) {} #endif /* CONFIG_VBOOT_VERIFY_FIRMWARE */ #endif /* CONFIG_CHROMEOS */
-static void lb_x86_rom_cache(struct lb_header *header) -{ -#if CONFIG_ARCH_X86 - long mtrr_index; - struct lb_x86_rom_mtrr *lb_x86_rom_mtrr; - - mtrr_index = x86_mtrr_rom_cache_var_index(); - - if (mtrr_index < 0) - return; - - lb_x86_rom_mtrr = (struct lb_x86_rom_mtrr *)lb_new_record(header); - lb_x86_rom_mtrr->tag = LB_TAG_X86_ROM_MTRR; - lb_x86_rom_mtrr->size = sizeof(struct lb_x86_rom_mtrr); - lb_x86_rom_mtrr->index = mtrr_index; -#endif -} - static void add_cbmem_pointers(struct lb_header *header) { /* @@ -529,8 +511,6 @@ unsigned long write_coreboot_table( lb_strings(head); /* Record our framebuffer */ lb_framebuffer(head); - /* Communicate x86 variable MTRR ROM cache information. */ - lb_x86_rom_cache(head);
#if CONFIG_CHROMEOS /* Record our GPIO settings (ChromeOS specific) */ diff --git a/src/mainboard/google/Kconfig b/src/mainboard/google/Kconfig index b86b0ad..85c8592 100644 --- a/src/mainboard/google/Kconfig +++ b/src/mainboard/google/Kconfig @@ -18,11 +18,6 @@ ## if VENDOR_GOOGLE
-# Auto select common options -config VENDOR_SPECIFIC_OPTIONS - def_bool y - select CACHE_ROM - choice prompt "Mainboard model"
diff --git a/src/mainboard/google/bolt/Kconfig b/src/mainboard/google/bolt/Kconfig index 5247070..47d41ed 100644 --- a/src/mainboard/google/bolt/Kconfig +++ b/src/mainboard/google/bolt/Kconfig @@ -18,7 +18,6 @@ config BOARD_SPECIFIC_OPTIONS # dummy select HAVE_SMI_HANDLER select MAINBOARD_HAS_CHROMEOS select EXTERNAL_MRC_BLOB - select CACHE_ROM select MONOTONIC_TIMER_MSR
config VBOOT_RAMSTAGE_INDEX diff --git a/src/mainboard/google/falco/Kconfig b/src/mainboard/google/falco/Kconfig index d800206..5d19ffc 100644 --- a/src/mainboard/google/falco/Kconfig +++ b/src/mainboard/google/falco/Kconfig @@ -18,7 +18,6 @@ config BOARD_SPECIFIC_OPTIONS # dummy select HAVE_SMI_HANDLER select MAINBOARD_HAS_CHROMEOS select EXTERNAL_MRC_BLOB - select CACHE_ROM select MONOTONIC_TIMER_MSR select DRIVERS_I2C_RTD2132
diff --git a/src/mainboard/google/peppy/Kconfig b/src/mainboard/google/peppy/Kconfig index 6cac476..f401feb 100644 --- a/src/mainboard/google/peppy/Kconfig +++ b/src/mainboard/google/peppy/Kconfig @@ -18,7 +18,6 @@ config BOARD_SPECIFIC_OPTIONS # dummy select HAVE_SMI_HANDLER select MAINBOARD_HAS_CHROMEOS select EXTERNAL_MRC_BLOB - select CACHE_ROM select MONOTONIC_TIMER_MSR
config VBOOT_RAMSTAGE_INDEX diff --git a/src/mainboard/google/slippy/Kconfig b/src/mainboard/google/slippy/Kconfig index 6980234..ba6a6fe 100644 --- a/src/mainboard/google/slippy/Kconfig +++ b/src/mainboard/google/slippy/Kconfig @@ -18,7 +18,6 @@ config BOARD_SPECIFIC_OPTIONS # dummy select HAVE_SMI_HANDLER select MAINBOARD_HAS_CHROMEOS select EXTERNAL_MRC_BLOB - select CACHE_ROM select MONOTONIC_TIMER_MSR select MAINBOARD_HAS_NATIVE_VGA_INIT select MAINBOARD_DO_NATIVE_VGA_INIT diff --git a/src/mainboard/intel/wtm2/Kconfig b/src/mainboard/intel/wtm2/Kconfig index 8e13108..00ea604 100644 --- a/src/mainboard/intel/wtm2/Kconfig +++ b/src/mainboard/intel/wtm2/Kconfig @@ -13,7 +13,6 @@ config BOARD_SPECIFIC_OPTIONS # dummy select HAVE_ACPI_RESUME select HAVE_SMI_HANDLER select MAINBOARD_HAS_CHROMEOS - select CACHE_ROM select MAINBOARD_HAS_NATIVE_VGA_INIT select MONOTONIC_TIMER_MSR
diff --git a/src/northbridge/intel/fsp_sandybridge/Kconfig b/src/northbridge/intel/fsp_sandybridge/Kconfig index 0c3d5f5..ce366ef 100644 --- a/src/northbridge/intel/fsp_sandybridge/Kconfig +++ b/src/northbridge/intel/fsp_sandybridge/Kconfig @@ -87,14 +87,10 @@ config CBFS_SIZE config ENABLE_FAST_BOOT bool "Enable Fast Boot" default y if CPU_INTEL_FSP_MODEL_306AX && SOUTHBRIDGE_INTEL_FSP_BD82X6X - depends on !CACHE_ROM help Enabling this feature will cause MRC data to be cached in NV storage which will speed up boot time on future reboots and/or power cycles.
- WARNING: This feature combined with the CACHE_ROM may result in undefined - behavior. - config MRC_CACHE_SIZE hex "MRC Data Cache Size" default 0x10000 diff --git a/src/northbridge/intel/nehalem/northbridge.c b/src/northbridge/intel/nehalem/northbridge.c index f9386de..d94bc09 100644 --- a/src/northbridge/intel/nehalem/northbridge.c +++ b/src/northbridge/intel/nehalem/northbridge.c @@ -340,8 +340,6 @@ static const struct pci_driver mc_driver_44 __pci_driver = { static void cpu_bus_init(device_t dev) { initialize_cpus(dev->link_list); - /* Enable ROM caching if option was selected. */ - x86_mtrr_enable_rom_caching(); }
static void cpu_bus_noop(device_t dev) diff --git a/src/northbridge/intel/sandybridge/northbridge.c b/src/northbridge/intel/sandybridge/northbridge.c index 7db9301..5440140 100644 --- a/src/northbridge/intel/sandybridge/northbridge.c +++ b/src/northbridge/intel/sandybridge/northbridge.c @@ -476,8 +476,6 @@ static const struct pci_driver mc_driver_1 __pci_driver = { static void cpu_bus_init(device_t dev) { initialize_cpus(dev->link_list); - /* Enable ROM caching if option was selected. */ - x86_mtrr_enable_rom_caching(); }
static void cpu_bus_noop(device_t dev) diff --git a/src/soc/intel/baytrail/Kconfig b/src/soc/intel/baytrail/Kconfig index d00acaa..2360b36 100644 --- a/src/soc/intel/baytrail/Kconfig +++ b/src/soc/intel/baytrail/Kconfig @@ -9,7 +9,6 @@ if SOC_INTEL_BAYTRAIL config CPU_SPECIFIC_OPTIONS def_bool y select CACHE_MRC_SETTINGS - select CACHE_ROM select CAR_MIGRATION select COLLECT_TIMESTAMPS select CPU_MICROCODE_IN_CBFS