There are just two callers of this function. It is identical to the
checks done in __trace_var.
The commit message of 9a86ac1aa3d2ebe1be05dc7fe78dd6759aa3241d
("xentrace 5/7: Additional tracing for the shadow code.") gives no
indication what the benefit of this function is.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 xen/arch/x86/hvm/svm/svm.c |  2 --
 xen/arch/x86/hvm/vmx/vmx.c |  2 --
 xen/common/trace.c         | 26 --------------------------
 xen/include/xen/trace.h    |  2 --
 4 files changed, 32 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 59a6e88dff..c10d0015e8 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2812,8 +2812,6 @@ void svm_vmexit_handler(void)
 
         if ( rc )
         {
-            if ( trace_will_trace_event(TRC_SHADOW) )
-                break;
             if ( hvm_long_mode_active(v) )
                 HVMTRACE_LONG_2D(PF_XEN, regs->error_code, TRC_PAR_LONG(va));
             else
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 40767b94c3..08dd297cae 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -4352,8 +4352,6 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
 
             if ( paging_fault(exit_qualification, regs) )
             {
-                if ( trace_will_trace_event(TRC_SHADOW) )
-                    break;
                 if ( hvm_long_mode_active(v) )
                     HVMTRACE_LONG_2D(PF_XEN, regs->error_code,
                                      TRC_PAR_LONG(exit_qualification) );
diff --git a/xen/common/trace.c b/xen/common/trace.c
index 77f8ce0ce5..60db45104e 100644
--- a/xen/common/trace.c
+++ b/xen/common/trace.c
@@ -308,32 +308,6 @@ static int tb_set_size(unsigned int pages)
     return alloc_trace_bufs(pages);
 }
 
-int trace_will_trace_event(u32 event)
-{
-    if ( !tb_init_done )
-        return 0;
-
-    /*
-     * Copied from __trace_var()
-     */
-    if ( (tb_event_mask & event) == 0 )
-        return 0;
-
-    /* match class */
-    if ( ((tb_event_mask >> TRC_CLS_SHIFT) & (event >> TRC_CLS_SHIFT)) == 0 )
-        return 0;
-
-    /* then match subclass */
-    if ( (((tb_event_mask >> TRC_SUBCLS_SHIFT) & 0xf )
-                & ((event >> TRC_SUBCLS_SHIFT) & 0xf )) == 0 )
-        return 0;
-
-    if ( !cpumask_test_cpu(smp_processor_id(), &tb_cpu_mask) )
-        return 0;
-
-    return 1;
-}
-
 /**
  * init_trace_bufs - performs initialization of the per-cpu trace buffers.
  *
diff --git a/xen/include/xen/trace.h b/xen/include/xen/trace.h
index 055883287e..6e9f80dd94 100644
--- a/xen/include/xen/trace.h
+++ b/xen/include/xen/trace.h
@@ -35,8 +35,6 @@ void init_trace_bufs(void);
 /* used to retrieve the physical address of the trace buffers */
 int tb_control(struct xen_sysctl_tbuf_op *tbc);
 
-int trace_will_trace_event(u32 event);
-
 void __trace_var(uint32_t event, bool cycles, unsigned int extra, const void *);
 
 static inline void trace_var(uint32_t event, bool cycles, unsigned int extra,

On 19/06/2023 1:18 pm, Olaf Hering wrote:
> There are just two callers of this function. It is identical to the
> checks done in __trace_var.
> The commit message of 9a86ac1aa3d2ebe1be05dc7fe78dd6759aa3241d
> ("xentrace 5/7: Additional tracing for the shadow code.") gives no
> indication what the benefit of this function is.
>
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

This looks like it is meant to avoid double-tracing the #PF if
TRC_SHADOW is active.

~Andrew

On 19.06.2023 14:18, Olaf Hering wrote:
> There are just two callers of this function. It is identical to the
> checks done in __trace_var.

But it's used differently.

> The commit message of 9a86ac1aa3d2ebe1be05dc7fe78dd6759aa3241d
> ("xentrace 5/7: Additional tracing for the shadow code.") gives no
> indication what the benefit of this function is.

My understanding is that its purpose is to avoid excess tracing, i.e.
when shadow code already traces the event (in greater detail), that the
more general tracing isn't needed / wanted. Note specifically how ...

> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2812,8 +2812,6 @@ void svm_vmexit_handler(void)
>  
>          if ( rc )
>          {
> -            if ( trace_will_trace_event(TRC_SHADOW) )
> -                break;

... this is _not_ "if ( !trace_will_trace_event(...) )" (and the same
on the VMX side.

Jan

>              if ( hvm_long_mode_active(v) )
>                  HVMTRACE_LONG_2D(PF_XEN, regs->error_code, TRC_PAR_LONG(va));
>              else