The MTRR infrastructure has two different copies of mtrr_attrib_to_str(), one
in .init and one in regular .text.  EPT has another variation.

All 3 are incomplete; they encode only the non-reserved values for the task,
but hiding reserved values with ?'s is detrimental to the diagnostic purpose
of these existing in the first place.

Implement a single function which covers all the architectural values.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>

Yes I know EPT tries to render the reserved reserved encodings numerically.
It's a cute trick, but does get foiled by the '[MTRR_NUM_TYPES] = "??";' row
which breaks things.

Putting this in traps.c isn't great, but there isn't an obviously better place
either.
---
 xen/arch/x86/cpu/mtrr/generic.c  | 21 ++++-----------------
 xen/arch/x86/cpu/mtrr/main.c     | 21 +++------------------
 xen/arch/x86/include/asm/traps.h |  1 +
 xen/arch/x86/mm/p2m-ept.c        | 22 +++-------------------
 xen/arch/x86/traps.c             | 16 ++++++++++++++++
 5 files changed, 27 insertions(+), 54 deletions(-)

diff --git a/xen/arch/x86/cpu/mtrr/generic.c b/xen/arch/x86/cpu/mtrr/generic.c
index ad6a24f055ec..0b30689a0b99 100644
--- a/xen/arch/x86/cpu/mtrr/generic.c
+++ b/xen/arch/x86/cpu/mtrr/generic.c
@@ -13,6 +13,7 @@
 #include <asm/msr.h>
 #include <asm/mtrr.h>
 #include <asm/system.h>
+#include <asm/traps.h>
 
 #include "mtrr.h"
 
@@ -125,20 +126,6 @@ void __init get_mtrr_state(void)
 static bool __initdata mtrr_show;
 boolean_param("mtrr.show", mtrr_show);
 
-static const char *__init mtrr_attrib_to_str(mtrr_type x)
-{
-	static const char __initconst strings[MTRR_NUM_TYPES][16] =
-	{
-		[X86_MT_UC] = "uncachable",
-		[X86_MT_WC] = "write-combining",
-		[X86_MT_WT] = "write-through",
-		[X86_MT_WP] = "write-protect",
-		[X86_MT_WB] = "write-back",
-	};
-
-	return (x < ARRAY_SIZE(strings) && strings[x][0]) ? strings[x] : "?";
-}
-
 static unsigned int __initdata last_fixed_start;
 static unsigned int __initdata last_fixed_end;
 static mtrr_type __initdata last_fixed_type;
@@ -149,7 +136,7 @@ static void __init print_fixed_last(const char *level)
 		return;
 
 	printk("%s  %05x-%05x %s\n", level, last_fixed_start,
-	       last_fixed_end - 1, mtrr_attrib_to_str(last_fixed_type));
+	       last_fixed_end - 1, x86_mt_name(last_fixed_type));
 
 	last_fixed_end = 0;
 }
@@ -188,7 +175,7 @@ static void __init print_mtrr_state(const char *level)
 	int width;
 
 	printk("%sMTRR default type: %s\n", level,
-	       mtrr_attrib_to_str(mtrr_state.def_type));
+	       x86_mt_name(mtrr_state.def_type));
 	if (mtrr_state.have_fixed) {
 		const mtrr_type *fr = mtrr_state.fixed_ranges;
 		const struct fixed_range_block *block = fixed_range_blocks;
@@ -214,7 +201,7 @@ static void __init print_mtrr_state(const char *level)
 			       level, i,
 			       width, mtrr_state.var_ranges[i].base >> 12,
 			       width, mtrr_state.var_ranges[i].mask >> 12,
-			       mtrr_attrib_to_str(mtrr_state.var_ranges[i].base &
+			       x86_mt_name(mtrr_state.var_ranges[i].base &
 			                          MTRR_PHYSBASE_TYPE_MASK));
 		else
 			printk("%s  %u disabled\n", level, i);
diff --git a/xen/arch/x86/cpu/mtrr/main.c b/xen/arch/x86/cpu/mtrr/main.c
index eff0500f0d06..f11fbf1a223b 100644
--- a/xen/arch/x86/cpu/mtrr/main.c
+++ b/xen/arch/x86/cpu/mtrr/main.c
@@ -39,6 +39,7 @@
 #include <asm/msr.h>
 #include <asm/mtrr.h>
 #include <asm/processor.h>
+#include <asm/traps.h>
 
 #include "mtrr.h"
 
@@ -63,22 +64,6 @@ static bool __ro_after_init mtrr_if;
 static void set_mtrr(unsigned int reg, unsigned long base,
 		     unsigned long size, mtrr_type type);
 
-static const char *const mtrr_strings[MTRR_NUM_TYPES] =
-{
-    "uncachable",               /* 0 */
-    "write-combining",          /* 1 */
-    "?",                        /* 2 */
-    "?",                        /* 3 */
-    "write-through",            /* 4 */
-    "write-protect",            /* 5 */
-    "write-back",               /* 6 */
-};
-
-static const char *mtrr_attrib_to_str(int x)
-{
-	return (x <= 6) ? mtrr_strings[x] : "?";
-}
-
 /*  This function returns the number of variable MTRRs  */
 static void __init set_num_var_ranges(void)
 {
@@ -364,8 +349,8 @@ int mtrr_add_page(unsigned long base, unsigned long size,
 			if (types_compatible(type, ltype))
 				continue;
 			printk (KERN_WARNING "mtrr: type mismatch for %lx000,%lx000 old: %s new: %s\n",
-			     base, size, mtrr_attrib_to_str(ltype),
-			     mtrr_attrib_to_str(type));
+			     base, size, x86_mt_name(ltype),
+			     x86_mt_name(type));
 			goto out;
 		}
 		if (increment)
diff --git a/xen/arch/x86/include/asm/traps.h b/xen/arch/x86/include/asm/traps.h
index 73097e957d05..00dde631ea5b 100644
--- a/xen/arch/x86/include/asm/traps.h
+++ b/xen/arch/x86/include/asm/traps.h
@@ -19,6 +19,7 @@ void percpu_traps_init(void);
 extern unsigned int ler_msr;
 
 const char *vector_name(unsigned int vec);
+const char *x86_mt_name(unsigned int mt);
 
 #endif /* ASM_TRAP_H */
 
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index 204bdb054a89..1950beb745af 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -21,6 +21,7 @@
 #include <asm/mtrr.h>
 #include <asm/p2m.h>
 #include <asm/paging.h>
+#include <asm/traps.h>
 
 #include <public/hvm/dm_op.h>
 
@@ -1489,21 +1490,6 @@ void ept_p2m_uninit(struct p2m_domain *p2m)
     free_cpumask_var(ept->invalidate);
 }
 
