Re: External USB disks not recognized with v6.1.8 when using Xen

[Borislav Petkov]

On Thu, Feb 02, 2023 at 12:24:30PM -0800, Linus Torvalds wrote:
> So I think that commit should be reverted as broken, and then people
> should *maybe* look at something like this (intentionally whitespace
> damaged, and people should *really* think about what the
> MTRR_TYPE_INVALID case should be - returning UC- is probably what is
> closest to "this is what the hardware does",

Yes, it is actually even documented that by default, all memory is UC-
if MTRRs are disabled.

> but maybe doesn't make sense for the largepage case, which might as
> well just always use largepages in that case?)

See below. I think it should be this way but I might be missing an
angle...

---
diff --git a/arch/x86/include/asm/mtrr.h b/arch/x86/include/asm/mtrr.h
index f0eeaf6e5f5f..4061f1e8d34c 100644
--- a/arch/x86/include/asm/mtrr.h
+++ b/arch/x86/include/asm/mtrr.h
@@ -53,7 +53,8 @@ static inline u8 mtrr_type_lookup(u64 addr, u64 end, u8 *uniform)
 	/*
 	 * Return no-MTRRs:
 	 */
-	return MTRR_TYPE_INVALID;
+	*uniform = 1;
+	return MTRR_TYPE_UNCACHABLE;
 }
 #define mtrr_save_fixed_ranges(arg) do {} while (0)
 #define mtrr_save_state() do {} while (0)
diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index ee09d359e08f..2a1ed63d2b24 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -255,17 +255,25 @@ static u8 mtrr_type_lookup_variable(u64 start, u64 end, u64 *partial_end,
 u8 mtrr_type_lookup(u64 start, u64 end, u8 *uniform)
 {
 	u8 type, prev_type, is_uniform = 1, dummy;
-	int repeat;
 	u64 partial_end;
+	int repeat;
 
 	/* Make end inclusive instead of exclusive */
 	end--;
 
+	/*
+	 * UC- by default because " [i]f the MTRRs are disabled in implementations
+	 * that support the MTRR mechanism, the default memory type is set to
+	 * uncacheable (UC)".
+	 */
+	 type = MTRR_TYPE_UNCACHABLE;
+
 	if (!mtrr_state_set)
-		return MTRR_TYPE_INVALID;
+		goto out;
 
+	type = mtrr_state.def_type;
 	if (!(mtrr_state.enabled & MTRR_STATE_MTRR_ENABLED))
-		return MTRR_TYPE_INVALID;
+		goto out;
 
 	/*
 	 * Look up the fixed ranges first, which take priority over
diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index e4f499eb0f29..ed914bc95345 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -721,8 +721,9 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
 	u8 mtrr, uniform;
 
 	mtrr = mtrr_type_lookup(addr, addr + PUD_SIZE, &uniform);
-	if ((mtrr != MTRR_TYPE_INVALID) && (!uniform) &&
-	    (mtrr != MTRR_TYPE_WRBACK))
+	if (mtrr != MTRR_TYPE_UNCACHABLE &&
+	    mtrr != MTRR_TYPE_WRBACK &&
+	    !uniform)
 		return 0;
 
 	/* Bail out if we are we on a populated non-leaf entry: */
@@ -748,8 +749,9 @@ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot)
 	u8 mtrr, uniform;
 
 	mtrr = mtrr_type_lookup(addr, addr + PMD_SIZE, &uniform);
-	if ((mtrr != MTRR_TYPE_INVALID) && (!uniform) &&
-	    (mtrr != MTRR_TYPE_WRBACK)) {
+	if (mtrr != MTRR_TYPE_UNCACHABLE &&
+	    mtrr != MTRR_TYPE_WRBACK &&
+	    !uniform) {
 		pr_warn_once("%s: Cannot satisfy [mem %#010llx-%#010llx] with a huge-page mapping due to MTRR override.\n",
 			     __func__, addr, addr + PMD_SIZE);
 		return 0;

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette