[PATCH] x86/shadow: suppress trace_emul_write_val hook when !TRACEBUFFER

Jan Beulich posted 1 patch 1 week, 3 days ago
Failed in applying to current master (apply log)
[PATCH] x86/shadow: suppress trace_emul_write_val hook when !TRACEBUFFER
Posted by Jan Beulich 1 week, 3 days ago
The hook is never invoked in that case, and hence needlessly offers an
extra valid indirect call target. With the hook suppressed, no consumer
of the three local per-CPU variables exists either, so they're
suppressed as well.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/include/asm/paging.h
+++ b/xen/arch/x86/include/asm/paging.h
@@ -90,10 +90,12 @@ struct shadow_paging_mode {
     int           (*guess_wrmap           )(struct vcpu *v, 
                                             unsigned long vaddr, mfn_t gmfn);
     void          (*pagetable_dying       )(paddr_t gpa);
+#ifdef CONFIG_TRACEBUFFER
     void          (*trace_emul_write_val  )(const void *ptr, unsigned long vaddr,
                                             const void *src, unsigned int bytes);
 #endif
 #endif
+#endif
     /* For outsiders to tell what mode we're in */
     unsigned int shadow_levels;
 };
--- a/xen/arch/x86/mm/shadow/hvm.c
+++ b/xen/arch/x86/mm/shadow/hvm.c
@@ -211,9 +211,11 @@ hvm_emulate_write(enum x86_segment seg,
     default: memcpy(ptr, p_data, bytes);                break;
     }
 
+#ifdef CONFIG_TRACEBUFFER
     if ( tb_init_done )
         v->arch.paging.mode->shadow.trace_emul_write_val(ptr, addr,
                                                          p_data, bytes);
+#endif
 
     sh_emulate_unmap_dest(v, ptr, bytes, sh_ctxt);
     shadow_audit_tables(v);
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -2044,6 +2044,7 @@ static void __maybe_unused sh_trace_gfn_
 }
 
 #ifdef CONFIG_HVM
+#ifdef CONFIG_TRACEBUFFER
 #if GUEST_PAGING_LEVELS == 3
 static DEFINE_PER_CPU(guest_va_t,trace_emulate_initial_va);
 static DEFINE_PER_CPU(int,trace_extra_emulation_count);
@@ -2071,9 +2072,11 @@ static void cf_check trace_emulate_write
     memcpy(&this_cpu(trace_emulate_write_val), src, bytes);
 #endif
 }
+#endif /* CONFIG_TRACEBUFFER */
 
 static inline void sh_trace_emulate(guest_l1e_t gl1e, unsigned long va)
 {
+#ifdef CONFIG_TRACEBUFFER
     if ( tb_init_done )
     {
         struct __packed {
@@ -2099,6 +2102,7 @@ static inline void sh_trace_emulate(gues
 
         sh_trace(TRC_SHADOW_EMULATE, sizeof(d), &d);
     }
+#endif /* CONFIG_TRACEBUFFER */
 }
 #endif /* CONFIG_HVM */
 
@@ -2678,7 +2682,9 @@ static int cf_check sh_page_fault(
     paging_unlock(d);
     put_gfn(d, gfn_x(gfn));
 
+#ifdef CONFIG_TRACEBUFFER
     this_cpu(trace_emulate_write_val) = (guest_l1e_t){};
+#endif
 
 #if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
  early_emulation:
@@ -2794,7 +2800,10 @@ static int cf_check sh_page_fault(
     if ( r == X86EMUL_OKAY && !emul_ctxt.ctxt.retire.raw )
     {
         int i, emulation_count=0;
+
+#ifdef CONFIG_TRACEBUFFER
         this_cpu(trace_emulate_initial_va) = va;
+#endif
 
         for ( i = 0 ; i < 4 ; i++ )
         {
@@ -2830,7 +2839,10 @@ static int cf_check sh_page_fault(
                 break; /* Don't emulate again if we failed! */
             }
         }
+
+#ifdef CONFIG_TRACEBUFFER
         this_cpu(trace_extra_emulation_count)=emulation_count;
+#endif
     }
 #endif /* PAE guest */
 
@@ -4130,7 +4142,9 @@ const struct paging_mode sh_paging_mode
     .shadow.guess_wrmap            = sh_guess_wrmap,
 #endif
     .shadow.pagetable_dying        = sh_pagetable_dying,
+#ifdef CONFIG_TRACEBUFFER
     .shadow.trace_emul_write_val   = trace_emulate_write_val,
+#endif
 #endif /* CONFIG_HVM */
     .shadow.shadow_levels          = SHADOW_PAGING_LEVELS,
 };
Re: [PATCH] x86/shadow: suppress trace_emul_write_val hook when !TRACEBUFFER
Posted by Andrew Cooper 1 week, 3 days ago
On 27/01/2026 1:20 pm, Jan Beulich wrote:
> The hook is never invoked in that case, and hence needlessly offers an
> extra valid indirect call target. With the hook suppressed, no consumer
> of the three local per-CPU variables exists either, so they're
> suppressed as well.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/include/asm/paging.h
> +++ b/xen/arch/x86/include/asm/paging.h
> @@ -90,10 +90,12 @@ struct shadow_paging_mode {
>      int           (*guess_wrmap           )(struct vcpu *v, 
>                                              unsigned long vaddr, mfn_t gmfn);
>      void          (*pagetable_dying       )(paddr_t gpa);
> +#ifdef CONFIG_TRACEBUFFER
>      void          (*trace_emul_write_val  )(const void *ptr, unsigned long vaddr,
>                                              const void *src, unsigned int bytes);
>  #endif
>  #endif
> +#endif

Can we get some comments on these endifs, and ...

> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -4130,7 +4142,9 @@ const struct paging_mode sh_paging_mode
>      .shadow.guess_wrmap            = sh_guess_wrmap,
>  #endif
>      .shadow.pagetable_dying        = sh_pagetable_dying,
> +#ifdef CONFIG_TRACEBUFFER
>      .shadow.trace_emul_write_val   = trace_emulate_write_val,
> +#endif
>  #endif /* CONFIG_HVM */

... this one too.

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Re: [PATCH] x86/shadow: suppress trace_emul_write_val hook when !TRACEBUFFER
Posted by Jan Beulich 1 week, 3 days ago
On 27.01.2026 14:27, Andrew Cooper wrote:
> On 27/01/2026 1:20 pm, Jan Beulich wrote:
>> The hook is never invoked in that case, and hence needlessly offers an
>> extra valid indirect call target. With the hook suppressed, no consumer
>> of the three local per-CPU variables exists either, so they're
>> suppressed as well.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/arch/x86/include/asm/paging.h
>> +++ b/xen/arch/x86/include/asm/paging.h
>> @@ -90,10 +90,12 @@ struct shadow_paging_mode {
>>      int           (*guess_wrmap           )(struct vcpu *v, 
>>                                              unsigned long vaddr, mfn_t gmfn);
>>      void          (*pagetable_dying       )(paddr_t gpa);
>> +#ifdef CONFIG_TRACEBUFFER
>>      void          (*trace_emul_write_val  )(const void *ptr, unsigned long vaddr,
>>                                              const void *src, unsigned int bytes);
>>  #endif
>>  #endif
>> +#endif
> 
> Can we get some comments on these endifs, and ...

I can touch the pre-existing ones here, but the new one spanning just two
lines here and ...

>> --- a/xen/arch/x86/mm/shadow/multi.c
>> +++ b/xen/arch/x86/mm/shadow/multi.c
>> @@ -4130,7 +4142,9 @@ const struct paging_mode sh_paging_mode
>>      .shadow.guess_wrmap            = sh_guess_wrmap,
>>  #endif
>>      .shadow.pagetable_dying        = sh_pagetable_dying,
>> +#ifdef CONFIG_TRACEBUFFER
>>      .shadow.trace_emul_write_val   = trace_emulate_write_val,
>> +#endif
>>  #endif /* CONFIG_HVM */
> 
> ... this one too.

... one line here I didn't think would need comments?

> Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks.

Jan