[coreboot-gerrit] Patch set updated for coreboot: 47bc952 Remove CACHE_ROM.

Vladimir Serbinenko (phcoder@gmail.com) gerrit at coreboot.org
Mon Feb 24 21:27:37 CET 2014


Vladimir Serbinenko (phcoder at gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/5146

-gerrit

commit 47bc952d4c7c5fe396494801263482fb76b720d1
Author: Vladimir Serbinenko <phcoder at 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 coherency
    in 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 at 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



More information about the coreboot-gerrit mailing list