[PATCH 12/16] x86/amd: Probe for NX support and re-activate if possible

Andrew Cooper posted 16 patches 1 week, 4 days ago
[PATCH 12/16] x86/amd: Probe for NX support and re-activate if possible
Posted by Andrew Cooper 1 week, 4 days ago
An AMD Ryzen system has been found with a firmware option to disable NX.  Like
we do on Intel systems, try to detect and undo this stupidity.

Link: https://xcp-ng.org/forum/post/80714
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>

Somewhat RFC.  I don't particularly like the double handling of
MSR_K8_EXT_FEATURE_MASK in this function, but I can't find any way of making
the logic legible while trying to dedup the MSR accesses.
---
 xen/arch/x86/cpu/amd.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index a14a12fb1d60..06646fc1af93 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -17,6 +17,7 @@
 #include <asm/msr.h>
 #include <asm/processor.h>
 #include <asm/spec_ctrl.h>
+#include <asm/trampoline.h>
 
 #include "cpu.h"
 
@@ -639,6 +640,40 @@ static void amd_early_adjust_cpuid_extd(void)
             printk(XENLOG_INFO "CPU: Re-enabling disabled Topology Extensions Support\n");
         }
     }
+
+    /*
+     * Probe for NX support if it appears to be unavailable.  All 64-bit
+     * capable AMD CPUs support it, but some firmwares have an option to hide
+     * it in CPUID, apparently for "feature parity" with Intel platforms.
+     *
+     * Unlike Intel, there's no active indication that this has been done.  A
+     * conversation with AMD suggests that if we can set EFER.NXE, then NX
+     * will work.  Use this as a heuristic to probe for support, coping with
+     * the fact that a hypervisor might have really disabled and blocked NX,
+     * and not emulate the mask MSRs either.
+     */
+    if ( !boot_cpu_has(X86_FEATURE_NX) )
+    {
+        uint64_t *this_efer = &this_cpu(efer);
+
+        if ( wrmsr_safe(MSR_EFER, *this_efer | EFER_NXE) == 0 )
+        {
+            *this_efer |= EFER_NXE;
+
+            if ( !rdmsr_safe(MSR_K8_EXT_FEATURE_MASK, &val) )
+            {
+                val |= 1ULL << cpufeat_bit(X86_FEATURE_NX);
+                wrmsr_safe(MSR_K8_EXT_FEATURE_MASK, val);
+                val = rdmsr(MSR_K8_EXT_FEATURE_MASK);
+                if ( val & (1ULL << cpufeat_bit(X86_FEATURE_NX)) )
+                {
+                    __set_bit(X86_FEATURE_NX, c->x86_capability);
+                    printk(XENLOG_INFO "re-enabled NX protection\n");
+                    bootsym(trampoline_efer) |= EFER_NXE;
+                }
+            }
+        }
+    }
 }
 
 void __init cf_check early_init_amd(void)
-- 
2.39.5


Re: [PATCH 12/16] x86/amd: Probe for NX support and re-activate if possible
Posted by Jan Beulich 1 week, 3 days ago
On 26.01.2026 18:53, Andrew Cooper wrote:
> An AMD Ryzen system has been found with a firmware option to disable NX.  Like
> we do on Intel systems, try to detect and undo this stupidity.
> 
> Link: https://xcp-ng.org/forum/post/80714
> 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>
> 
> Somewhat RFC.  I don't particularly like the double handling of
> MSR_K8_EXT_FEATURE_MASK in this function, but I can't find any way of making
> the logic legible while trying to dedup the MSR accesses.

Looks reasonable to me as is. If later someone has a neat idea for de-duplication,
it can always be done incrementally.

> @@ -639,6 +640,40 @@ static void amd_early_adjust_cpuid_extd(void)
>              printk(XENLOG_INFO "CPU: Re-enabling disabled Topology Extensions Support\n");
>          }
>      }
> +
> +    /*
> +     * Probe for NX support if it appears to be unavailable.  All 64-bit
> +     * capable AMD CPUs support it, but some firmwares have an option to hide
> +     * it in CPUID, apparently for "feature parity" with Intel platforms.
> +     *
> +     * Unlike Intel, there's no active indication that this has been done.  A
> +     * conversation with AMD suggests that if we can set EFER.NXE, then NX
> +     * will work.  Use this as a heuristic to probe for support, coping with
> +     * the fact that a hypervisor might have really disabled and blocked NX,
> +     * and not emulate the mask MSRs either.
> +     */
> +    if ( !boot_cpu_has(X86_FEATURE_NX) )
> +    {
> +        uint64_t *this_efer = &this_cpu(efer);
> +
> +        if ( wrmsr_safe(MSR_EFER, *this_efer | EFER_NXE) == 0 )

Would you mind using ! here, just like you do ...

> +        {
> +            *this_efer |= EFER_NXE;
> +
> +            if ( !rdmsr_safe(MSR_K8_EXT_FEATURE_MASK, &val) )

... here?

> +            {
> +                val |= 1ULL << cpufeat_bit(X86_FEATURE_NX);
> +                wrmsr_safe(MSR_K8_EXT_FEATURE_MASK, val);
> +                val = rdmsr(MSR_K8_EXT_FEATURE_MASK);
> +                if ( val & (1ULL << cpufeat_bit(X86_FEATURE_NX)) )
> +                {
> +                    __set_bit(X86_FEATURE_NX, c->x86_capability);

Again this shouldn't be needed, as identify_cpu() takes care. Unless in this
case it matters for the time in between?

Jan