[PATCH 2/4] x86/hvm: Disable non-FEP cross-vendor handling in #UD handler

Alejandro Vallejo posted 4 patches 2 weeks, 4 days ago
There is a newer version of this series
[PATCH 2/4] x86/hvm: Disable non-FEP cross-vendor handling in #UD handler
Posted by Alejandro Vallejo 2 weeks, 4 days ago
Remove cross-vendor support now that VMs can no longer have a different
vendor than the host, leaving FEP as the sole raison-d'être for #UD
interception.

Not a functional change.

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
 xen/arch/x86/hvm/hvm.c     | 25 ++++---------------------
 xen/arch/x86/hvm/svm/svm.c |  4 ++--
 xen/arch/x86/hvm/vmx/vmx.c |  4 ++--
 3 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 4d37a93c57..611ff83a60 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3832,28 +3832,13 @@ int hvm_descriptor_access_intercept(uint64_t exit_info,
     return X86EMUL_OKAY;
 }
 
-static bool cf_check is_cross_vendor(
-    const struct x86_emulate_state *state, const struct x86_emulate_ctxt *ctxt)
-{
-    switch ( ctxt->opcode )
-    {
-    case X86EMUL_OPC(0x0f, 0x05): /* syscall */
-    case X86EMUL_OPC(0x0f, 0x34): /* sysenter */
-    case X86EMUL_OPC(0x0f, 0x35): /* sysexit */
-        return true;
-    }
-
-    return false;
-}
-
+#ifdef CONFIG_HVM_FEP
 void hvm_ud_intercept(struct cpu_user_regs *regs)
 {
     struct vcpu *cur = current;
-    bool should_emulate =
-        cur->domain->arch.cpuid->x86_vendor != boot_cpu_data.x86_vendor;
     struct hvm_emulate_ctxt ctxt;
 
-    hvm_emulate_init_once(&ctxt, opt_hvm_fep ? NULL : is_cross_vendor, regs);
+    hvm_emulate_init_once(&ctxt, NULL, regs);
 
     if ( opt_hvm_fep )
     {
@@ -3878,12 +3863,9 @@ void hvm_ud_intercept(struct cpu_user_regs *regs)
                 regs->rip = (uint32_t)regs->rip;
 
             add_taint(TAINT_HVM_FEP);
-
-            should_emulate = true;
         }
     }
-
-    if ( !should_emulate )
+    else
     {
         hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC);
         return;
@@ -3903,6 +3885,7 @@ void hvm_ud_intercept(struct cpu_user_regs *regs)
         break;
     }
 }
