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

[Jan Beulich <jbeulich@xxxxxxxx>]

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/
>