From: Grygorii Strashko <grygorii_strashko@epam.com>

The VMTRACE feature depends on Platform/Arch HW and code support and now
can be used only on x86 HVM with Intel VT-x (INTEL_VMX) enabled.
This makes VMTRACE support optional by introducing two Kconfig options:
- CONFIG_HAS_VMTRACE which allows Platform/Arch to advertise VMTRACE
support readiness
- CONFIG_VMTRACE to enable/disable the feature.

Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
---
Based on top of patch "domain: adjust soft-reset arch dependency" [1]

[1] https://patchwork.kernel.org/project/xen-devel/patch/c9c8c9c6-a155-4986-bf77-5766cdcd6024@suse.com/

 xen/Kconfig.debug                       | 15 +++++++++++++++
 xen/arch/x86/domctl.c                   |  4 ++++
 xen/arch/x86/hvm/Kconfig                |  1 +
 xen/arch/x86/hvm/vmx/vmcs.c             |  2 ++
 xen/arch/x86/hvm/vmx/vmx.c              | 10 ++++++++++
 xen/arch/x86/include/asm/guest-msr.h    |  2 ++
 xen/arch/x86/include/asm/hvm/hvm.h      |  9 +++++++++
 xen/arch/x86/include/asm/hvm/vmx/vmcs.h |  2 ++
 xen/arch/x86/mm/mem_sharing.c           |  2 ++
 xen/arch/x86/vm_event.c                 |  6 ++++--
 xen/common/domain.c                     | 10 ++++++++++
 xen/common/memory.c                     |  6 ++++++
 xen/common/vm_event.c                   |  3 ++-
 xen/include/xen/domain.h                |  4 ++++
 xen/include/xen/sched.h                 |  4 ++++
 15 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index d900d926c555..70ec4f0d14a5 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -155,4 +155,19 @@ config DEBUG_INFO
 	  "make install-xen" for installing xen.efi, stripping needs to be
 	  done outside the Xen build environment).
 
+config HAS_VMTRACE
+    bool
+
+config VMTRACE
+    bool "HW VM tracing support"
+    depends on HAS_VMTRACE
+    default y
+    help
+      Enables HW VM tracing support which allows to configure HW processor
+      features (vmtrace_op) to enable capturing information about software
+      execution using dedicated hardware facilities with minimal interference
+      to the software being traced. The trace date can be retrieved using buffer
+      shared between Xen and domain
+      (see XENMEM_acquire_resource(XENMEM_resource_vmtrace_buf)).
+
 endmenu
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 6153e3c07e2d..d9521808dcba 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -154,6 +154,7 @@ void arch_get_domain_info(const struct domain *d,
 static int do_vmtrace_op(struct domain *d, struct xen_domctl_vmtrace_op *op,
                          XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
 {
+#ifdef CONFIG_VMTRACE
     struct vcpu *v;
     int rc;
 
@@ -198,6 +199,9 @@ static int do_vmtrace_op(struct domain *d, struct xen_domctl_vmtrace_op *op,
     vcpu_unpause(v);
 
     return rc;
+#else
+    return -EOPNOTSUPP;
+#endif
 }
 
 #define MAX_IOPORTS 0x10000
diff --git a/xen/arch/x86/hvm/Kconfig b/xen/arch/x86/hvm/Kconfig
index c1a131d1851a..c017a77fffe0 100644
--- a/xen/arch/x86/hvm/Kconfig
+++ b/xen/arch/x86/hvm/Kconfig
@@ -29,6 +29,7 @@ config INTEL_VMX
 	bool "Intel VT-x" if EXPERT
 	default INTEL
 	select ARCH_VCPU_IOREQ_COMPLETION
+	select HAS_VMTRACE
 	help
 	  Enables virtual machine extensions on platforms that implement the
 	  Intel Virtualization Technology (Intel VT-x).
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index ab8b1c87ec0f..3728a9140223 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -300,6 +300,7 @@ static int vmx_init_vmcs_config(bool bsp)
     rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap);
 
     /* Check whether IPT is supported in VMX operation. */
+#ifdef CONFIG_VMTRACE
     if ( bsp )
         vmtrace_available = cpu_has_proc_trace &&
                             (_vmx_misc_cap & VMX_MISC_PROC_TRACE);
@@ -310,6 +311,7 @@ static int vmx_init_vmcs_config(bool bsp)
                smp_processor_id());
         return -EINVAL;
     }
+#endif
 
     if ( caps.cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS )
     {
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index b65981077295..f1588cd90b2d 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -622,6 +622,7 @@ static void cf_check domain_creation_finished(struct domain *d)
 
 static void vmx_init_ipt(struct vcpu *v)
 {
+#ifdef CONFIG_VMTRACE
     unsigned int size = v->domain->vmtrace_size;
 
     if ( !size )
@@ -632,6 +633,7 @@ static void vmx_init_ipt(struct vcpu *v)
     ASSERT(size >= PAGE_SIZE && (size & (size - 1)) == 0);
 
     v->arch.msrs->rtit.output_limit = size - 1;
+#endif
 }
 
 static int cf_check vmx_vcpu_initialise(struct vcpu *v)
@@ -724,11 +726,13 @@ static void vmx_save_guest_msrs(struct vcpu *v)
      */
     v->arch.hvm.vmx.shadow_gs = read_gs_shadow();
 
+#ifdef CONFIG_VMTRACE
     if ( v->arch.hvm.vmx.ipt_active )
     {
         rdmsrl(MSR_RTIT_OUTPUT_MASK, msrs->rtit.output_mask);
         rdmsrl(MSR_RTIT_STATUS, msrs->rtit.status);
     }
+#endif
 
     if ( cp->feat.pks )
         msrs->pkrs = rdpkrs_and_cache();
@@ -747,12 +751,14 @@ static void vmx_restore_guest_msrs(struct vcpu *v)
     if ( cpu_has_msr_tsc_aux )
         wrmsr_tsc_aux(msrs->tsc_aux);
 
+#ifdef CONFIG_VMTRACE
     if ( v->arch.hvm.vmx.ipt_active )
     {
         wrmsrl(MSR_RTIT_OUTPUT_BASE, page_to_maddr(v->vmtrace.pg));
         wrmsrl(MSR_RTIT_OUTPUT_MASK, msrs->rtit.output_mask);
         wrmsrl(MSR_RTIT_STATUS, msrs->rtit.status);
     }
+#endif
 
     if ( cp->feat.pks )
         wrpkrs(msrs->pkrs);
@@ -2626,6 +2632,7 @@ static bool cf_check vmx_get_pending_event(
     return true;
 }
 
+#ifdef CONFIG_VMTRACE
 /*
  * We only let vmtrace agents see and modify a subset of bits in MSR_RTIT_CTL.
  * These all pertain to data-emitted into the trace buffer(s).  Must not
@@ -2768,6 +2775,7 @@ static int cf_check vmtrace_reset(struct vcpu *v)
     v->arch.msrs->rtit.status = 0;
     return 0;
 }
+#endif
 
 static uint64_t cf_check vmx_get_reg(struct vcpu *v, unsigned int reg)
 {
@@ -2940,11 +2948,13 @@ static struct hvm_function_table __initdata_cf_clobber vmx_function_table = {
     .altp2m_vcpu_emulate_ve = vmx_vcpu_emulate_ve,
     .altp2m_vcpu_emulate_vmfunc = vmx_vcpu_emulate_vmfunc,
 #endif
+#ifdef CONFIG_VMTRACE
     .vmtrace_control = vmtrace_control,
     .vmtrace_output_position = vmtrace_output_position,
     .vmtrace_set_option = vmtrace_set_option,
     .vmtrace_get_option = vmtrace_get_option,
     .vmtrace_reset = vmtrace_reset,
+#endif
 
     .get_reg = vmx_get_reg,
     .set_reg = vmx_set_reg,
diff --git a/xen/arch/x86/include/asm/guest-msr.h b/xen/arch/x86/include/asm/guest-msr.h
index 5f0cb0a93995..702f47fe1e16 100644
--- a/xen/arch/x86/include/asm/guest-msr.h
+++ b/xen/arch/x86/include/asm/guest-msr.h
@@ -50,6 +50,7 @@ struct vcpu_msrs
         };
     } misc_features_enables;
 
+#ifdef CONFIG_VMTRACE
     /*
      * 0x00000560 ... 57x - MSR_RTIT_*
      *
@@ -81,6 +82,7 @@ struct vcpu_msrs
             };
         };
     } rtit;
+#endif
 
     /*
      * 0x000006e1 - MSR_PKRS - Protection Key Supervisor.
diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h
index 7412256a2dab..728b9624522f 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -234,12 +234,14 @@ struct hvm_function_table {
     int (*altp2m_vcpu_emulate_vmfunc)(const struct cpu_user_regs *regs);
 #endif
 
+#ifdef CONFIG_VMTRACE
     /* vmtrace */
     int (*vmtrace_control)(struct vcpu *v, bool enable, bool reset);
     int (*vmtrace_output_position)(struct vcpu *v, uint64_t *pos);
     int (*vmtrace_set_option)(struct vcpu *v, uint64_t key, uint64_t value);
     int (*vmtrace_get_option)(struct vcpu *v, uint64_t key, uint64_t *value);
     int (*vmtrace_reset)(struct vcpu *v);
+#endif
 
     uint64_t (*get_reg)(struct vcpu *v, unsigned int reg);
     void (*set_reg)(struct vcpu *v, unsigned int reg, uint64_t val);
@@ -738,6 +740,7 @@ static inline bool altp2m_vcpu_emulate_ve(struct vcpu *v)
 bool altp2m_vcpu_emulate_ve(struct vcpu *v);
 #endif /* CONFIG_ALTP2M */
 
+#ifdef CONFIG_VMTRACE
 static inline int hvm_vmtrace_control(struct vcpu *v, bool enable, bool reset)
 {
     if ( hvm_funcs.vmtrace_control )
@@ -780,6 +783,12 @@ static inline int hvm_vmtrace_reset(struct vcpu *v)
 
     return -EOPNOTSUPP;
 }
+#else
+static inline int hvm_vmtrace_reset(struct vcpu *v)
+{
+    return 0;
+}
+#endif
 
 /*
  * Accessors for registers which have per-guest-type or per-vendor locations
diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
index 8ff7c8045fc6..d28a2682e9df 100644
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -155,7 +155,9 @@ struct vmx_vcpu {
     bool                 ept_spurious_misconfig;
 
     /* Processor Trace configured and enabled for the vcpu. */
+#ifdef CONFIG_VMTRACE
     bool                 ipt_active;
+#endif
 
     /* Is the guest in real mode? */
     uint8_t              vmx_realmode;
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 4787b2796479..074f1b2562b3 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1888,7 +1888,9 @@ static int fork(struct domain *cd, struct domain *d)
         domain_pause(d);
         cd->max_pages = d->max_pages;
         *cd->arch.cpu_policy = *d->arch.cpu_policy;
+#ifdef CONFIG_VMTRACE
         cd->vmtrace_size = d->vmtrace_size;
+#endif
         cd->parent = d;
     }
 
diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
index fc349270b9c5..f4c8696ce54e 100644
--- a/xen/arch/x86/vm_event.c
+++ b/xen/arch/x86/vm_event.c
@@ -253,7 +253,9 @@ void vm_event_fill_regs(vm_event_request_t *req)
     req->data.regs.x86.shadow_gs = ctxt.shadow_gs;
     req->data.regs.x86.dr6 = ctxt.dr6;
 
+#ifdef CONFIG_VMTRACE
     if ( hvm_vmtrace_output_position(curr, &req->data.regs.x86.vmtrace_pos) != 1 )
+#endif
         req->data.regs.x86.vmtrace_pos = ~0;
 #endif
 }
@@ -303,12 +305,12 @@ void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp)
 #endif
 }
 
+#ifdef CONFIG_VMTRACE
 void vm_event_reset_vmtrace(struct vcpu *v)
 {
-#ifdef CONFIG_HVM
     hvm_vmtrace_reset(v);
-#endif
 }
+#endif
 
 /*
  * Local variables:
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 7dcd466e5a12..2be6ee03d004 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -136,7 +136,9 @@ struct vcpu *idle_vcpu[NR_CPUS] __read_mostly;
 
 vcpu_info_t dummy_vcpu_info;
 
+#ifdef CONFIG_VMTRACE
 bool __read_mostly vmtrace_available;
+#endif
 
 bool __read_mostly vpmu_is_available;
 
@@ -318,6 +320,7 @@ static void vcpu_info_reset(struct vcpu *v)
 
 static void vmtrace_free_buffer(struct vcpu *v)
 {
+#ifdef CONFIG_VMTRACE
     const struct domain *d = v->domain;
     struct page_info *pg = v->vmtrace.pg;
     unsigned int i;
@@ -332,10 +335,12 @@ static void vmtrace_free_buffer(struct vcpu *v)
         put_page_alloc_ref(&pg[i]);
         put_page_and_type(&pg[i]);
     }
+#endif
 }
 
 static int vmtrace_alloc_buffer(struct vcpu *v)
 {
+#ifdef CONFIG_VMTRACE
     struct domain *d = v->domain;
     struct page_info *pg;
     unsigned int i;
@@ -377,6 +382,9 @@ static int vmtrace_alloc_buffer(struct vcpu *v)
     }
 
     return -ENODATA;
+#else
+    return 0;
+#endif
 }
 
 /*
@@ -825,7 +833,9 @@ struct domain *domain_create(domid_t domid,
         ASSERT(!config->altp2m.nr);
 #endif
 
+#ifdef CONFIG_VMTRACE
         d->vmtrace_size = config->vmtrace_size;
+#endif
     }
 
     /* Sort out our idea of is_control_domain(). */
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 3688e6dd5032..66dc7f7a0a41 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -1155,8 +1155,10 @@ static unsigned int resource_max_frames(const struct domain *d,
     case XENMEM_resource_ioreq_server:
         return ioreq_server_max_frames(d);
 
+#ifdef CONFIG_VMTRACE
     case XENMEM_resource_vmtrace_buf:
         return d->vmtrace_size >> PAGE_SHIFT;
+#endif
 
     default:
         return 0;
@@ -1198,6 +1200,7 @@ static int acquire_ioreq_server(struct domain *d,
 #endif
 }
 
+#ifdef CONFIG_VMTRACE
 static int acquire_vmtrace_buf(
     struct domain *d, unsigned int id, unsigned int frame,
     unsigned int nr_frames, xen_pfn_t mfn_list[])
@@ -1220,6 +1223,7 @@ static int acquire_vmtrace_buf(
 
     return nr_frames;
 }
+#endif
 
 /*
  * Returns -errno on error, or positive in the range [1, nr_frames] on
@@ -1238,8 +1242,10 @@ static int _acquire_resource(
     case XENMEM_resource_ioreq_server:
         return acquire_ioreq_server(d, id, frame, nr_frames, mfn_list);
 
+#ifdef CONFIG_VMTRACE
     case XENMEM_resource_vmtrace_buf:
         return acquire_vmtrace_buf(d, id, frame, nr_frames, mfn_list);
+#endif
 
     default:
         ASSERT_UNREACHABLE();
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index b2787c010890..cf0258223f50 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -441,7 +441,8 @@ static int vm_event_resume(struct domain *d, struct vm_event_domain *ved)
             if ( rsp.flags & VM_EVENT_FLAG_GET_NEXT_INTERRUPT )
                 vm_event_monitor_next_interrupt(v);
 
-            if ( rsp.flags & VM_EVENT_FLAG_RESET_VMTRACE )
+            if ( IS_ENABLED(CONFIG_VMTRACE) &&
+                 (rsp.flags & VM_EVENT_FLAG_RESET_VMTRACE) )
                 vm_event_reset_vmtrace(v);
 
             if ( rsp.flags & VM_EVENT_FLAG_VCPU_PAUSED )
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 8aab05ae93c8..aa86a9f46022 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -191,7 +191,11 @@ void vnuma_destroy(struct vnuma_info *vnuma);
 static inline void vnuma_destroy(struct vnuma_info *vnuma) { ASSERT(!vnuma); }
 #endif
 
+#ifdef CONFIG_VMTRACE
 extern bool vmtrace_available;
+#else
+#define vmtrace_available false
+#endif
 
 extern bool vpmu_is_available;
 
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 02bdc256ce37..dcd8647e0d36 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -300,9 +300,11 @@ struct vcpu
     /* vPCI per-vCPU area, used to store data for long running operations. */
     struct vpci_vcpu vpci;
 
+#ifdef CONFIG_VMTRACE
     struct {
         struct page_info *pg; /* One contiguous allocation of d->vmtrace_size */
     } vmtrace;
+#endif
 
     struct arch_vcpu arch;
 
@@ -623,7 +625,9 @@ struct domain
     unsigned int nr_altp2m;    /* Number of altp2m tables. */
 #endif
 
+#ifdef CONFIG_VMTRACE
     unsigned int vmtrace_size; /* Buffer size in bytes, or 0 to disable. */
+#endif
 
 #ifdef CONFIG_ARGO
     /* Argo interdomain communication support */
-- 
2.34.1

On 31.10.2025 22:20, Grygorii Strashko wrote:
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -155,4 +155,19 @@ config DEBUG_INFO
>  	  "make install-xen" for installing xen.efi, stripping needs to be
>  	  done outside the Xen build environment).
>  
> +config HAS_VMTRACE
> +    bool
> +
> +config VMTRACE
> +    bool "HW VM tracing support"
> +    depends on HAS_VMTRACE
> +    default y
> +    help
> +      Enables HW VM tracing support which allows to configure HW processor
> +      features (vmtrace_op) to enable capturing information about software
> +      execution using dedicated hardware facilities with minimal interference
> +      to the software being traced. The trace date can be retrieved using buffer

