diff mbox series

[XEN,v5,09/13] x86/vmx: guard access to cpu_has_vmx_* in common code

Message ID c2961c8b67041883ce5a5f6d0511a31dc7fbe22d.1722333634.git.Sergiy_Kibrik@epam.com (mailing list archive)
State Superseded
Headers show
Series x86: make CPU virtualisation support configurable | expand

Commit Message

Sergiy Kibrik July 30, 2024, 10:33 a.m. UTC 
There're several places in common code, outside of arch/x86/hvm/vmx,
where cpu_has_vmx_* get accessed without checking whether VMX supported first.
These macros rely on global variables defined in vmx code, so when VMX support
is disabled accesses to these variables turn into build failures.

To overcome these failures, build-time check is done before accessing global
variables, so that DCE would remove these variables.

Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
Acked-by: Paul Durrant <paul@xen.org>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
---
changes in v5:
 - change kconfig option name VMX -> INTEL_VMX
 - do not change .c files, only modify macros in vmcs.h
changes in v4:
 - use IS_ENABLED(CONFIG_VMX) instead of using_vmx
changes in v3:
 - using_vmx instead of cpu_has_vmx
 - clarify description on why this change needed
---
 xen/arch/x86/include/asm/hvm/vmx/vmcs.h | 33 ++++++++++++++++---------
 1 file changed, 22 insertions(+), 11 deletions(-)

Comments

Jan Beulich July 31, 2024, 12:23 p.m. UTC | #1 
On 30.07.2024 12:33, Sergiy Kibrik wrote:
> There're several places in common code, outside of arch/x86/hvm/vmx,
> where cpu_has_vmx_* get accessed without checking whether VMX supported first.
> These macros rely on global variables defined in vmx code, so when VMX support
> is disabled accesses to these variables turn into build failures.
> 
> To overcome these failures, build-time check is done before accessing global
> variables, so that DCE would remove these variables.
> 
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> Acked-by: Paul Durrant <paul@xen.org>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> ---
> changes in v5:
>  - change kconfig option name VMX -> INTEL_VMX
>  - do not change .c files, only modify macros in vmcs.h

Better, yet still not going far enough, as indicated earlier:

> --- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
> +++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
> @@ -300,13 +300,15 @@ extern u64 vmx_ept_vpid_cap;
>  #define cpu_has_wbinvd_exiting \
>      (vmx_secondary_exec_control & SECONDARY_EXEC_WBINVD_EXITING)
>  #define cpu_has_vmx_virtualize_apic_accesses \
> -    (vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)
> +    (IS_ENABLED(CONFIG_INTEL_VMX) && \
> +     vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)

Why does this change but the earlier cpu_has_wbinvd_exiting or ...

>  #define cpu_has_vmx_tpr_shadow \
>      (vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW)
>  #define cpu_has_vmx_vnmi \
>      (vmx_pin_based_exec_control & PIN_BASED_VIRTUAL_NMIS)

... these two (and several more elsewhere) don't?

Jan
Sergiy Kibrik Aug. 2, 2024, 10:53 a.m. UTC | #2 
31.07.24 15:23, Jan Beulich:
> On 30.07.2024 12:33, Sergiy Kibrik wrote:
>> There're several places in common code, outside of arch/x86/hvm/vmx,
>> where cpu_has_vmx_* get accessed without checking whether VMX supported first.
>> These macros rely on global variables defined in vmx code, so when VMX support
>> is disabled accesses to these variables turn into build failures.
>>
>> To overcome these failures, build-time check is done before accessing global
>> variables, so that DCE would remove these variables.
>>
>> Signed-off-by: Sergiy Kibrik<Sergiy_Kibrik@epam.com>
>> Acked-by: Paul Durrant<paul@xen.org>
>> CC: Andrew Cooper<andrew.cooper3@citrix.com>
>> CC: Jan Beulich<jbeulich@suse.com>
>> ---
>> changes in v5:
>>   - change kconfig option name VMX -> INTEL_VMX
>>   - do not change .c files, only modify macros in vmcs.h
> Better, yet still not going far enough, as indicated earlier:

I must've misunderstood your earlier suggestion. Are we talking about 
modifying all of cpu_has_vmx_* macros -- even though most of them used 
internally to VMX code and don't cause any trouble in common code?

   -Sergiy
Jan Beulich Aug. 2, 2024, 11:10 a.m. UTC | #3 
On 02.08.2024 12:53, Sergiy Kibrik wrote:
> 31.07.24 15:23, Jan Beulich:
>> On 30.07.2024 12:33, Sergiy Kibrik wrote:
>>> There're several places in common code, outside of arch/x86/hvm/vmx,
>>> where cpu_has_vmx_* get accessed without checking whether VMX supported first.
>>> These macros rely on global variables defined in vmx code, so when VMX support
>>> is disabled accesses to these variables turn into build failures.
>>>
>>> To overcome these failures, build-time check is done before accessing global
>>> variables, so that DCE would remove these variables.
>>>
>>> Signed-off-by: Sergiy Kibrik<Sergiy_Kibrik@epam.com>
>>> Acked-by: Paul Durrant<paul@xen.org>
>>> CC: Andrew Cooper<andrew.cooper3@citrix.com>
>>> CC: Jan Beulich<jbeulich@suse.com>
>>> ---
>>> changes in v5:
>>>   - change kconfig option name VMX -> INTEL_VMX
>>>   - do not change .c files, only modify macros in vmcs.h
>> Better, yet still not going far enough, as indicated earlier:
> 
> I must've misunderstood your earlier suggestion. Are we talking about 
> modifying all of cpu_has_vmx_* macros -- even though most of them used 
> internally to VMX code and don't cause any trouble in common code?

