init_amd() tries to re-activate TOPOEXT on certain systems, but only after
amd_init_levelling() has calculated the levelling defaults, meaning that the
re-activation will be lost on the next context switch.

Move the logic into early_init_amd() so it takes effect before calculating the
levelling defaults.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Julian Vetter <julian.vetter@vates.tech>
CC: Teddy Astie <teddy.astie@vates.tech>
---
 xen/arch/x86/cpu/amd.c | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index e8daf7415bb0..a14a12fb1d60 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -620,9 +620,32 @@ void amd_process_freq(const struct cpuinfo_x86 *c,
 		*low_mhz = amd_parse_freq(c->x86, lo);
 }
 
+static void amd_early_adjust_cpuid_extd(void)
+{
+    struct cpuinfo_x86 *c = &boot_cpu_data;
+    uint64_t val;
+
+    /* Re-enable TopologyExtensions if switched off by BIOS */
+    if ( c->family == 0x15 && c->model >= 0x10 && c->model <= 0x1f &&
+         !boot_cpu_has(X86_FEATURE_TOPOEXT) &&
+         !rdmsr_safe(MSR_K8_EXT_FEATURE_MASK, &val) )
+    {
+        val |= 1ULL << 54;
+        wrmsr_safe(MSR_K8_EXT_FEATURE_MASK, val);
+        val = rdmsr(MSR_K8_EXT_FEATURE_MASK);
+        if ( val & (1ULL << 54) )
+        {
+            __set_bit(X86_FEATURE_TOPOEXT, c->x86_capability);
+            printk(XENLOG_INFO "CPU: Re-enabling disabled Topology Extensions Support\n");
+        }
+    }
+}
+
 void __init cf_check early_init_amd(void)
 {
-    amd_init_levelling();
+    amd_early_adjust_cpuid_extd();
+
+    amd_init_levelling(); /* Capture defaults after early CPUID adjustments */
 }
 
 void amd_log_freq(const struct cpuinfo_x86 *c)
@@ -1145,21 +1168,6 @@ static void cf_check init_amd(struct cpuinfo_x86 *c)
 		}
 	}
 
-	/* re-enable TopologyExtensions if switched off by BIOS */
-	if ((c->x86 == 0x15) &&
-	    (c->x86_model >= 0x10) && (c->x86_model <= 0x1f) &&
-	    !cpu_has(c, X86_FEATURE_TOPOEXT) &&
-	    !rdmsr_safe(MSR_K8_EXT_FEATURE_MASK, &value)) {
-		value |= 1ULL << 54;
-		wrmsr_safe(MSR_K8_EXT_FEATURE_MASK, value);
-		rdmsrl(MSR_K8_EXT_FEATURE_MASK, value);
-		if (value & (1ULL << 54)) {
-			__set_bit(X86_FEATURE_TOPOEXT, c->x86_capability);
-			printk(KERN_INFO "CPU: Re-enabling disabled "
-			       "Topology Extensions Support\n");
-		}
-	}
-
 	/*
 	 * The way access filter has a performance penalty on some workloads.
 	 * Disable it on the affected CPUs.
-- 
2.39.5

On 26.01.2026 18:53, Andrew Cooper wrote:
> init_amd() tries to re-activate TOPOEXT on certain systems, but only after
> amd_init_levelling() has calculated the levelling defaults, meaning that the
> re-activation will be lost on the next context switch.
> 
> Move the logic into early_init_amd() so it takes effect before calculating the
> levelling defaults.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

I'm sorry, but as this is being moved and tidied some, I've got some remarks:

> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -620,9 +620,32 @@ void amd_process_freq(const struct cpuinfo_x86 *c,
>  		*low_mhz = amd_parse_freq(c->x86, lo);
>  }
>  
> +static void amd_early_adjust_cpuid_extd(void)

__init?

> +{
> +    struct cpuinfo_x86 *c = &boot_cpu_data;
> +    uint64_t val;
> +
> +    /* Re-enable TopologyExtensions if switched off by BIOS */
> +    if ( c->family == 0x15 && c->model >= 0x10 && c->model <= 0x1f &&

If this is done by BIOSes, why would it be constrained to a certain model
range of a certain family?

> +         !boot_cpu_has(X86_FEATURE_TOPOEXT) &&
> +         !rdmsr_safe(MSR_K8_EXT_FEATURE_MASK, &val) )
> +    {
> +        val |= 1ULL << 54;
> +        wrmsr_safe(MSR_K8_EXT_FEATURE_MASK, val);
> +        val = rdmsr(MSR_K8_EXT_FEATURE_MASK);
> +        if ( val & (1ULL << 54) )
> +        {
> +            __set_bit(X86_FEATURE_TOPOEXT, c->x86_capability);

This shouldn't be needed, as identify_cpu() will obtain the updated value
anyway.

Jan