On 24.05.2024 01:36, Stefano Stabellini wrote: > On Thu, 23 May 2024, Jan Beulich wrote: >> On 23.05.2024 15:07, Sergiy Kibrik wrote: >>> 16.05.24 14:12, Jan Beulich: >>>> On 15.05.2024 11:12, Sergiy Kibrik wrote: >>>>> --- a/xen/arch/x86/include/asm/cpufeature.h >>>>> +++ b/xen/arch/x86/include/asm/cpufeature.h >>>>> @@ -81,7 +81,8 @@ static inline bool boot_cpu_has(unsigned int feat) >>>>> #define cpu_has_sse3 boot_cpu_has(X86_FEATURE_SSE3) >>>>> #define cpu_has_pclmulqdq boot_cpu_has(X86_FEATURE_PCLMULQDQ) >>>>> #define cpu_has_monitor boot_cpu_has(X86_FEATURE_MONITOR) >>>>> -#define cpu_has_vmx boot_cpu_has(X86_FEATURE_VMX) >>>>> +#define cpu_has_vmx ( IS_ENABLED(CONFIG_VMX) && \ >>>>> + boot_cpu_has(X86_FEATURE_VMX)) >>>>> #define cpu_has_eist boot_cpu_has(X86_FEATURE_EIST) >>>>> #define cpu_has_ssse3 boot_cpu_has(X86_FEATURE_SSSE3) >>>>> #define cpu_has_fma boot_cpu_has(X86_FEATURE_FMA) >>>>> @@ -109,7 +110,8 @@ static inline bool boot_cpu_has(unsigned int feat) >>>>> >>>>> /* CPUID level 0x80000001.ecx */ >>>>> #define cpu_has_cmp_legacy boot_cpu_has(X86_FEATURE_CMP_LEGACY) >>>>> -#define cpu_has_svm boot_cpu_has(X86_FEATURE_SVM) >>>>> +#define cpu_has_svm ( IS_ENABLED(CONFIG_SVM) && \ >>>>> + boot_cpu_has(X86_FEATURE_SVM)) >>>>> #define cpu_has_sse4a boot_cpu_has(X86_FEATURE_SSE4A) >>>>> #define cpu_has_xop boot_cpu_has(X86_FEATURE_XOP) >>>>> #define cpu_has_skinit boot_cpu_has(X86_FEATURE_SKINIT) >>>> >>>> Hmm, leaving aside the style issue (stray blanks after opening parentheses, >>>> and as a result one-off indentation on the wrapped lines) I'm not really >>>> certain we can do this. The description goes into detail why we would want >>>> this, but it doesn't cover at all why it is safe for all present (and >>>> ideally also future) uses. I wouldn't be surprised if we had VMX/SVM checks >>>> just to derive further knowledge from that, without them being directly >>>> related to the use of VMX/SVM. Take a look at calculate_hvm_max_policy(), >>>> for example. While it looks to be okay there, it may give you an idea of >>>> what I mean. >>>> >>>> Things might become better separated if instead for such checks we used >>>> host and raw CPU policies instead of cpuinfo_x86.x86_capability[]. But >>>> that's still pretty far out, I'm afraid. >>> >>> I've followed a suggestion you made for patch in previous series: >>> >>> https://lore.kernel.org/xen-devel/8fbd604e-5e5d-410c-880f-2ad257bbe...@suse.com/ >> >> See the "If not, ..." that I had put there. Doing the change just >> mechanically >> isn't enough, you also need to make clear (in the description) that you >> verified it's safe to have this way. > > What does it mean to "verified it's safe to have this way"? "Safe" in > what way?
"Safe" as in "not breaking existing logic", anywhere. Jan
