Re: [XEN PATCH v2 12/15] x86/vmx: guard access to cpu_has_vmx_* in common code

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.05.2024 02:50, Stefano Stabellini wrote:
> On Wed, 15 May 2024, Sergiy Kibrik wrote:
>> There're several places in common code, outside of arch/x86/hvm/vmx,
>> where cpu_has_vmx_* get accessed without checking if VMX present first.
>> We may want to guard these macros, as they read global variables defined
>> inside vmx-specific files -- so VMX can be made optional later on.
>>
>> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
>> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> CC: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> Here I've tried a different approach from prev.patches [1,2] -- instead of
>> modifying whole set of cpu_has_{svm/vmx}_* macros, we can:
>>  1) do not touch SVM part at all, because just as Andrew pointed out they're
>> used inside arch/x86/hvm/svm only.
>>  2) track several places in common code where cpu_has_vmx_* features are
>> checked out and guard them using cpu_has_vmx condition
>>  3) two of cpu_has_vmx_* macros being used in common code are checked in a 
>> bit
>> more tricky way, so instead of making complex conditionals even more 
>> complicated,
>> we can instead integrate cpu_has_vmx condition inside these two macros.
>>
>> This patch aims to replace [1,2] from v1 series by doing steps above.
>>
>>  1. 
>> https://lore.kernel.org/xen-devel/20240416064402.3469959-1-Sergiy_Kibrik@xxxxxxxx/
>>  2. 
>> https://lore.kernel.org/xen-devel/20240416064606.3470052-1-Sergiy_Kibrik@xxxxxxxx/
> 
> I am missing some of the previous discussions but why can't we just fix
> all of the cpu_has_vmx_* #defines in vmcs.h to also check for
> cpu_has_vmx?
> 
> That seems easier and simpler than to add add-hoc checks at the invocations?

I'd like to take the question on step further: Following 0b5f149338e3
("x86/HVM: hide SVM/VMX when their enabling is prohibited by firmware"),
is this change needed at all? IOW is there a path left where cpu_has_vmx
may be false, by any cpu_has_vmx_* may still yield true?

Jan