Nit: s/date/data/

> +      shared between Xen and domain
> +      (see XENMEM_acquire_resource(XENMEM_resource_vmtrace_buf)).
> +

I was actually meaning to ask that "VMX only" should somehow be mentioned here,
but then I noticed this is an arch-independent location. I'm not quite sure we
want it like this (just yet).

> @@ -2940,11 +2948,13 @@ static struct hvm_function_table __initdata_cf_clobber vmx_function_table = {
>      .altp2m_vcpu_emulate_ve = vmx_vcpu_emulate_ve,
>      .altp2m_vcpu_emulate_vmfunc = vmx_vcpu_emulate_vmfunc,
>  #endif
> +#ifdef CONFIG_VMTRACE
>      .vmtrace_control = vmtrace_control,
>      .vmtrace_output_position = vmtrace_output_position,
>      .vmtrace_set_option = vmtrace_set_option,
>      .vmtrace_get_option = vmtrace_get_option,
>      .vmtrace_reset = vmtrace_reset,
> +#endif
>  
>      .get_reg = vmx_get_reg,
>      .set_reg = vmx_set_reg,

Blank line ahead of the new #ifdef?

> @@ -738,6 +740,7 @@ static inline bool altp2m_vcpu_emulate_ve(struct vcpu *v)
>  bool altp2m_vcpu_emulate_ve(struct vcpu *v);
>  #endif /* CONFIG_ALTP2M */
>  
> +#ifdef CONFIG_VMTRACE
>  static inline int hvm_vmtrace_control(struct vcpu *v, bool enable, bool reset)
>  {
>      if ( hvm_funcs.vmtrace_control )
> @@ -780,6 +783,12 @@ static inline int hvm_vmtrace_reset(struct vcpu *v)
>  
>      return -EOPNOTSUPP;
>  }
> +#else
> +static inline int hvm_vmtrace_reset(struct vcpu *v)
> +{
> +    return 0;
> +}
> +#endif

#ifdef inside the function body please, to reduce redundancy and to reduce the
risk of overlooking multiple places which need editing (when e.g. function
parameters change).

> --- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
> +++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
> @@ -155,7 +155,9 @@ struct vmx_vcpu {
>      bool                 ept_spurious_misconfig;
>  
>      /* Processor Trace configured and enabled for the vcpu. */
> +#ifdef CONFIG_VMTRACE
>      bool                 ipt_active;
> +#endif

Imo such an #ifdef would better enclose the comment as well.

> --- a/xen/arch/x86/vm_event.c
> +++ b/xen/arch/x86/vm_event.c
> @@ -253,7 +253,9 @@ void vm_event_fill_regs(vm_event_request_t *req)
>      req->data.regs.x86.shadow_gs = ctxt.shadow_gs;
>      req->data.regs.x86.dr6 = ctxt.dr6;
>  
> +#ifdef CONFIG_VMTRACE
>      if ( hvm_vmtrace_output_position(curr, &req->data.regs.x86.vmtrace_pos) != 1 )
> +#endif
>          req->data.regs.x86.vmtrace_pos = ~0;
>  #endif
>  }

Use IS_ENABLED() together with a function declaration (but no definition) in the
VMTRACE=n case?

Jan

Hi Jan,

On 06.11.25 14:00, Jan Beulich wrote:
> On 31.10.2025 22:20, Grygorii Strashko wrote:
>> --- a/xen/Kconfig.debug
>> +++ b/xen/Kconfig.debug
>> @@ -155,4 +155,19 @@ config DEBUG_INFO
>>   	  "make install-xen" for installing xen.efi, stripping needs to be
>>   	  done outside the Xen build environment).
>>   
>> +config HAS_VMTRACE
>> +    bool
>> +
>> +config VMTRACE
>> +    bool "HW VM tracing support"
>> +    depends on HAS_VMTRACE
>> +    default y
>> +    help
>> +      Enables HW VM tracing support which allows to configure HW processor
>> +      features (vmtrace_op) to enable capturing information about software
>> +      execution using dedicated hardware facilities with minimal interference
>> +      to the software being traced. The trace date can be retrieved using buffer
> 
> Nit: s/date/data/
> 
>> +      shared between Xen and domain
>> +      (see XENMEM_acquire_resource(XENMEM_resource_vmtrace_buf)).
>> +
> 
> I was actually meaning to ask that "VMX only" should somehow be mentioned here,
> but then I noticed this is an arch-independent location. 

Right, Arch code advertise VMTRACE support with HAS_VMTRACE.
In this particular case:
config INTEL_VMX
...
	select HAS_VMTRACE


> I'm not quite sure we want it like this (just yet).

?

