Re: [XEN PATCH v1 13/15] x86: wire cpu_has_{svm/vmx}_* to false when svm/vmx not enabled

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/04/2024 7:46 am, Sergiy Kibrik wrote:
> From: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>
>
> To be able to use cpu_has_{svm/vmx}_* macros in common code without enclosing
> them inside #ifdef guards when the respective virtualization technology is
> not enabled, define corresponding helper routines as false when not 
> applicable.
>
> No functional change intended.
>
> Signed-off-by: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
> ---
>  xen/arch/x86/include/asm/hvm/svm/svm.h  | 8 ++++++++
>  xen/arch/x86/include/asm/hvm/vmx/vmcs.h | 7 +++++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/xen/arch/x86/include/asm/hvm/svm/svm.h 
> b/xen/arch/x86/include/asm/hvm/svm/svm.h
> index 4eeeb25da9..7e8cdb4a27 100644
> --- a/xen/arch/x86/include/asm/hvm/svm/svm.h
> +++ b/xen/arch/x86/include/asm/hvm/svm/svm.h
> @@ -38,10 +38,18 @@ 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_SVM
>  static inline bool cpu_has_svm_feature(unsigned int feat)
>  {
>      return svm_feature_flags & (1u << feat);
>  }
> +#else
> +static inline bool cpu_has_svm_feature(unsigned int feat)
> +{
> +    return false;
> +}
> +#endif
> +
>  #define cpu_has_svm_npt       cpu_has_svm_feature(SVM_FEATURE_NPT)
>  #define cpu_has_svm_lbrv      cpu_has_svm_feature(SVM_FEATURE_LBRV)
>  #define cpu_has_svm_svml      cpu_has_svm_feature(SVM_FEATURE_SVML)
> diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h 
> b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
> index fd197e2603..2d927d3100 100644
> --- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
> +++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
> @@ -287,10 +287,17 @@ extern uint64_t vmx_tertiary_exec_control;
>  #define VMX_VPID_INVVPID_SINGLE_CONTEXT_RETAINING_GLOBAL 0x80000000000ULL
>  extern u64 vmx_ept_vpid_cap;
>  
> +#ifdef CONFIG_VMX
>  static inline bool vmx_ctrl_has_feature(uint64_t control, unsigned long 
> feature)
>  {
>      return control & feature;
>  }
> +#else
> +static inline bool vmx_ctrl_has_feature(uint64_t control, unsigned long 
> feature)
> +{
> +    return false;
> +}
> +#endif
>  
>  #define VMX_MISC_ACTIVITY_MASK                  0x000001c0
>  #define VMX_MISC_PROC_TRACE                     0x00004000

I'm afraid this is going in an unhelpful direction.  We want to move
both of these files to be local to arch/x86/hvm/{vmx,svm}/.

cpu_has_svm_* isn't actually used outside of svm/; only the plain
SVM_FEATURE_* constants are, and that's only because they're not
expressed as plain cpu features yet.

cpu_has_vmx_* has a few more users, but most are unlikely to remain in
this form.  One critical set of changes to fix vulnerabilities in
nested-virt is to make almost of of these decisions based on per-domain
state, not host state.  The aspects which are host state should be in
regular cpu features.

I already volunteered to sort out the SEV feature leaf properly, and I
was going to do the SVM leaf while I was at it.  If you can wait a few
days, I might be able to make half of this problem disappear.

~Andrew