Re: [PATCH v6 00/16] x86/mtrr: fix handling with PAT but without MTRR

[Juergen Gross <jgross@suse.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 481 bytes --]

On 31.05.23 19:48, Borislav Petkov wrote:
> On Wed, May 31, 2023 at 04:20:08PM +0200, Juergen Gross wrote:
>> One other note: why does mtrr_cleanup() think that using 8 instead of 6
>> variable MTRRs would be an "optimal setting"?
> 
> Maybe the more extensive debug output below would help answer that...
> 
>> IMO it should replace the original setup only in case it is using _less_
>> MTRRs than before.
> 
> Right.

The attached patch will do that.


Juergen


[-- Attachment #1.1.2: v7-0001-x86-mtrr-Let-mtrr_cleanup-not-increase-number-of-.patch --]
[-- Type: text/x-patch, Size: 2367 bytes --]

From 7989ef9822115a708fc2ba3f7740888a350cb40f Mon Sep 17 00:00:00 2001
From: Juergen Gross <jgross@suse.com>
Date: Thu, 1 Jun 2023 14:40:58 +0200
Subject: [PATCH v7] x86/mtrr: Let mtrr_cleanup() not increase number of used
 MTRRs

Today mtrr_cleanup() will always use the best found alternative MTRR
setting, even if this setting is using more variable MTRRs than the
BIOS provided setup.

Add a check that only settings with less variable MTRRs are used.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V7:
- new patch
---
 arch/x86/kernel/cpu/mtrr/cleanup.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/mtrr/cleanup.c b/arch/x86/kernel/cpu/mtrr/cleanup.c
index a7cb5d32d03d..a5d331722092 100644
--- a/arch/x86/kernel/cpu/mtrr/cleanup.c
+++ b/arch/x86/kernel/cpu/mtrr/cleanup.c
@@ -567,7 +567,7 @@ static int __init mtrr_need_cleanup(void)
 	    num_var_ranges - num[MTRR_NUM_TYPES])
 		return 0;
 
-	return 1;
+	return num_var_ranges - num[MTRR_NUM_TYPES];
 }
 
 static unsigned long __initdata range_sums;
@@ -673,6 +673,7 @@ int __init mtrr_cleanup(void)
 	u64 chunk_size, gran_size;
 	mtrr_type type;
 	int index_good;
+	int num_used;
 	int i;
 
 	if (!cpu_feature_enabled(X86_FEATURE_MTRR) || enable_mtrr_cleanup < 1)
@@ -693,7 +694,8 @@ int __init mtrr_cleanup(void)
 	}
 
 	/* Check if we need handle it and can handle it: */
-	if (!mtrr_need_cleanup())
+	num_used = mtrr_need_cleanup();
+	if (!num_used)
 		return 0;
 
 	/* Print original var MTRRs at first, for debugging: */
@@ -728,6 +730,10 @@ int __init mtrr_cleanup(void)
 		mtrr_print_out_one_result(i);
 
 		if (!result[i].bad) {
+			if (result[i].num_reg >= num_used) {
+				Dprintk("BIOS provided MTRR setting is better than found one\n");
+				return 0;
+			}
 			set_var_mtrr_all();
 			Dprintk("New variable MTRRs\n");
 			print_out_mtrr_range_state();
@@ -762,8 +768,12 @@ int __init mtrr_cleanup(void)
 	index_good = mtrr_search_optimal_index();
 
 	if (index_good != -1) {
-		pr_info("Found optimal setting for mtrr clean up\n");
 		i = index_good;
+		if (result[i].num_reg >= num_used) {
+			Dprintk("BIOS provided MTRR setting is better than found one\n");
+			return 0;
+		}
+		pr_info("Found optimal setting for mtrr clean up\n");
 		mtrr_print_out_one_result(i);
 
 		/* Convert ranges to var ranges state: */
-- 
2.35.3


[-- Attachment #1.1.3: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]