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
