Re: [RFC 02/10] x86/hvm: separate AMD-V and Intel VT-x hvm_function_table initializers

2023-02-13 Thread Xenia Ragiadakou 



On 2/13/23 18:47, Jan Beulich wrote:

On 13.02.2023 15:57, Xenia Ragiadakou wrote:

--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -261,8 +261,16 @@ extern struct hvm_function_table hvm_funcs;
  extern bool_t hvm_enabled;
  extern s8 hvm_port80_allowed;
  
+#ifdef CONFIG_AMD_SVM

  extern const struct hvm_function_table *start_svm(void);
+#else
+static inline const struct hvm_function_table *start_svm(void) { return NULL; }
+#endif
+#ifdef CONFIG_INTEL_VMX
  extern const struct hvm_function_table *start_vmx(void);
+#else
+static inline const struct hvm_function_table *start_vmx(void) { return NULL; }
+#endif
  
  int hvm_domain_initialise(struct domain *d,

const struct xen_domctl_createdomain *config);


Instead of this (which I consider harder to read), may I suggest

 if ( IS_ENABLED(CONFIG_VMX) && cpu_has_vmx )
 fns = start_vmx();
 else if ( IS_ENABLED(CONFIG_SVM) && cpu_has_svm )
 fns = start_svm();

in hvm_enable() instead (with DCE taking care of removing the dead
calls)?


Sure, it looks much better this way.



Jan


--
Xenia

Re: [RFC 02/10] x86/hvm: separate AMD-V and Intel VT-x hvm_function_table initializers

2023-02-13 Thread Jan Beulich 
On 13.02.2023 15:57, Xenia Ragiadakou wrote:
> --- a/xen/arch/x86/include/asm/hvm/hvm.h
> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
> @@ -261,8 +261,16 @@ extern struct hvm_function_table hvm_funcs;
>  extern bool_t hvm_enabled;
>  extern s8 hvm_port80_allowed;
>  
> +#ifdef CONFIG_AMD_SVM
>  extern const struct hvm_function_table *start_svm(void);
> +#else
> +static inline const struct hvm_function_table *start_svm(void) { return 
> NULL; }
> +#endif
> +#ifdef CONFIG_INTEL_VMX
>  extern const struct hvm_function_table *start_vmx(void);
> +#else
> +static inline const struct hvm_function_table *start_vmx(void) { return 
> NULL; }
> +#endif
>  
>  int hvm_domain_initialise(struct domain *d,
>const struct xen_domctl_createdomain *config);

Instead of this (which I consider harder to read), may I suggest

if ( IS_ENABLED(CONFIG_VMX) && cpu_has_vmx )
fns = start_vmx();
else if ( IS_ENABLED(CONFIG_SVM) && cpu_has_svm )
fns = start_svm();

in hvm_enable() instead (with DCE taking care of removing the dead
calls)?

Jan

[RFC 02/10] x86/hvm: separate AMD-V and Intel VT-x hvm_function_table initializers

2023-02-13 Thread Xenia Ragiadakou 
Since start_svm() is AMD-V specific while start_vmx() is Intel VT-x specific,
need to be guarded with AMD_SVM and INTEL_VMX, respectively.
Instead of adding #ifdef guards around the function calls, implement them as
static inline null-returning functions when the respective technology is not
enabled.

No functional change intended.

Signed-off-by: Xenia Ragiadakou 
---
 xen/arch/x86/include/asm/hvm/hvm.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/xen/arch/x86/include/asm/hvm/hvm.h 
b/xen/arch/x86/include/asm/hvm/hvm.h
index 43d3fc2498..353e48f4e3 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -261,8 +261,16 @@ extern struct hvm_function_table hvm_funcs;
 extern bool_t hvm_enabled;
 extern s8 hvm_port80_allowed;
 
+#ifdef CONFIG_AMD_SVM
 extern const struct hvm_function_table *start_svm(void);
+#else
+static inline const struct hvm_function_table *start_svm(void) { return NULL; }
+#endif
+#ifdef CONFIG_INTEL_VMX
 extern const struct hvm_function_table *start_vmx(void);
+#else
+static inline const struct hvm_function_table *start_vmx(void) { return NULL; }
+#endif
 
 int hvm_domain_initialise(struct domain *d,
   const struct xen_domctl_createdomain *config);
-- 
2.37.2