> 
>> @@ -2940,11 +2948,13 @@ static struct hvm_function_table __initdata_cf_clobber vmx_function_table = {
>>       .altp2m_vcpu_emulate_ve = vmx_vcpu_emulate_ve,
>>       .altp2m_vcpu_emulate_vmfunc = vmx_vcpu_emulate_vmfunc,
>>   #endif
>> +#ifdef CONFIG_VMTRACE
>>       .vmtrace_control = vmtrace_control,
>>       .vmtrace_output_position = vmtrace_output_position,
>>       .vmtrace_set_option = vmtrace_set_option,
>>       .vmtrace_get_option = vmtrace_get_option,
>>       .vmtrace_reset = vmtrace_reset,
>> +#endif
>>   
>>       .get_reg = vmx_get_reg,
>>       .set_reg = vmx_set_reg,
> 
> Blank line ahead of the new #ifdef?
> 
>> @@ -738,6 +740,7 @@ static inline bool altp2m_vcpu_emulate_ve(struct vcpu *v)
>>   bool altp2m_vcpu_emulate_ve(struct vcpu *v);
>>   #endif /* CONFIG_ALTP2M */
>>   
>> +#ifdef CONFIG_VMTRACE
>>   static inline int hvm_vmtrace_control(struct vcpu *v, bool enable, bool reset)
>>   {
>>       if ( hvm_funcs.vmtrace_control )
>> @@ -780,6 +783,12 @@ static inline int hvm_vmtrace_reset(struct vcpu *v)
>>   
>>       return -EOPNOTSUPP;
>>   }
>> +#else
>> +static inline int hvm_vmtrace_reset(struct vcpu *v)
>> +{
>> +    return 0;
>> +}
>> +#endif
> 
> #ifdef inside the function body please, to reduce redundancy and to reduce the
> risk of overlooking multiple places which need editing (when e.g. function
> parameters change).

All hvm_vmtrace_x() functions are inline - do you mean like below for all of them?

  static inline int hvm_vmtrace_get_option(
      struct vcpu *v, uint64_t key, uint64_t *value)
  {
+#ifdef CONFIG_VMTRACE
      if ( hvm_funcs.vmtrace_get_option )
          return alternative_call(hvm_funcs.vmtrace_get_option, v, key, value);
+#endif
  
      return -EOPNOTSUPP;
  }


> 
>> --- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
>> +++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
>> @@ -155,7 +155,9 @@ struct vmx_vcpu {
>>       bool                 ept_spurious_misconfig;
>>   
>>       /* Processor Trace configured and enabled for the vcpu. */
>> +#ifdef CONFIG_VMTRACE
>>       bool                 ipt_active;
>> +#endif
> 
> Imo such an #ifdef would better enclose the comment as well.
> 
>> --- a/xen/arch/x86/vm_event.c
>> +++ b/xen/arch/x86/vm_event.c
>> @@ -253,7 +253,9 @@ void vm_event_fill_regs(vm_event_request_t *req)
>>       req->data.regs.x86.shadow_gs = ctxt.shadow_gs;
>>       req->data.regs.x86.dr6 = ctxt.dr6;
>>   
>> +#ifdef CONFIG_VMTRACE
>>       if ( hvm_vmtrace_output_position(curr, &req->data.regs.x86.vmtrace_pos) != 1 )
>> +#endif
>>           req->data.regs.x86.vmtrace_pos = ~0;
>>   #endif
>>   }
> 
> Use IS_ENABLED() together with a function declaration (but no definition) in the
> VMTRACE=n case?


-- 
Best regards,
-grygorii

On 06.11.2025 14:50, Grygorii Strashko wrote:
> On 06.11.25 14:00, Jan Beulich wrote:
>> On 31.10.2025 22:20, Grygorii Strashko wrote:
>>> --- a/xen/Kconfig.debug
>>> +++ b/xen/Kconfig.debug
>>> @@ -155,4 +155,19 @@ config DEBUG_INFO
>>>   	  "make install-xen" for installing xen.efi, stripping needs to be
>>>   	  done outside the Xen build environment).
>>>   
>>> +config HAS_VMTRACE
>>> +    bool
>>> +
>>> +config VMTRACE
>>> +    bool "HW VM tracing support"
>>> +    depends on HAS_VMTRACE
>>> +    default y
>>> +    help
>>> +      Enables HW VM tracing support which allows to configure HW processor
>>> +      features (vmtrace_op) to enable capturing information about software
>>> +      execution using dedicated hardware facilities with minimal interference
>>> +      to the software being traced. The trace date can be retrieved using buffer
>>
>> Nit: s/date/data/
>>
>>> +      shared between Xen and domain
>>> +      (see XENMEM_acquire_resource(XENMEM_resource_vmtrace_buf)).
>>> +
>>
>> I was actually meaning to ask that "VMX only" should somehow be mentioned here,
>> but then I noticed this is an arch-independent location. 
> 
> Right, Arch code advertise VMTRACE support with HAS_VMTRACE.
> In this particular case:
> config INTEL_VMX
> ...
> 	select HAS_VMTRACE
> 
> 
>> I'm not quite sure we want it like this (just yet).
> 
> ?

To rephrase the question: Are we expecting anything other than VMX to support
VMTRACE any time soon?

>>> @@ -738,6 +740,7 @@ static inline bool altp2m_vcpu_emulate_ve(struct vcpu *v)
>>>   bool altp2m_vcpu_emulate_ve(struct vcpu *v);
>>>   #endif /* CONFIG_ALTP2M */
>>>   
>>> +#ifdef CONFIG_VMTRACE
>>>   static inline int hvm_vmtrace_control(struct vcpu *v, bool enable, bool reset)
>>>   {
>>>       if ( hvm_funcs.vmtrace_control )
>>> @@ -780,6 +783,12 @@ static inline int hvm_vmtrace_reset(struct vcpu *v)
>>>   
>>>       return -EOPNOTSUPP;
>>>   }
>>> +#else
>>> +static inline int hvm_vmtrace_reset(struct vcpu *v)
>>> +{
>>> +    return 0;
>>> +}
>>> +#endif
>>
>> #ifdef inside the function body please, to reduce redundancy and to reduce the
>> risk of overlooking multiple places which need editing (when e.g. function
>> parameters change).
> 
> All hvm_vmtrace_x() functions are inline - do you mean like below for all of them?
> 
>   static inline int hvm_vmtrace_get_option(
>       struct vcpu *v, uint64_t key, uint64_t *value)
>   {
> +#ifdef CONFIG_VMTRACE
>       if ( hvm_funcs.vmtrace_get_option )
>           return alternative_call(hvm_funcs.vmtrace_get_option, v, key, value);
> +#endif
>   
>       return -EOPNOTSUPP;
>   }

No, the request was for just the single function that you add a 2nd instance of.

Jan

Hi

On 06.11.25 16:09, Jan Beulich wrote:
> On 06.11.2025 14:50, Grygorii Strashko wrote:
>> On 06.11.25 14:00, Jan Beulich wrote:
>>> On 31.10.2025 22:20, Grygorii Strashko wrote:
>>>> --- a/xen/Kconfig.debug
>>>> +++ b/xen/Kconfig.debug
>>>> @@ -155,4 +155,19 @@ config DEBUG_INFO
>>>>    	  "make install-xen" for installing xen.efi, stripping needs to be
>>>>    	  done outside the Xen build environment).
>>>>    
>>>> +config HAS_VMTRACE
>>>> +    bool
>>>> +
>>>> +config VMTRACE
>>>> +    bool "HW VM tracing support"
>>>> +    depends on HAS_VMTRACE
>>>> +    default y
>>>> +    help
>>>> +      Enables HW VM tracing support which allows to configure HW processor
>>>> +      features (vmtrace_op) to enable capturing information about software
>>>> +      execution using dedicated hardware facilities with minimal interference
>>>> +      to the software being traced. The trace date can be retrieved using buffer
>>>
>>> Nit: s/date/data/
>>>
>>>> +      shared between Xen and domain
>>>> +      (see XENMEM_acquire_resource(XENMEM_resource_vmtrace_buf)).
>>>> +
>>>
>>> I was actually meaning to ask that "VMX only" should somehow be mentioned here,
>>> but then I noticed this is an arch-independent location.
>>
>> Right, Arch code advertise VMTRACE support with HAS_VMTRACE.
>> In this particular case:
>> config INTEL_VMX
>> ...
>> 	select HAS_VMTRACE
>>
>>
>>> I'm not quite sure we want it like this (just yet).
>>
>> ?
> 
> To rephrase the question: Are we expecting anything other than VMX to support
> VMTRACE any time soon?
> 

That's I do not know.

I assume your point is similar to what Teddy noted [1].

I think vmtrace code can be consolidate, but question is on what level(s):

only:
  xen/arch/x86/hvm/vmx/
  |- vmtrace.c

or:
  xen/arch/x86/hvm
  |- vmtrace.c
     <- vmtrace_alloc/free_buffer(), acquire_vmtrace_buf(), do_vmtrace_op()
  xen/arch/x86/hvm/vmx/
  |- vmtrace.c

it will require more work comparing to the current change.

[1] https://patchwork.kernel.org/comment/26637627/

-- 
Best regards,
-grygorii

