Re: [XEN][PATCH v3] xen: make VMTRACE support optional

[Jan Beulich <jbeulich@suse.com>]

On 18.11.2025 13:42, Grygorii Strashko wrote:
> On 17.11.25 18:52, Jan Beulich wrote:
>> On 14.11.2025 15:22, Grygorii Strashko wrote:
>>> --- a/xen/arch/x86/hvm/Kconfig
>>> +++ b/xen/arch/x86/hvm/Kconfig
>>> @@ -35,6 +35,18 @@ config INTEL_VMX
>>>   	  If your system includes a processor with Intel VT-x support, say Y.
>>>   	  If in doubt, say Y.
>>>   
>>> +config VMTRACE
>>> +    bool "HW VM tracing support"
>>> +    depends on INTEL_VMX
>>> +    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 data can be retrieved using buffer
>>> +      shared between Xen and domain
>>> +      (see XENMEM_acquire_resource(XENMEM_resource_vmtrace_buf)).
>>
>> Please check adjacent options above or ...
>>
>>>   config HVM_FEP
>>>   	bool "HVM Forced Emulation Prefix support (UNSUPPORTED)" if UNSUPPORTED
>>>   	default DEBUG
>>
>> ... below for how proper indentation would look like here.
> 
> There is a mix in Kconfigs - some places <Tabs> some places <Spaces> :(
> Will change to <Tabs>

Any place where only spaces are used is malformed. "help" text with its special
indentation is a separate thing, of course.

>>> --- a/xen/arch/x86/vm_event.c
>>> +++ b/xen/arch/x86/vm_event.c
>>> @@ -253,7 +253,8 @@ 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;
>>>   
>>> -    if ( hvm_vmtrace_output_position(curr, &req->data.regs.x86.vmtrace_pos) != 1 )
>>> +    if ( IS_ENABLED(CONFIG_VMTRACE) &&
>>> +         hvm_vmtrace_output_position(curr, &req->data.regs.x86.vmtrace_pos) != 1 )
>>
>> Would be nice if the too-long-line issue here was also address, when the line
>> needs touching anyway.
> 
> I left it as is for better readability as an exception.
> Will below be ok:
> 
>       if ( IS_ENABLED(CONFIG_VMTRACE) &&
> -         hvm_vmtrace_output_position(curr, &req->data.regs.x86.vmtrace_pos) != 1 )
> +         hvm_vmtrace_output_position(curr,
> +                                     &req->data.regs.x86.vmtrace_pos) != 1 )
>           req->data.regs.x86.vmtrace_pos = ~0;

Almost, albeit the off-by-1 indentation may also merely be an effect of your mailer.

Jan