+#endif /* CONFIG_HVM_FEP */
 
 enum hvm_intblk hvm_interrupt_blocked(struct vcpu *v, struct hvm_intack intack)
 {
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 18ba837738..0658ca990f 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -589,8 +589,7 @@ static void cf_check svm_cpuid_policy_changed(struct vcpu *v)
     const struct cpu_policy *cp = v->domain->arch.cpu_policy;
     u32 bitmap = vmcb_get_exception_intercepts(vmcb);
 
-    if ( opt_hvm_fep ||
-         (v->domain->arch.cpuid->x86_vendor != boot_cpu_data.x86_vendor) )
+    if ( opt_hvm_fep )
         bitmap |= (1U << X86_EXC_UD);
     else
         bitmap &= ~(1U << X86_EXC_UD);
@@ -2810,6 +2809,7 @@ void asmlinkage svm_vmexit_handler(void)
         break;
 
     case VMEXIT_EXCEPTION_UD:
+        BUG_ON(!IS_ENABLED(CONFIG_HVM_FEP));
         hvm_ud_intercept(regs);
         break;
 
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 40e4c71244..34e988ee61 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -797,8 +797,7 @@ static void cf_check vmx_cpuid_policy_changed(struct vcpu *v)
     const struct cpu_policy *cp = v->domain->arch.cpu_policy;
     int rc = 0;
 
-    if ( opt_hvm_fep ||
-         (v->domain->arch.cpuid->x86_vendor != boot_cpu_data.x86_vendor) )
+    if ( opt_hvm_fep )
         v->arch.hvm.vmx.exception_bitmap |= (1U << X86_EXC_UD);
     else
         v->arch.hvm.vmx.exception_bitmap &= ~(1U << X86_EXC_UD);
@@ -4576,6 +4575,7 @@ void asmlinkage vmx_vmexit_handler(struct cpu_user_regs *regs)
             /* Already handled above. */
             break;
         case X86_EXC_UD:
+            BUG_ON(!IS_ENABLED(CONFIG_HVM_FEP));
             TRACE(TRC_HVM_TRAP, vector);
             hvm_ud_intercept(regs);
             break;
-- 
2.43.0


Re: [PATCH 2/4] x86/hvm: Disable non-FEP cross-vendor handling in #UD handler
Posted by Alejandro Vallejo 1 week, 5 days ago
On Thu Jan 22, 2026 at 5:49 PM CET, Alejandro Vallejo wrote:
> Remove cross-vendor support now that VMs can no longer have a different
> vendor than the host, leaving FEP as the sole raison-d'être for #UD
> interception.
>
> Not a functional change.
>
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> ---
>  xen/arch/x86/hvm/hvm.c     | 25 ++++---------------------
>  xen/arch/x86/hvm/svm/svm.c |  4 ++--
>  xen/arch/x86/hvm/vmx/vmx.c |  4 ++--
>  3 files changed, 8 insertions(+), 25 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 4d37a93c57..611ff83a60 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3832,28 +3832,13 @@ int hvm_descriptor_access_intercept(uint64_t exit_info,
>      return X86EMUL_OKAY;
>  }
>  
> -static bool cf_check is_cross_vendor(
> -    const struct x86_emulate_state *state, const struct x86_emulate_ctxt *ctxt)
> -{
> -    switch ( ctxt->opcode )
> -    {
> -    case X86EMUL_OPC(0x0f, 0x05): /* syscall */
> -    case X86EMUL_OPC(0x0f, 0x34): /* sysenter */
> -    case X86EMUL_OPC(0x0f, 0x35): /* sysexit */
> -        return true;
> -    }
> -
> -    return false;
> -}
> -
> +#ifdef CONFIG_HVM_FEP
>  void hvm_ud_intercept(struct cpu_user_regs *regs)
>  {
>      struct vcpu *cur = current;
> -    bool should_emulate =
> -        cur->domain->arch.cpuid->x86_vendor != boot_cpu_data.x86_vendor;
>      struct hvm_emulate_ctxt ctxt;
>  
> -    hvm_emulate_init_once(&ctxt, opt_hvm_fep ? NULL : is_cross_vendor, regs);
> +    hvm_emulate_init_once(&ctxt, NULL, regs);
>  
>      if ( opt_hvm_fep )
>      {
> @@ -3878,12 +3863,9 @@ void hvm_ud_intercept(struct cpu_user_regs *regs)
>                  regs->rip = (uint32_t)regs->rip;
>  
>              add_taint(TAINT_HVM_FEP);
> -
> -            should_emulate = true;
>          }
>      }
> -
> -    if ( !should_emulate )
> +    else

review to self. This is buggy. It allows instruction emulation when HVM_FEP is
enabled, but the FEP is absent in the particular instruction that caused the
exception.

#UD should be re-injected when the instruction doesn't have the prefix.

Cheers,
Alejandro
Re: [PATCH 2/4] x86/hvm: Disable non-FEP cross-vendor handling in #UD handler
Posted by Andrew Cooper 2 weeks, 3 days ago
On 22/01/2026 4:49 pm, Alejandro Vallejo wrote:
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 40e4c71244..34e988ee61 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -797,8 +797,7 @@ static void cf_check vmx_cpuid_policy_changed(struct vcpu *v)
>      const struct cpu_policy *cp = v->domain->arch.cpu_policy;
>      int rc = 0;
>  
> -    if ( opt_hvm_fep ||
> -         (v->domain->arch.cpuid->x86_vendor != boot_cpu_data.x86_vendor) )
> +    if ( opt_hvm_fep )
>          v->arch.hvm.vmx.exception_bitmap |= (1U << X86_EXC_UD);
>      else
>          v->arch.hvm.vmx.exception_bitmap &= ~(1U << X86_EXC_UD);
> @@ -4576,6 +4575,7 @@ void asmlinkage vmx_vmexit_handler(struct cpu_user_regs *regs)
>              /* Already handled above. */
>              break;
>          case X86_EXC_UD:
> +            BUG_ON(!IS_ENABLED(CONFIG_HVM_FEP));
>              TRACE(TRC_HVM_TRAP, vector);
>              hvm_ud_intercept(regs);
>              break;

Again, nested virt makes this more complicated than to simply believe it
doesn't happen.

Also, more often than I'd like, I enable #UD interception for other
reasons, and I'd prefer that that doesn't get any harder than it does
right now.

In an ideal world I'd have already upstreamed the logic to decompose
double/triple faults...

~Andrew
Re: [PATCH 2/4] x86/hvm: Disable non-FEP cross-vendor handling in #UD handler
Posted by Alejandro Vallejo 2 weeks ago
On Fri Jan 23, 2026 at 7:40 PM CET, Andrew Cooper wrote:
> On 22/01/2026 4:49 pm, Alejandro Vallejo wrote:
>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>> index 40e4c71244..34e988ee61 100644
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -797,8 +797,7 @@ static void cf_check vmx_cpuid_policy_changed(struct vcpu *v)
>>      const struct cpu_policy *cp = v->domain->arch.cpu_policy;
>>      int rc = 0;
>>  
>> -    if ( opt_hvm_fep ||
>> -         (v->domain->arch.cpuid->x86_vendor != boot_cpu_data.x86_vendor) )
>> +    if ( opt_hvm_fep )
>>          v->arch.hvm.vmx.exception_bitmap |= (1U << X86_EXC_UD);
>>      else
>>          v->arch.hvm.vmx.exception_bitmap &= ~(1U << X86_EXC_UD);
>> @@ -4576,6 +4575,7 @@ void asmlinkage vmx_vmexit_handler(struct cpu_user_regs *regs)
>>              /* Already handled above. */
>>              break;
>>          case X86_EXC_UD:
>> +            BUG_ON(!IS_ENABLED(CONFIG_HVM_FEP));
>>              TRACE(TRC_HVM_TRAP, vector);
>>              hvm_ud_intercept(regs);
>>              break;
>
> Again, nested virt makes this more complicated than to simply believe it
> doesn't happen.

How so? nested vmexits go on a separate function (nvmx_n2_vmexit_handler()),
which is purposefully dispatched earlier. This switch is strictly for non-nested
exits or it would be all sorts of wrong for other reasons.

Either (non-nested) #UD does happen, in which case I want to know how. Or
it doesn't, in which case we have dead code. Both cannot be simultaneously
true. The #UD handler is (after the should_emulate fixup) just doing FEP and
re-injecting otherwise. Whether there is an "otherwise" is relevant for the
refactor and Teddy's rightful request.

>
> Also, more often than I'd like, I enable #UD interception for other
> reasons, and I'd prefer that that doesn't get any harder than it does
> right now.

It's equally simple with hvm_fep=1 in the cmdline for an unmodified Xen (to
get the tracepoint). With a modified Xen it's a BUG_ON() removal, or running
with HVM_FEP, which affects security but not performance (so it's ok for one-off
tests). I could also conditionalise it to #ifndef CONFIG_DEBUG, as the
overwhelming majority of the time you'll run your tests in debug mode.

Do any of those options sound fine? Shipping a dead function to users/customers
because it's occasionally useful during development sounds like bad policy to
me.

>
> In an ideal world I'd have already upstreamed the logic to decompose
> double/triple faults...

Sorry, I'm afraid I don't follow what this ties in with. Is this why you find
#UD interception helpful?

Cheers,
Alejandro
Re: [PATCH 2/4] x86/hvm: Disable non-FEP cross-vendor handling in #UD handler
Posted by Teddy Astie 2 weeks, 4 days ago
Le 22/01/2026 à 17:52, Alejandro Vallejo a écrit :
> Remove cross-vendor support now that VMs can no longer have a different
> vendor than the host, leaving FEP as the sole raison-d'être for #UD
> interception.
> 
> Not a functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> ---
>   xen/arch/x86/hvm/hvm.c     | 25 ++++---------------------
>   xen/arch/x86/hvm/svm/svm.c |  4 ++--
>   xen/arch/x86/hvm/vmx/vmx.c |  4 ++--
>   3 files changed, 8 insertions(+), 25 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 4d37a93c57..611ff83a60 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3832,28 +3832,13 @@ int hvm_descriptor_access_intercept(uint64_t exit_info,
>       return X86EMUL_OKAY;
>   }
>   
> -static bool cf_check is_cross_vendor(
> -    const struct x86_emulate_state *state, const struct x86_emulate_ctxt *ctxt)
> -{
> -    switch ( ctxt->opcode )
> -    {
> -    case X86EMUL_OPC(0x0f, 0x05): /* syscall */
> -    case X86EMUL_OPC(0x0f, 0x34): /* sysenter */
> -    case X86EMUL_OPC(0x0f, 0x35): /* sysexit */
> -        return true;
> -    }
> -
> -    return false;
> -}
> -
> +#ifdef CONFIG_HVM_FEP

