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