On 07.11.2025 16:47, Grygorii Strashko wrote:
> Hi
> 
> On 06.11.25 16:09, Jan Beulich wrote:
>> On 06.11.2025 14:50, Grygorii Strashko wrote:
>>> On 06.11.25 14:00, Jan Beulich wrote:
>>>> On 31.10.2025 22:20, Grygorii Strashko wrote:
>>>>> --- a/xen/Kconfig.debug
>>>>> +++ b/xen/Kconfig.debug
>>>>> @@ -155,4 +155,19 @@ config DEBUG_INFO
>>>>>          "make install-xen" for installing xen.efi, stripping needs to be
>>>>>          done outside the Xen build environment).
>>>>>    +config HAS_VMTRACE
>>>>> +    bool
>>>>> +
>>>>> +config VMTRACE
>>>>> +    bool "HW VM tracing support"
>>>>> +    depends on HAS_VMTRACE
>>>>> +    default y
>>>>> +    help
>>>>> +      Enables HW VM tracing support which allows to configure HW processor
>>>>> +      features (vmtrace_op) to enable capturing information about software
>>>>> +      execution using dedicated hardware facilities with minimal interference
>>>>> +      to the software being traced. The trace date can be retrieved using buffer
>>>>
>>>> Nit: s/date/data/
>>>>
>>>>> +      shared between Xen and domain
>>>>> +      (see XENMEM_acquire_resource(XENMEM_resource_vmtrace_buf)).
>>>>> +
>>>>
>>>> I was actually meaning to ask that "VMX only" should somehow be mentioned here,
>>>> but then I noticed this is an arch-independent location.
>>>
>>> Right, Arch code advertise VMTRACE support with HAS_VMTRACE.
>>> In this particular case:
>>> config INTEL_VMX
>>> ...
>>>     select HAS_VMTRACE
>>>
>>>
>>>> I'm not quite sure we want it like this (just yet).
>>>
>>> ?
>>
>> To rephrase the question: Are we expecting anything other than VMX to support
>> VMTRACE any time soon?
>>
> 
> That's I do not know.
> 
> I assume your point is similar to what Teddy noted [1].
> 
> I think vmtrace code can be consolidate, but question is on what level(s):
> 
> only:
>  xen/arch/x86/hvm/vmx/
>  |- vmtrace.c
> 
> or:
>  xen/arch/x86/hvm
>  |- vmtrace.c
>     <- vmtrace_alloc/free_buffer(), acquire_vmtrace_buf(), do_vmtrace_op()
>  xen/arch/x86/hvm/vmx/
>  |- vmtrace.c
> 
> it will require more work comparing to the current change.

Well, I don't think code movement is strictly necessary here. But as I'm unconvinced
of Kconfig.debug (in whatever subdir) being an appropriate place to add this, for
the time being merely putting the new Kconfig option directly next to INTEL_VMX (and
then without any new HAS_*) wants at least considering, imo. If and when some other
use appears, HAS_* can be introduced and the whole thing moved.

As otoh abstracting things in an arch-independent way also has its merits, it is -
as said - not quite clear to me which way we'd prefer to have it.

Jan

> [1] https://patchwork.kernel.org/comment/26637627/
> 

On 10.11.25 08:36, Jan Beulich wrote:
> On 07.11.2025 16:47, Grygorii Strashko wrote:
>> Hi
>>
>> On 06.11.25 16:09, Jan Beulich wrote:
>>> On 06.11.2025 14:50, Grygorii Strashko wrote:
>>>> On 06.11.25 14:00, Jan Beulich wrote:
>>>>> On 31.10.2025 22:20, Grygorii Strashko wrote:
>>>>>> --- a/xen/Kconfig.debug
>>>>>> +++ b/xen/Kconfig.debug
>>>>>> @@ -155,4 +155,19 @@ config DEBUG_INFO
>>>>>>           "make install-xen" for installing xen.efi, stripping needs to be
>>>>>>           done outside the Xen build environment).
>>>>>>     +config HAS_VMTRACE
>>>>>> +    bool
>>>>>> +
>>>>>> +config VMTRACE
>>>>>> +    bool "HW VM tracing support"
>>>>>> +    depends on HAS_VMTRACE
>>>>>> +    default y
>>>>>> +    help
>>>>>> +      Enables HW VM tracing support which allows to configure HW processor
>>>>>> +      features (vmtrace_op) to enable capturing information about software
>>>>>> +      execution using dedicated hardware facilities with minimal interference
>>>>>> +      to the software being traced. The trace date can be retrieved using buffer
>>>>>
>>>>> Nit: s/date/data/
>>>>>
>>>>>> +      shared between Xen and domain
>>>>>> +      (see XENMEM_acquire_resource(XENMEM_resource_vmtrace_buf)).
>>>>>> +
>>>>>
>>>>> I was actually meaning to ask that "VMX only" should somehow be mentioned here,
>>>>> but then I noticed this is an arch-independent location.
>>>>
>>>> Right, Arch code advertise VMTRACE support with HAS_VMTRACE.
>>>> In this particular case:
>>>> config INTEL_VMX
>>>> ...
>>>>      select HAS_VMTRACE
>>>>
>>>>
>>>>> I'm not quite sure we want it like this (just yet).
>>>>
>>>> ?
>>>
>>> To rephrase the question: Are we expecting anything other than VMX to support
>>> VMTRACE any time soon?
>>>
>>
>> That's I do not know.
>>
>> I assume your point is similar to what Teddy noted [1].
>>
>> I think vmtrace code can be consolidate, but question is on what level(s):
>>
>> only:
>>   xen/arch/x86/hvm/vmx/
>>   |- vmtrace.c
>>
>> or:
>>   xen/arch/x86/hvm
>>   |- vmtrace.c
>>      <- vmtrace_alloc/free_buffer(), acquire_vmtrace_buf(), do_vmtrace_op()
>>   xen/arch/x86/hvm/vmx/
>>   |- vmtrace.c
>>
>> it will require more work comparing to the current change.
> 
> Well, I don't think code movement is strictly necessary here. But as I'm unconvinced
> of Kconfig.debug (in whatever subdir) being an appropriate place to add this, for
> the time being merely putting the new Kconfig option directly next to INTEL_VMX (and
> then without any new HAS_*) wants at least considering, imo. If and when some other
> use appears, HAS_* can be introduced and the whole thing moved.

For me it will be fine.

> 
> As otoh abstracting things in an arch-independent way also has its merits, it is -
> as said - not quite clear to me which way we'd prefer to have it.
> 
> Jan
> 
>> [1] https://patchwork.kernel.org/comment/26637627/
>>
> 

-- 
Best regards,
-grygorii

Le 31/10/2025 à 22:24, Grygorii Strashko a écrit :
> From: Grygorii Strashko <grygorii_strashko@epam.com>
> 
> The VMTRACE feature depends on Platform/Arch HW and code support and now
> can be used only on x86 HVM with Intel VT-x (INTEL_VMX) enabled.
> This makes VMTRACE support optional by introducing two Kconfig options:
> - CONFIG_HAS_VMTRACE which allows Platform/Arch to advertise VMTRACE
> support readiness
> - CONFIG_VMTRACE to enable/disable the feature.
> 

I like the idea of making it optional since it's only used in specific 
contexts.

> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
> ---
> Based on top of patch "domain: adjust soft-reset arch dependency" [1]
> 
> [1] https://patchwork.kernel.org/project/xen-devel/patch/c9c8c9c6-a155-4986-bf77-5766cdcd6024@suse.com/
> 
>   xen/Kconfig.debug                       | 15 +++++++++++++++
>   xen/arch/x86/domctl.c                   |  4 ++++
>   xen/arch/x86/hvm/Kconfig                |  1 +
>   xen/arch/x86/hvm/vmx/vmcs.c             |  2 ++
>   xen/arch/x86/hvm/vmx/vmx.c              | 10 ++++++++++
>   xen/arch/x86/include/asm/guest-msr.h    |  2 ++
>   xen/arch/x86/include/asm/hvm/hvm.h      |  9 +++++++++
>   xen/arch/x86/include/asm/hvm/vmx/vmcs.h |  2 ++
>   xen/arch/x86/mm/mem_sharing.c           |  2 ++
>   xen/arch/x86/vm_event.c                 |  6 ++++--
>   xen/common/domain.c                     | 10 ++++++++++
>   xen/common/memory.c                     |  6 ++++++
>   xen/common/vm_event.c                   |  3 ++-
>   xen/include/xen/domain.h                |  4 ++++
>   xen/include/xen/sched.h                 |  4 ++++
>   15 files changed, 77 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index d900d926c555..70ec4f0d14a5 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -155,4 +155,19 @@ config DEBUG_INFO
>   	  "make install-xen" for installing xen.efi, stripping needs to be
>   	  done outside the Xen build environment).
>   
> +config HAS_VMTRACE
> +    bool
> +
> +config VMTRACE
> +    bool "HW VM tracing support"
> +    depends on HAS_VMTRACE
> +    default y
> +    help
> +      Enables HW VM tracing support which allows to configure HW processor
> +      features (vmtrace_op) to enable capturing information about software
> +      execution using dedicated hardware facilities with minimal interference
> +      to the software being traced. The trace date can be retrieved using buffer
> +      shared between Xen and domain
> +      (see XENMEM_acquire_resource(XENMEM_resource_vmtrace_buf)).
> +
>   endmenu
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 6153e3c07e2d..d9521808dcba 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -154,6 +154,7 @@ void arch_get_domain_info(const struct domain *d,
>   static int do_vmtrace_op(struct domain *d, struct xen_domctl_vmtrace_op *op,
>                            XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>   {
> +#ifdef CONFIG_VMTRACE
>       struct vcpu *v;
>       int rc;
>   
> @@ -198,6 +199,9 @@ static int do_vmtrace_op(struct domain *d, struct xen_domctl_vmtrace_op *op,
>       vcpu_unpause(v);
>   
>       return rc;
> +#else
> +    return -EOPNOTSUPP;
> +#endif
>   }
>   
>   #define MAX_IOPORTS 0x10000
> diff --git a/xen/arch/x86/hvm/Kconfig b/xen/arch/x86/hvm/Kconfig
> index c1a131d1851a..c017a77fffe0 100644
> --- a/xen/arch/x86/hvm/Kconfig
> +++ b/xen/arch/x86/hvm/Kconfig
> @@ -29,6 +29,7 @@ config INTEL_VMX
>   	bool "Intel VT-x" if EXPERT
>   	default INTEL
>   	select ARCH_VCPU_IOREQ_COMPLETION
> +	select HAS_VMTRACE
>   	help
>   	  Enables virtual machine extensions on platforms that implement the
>   	  Intel Virtualization Technology (Intel VT-x).
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index ab8b1c87ec0f..3728a9140223 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -300,6 +300,7 @@ static int vmx_init_vmcs_config(bool bsp)
>       rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap);
>   
>       /* Check whether IPT is supported in VMX operation. */
> +#ifdef CONFIG_VMTRACE
>       if ( bsp )
>           vmtrace_available = cpu_has_proc_trace &&
>                               (_vmx_misc_cap & VMX_MISC_PROC_TRACE);
> @@ -310,6 +311,7 @@ static int vmx_init_vmcs_config(bool bsp)
>                  smp_processor_id());
>           return -EINVAL;
>       }
> +#endif
>   
>       if ( caps.cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS )
>       {
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index b65981077295..f1588cd90b2d 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -622,6 +622,7 @@ static void cf_check domain_creation_finished(struct domain *d)
>   
>   static void vmx_init_ipt(struct vcpu *v)
>   {
> +#ifdef CONFIG_VMTRACE
>       unsigned int size = v->domain->vmtrace_size;
>   
>       if ( !size )
> @@ -632,6 +633,7 @@ static void vmx_init_ipt(struct vcpu *v)
>       ASSERT(size >= PAGE_SIZE && (size & (size - 1)) == 0);
>   
>       v->arch.msrs->rtit.output_limit = size - 1;
> +#endif
>   }
>   
>   static int cf_check vmx_vcpu_initialise(struct vcpu *v)
> @@ -724,11 +726,13 @@ static void vmx_save_guest_msrs(struct vcpu *v)
>        */
>       v->arch.hvm.vmx.shadow_gs = read_gs_shadow();
>   
> +#ifdef CONFIG_VMTRACE
>       if ( v->arch.hvm.vmx.ipt_active )
>       {
>           rdmsrl(MSR_RTIT_OUTPUT_MASK, msrs->rtit.output_mask);
>           rdmsrl(MSR_RTIT_STATUS, msrs->rtit.status);
>       }
> +#endif
>   
>       if ( cp->feat.pks )
>           msrs->pkrs = rdpkrs_and_cache();
> @@ -747,12 +751,14 @@ static void vmx_restore_guest_msrs(struct vcpu *v)
>       if ( cpu_has_msr_tsc_aux )
>           wrmsr_tsc_aux(msrs->tsc_aux);
>   
> +#ifdef CONFIG_VMTRACE
>       if ( v->arch.hvm.vmx.ipt_active )
>       {
>           wrmsrl(MSR_RTIT_OUTPUT_BASE, page_to_maddr(v->vmtrace.pg));
>           wrmsrl(MSR_RTIT_OUTPUT_MASK, msrs->rtit.output_mask);
>           wrmsrl(MSR_RTIT_STATUS, msrs->rtit.status);
>       }
> +#endif
>   
>       if ( cp->feat.pks )
>           wrpkrs(msrs->pkrs);
> @@ -2626,6 +2632,7 @@ static bool cf_check vmx_get_pending_event(
>       return true;
>   }
>   
> +#ifdef CONFIG_VMTRACE
>   /*
>    * We only let vmtrace agents see and modify a subset of bits in MSR_RTIT_CTL.
>    * These all pertain to data-emitted into the trace buffer(s).  Must not
> @@ -2768,6 +2775,7 @@ static int cf_check vmtrace_reset(struct vcpu *v)
>       v->arch.msrs->rtit.status = 0;
>       return 0;
>   }
> +#endif
>   
>   static uint64_t cf_check vmx_get_reg(struct vcpu *v, unsigned int reg)
>   {
> @@ -2940,11 +2948,13 @@ static struct hvm_function_table __initdata_cf_clobber vmx_function_table = {
>       .altp2m_vcpu_emulate_ve = vmx_vcpu_emulate_ve,
>       .altp2m_vcpu_emulate_vmfunc = vmx_vcpu_emulate_vmfunc,
>   #endif
> +#ifdef CONFIG_VMTRACE
>       .vmtrace_control = vmtrace_control,
>       .vmtrace_output_position = vmtrace_output_position,
>       .vmtrace_set_option = vmtrace_set_option,
>       .vmtrace_get_option = vmtrace_get_option,
>       .vmtrace_reset = vmtrace_reset,
> +#endif
>   
>       .get_reg = vmx_get_reg,
>       .set_reg = vmx_set_reg,
> diff --git a/xen/arch/x86/include/asm/guest-msr.h b/xen/arch/x86/include/asm/guest-msr.h
> index 5f0cb0a93995..702f47fe1e16 100644
> --- a/xen/arch/x86/include/asm/guest-msr.h
> +++ b/xen/arch/x86/include/asm/guest-msr.h
> @@ -50,6 +50,7 @@ struct vcpu_msrs
>           };
>       } misc_features_enables;
>   
> +#ifdef CONFIG_VMTRACE
>       /*
>        * 0x00000560 ... 57x - MSR_RTIT_*
>        *
> @@ -81,6 +82,7 @@ struct vcpu_msrs
>               };
>           };
>       } rtit;
> +#endif
>   
>       /*
>        * 0x000006e1 - MSR_PKRS - Protection Key Supervisor.
> diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h
> index 7412256a2dab..728b9624522f 100644
> --- a/xen/arch/x86/include/asm/hvm/hvm.h
> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
> @@ -234,12 +234,14 @@ struct hvm_function_table {
>       int (*altp2m_vcpu_emulate_vmfunc)(const struct cpu_user_regs *regs);
>   #endif
>   
> +#ifdef CONFIG_VMTRACE
>       /* vmtrace */
>       int (*vmtrace_control)(struct vcpu *v, bool enable, bool reset);
>       int (*vmtrace_output_position)(struct vcpu *v, uint64_t *pos);
>       int (*vmtrace_set_option)(struct vcpu *v, uint64_t key, uint64_t value);
>       int (*vmtrace_get_option)(struct vcpu *v, uint64_t key, uint64_t *value);
>       int (*vmtrace_reset)(struct vcpu *v);
> +#endif
>   
>       uint64_t (*get_reg)(struct vcpu *v, unsigned int reg);
>       void (*set_reg)(struct vcpu *v, unsigned int reg, uint64_t val);
> @@ -738,6 +740,7 @@ static inline bool altp2m_vcpu_emulate_ve(struct vcpu *v)
>   bool altp2m_vcpu_emulate_ve(struct vcpu *v);
>   #endif /* CONFIG_ALTP2M */
>   
> +#ifdef CONFIG_VMTRACE
>   static inline int hvm_vmtrace_control(struct vcpu *v, bool enable, bool reset)
>   {
>       if ( hvm_funcs.vmtrace_control )
> @@ -780,6 +783,12 @@ static inline int hvm_vmtrace_reset(struct vcpu *v)
>   
>       return -EOPNOTSUPP;
>   }
> +#else
> +static inline int hvm_vmtrace_reset(struct vcpu *v)
> +{
> +    return 0;
> +}
> +#endif
>   
>   /*
>    * Accessors for registers which have per-guest-type or per-vendor locations
> diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
> index 8ff7c8045fc6..d28a2682e9df 100644
> --- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
> +++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
> @@ -155,7 +155,9 @@ struct vmx_vcpu {
>       bool                 ept_spurious_misconfig;
>   
>       /* Processor Trace configured and enabled for the vcpu. */
> +#ifdef CONFIG_VMTRACE
>       bool                 ipt_active;
> +#endif
>   
>       /* Is the guest in real mode? */
>       uint8_t              vmx_realmode;
> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
> index 4787b2796479..074f1b2562b3 100644
> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -1888,7 +1888,9 @@ static int fork(struct domain *cd, struct domain *d)
>           domain_pause(d);
>           cd->max_pages = d->max_pages;
>           *cd->arch.cpu_policy = *d->arch.cpu_policy;
> +#ifdef CONFIG_VMTRACE
>           cd->vmtrace_size = d->vmtrace_size;
> +#endif
>           cd->parent = d;
>       }
>   
> diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
> index fc349270b9c5..f4c8696ce54e 100644
> --- a/xen/arch/x86/vm_event.c
> +++ b/xen/arch/x86/vm_event.c
> @@ -253,7 +253,9 @@ void vm_event_fill_regs(vm_event_request_t *req)
>       req->data.regs.x86.shadow_gs = ctxt.shadow_gs;
>       req->data.regs.x86.dr6 = ctxt.dr6;
>   
> +#ifdef CONFIG_VMTRACE
>       if ( hvm_vmtrace_output_position(curr, &req->data.regs.x86.vmtrace_pos) != 1 )
> +#endif
>           req->data.regs.x86.vmtrace_pos = ~0;

This if-def looks very oddly placed.

>   #endif
>   }
> @@ -303,12 +305,12 @@ void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp)
>   #endif
>   }
>   
> +#ifdef CONFIG_VMTRACE
>   void vm_event_reset_vmtrace(struct vcpu *v)
>   {
> -#ifdef CONFIG_HVM
>       hvm_vmtrace_reset(v);
> -#endif
>   }
> +#endif
>   
>   /*
>    * Local variables:
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 7dcd466e5a12..2be6ee03d004 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -136,7 +136,9 @@ struct vcpu *idle_vcpu[NR_CPUS] __read_mostly;
>   
>   vcpu_info_t dummy_vcpu_info;
>   
> +#ifdef CONFIG_VMTRACE
>   bool __read_mostly vmtrace_available;
> +#endif
>   
>   bool __read_mostly vpmu_is_available;
>   
> @@ -318,6 +320,7 @@ static void vcpu_info_reset(struct vcpu *v)
>   
>   static void vmtrace_free_buffer(struct vcpu *v)
>   {
> +#ifdef CONFIG_VMTRACE
>       const struct domain *d = v->domain;
>       struct page_info *pg = v->vmtrace.pg;
>       unsigned int i;
> @@ -332,10 +335,12 @@ static void vmtrace_free_buffer(struct vcpu *v)
>           put_page_alloc_ref(&pg[i]);
>           put_page_and_type(&pg[i]);
>       }
> +#endif
>   }
>   
>   static int vmtrace_alloc_buffer(struct vcpu *v)
>   {
> +#ifdef CONFIG_VMTRACE
>       struct domain *d = v->domain;
>       struct page_info *pg;
>       unsigned int i;
> @@ -377,6 +382,9 @@ static int vmtrace_alloc_buffer(struct vcpu *v)
>       }
>   
>       return -ENODATA;
> +#else
> +    return 0;
> +#endif
>   }
>   
>   /*
> @@ -825,7 +833,9 @@ struct domain *domain_create(domid_t domid,
>           ASSERT(!config->altp2m.nr);
>   #endif
>   
> +#ifdef CONFIG_VMTRACE
>           d->vmtrace_size = config->vmtrace_size;
> +#endif
>       }
>   
>       /* Sort out our idea of is_control_domain(). */
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index 3688e6dd5032..66dc7f7a0a41 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -1155,8 +1155,10 @@ static unsigned int resource_max_frames(const struct domain *d,
>       case XENMEM_resource_ioreq_server:
>           return ioreq_server_max_frames(d);
>   
> +#ifdef CONFIG_VMTRACE
>       case XENMEM_resource_vmtrace_buf:
>           return d->vmtrace_size >> PAGE_SHIFT;
> +#endif
>   
>       default:
>           return 0;
> @@ -1198,6 +1200,7 @@ static int acquire_ioreq_server(struct domain *d,
>   #endif
>   }
>   
> +#ifdef CONFIG_VMTRACE
>   static int acquire_vmtrace_buf(
>       struct domain *d, unsigned int id, unsigned int frame,
>       unsigned int nr_frames, xen_pfn_t mfn_list[])
> @@ -1220,6 +1223,7 @@ static int acquire_vmtrace_buf(
>   
>       return nr_frames;
>   }
> +#endif
>   

Given that vmtrace feature is fairly isolated in VMX code, wouldn't it 
be better to move all vmtrace related functions (including vmx_init_ipt) 
in a separate vmtrace.c file and make such file gated behind 
CONFIG_VMTRACE ?

>   /*
>    * Returns -errno on error, or positive in the range [1, nr_frames] on
> @@ -1238,8 +1242,10 @@ static int _acquire_resource(
>       case XENMEM_resource_ioreq_server:
>           return acquire_ioreq_server(d, id, frame, nr_frames, mfn_list);
>   
> +#ifdef CONFIG_VMTRACE
>       case XENMEM_resource_vmtrace_buf:
>           return acquire_vmtrace_buf(d, id, frame, nr_frames, mfn_list);
> +#endif
>   
>       default:
>           ASSERT_UNREACHABLE();
> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
> index b2787c010890..cf0258223f50 100644
> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -441,7 +441,8 @@ static int vm_event_resume(struct domain *d, struct vm_event_domain *ved)
>               if ( rsp.flags & VM_EVENT_FLAG_GET_NEXT_INTERRUPT )
>                   vm_event_monitor_next_interrupt(v);
>   
> -            if ( rsp.flags & VM_EVENT_FLAG_RESET_VMTRACE )
> +            if ( IS_ENABLED(CONFIG_VMTRACE) &&
> +                 (rsp.flags & VM_EVENT_FLAG_RESET_VMTRACE) )
>                   vm_event_reset_vmtrace(v);
>   
>               if ( rsp.flags & VM_EVENT_FLAG_VCPU_PAUSED )
> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> index 8aab05ae93c8..aa86a9f46022 100644
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -191,7 +191,11 @@ void vnuma_destroy(struct vnuma_info *vnuma);
>   static inline void vnuma_destroy(struct vnuma_info *vnuma) { ASSERT(!vnuma); }
>   #endif
>   
> +#ifdef CONFIG_VMTRACE
>   extern bool vmtrace_available;
> +#else
> +#define vmtrace_available false
> +#endif
>   
>   extern bool vpmu_is_available;
>   
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 02bdc256ce37..dcd8647e0d36 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -300,9 +300,11 @@ struct vcpu
>       /* vPCI per-vCPU area, used to store data for long running operations. */
>       struct vpci_vcpu vpci;
>   
> +#ifdef CONFIG_VMTRACE
>       struct {
>           struct page_info *pg; /* One contiguous allocation of d->vmtrace_size */
>       } vmtrace;
> +#endif
>   
>       struct arch_vcpu arch;
>   
> @@ -623,7 +625,9 @@ struct domain
>       unsigned int nr_altp2m;    /* Number of altp2m tables. */
>   #endif
>   
> +#ifdef CONFIG_VMTRACE
>       unsigned int vmtrace_size; /* Buffer size in bytes, or 0 to disable. */
> +#endif
>   
>   #ifdef CONFIG_ARGO
>       /* Argo interdomain communication support */



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

On 03.11.25 11:47, Teddy Astie wrote:
> Le 31/10/2025 à 22:24, Grygorii Strashko a écrit :
>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>>
>> The VMTRACE feature depends on Platform/Arch HW and code support and now
>> can be used only on x86 HVM with Intel VT-x (INTEL_VMX) enabled.
>> This makes VMTRACE support optional by introducing two Kconfig options:
>> - CONFIG_HAS_VMTRACE which allows Platform/Arch to advertise VMTRACE
>> support readiness
>> - CONFIG_VMTRACE to enable/disable the feature.
>>
> 
> I like the idea of making it optional since it's only used in specific
> contexts.
> 
>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
>> ---
>> Based on top of patch "domain: adjust soft-reset arch dependency" [1]
>>
>> [1] https://patchwork.kernel.org/project/xen-devel/patch/c9c8c9c6-a155-4986-bf77-5766cdcd6024@suse.com/
>>
>>    xen/Kconfig.debug                       | 15 +++++++++++++++
>>    xen/arch/x86/domctl.c                   |  4 ++++
>>    xen/arch/x86/hvm/Kconfig                |  1 +
>>    xen/arch/x86/hvm/vmx/vmcs.c             |  2 ++
>>    xen/arch/x86/hvm/vmx/vmx.c              | 10 ++++++++++
>>    xen/arch/x86/include/asm/guest-msr.h    |  2 ++
>>    xen/arch/x86/include/asm/hvm/hvm.h      |  9 +++++++++
>>    xen/arch/x86/include/asm/hvm/vmx/vmcs.h |  2 ++
>>    xen/arch/x86/mm/mem_sharing.c           |  2 ++
>>    xen/arch/x86/vm_event.c                 |  6 ++++--
>>    xen/common/domain.c                     | 10 ++++++++++
>>    xen/common/memory.c                     |  6 ++++++
>>    xen/common/vm_event.c                   |  3 ++-
>>    xen/include/xen/domain.h                |  4 ++++
>>    xen/include/xen/sched.h                 |  4 ++++
>>    15 files changed, 77 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
>> index d900d926c555..70ec4f0d14a5 100644
>> --- a/xen/Kconfig.debug
>> +++ b/xen/Kconfig.debug
>> @@ -155,4 +155,19 @@ config DEBUG_INFO
>>    	  "make install-xen" for installing xen.efi, stripping needs to be
>>    	  done outside the Xen build environment).
>>    
>> +config HAS_VMTRACE
>> +    bool
>> +
>> +config VMTRACE
>> +    bool "HW VM tracing support"
>> +    depends on HAS_VMTRACE
>> +    default y
>> +    help
>> +      Enables HW VM tracing support which allows to configure HW processor
>> +      features (vmtrace_op) to enable capturing information about software
>> +      execution using dedicated hardware facilities with minimal interference
>> +      to the software being traced. The trace date can be retrieved using buffer
>> +      shared between Xen and domain
>> +      (see XENMEM_acquire_resource(XENMEM_resource_vmtrace_buf)).
>> +
>>    endmenu

