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

[Juergen Gross]

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

From 7989ef9822115a708fc2ba3f7740888a350cb40f Mon Sep 17 00:00:00 2001
From: Juergen Gross <jgross@xxxxxxxx>
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@xxxxxxxx>
---
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: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature