On 13.11.2025 21:40, Grygorii Strashko wrote:
> On 13.11.25 14:58, Jan Beulich wrote:
>> On 13.11.2025 13:53, Grygorii Strashko wrote:
>>> On 13.11.25 10:36, Jan Beulich wrote:
>>>> On 12.11.2025 21:24, Grygorii Strashko wrote:
>>>>> --- a/xen/arch/x86/hvm/vmx/vmcs.c
>>>>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>>>>> @@ -307,6 +307,7 @@ static int vmx_init_vmcs_config(bool bsp)
>>>>>        rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap);
>>>>>    
>>>>>        /* Check whether IPT is supported in VMX operation. */
>>>>> +#ifdef CONFIG_VMTRACE
>>>>>        if ( bsp )
>>>>>            vmtrace_available = cpu_has_proc_trace &&
>>>>>                                (_vmx_misc_cap & VMX_MISC_PROC_TRACE);
>>>>> @@ -317,6 +318,7 @@ static int vmx_init_vmcs_config(bool bsp)
>>>>>                   smp_processor_id());
>>>>>            return -EINVAL;
>>>>>        }
>>>>> +#endif
>>>>
>>>> Initially I was inclined to ask for use of IS_ENABLED() here, but that 
>>>> wouldn't
>>>> work since vmtrace_available isn't an lvalue when VMTRACE=n. Hence why 
>>>> generally
>>>> I think it is better to check the particular identifier in such cases, 
>>>> rather
>>>> than the original CONFIG_* (i.e. "#ifndef vmtrace_available" here). I'm not
>>>> going to insist though, as I expect opinions may differ on this matter.
>>>
>>> Yep. assignment required ifdef wrapping.
>>>
>>> "#ifndef vmtrace_available" will not work out of the box as there are
>>>
>>> "if (vmtrace_available)" in code. So, can't just "not define"/undef 
>>> "vmtrace_available".
>>
>> I meant this just for the case here, though. Elsewhere you want to stick to
>> checking CONFIG_VMTRACE.
> 
> I'd prefer to keep this part as is. Please tell me if you insist.

I won't insist, first and foremost again because I expect opinions on this
matter would generally differ anyway.

Jan

Reply via email to