[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC 08/10] x86: wire cpu_has_svm/vmx_* to false when svm/vmx not enabled
On 13.02.2023 15:57, Xenia Ragiadakou wrote: > To be able to use cpu_has_svm/vmx_* macros in common code without enclosing Both in the title and here the spelling you use made me first think that you talk about "cpu_has_svm". May I suggest you follow what we typically do and use "cpu_has_{svm,vmx}_*" instead in such a case? However, the title may want changing anyway: > --- a/xen/arch/x86/include/asm/hvm/svm/svm.h > +++ b/xen/arch/x86/include/asm/hvm/svm/svm.h > @@ -80,6 +80,7 @@ extern u32 svm_feature_flags; > #define SVM_FEATURE_SSS 19 /* NPT Supervisor Shadow Stacks */ > #define SVM_FEATURE_SPEC_CTRL 20 /* MSR_SPEC_CTRL virtualisation */ > > +#ifdef CONFIG_AMD_SVM > #define cpu_has_svm_feature(f) (svm_feature_flags & (1u << (f))) Why don't you simply provide a 2nd form of this one macro, leaving all others alone? > --- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h > +++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h > @@ -294,6 +294,7 @@ extern u64 vmx_ept_vpid_cap; > > #define VMX_TSC_MULTIPLIER_MAX 0xffffffffffffffffULL > > +#ifdef CONFIG_INTEL_VMX > #define cpu_has_wbinvd_exiting \ > (vmx_secondary_exec_control & SECONDARY_EXEC_WBINVD_EXITING) > #define cpu_has_vmx_virtualize_apic_accesses \ > @@ -352,6 +353,36 @@ extern u64 vmx_ept_vpid_cap; > (vmx_secondary_exec_control & SECONDARY_EXEC_BUS_LOCK_DETECTION) > #define cpu_has_vmx_notify_vm_exiting \ > (vmx_secondary_exec_control & SECONDARY_EXEC_NOTIFY_VM_EXITING) > +#else > +#define cpu_has_wbinvd_exiting false > +#define cpu_has_vmx_virtualize_apic_accesses false > +#define cpu_has_vmx_tpr_shadow false > +#define cpu_has_vmx_vnmi false > +#define cpu_has_vmx_msr_bitmap false > +#define cpu_has_vmx_secondary_exec_control false > +#define cpu_has_vmx_ept false > +#define cpu_has_vmx_dt_exiting false > +#define cpu_has_vmx_vpid false > +#define cpu_has_monitor_trap_flag false > +#define cpu_has_vmx_pat false > +#define cpu_has_vmx_efer false > +#define cpu_has_vmx_unrestricted_guest false > +#define vmx_unrestricted_guest(v) false > +#define cpu_has_vmx_ple false > +#define cpu_has_vmx_apic_reg_virt false > +#define cpu_has_vmx_virtual_intr_delivery false > +#define cpu_has_vmx_virtualize_x2apic_mode false > +#define cpu_has_vmx_posted_intr_processing false > +#define cpu_has_vmx_vmcs_shadowing false > +#define cpu_has_vmx_vmfunc false > +#define cpu_has_vmx_virt_exceptions false > +#define cpu_has_vmx_pml false > +#define cpu_has_vmx_mpx false > +#define cpu_has_vmx_xsaves false > +#define cpu_has_vmx_tsc_scaling false > +#define cpu_has_vmx_bus_lock_detection false > +#define cpu_has_vmx_notify_vm_exiting false > +#endif /* CONFIG_INTEL_VMX */ For VMX you first of all want to separate out vmx_unrestricted_guest(v). Maybe we can even get away without a 2nd form. Then I think it would be much neater a change if, like we have for SVM, a couple (three I think) helper macros were introduced, which then is all that needs providing a 2nd form for. (Both steps may want doing in separate, prereq patches.) > @@ -374,8 +405,12 @@ extern u64 vmx_ept_vpid_cap; > #define VMX_BASIC_DEFAULT1_ZERO (1ULL << 55) > > extern u64 vmx_basic_msr; > +#ifdef CONFIG_INTEL_VMX > #define cpu_has_vmx_ins_outs_instr_info \ > (!!(vmx_basic_msr & VMX_BASIC_INS_OUT_INFO)) > +#else > +#define cpu_has_vmx_ins_outs_instr_info false > +#endif /* CONFIG_INTEL_VMX */ I don't think you need an alternative here - it's used solely in VMX code. If one wanted to this could even be moved to vmcs.c. > --- a/xen/arch/x86/include/asm/hvm/vmx/vmx.h > +++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h > @@ -289,6 +289,7 @@ typedef union cr_access_qual { > > extern uint8_t posted_intr_vector; > > +#ifdef CONFIG_INTEL_VMX > #define cpu_has_vmx_ept_exec_only_supported \ > (vmx_ept_vpid_cap & VMX_EPT_EXEC_ONLY_SUPPORTED) > > @@ -301,6 +302,17 @@ extern uint8_t posted_intr_vector; > #define cpu_has_vmx_ept_ad (vmx_ept_vpid_cap & VMX_EPT_AD_BIT) > #define cpu_has_vmx_ept_invept_single_context \ > (vmx_ept_vpid_cap & VMX_EPT_INVEPT_SINGLE_CONTEXT) > +#else > +#define cpu_has_vmx_ept_exec_only_supported false > + > +#define cpu_has_vmx_ept_wl4_supported false > +#define cpu_has_vmx_ept_mt_uc false > +#define cpu_has_vmx_ept_mt_wb false > +#define cpu_has_vmx_ept_2mb false > +#define cpu_has_vmx_ept_1gb false > +#define cpu_has_vmx_ept_ad false > +#define cpu_has_vmx_ept_invept_single_context false > +#endif /* CONFIG_INTEL_VMX */ Same suggestion for introducing a helper macro here, which would then also be usable ... > @@ -310,12 +322,18 @@ extern uint8_t posted_intr_vector; > #define INVEPT_SINGLE_CONTEXT 1 > #define INVEPT_ALL_CONTEXT 2 > > +#ifdef CONFIG_INTEL_VMX > #define cpu_has_vmx_vpid_invvpid_individual_addr \ > (vmx_ept_vpid_cap & VMX_VPID_INVVPID_INDIVIDUAL_ADDR) > #define cpu_has_vmx_vpid_invvpid_single_context \ > (vmx_ept_vpid_cap & VMX_VPID_INVVPID_SINGLE_CONTEXT) > #define cpu_has_vmx_vpid_invvpid_single_context_retaining_global \ > (vmx_ept_vpid_cap & VMX_VPID_INVVPID_SINGLE_CONTEXT_RETAINING_GLOBAL) > +#else > +#define cpu_has_vmx_vpid_invvpid_individual_addr false > +#define cpu_has_vmx_vpid_invvpid_single_context false > +#define cpu_has_vmx_vpid_invvpid_single_context_retaining_global false > +#endif /* CONFIG_INTEL_VMX */ ... here. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |