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