I'm not sure it is wise to put it being ifdef given that we have it in 
support.h.

Given that this function now assume we have FEP enabled (since it's only 
called in that case), I think we should rename it to reflect that, like 
"hvm_fep_intercept" and drop the non-FEP logic.

>   void hvm_ud_intercept(struct cpu_user_regs *regs)
>   {
>       struct vcpu *cur = current;
> -    bool should_emulate =
> -        cur->domain->arch.cpuid->x86_vendor != boot_cpu_data.x86_vendor;
>       struct hvm_emulate_ctxt ctxt;
>   
> -    hvm_emulate_init_once(&ctxt, opt_hvm_fep ? NULL : is_cross_vendor, regs);
> +    hvm_emulate_init_once(&ctxt, NULL, regs);
>   
>       if ( opt_hvm_fep )
>       {
> @@ -3878,12 +3863,9 @@ void hvm_ud_intercept(struct cpu_user_regs *regs)
>                   regs->rip = (uint32_t)regs->rip;
>   
>               add_taint(TAINT_HVM_FEP);
> -
> -            should_emulate = true;
>           }
>       }
> -
> -    if ( !should_emulate )
> +    else
>       {
>           hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC);
>           return;
> @@ -3903,6 +3885,7 @@ void hvm_ud_intercept(struct cpu_user_regs *regs)
>           break;
>       }
>   }
> +#endif /* CONFIG_HVM_FEP */
>   
>   enum hvm_intblk hvm_interrupt_blocked(struct vcpu *v, struct hvm_intack intack)
>   {
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 18ba837738..0658ca990f 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -589,8 +589,7 @@ static void cf_check svm_cpuid_policy_changed(struct vcpu *v)
>       const struct cpu_policy *cp = v->domain->arch.cpu_policy;
>       u32 bitmap = vmcb_get_exception_intercepts(vmcb);
>   
> -    if ( opt_hvm_fep ||
> -         (v->domain->arch.cpuid->x86_vendor != boot_cpu_data.x86_vendor) )
> +    if ( opt_hvm_fep )
>           bitmap |= (1U << X86_EXC_UD);
>       else
>           bitmap &= ~(1U << X86_EXC_UD);
> @@ -2810,6 +2809,7 @@ void asmlinkage svm_vmexit_handler(void)
>           break;
>   
>       case VMEXIT_EXCEPTION_UD:
> +        BUG_ON(!IS_ENABLED(CONFIG_HVM_FEP));
>           hvm_ud_intercept(regs);
>           break;
>   
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 40e4c71244..34e988ee61 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -797,8 +797,7 @@ static void cf_check vmx_cpuid_policy_changed(struct vcpu *v)
>       const struct cpu_policy *cp = v->domain->arch.cpu_policy;
>       int rc = 0;
>   
> -    if ( opt_hvm_fep ||
> -         (v->domain->arch.cpuid->x86_vendor != boot_cpu_data.x86_vendor) )
> +    if ( opt_hvm_fep )
>           v->arch.hvm.vmx.exception_bitmap |= (1U << X86_EXC_UD);
>       else
>           v->arch.hvm.vmx.exception_bitmap &= ~(1U << X86_EXC_UD);
> @@ -4576,6 +4575,7 @@ void asmlinkage vmx_vmexit_handler(struct cpu_user_regs *regs)
>               /* Already handled above. */
>               break;
>           case X86_EXC_UD:
> +            BUG_ON(!IS_ENABLED(CONFIG_HVM_FEP));
>               TRACE(TRC_HVM_TRAP, vector);
>               hvm_ud_intercept(regs);
>               break;



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
Re: [PATCH 2/4] x86/hvm: Disable non-FEP cross-vendor handling in #UD handler
Posted by Alejandro Vallejo 2 weeks, 3 days ago
On Thu Jan 22, 2026 at 6:28 PM CET, Teddy Astie wrote:
> Le 22/01/2026 à 17:52, Alejandro Vallejo a écrit :
>> Remove cross-vendor support now that VMs can no longer have a different
>> vendor than the host, leaving FEP as the sole raison-d'être for #UD
>> interception.
>> 
>> Not a functional change.
>> 
>> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
>> ---
>>   xen/arch/x86/hvm/hvm.c     | 25 ++++---------------------
>>   xen/arch/x86/hvm/svm/svm.c |  4 ++--
>>   xen/arch/x86/hvm/vmx/vmx.c |  4 ++--
>>   3 files changed, 8 insertions(+), 25 deletions(-)
>> 
>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>> index 4d37a93c57..611ff83a60 100644
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -3832,28 +3832,13 @@ int hvm_descriptor_access_intercept(uint64_t exit_info,
>>       return X86EMUL_OKAY;
>>   }
>>   
>> -static bool cf_check is_cross_vendor(
>> -    const struct x86_emulate_state *state, const struct x86_emulate_ctxt *ctxt)
>> -{
>> -    switch ( ctxt->opcode )
>> -    {
>> -    case X86EMUL_OPC(0x0f, 0x05): /* syscall */
>> -    case X86EMUL_OPC(0x0f, 0x34): /* sysenter */
>> -    case X86EMUL_OPC(0x0f, 0x35): /* sysexit */
>> -        return true;
>> -    }
>> -
>> -    return false;
>> -}
>> -
>> +#ifdef CONFIG_HVM_FEP
>
> I'm not sure it is wise to put it being ifdef given that we have it in 
> support.h.

We already abuse code elision in this manner. See domain_soft_reset(). It's
intentional, to avoid polluting the headers.

You'll get a link error anyway (as opposed to a compile time error).

>
> Given that this function now assume we have FEP enabled (since it's only 
> called in that case), I think we should rename it to reflect that, like 
> "hvm_fep_intercept" and drop the non-FEP logic.

I'm not a big fan of renaming the handler, because it'd force future changes
where #UD is invoked in more cases than HVM_FEP to rename it back.

But yes to the removal of the non-FEP logic.

Cheers,
Alejandro