While the #UD handler is dead code when CONFIG_HVM_FEP is disabled it's
helpful to keep the function around. Reinject #UD in the non-FEP case so
DCE can remove the rest.

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
 xen/arch/x86/hvm/hvm.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 4c00cf4c4fe..0eaeb728382 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3835,13 +3835,18 @@ int hvm_descriptor_access_intercept(uint64_t exit_info,
 void hvm_ud_intercept(struct cpu_user_regs *regs)
 {
     struct vcpu *cur = current;
-    bool should_emulate = false;
     struct hvm_emulate_ctxt ctxt;
     const struct segment_register *cs;
     uint32_t walk;
     unsigned long addr;
     char sig[5]; /* ud2; .ascii "xen" */
 
+    if ( !opt_hvm_fep )
+    {
+        ASSERT_UNREACHABLE();
+        goto reinject;
+    }
+
     hvm_emulate_init_once(&ctxt, NULL, regs);
 
     cs = &ctxt.seg_reg[x86_seg_cs];
@@ -3863,20 +3868,15 @@ 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 )
-    {
-        hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC);
-        return;
     }
+    else
+        goto reinject;
 
     switch ( hvm_emulate_one(&ctxt, VIO_no_completion) )
     {
     case X86EMUL_UNHANDLEABLE:
     case X86EMUL_UNIMPLEMENTED:
+    reinject:
         hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC);
         break;
     case X86EMUL_EXCEPTION:
-- 
2.43.0

On 12.03.2026 14:45, Alejandro Vallejo wrote:
> While the #UD handler is dead code when CONFIG_HVM_FEP is disabled it's
> helpful to keep the function around. Reinject #UD in the non-FEP case so
> DCE can remove the rest.

For the code change - I don't think this should be a separate change. I also
didn't understand Andrew's earlier request to split the original change to
go this far.

As to the "dead code" aspect (using Misra terminology it's really
"unreachable code" aiui), we may need to deal with that, as we've accepted
that rule. Yes, Eclair is unlikely to be able to spot this, yet that doesn't
make the issue go away.

Jan

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3835,13 +3835,18 @@ int hvm_descriptor_access_intercept(uint64_t exit_info,
>  void hvm_ud_intercept(struct cpu_user_regs *regs)
>  {
>      struct vcpu *cur = current;
> -    bool should_emulate = false;
>      struct hvm_emulate_ctxt ctxt;
>      const struct segment_register *cs;
>      uint32_t walk;
>      unsigned long addr;
>      char sig[5]; /* ud2; .ascii "xen" */
>  
> +    if ( !opt_hvm_fep )
> +    {
> +        ASSERT_UNREACHABLE();
> +        goto reinject;
> +    }
> +
>      hvm_emulate_init_once(&ctxt, NULL, regs);
>  
>      cs = &ctxt.seg_reg[x86_seg_cs];
> @@ -3863,20 +3868,15 @@ 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 )
> -    {
> -        hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC);
> -        return;
>      }
> +    else
> +        goto reinject;
>  
>      switch ( hvm_emulate_one(&ctxt, VIO_no_completion) )
>      {
>      case X86EMUL_UNHANDLEABLE:
>      case X86EMUL_UNIMPLEMENTED:
> +    reinject:
>          hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC);
>          break;
>      case X86EMUL_EXCEPTION:

On Thu Mar 12, 2026 at 4:24 PM CET, Jan Beulich wrote:
> On 12.03.2026 14:45, Alejandro Vallejo wrote:
>> While the #UD handler is dead code when CONFIG_HVM_FEP is disabled it's
>> helpful to keep the function around. Reinject #UD in the non-FEP case so
>> DCE can remove the rest.
>
> For the code change - I don't think this should be a separate change. I also
> didn't understand Andrew's earlier request to split the original change to
> go this far.

I don't mind either way. Fold it with the prior patch if you think it's better
that way.

>
> As to the "dead code" aspect (using Misra terminology it's really
> "unreachable code" aiui), we may need to deal with that, as we've accepted
> that rule. Yes, Eclair is unlikely to be able to spot this, yet that doesn't
> make the issue go away.

That's also the case for handlers of other VMEXIT error codes under certain
configurations. The original patch I sent fully removed the #UD handler to
restrict the unreachable part to just a BUG_ON(). But even that was incomplete.

The solution is some creative filtering at the switch condition, but that's
something I haven't had time to experiment with yet.

Cheers,
Alejandro