Yes. Where exactly they're used can change any moment (in principle at
least).

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
index 58140af691..34898d9612 100644
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -300,13 +300,15 @@  extern u64 vmx_ept_vpid_cap;
 #define cpu_has_wbinvd_exiting \
     (vmx_secondary_exec_control & SECONDARY_EXEC_WBINVD_EXITING)
 #define cpu_has_vmx_virtualize_apic_accesses \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)
+    (IS_ENABLED(CONFIG_INTEL_VMX) && \
+     vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)
 #define cpu_has_vmx_tpr_shadow \
     (vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW)
 #define cpu_has_vmx_vnmi \
     (vmx_pin_based_exec_control & PIN_BASED_VIRTUAL_NMIS)
 #define cpu_has_vmx_msr_bitmap \
-    (vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_MSR_BITMAP)
+    (IS_ENABLED(CONFIG_INTEL_VMX) && \
+     vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_MSR_BITMAP)
 #define cpu_has_vmx_secondary_exec_control \
     (vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS)
 #define cpu_has_vmx_tertiary_exec_control \
@@ -316,11 +318,13 @@  extern u64 vmx_ept_vpid_cap;
 #define cpu_has_vmx_dt_exiting \
     (vmx_secondary_exec_control & SECONDARY_EXEC_DESCRIPTOR_TABLE_EXITING)
 #define cpu_has_vmx_rdtscp \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_RDTSCP)
+    (IS_ENABLED(CONFIG_INTEL_VMX) && \
+     vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_RDTSCP)
 #define cpu_has_vmx_vpid \
     (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VPID)
 #define cpu_has_monitor_trap_flag \
-    (vmx_cpu_based_exec_control & CPU_BASED_MONITOR_TRAP_FLAG)
+    (IS_ENABLED(CONFIG_INTEL_VMX) && \
+     vmx_cpu_based_exec_control & CPU_BASED_MONITOR_TRAP_FLAG)
 #define cpu_has_vmx_pat \
     (vmx_vmentry_control & VM_ENTRY_LOAD_GUEST_PAT)
 #define cpu_has_vmx_efer \
@@ -333,13 +337,17 @@  extern u64 vmx_ept_vpid_cap;
 #define cpu_has_vmx_ple \
     (vmx_secondary_exec_control & SECONDARY_EXEC_PAUSE_LOOP_EXITING)
 #define cpu_has_vmx_invpcid \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_INVPCID)
+    (IS_ENABLED(CONFIG_INTEL_VMX) && \
+     vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_INVPCID)
 #define cpu_has_vmx_apic_reg_virt \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_APIC_REGISTER_VIRT)
+    (IS_ENABLED(CONFIG_INTEL_VMX) && \
+     vmx_secondary_exec_control & SECONDARY_EXEC_APIC_REGISTER_VIRT)
 #define cpu_has_vmx_virtual_intr_delivery \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY)
+    (IS_ENABLED(CONFIG_INTEL_VMX) && \
+     vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY)
 #define cpu_has_vmx_virtualize_x2apic_mode \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE)
+    (IS_ENABLED(CONFIG_INTEL_VMX) && \
+     vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE)
 #define cpu_has_vmx_posted_intr_processing \
     (vmx_pin_based_exec_control & PIN_BASED_POSTED_INTERRUPT)
 #define cpu_has_vmx_vmcs_shadowing \
@@ -347,14 +355,17 @@  extern u64 vmx_ept_vpid_cap;
 #define cpu_has_vmx_vmfunc \
     (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VM_FUNCTIONS)
 #define cpu_has_vmx_virt_exceptions \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS)
+    (IS_ENABLED(CONFIG_INTEL_VMX) && \
+     vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS)
 #define cpu_has_vmx_pml \
     (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_PML)
 #define cpu_has_vmx_mpx \
-    ((vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) && \
+    (IS_ENABLED(CONFIG_INTEL_VMX) && \
+     (vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) && \
      (vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS))
 #define cpu_has_vmx_xsaves \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_XSAVES)
+    (IS_ENABLED(CONFIG_INTEL_VMX) && \
+     vmx_secondary_exec_control & SECONDARY_EXEC_XSAVES)
 #define cpu_has_vmx_tsc_scaling \
     (vmx_secondary_exec_control & SECONDARY_EXEC_TSC_SCALING)
 #define cpu_has_vmx_bus_lock_detection \