[...]

>>    
>> diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
>> index fc349270b9c5..f4c8696ce54e 100644
>> --- a/xen/arch/x86/vm_event.c
>> +++ b/xen/arch/x86/vm_event.c
>> @@ -253,7 +253,9 @@ void vm_event_fill_regs(vm_event_request_t *req)
>>        req->data.regs.x86.shadow_gs = ctxt.shadow_gs;
>>        req->data.regs.x86.dr6 = ctxt.dr6;
>>    
>> +#ifdef CONFIG_VMTRACE
>>        if ( hvm_vmtrace_output_position(curr, &req->data.regs.x86.vmtrace_pos) != 1 )
>> +#endif
>>            req->data.regs.x86.vmtrace_pos = ~0;
> 
> This if-def looks very oddly placed.

I assume you are asking for hvm_vmtrace_output_position() stub, Right?

> 
>>    #endif
>>    }
>> @@ -303,12 +305,12 @@ void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp)
>>    #endif
>>    }
>>    
>> +#ifdef CONFIG_VMTRACE
>>    void vm_event_reset_vmtrace(struct vcpu *v)
>>    {
>> -#ifdef CONFIG_HVM
>>        hvm_vmtrace_reset(v);
>> -#endif
>>    }
>> +#endif
>>    
>>    /*
>>     * Local variables:
>> diff --git a/xen/common/domain.c b/xen/common/domain.c
>> index 7dcd466e5a12..2be6ee03d004 100644
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -136,7 +136,9 @@ struct vcpu *idle_vcpu[NR_CPUS] __read_mostly;
>>    
>>    vcpu_info_t dummy_vcpu_info;
>>    
>> +#ifdef CONFIG_VMTRACE
>>    bool __read_mostly vmtrace_available;
>> +#endif
>>    
>>    bool __read_mostly vpmu_is_available;
>>    
>> @@ -318,6 +320,7 @@ static void vcpu_info_reset(struct vcpu *v)
>>    
>>    static void vmtrace_free_buffer(struct vcpu *v)
>>    {
>> +#ifdef CONFIG_VMTRACE
>>        const struct domain *d = v->domain;
>>        struct page_info *pg = v->vmtrace.pg;
>>        unsigned int i;
>> @@ -332,10 +335,12 @@ static void vmtrace_free_buffer(struct vcpu *v)
>>            put_page_alloc_ref(&pg[i]);
>>            put_page_and_type(&pg[i]);
>>        }
>> +#endif
>>    }
>>    
>>    static int vmtrace_alloc_buffer(struct vcpu *v)
>>    {
>> +#ifdef CONFIG_VMTRACE
>>        struct domain *d = v->domain;
>>        struct page_info *pg;
>>        unsigned int i;
>> @@ -377,6 +382,9 @@ static int vmtrace_alloc_buffer(struct vcpu *v)
>>        }
>>    
>>        return -ENODATA;
>> +#else
>> +    return 0;
>> +#endif
>>    }
>>    
>>    /*
>> @@ -825,7 +833,9 @@ struct domain *domain_create(domid_t domid,
>>            ASSERT(!config->altp2m.nr);
>>    #endif
>>    
>> +#ifdef CONFIG_VMTRACE
>>            d->vmtrace_size = config->vmtrace_size;
>> +#endif
>>        }
>>    
>>        /* Sort out our idea of is_control_domain(). */
>> diff --git a/xen/common/memory.c b/xen/common/memory.c
>> index 3688e6dd5032..66dc7f7a0a41 100644
>> --- a/xen/common/memory.c
>> +++ b/xen/common/memory.c
>> @@ -1155,8 +1155,10 @@ static unsigned int resource_max_frames(const struct domain *d,
>>        case XENMEM_resource_ioreq_server:
>>            return ioreq_server_max_frames(d);
>>    
>> +#ifdef CONFIG_VMTRACE
>>        case XENMEM_resource_vmtrace_buf:
>>            return d->vmtrace_size >> PAGE_SHIFT;
>> +#endif
>>    
>>        default:
>>            return 0;
>> @@ -1198,6 +1200,7 @@ static int acquire_ioreq_server(struct domain *d,
>>    #endif
>>    }
>>    
>> +#ifdef CONFIG_VMTRACE
>>    static int acquire_vmtrace_buf(
>>        struct domain *d, unsigned int id, unsigned int frame,
>>        unsigned int nr_frames, xen_pfn_t mfn_list[])
>> @@ -1220,6 +1223,7 @@ static int acquire_vmtrace_buf(
>>    
>>        return nr_frames;
>>    }
>> +#endif
>>    
> 
> Given that vmtrace feature is fairly isolated in VMX code, wouldn't it
> be better to move all vmtrace related functions (including vmx_init_ipt)
> in a separate vmtrace.c file and make such file gated behind
> CONFIG_VMTRACE ?
> 

Are you thinking about:
  xen/common/
  |- vmtrace.c
  xen/arch/x86/hvm/vmx/
  |- vmtrace.c
?

or smth else?

[...]

-- 
Best regards,
-grygorii