-static const char *memory_type_to_str(unsigned int x)
-{
-    static const char memory_types[8][3] = {
-        [X86_MT_UC]      = "UC",
-        [X86_MT_WC]      = "WC",
-        [X86_MT_WT]      = "WT",
-        [X86_MT_WP]      = "WP",
-        [X86_MT_WB]      = "WB",
-        [MTRR_NUM_TYPES] = "??",
-    };
-
-    ASSERT(x < ARRAY_SIZE(memory_types));
-    return memory_types[x][0] ? memory_types[x] : "?";
-}
-
 static void cf_check ept_dump_p2m_table(unsigned char key)
 {
     struct domain *d;
@@ -1551,14 +1537,12 @@ static void cf_check ept_dump_p2m_table(unsigned char key)
                 if ( p2m_is_pod(ept_entry->sa_p2mt) )
                     printk("gfn: %13lx order: %2d PoD\n", gfn, order);
                 else
-                    printk("gfn: %13lx order: %2d mfn: %13lx %c%c%c %c%c%c\n",
+                    printk("gfn: %13lx order: %2d mfn: %13lx %c%c%c %s%c\n",
                            gfn, order, ept_entry->mfn + 0UL,
                            ept_entry->r ? 'r' : ' ',
                            ept_entry->w ? 'w' : ' ',
                            ept_entry->x ? 'x' : ' ',
-                           memory_type_to_str(ept_entry->emt)[0],
-                           memory_type_to_str(ept_entry->emt)[1]
-                           ?: ept_entry->emt + '0',
+                           x86_mt_name(ept_entry->emt),
                            c ?: ept_entry->ipat ? '!' : ' ');
 
                 if ( !(record_counter++ % 100) )
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index f621b99a5fcc..0d951762bce2 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1044,6 +1044,22 @@ const char *vector_name(unsigned int vec)
     return (vec < ARRAY_SIZE(names) && names[vec][0]) ? names[vec] : "???";
 }
 
+const char *x86_mt_name(unsigned int mt)
+{
+    static const char names[8][4] = {
+        [X86_MT_UC]      = "UC",
+        [X86_MT_WC]      = "WC",
+        [X86_MT_RSVD_2]  = "Rs2",
+        [X86_MT_RSVD_3]  = "Rs3",
+        [X86_MT_WT]      = "WT",
+        [X86_MT_WP]      = "WP",
+        [X86_MT_WB]      = "WB",
+        [X86_MT_UCM]     = "UC-",
+    };
+
+    return mt < ARRAY_SIZE(names) ? names[mt] : "?";
+}
+
 void asmlinkage do_double_fault(struct cpu_user_regs *regs)
 {
     unsigned int cpu;
-- 
2.39.5

On 06.02.2026 14:13, Andrew Cooper wrote:
> The MTRR infrastructure has two different copies of mtrr_attrib_to_str(), one
> in .init and one in regular .text.  EPT has another variation.
> 
> All 3 are incomplete; they encode only the non-reserved values for the task,
> but hiding reserved values with ?'s is detrimental to the diagnostic purpose
> of these existing in the first place.
> 
> Implement a single function which covers all the architectural values.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> 
> Yes I know EPT tries to render the reserved reserved encodings numerically.
> It's a cute trick, but does get foiled by the '[MTRR_NUM_TYPES] = "??";' row
> which breaks things.

Does it? 7 isn't UC- there, but is instead reserved, which we leverage to get
EPT-misconfig exits. 7 also isn't UC- when used in MTRRs; that's a PAT-only
type. I think we better wouldn't mix those. Therefore I'm also not really
happy with x86_mt_name() as a name - it should be clear from the name whether
this is about MTRR (and EPT memory type) or PAT.

> Putting this in traps.c isn't great, but there isn't an obviously better place
> either.

Any reason not to put them in one of the two mtrr/*.c files? Are we entertaining
the idea of allowing to compile out mtrr/?

> @@ -214,7 +201,7 @@ static void __init print_mtrr_state(const char *level)
>  			       level, i,
>  			       width, mtrr_state.var_ranges[i].base >> 12,
>  			       width, mtrr_state.var_ranges[i].mask >> 12,
> -			       mtrr_attrib_to_str(mtrr_state.var_ranges[i].base &
> +			       x86_mt_name(mtrr_state.var_ranges[i].base &
>  			                          MTRR_PHYSBASE_TYPE_MASK));

Nit: Indentation of this line then wants changing, too.

Jan