From 6fcc46d1a41d8818e1dc791cadd173fd05afe54b Mon Sep 17 00:00:00 2001 From: Tim Wawrzynczak Date: Mon, 19 Apr 2021 13:47:36 -0600 Subject: [PATCH] cpu/x86/mtrr: Use a Kconfig for reserving MTRRs for OS Some platforms which have large amounts of RAM and also write-combining regions may decide to drop the WC regions in favor of the default when preserving MTRRs for the OS. From a data safety perspective, this is safe to do, but if, say, the graphics framebuffer is the region that is changed from WC to UC/WB, then the performance of writing to the framebuffer will decrease dramatically. Modern OSes typically use Page Attribute Tables (PAT) to determine the cacheability on a page level and usually do not touch the MTRRs. Thus, it is believed to be safe to stop reserving MTRRs for the OS, in general; PentiumII is the exception here in that OSes that still support that may still require MTRRs to be available. In any case, if the OS wants to reprogram all of the MTRRs, it is of course still free to do so (after consulting the e820 table). BUG=b:185452338 TEST=Verify MTRR programming on a brya (where `sa_add_dram_resources` was faked to think it had 32 GiB of DRAM installed) and variable MTRR map includes a WC entry for the framebuffer (and all the RAM): MTRR: default type WB/UC MTRR counts: 13/9. MTRR: UC selected as default type. MTRR: 0 base 0x0000000000000000 mask 0x00003fff80000000 type 6 MTRR: 1 base 0x0000000077000000 mask 0x00003fffff000000 type 0 MTRR: 2 base 0x0000000078000000 mask 0x00003ffff8000000 type 0 MTRR: 3 base 0x0000000090000000 mask 0x00003ffff0000000 type 1 MTRR: 4 base 0x0000000100000000 mask 0x00003fff00000000 type 6 MTRR: 5 base 0x0000000200000000 mask 0x00003ffe00000000 type 6 MTRR: 6 base 0x0000000400000000 mask 0x00003ffc00000000 type 6 MTRR: 7 base 0x0000000800000000 mask 0x00003fff80000000 type 6 MTRR: 8 base 0x000000087fc00000 mask 0x00003fffffc00000 type 0 ADL has 9 variable-range MTRRs, previously 8 of them were used, and there was no separate entry for the framebuffer, thus leaving the default MTRR in place of uncached. Change-Id: I2ae2851248c95fd516627b101ebcb36ec59c29c3 Signed-off-by: Tim Wawrzynczak Reviewed-on: https://review.coreboot.org/c/coreboot/+/52522 Tested-by: build bot (Jenkins) Reviewed-by: Angel Pons Reviewed-by: Furquan Shaikh --- src/cpu/intel/slot_1/Kconfig | 1 + src/cpu/x86/Kconfig | 10 ++++++++++ src/cpu/x86/mtrr/mtrr.c | 31 +++++++++++++++++++------------ 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/cpu/intel/slot_1/Kconfig b/src/cpu/intel/slot_1/Kconfig index cc24833fcf..c30e066444 100644 --- a/src/cpu/intel/slot_1/Kconfig +++ b/src/cpu/intel/slot_1/Kconfig @@ -17,6 +17,7 @@ config SLOT_SPECIFIC_OPTIONS select TSC_MONOTONIC_TIMER select UNKNOWN_TSC_RATE select SETUP_XIP_CACHE + select RESERVE_MTRRS_FOR_OS config DCACHE_RAM_BASE hex diff --git a/src/cpu/x86/Kconfig b/src/cpu/x86/Kconfig index 5394cd023d..bcaf0bfad7 100644 --- a/src/cpu/x86/Kconfig +++ b/src/cpu/x86/Kconfig @@ -160,3 +160,13 @@ config SOC_SETS_MSRS help The SoC requires different access methods for reading and writing the MSRs. Use SoC specific routines to handle the MSR access. + +config RESERVE_MTRRS_FOR_OS + bool + default n + help + This option allows a platform to reserve 2 MTRRs for the OS usage. + The Intel SDM documents that the the first 6 MTRRs are intended for + the system BIOS and the last 2 are to be reserved for OS usage. + However, modern OSes use PAT to control cacheability instead of + using MTRRs. diff --git a/src/cpu/x86/mtrr/mtrr.c b/src/cpu/x86/mtrr/mtrr.c index cb7ecdc963..f1d36dac1d 100644 --- a/src/cpu/x86/mtrr/mtrr.c +++ b/src/cpu/x86/mtrr/mtrr.c @@ -28,10 +28,8 @@ #define MTRR_FIXED_WRBACK_BITS 0 #endif -/* 2 MTRRS are reserved for the operating system */ -#define BIOS_MTRRS 6 -#define OS_MTRRS 2 -#define MTRRS (BIOS_MTRRS + OS_MTRRS) +#define MIN_MTRRS 8 + /* * Static storage size for variable MTRRs. It's sized sufficiently large to * handle different types of CPUs. Empirically, 16 variable MTRRs has not @@ -39,8 +37,7 @@ */ #define NUM_MTRR_STATIC_STORAGE 16 -static int total_mtrrs = MTRRS; -static int bios_mtrrs = BIOS_MTRRS; +static int total_mtrrs; static void detect_var_mtrrs(void) { @@ -56,7 +53,6 @@ static void detect_var_mtrrs(void) total_mtrrs, NUM_MTRR_STATIC_STORAGE); total_mtrrs = NUM_MTRR_STATIC_STORAGE; } - bios_mtrrs = total_mtrrs - OS_MTRRS; } void enable_fixed_mtrr(void) @@ -399,6 +395,11 @@ static void clear_var_mtrr(int index) wrmsr(MTRR_PHYS_MASK(index), msr); } +static int get_os_reserved_mtrrs(void) +{ + return CONFIG(RESERVE_MTRRS_FOR_OS) ? 2 : 0; +} + static void prep_var_mtrr(struct var_mtrr_state *var_state, uint64_t base, uint64_t size, int mtrr_type) { @@ -407,17 +408,20 @@ static void prep_var_mtrr(struct var_mtrr_state *var_state, resource_t rsize; resource_t mask; - /* Some variable MTRRs are attempted to be saved for the OS use. - * However, it's more important to try to map the full address space - * properly. */ - if (var_state->mtrr_index >= bios_mtrrs) - printk(BIOS_WARNING, "Taking a reserved OS MTRR.\n"); if (var_state->mtrr_index >= total_mtrrs) { printk(BIOS_ERR, "ERROR: Not enough MTRRs available! MTRR index is %d with %d MTRRs in total.\n", var_state->mtrr_index, total_mtrrs); return; } + /* + * If desired, 2 variable MTRRs are attempted to be saved for the OS to + * use. However, it's more important to try to map the full address + * space properly. + */ + if (var_state->mtrr_index >= total_mtrrs - get_os_reserved_mtrrs()) + printk(BIOS_WARNING, "Taking a reserved OS MTRR.\n"); + rbase = base; rsize = size; @@ -710,6 +714,7 @@ static int calc_var_mtrrs(struct memranges *addr_space, __calc_var_mtrrs(addr_space, above4gb, address_bits, &wb_deftype_count, &uc_deftype_count); + const int bios_mtrrs = total_mtrrs - get_os_reserved_mtrrs(); if (wb_deftype_count > bios_mtrrs && uc_deftype_count > bios_mtrrs) { printk(BIOS_DEBUG, "MTRR: Removing WRCOMB type. " "WB/UC MTRR counts: %d/%d > %d.\n", @@ -813,6 +818,8 @@ static void _x86_setup_mtrrs(unsigned int above4gb) void x86_setup_mtrrs(void) { + /* Without detect, assume the minimum */ + total_mtrrs = MIN_MTRRS; /* Always handle addresses above 4GiB. */ _x86_setup_mtrrs(1); }