Re: [XEN PATCH v1 1/7] x86/vpmu: separate amd/intel vPMU code

2024-04-30 Thread Sergiy Kibrik 

29.04.24 18:28, Jan Beulich:

Any reason you don't follow the approach used in patch 7, putting simple
#ifdef in the switch() in vpmu_init()? That would avoid the need for the
three almost identical stubs.


I didn't want to put that many preprocessor statements into this small 
switch() block (4 #ifdef/#endif-s per 15 LOC) but if it's ok I'll do it.


  -Sergiy

Re: [XEN PATCH v1 1/7] x86/vpmu: separate amd/intel vPMU code

2024-04-29 Thread Jan Beulich 
On 23.04.2024 10:48, Sergiy Kibrik wrote:
> --- a/xen/arch/x86/include/asm/vpmu.h
> +++ b/xen/arch/x86/include/asm/vpmu.h
> @@ -11,6 +11,7 @@
>  #define __ASM_X86_HVM_VPMU_H_
>  
>  #include 
> +#include 
>  
>  #define vcpu_vpmu(vcpu)   (&(vcpu)->arch.vpmu)
>  #define vpmu_vcpu(vpmu)   container_of((vpmu), struct vcpu, arch.vpmu)
> @@ -42,9 +43,27 @@ struct arch_vpmu_ops {
>  #endif
>  };
>  
> +#ifdef CONFIG_INTEL
>  const struct arch_vpmu_ops *core2_vpmu_init(void);
> +#else
> +static inline const struct arch_vpmu_ops *core2_vpmu_init(void)
> +{
> +return ERR_PTR(-ENODEV);
> +}
> +#endif
> +#ifdef CONFIG_AMD
>  const struct arch_vpmu_ops *amd_vpmu_init(void);
>  const struct arch_vpmu_ops *hygon_vpmu_init(void);
> +#else
> +static inline const struct arch_vpmu_ops *amd_vpmu_init(void)
> +{
> +return ERR_PTR(-ENODEV);
> +}
> +static inline const struct arch_vpmu_ops *hygon_vpmu_init(void)
> +{
> +return ERR_PTR(-ENODEV);
> +}
> +#endif

Any reason you don't follow the approach used in patch 7, putting simple
#ifdef in the switch() in vpmu_init()? That would avoid the need for the
three almost identical stubs.

Jan

Re: [XEN PATCH v1 1/7] x86/vpmu: separate amd/intel vPMU code

2024-04-26 Thread Stefano Stabellini 
On Tue, 23 Apr 2024, Sergiy Kibrik wrote:
> Build AMD vPMU when CONFIG_AMD is on, and Intel vPMU when CONFIG_INTEL
> is on respectively, allowing for a plaftorm-specific build. Also separate
> arch_vpmu_ops initializers using these options and static inline stubs.
> 
> No functional change intended.
> 
> Signed-off-by: Sergiy Kibrik 
> CC: Andrew Cooper 
> CC: Jan Beulich 
> 
> ---
> changes in v1:
>  - switch to CONFIG_{AMD,INTEL} instead of CONFIG_{SVM,VMX}
> 
> 
>  xen/arch/x86/cpu/Makefile   |  4 +++-
>  xen/arch/x86/include/asm/vpmu.h | 19 +++
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile
> index 35561fe51d..eafce5f204 100644
> --- a/xen/arch/x86/cpu/Makefile
> +++ b/xen/arch/x86/cpu/Makefile
> @@ -10,4 +10,6 @@ obj-y += intel.o
>  obj-y += intel_cacheinfo.o
>  obj-y += mwait-idle.o
>  obj-y += shanghai.o
> -obj-y += vpmu.o vpmu_amd.o vpmu_intel.o
> +obj-y += vpmu.o
> +obj-$(CONFIG_AMD) += vpmu_amd.o
> +obj-$(CONFIG_INTEL) += vpmu_intel.o
> diff --git a/xen/arch/x86/include/asm/vpmu.h b/xen/arch/x86/include/asm/vpmu.h
> index dae9b43dac..e7a8f211f8 100644
> --- a/xen/arch/x86/include/asm/vpmu.h
> +++ b/xen/arch/x86/include/asm/vpmu.h
> @@ -11,6 +11,7 @@
>  #define __ASM_X86_HVM_VPMU_H_
>  
>  #include 
> +#include 
>  
>  #define vcpu_vpmu(vcpu)   (&(vcpu)->arch.vpmu)
>  #define vpmu_vcpu(vpmu)   container_of((vpmu), struct vcpu, arch.vpmu)
> @@ -42,9 +43,27 @@ struct arch_vpmu_ops {
>  #endif
>  };
>  
> +#ifdef CONFIG_INTEL
>  const struct arch_vpmu_ops *core2_vpmu_init(void);
> +#else
> +static inline const struct arch_vpmu_ops *core2_vpmu_init(void)
> +{
> +return ERR_PTR(-ENODEV);
> +}
> +#endif
> +#ifdef CONFIG_AMD
>  const struct arch_vpmu_ops *amd_vpmu_init(void);
>  const struct arch_vpmu_ops *hygon_vpmu_init(void);
> +#else
> +static inline const struct arch_vpmu_ops *amd_vpmu_init(void)
> +{
> +return ERR_PTR(-ENODEV);
> +}
> +static inline const struct arch_vpmu_ops *hygon_vpmu_init(void)
> +{
> +return ERR_PTR(-ENODEV);
> +}
> +#endif

At some point we'll need to discuss how to separate out hygon as well.
But for now:

Reviewed-by: Stefano Stabellini

[XEN PATCH v1 1/7] x86/vpmu: separate amd/intel vPMU code

2024-04-23 Thread Sergiy Kibrik 
Build AMD vPMU when CONFIG_AMD is on, and Intel vPMU when CONFIG_INTEL
is on respectively, allowing for a plaftorm-specific build. Also separate
arch_vpmu_ops initializers using these options and static inline stubs.

No functional change intended.

Signed-off-by: Sergiy Kibrik 
CC: Andrew Cooper 
CC: Jan Beulich 

---
changes in v1:
 - switch to CONFIG_{AMD,INTEL} instead of CONFIG_{SVM,VMX}


 xen/arch/x86/cpu/Makefile   |  4 +++-
 xen/arch/x86/include/asm/vpmu.h | 19 +++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile
index 35561fe51d..eafce5f204 100644
--- a/xen/arch/x86/cpu/Makefile
+++ b/xen/arch/x86/cpu/Makefile
@@ -10,4 +10,6 @@ obj-y += intel.o
 obj-y += intel_cacheinfo.o
 obj-y += mwait-idle.o
 obj-y += shanghai.o
-obj-y += vpmu.o vpmu_amd.o vpmu_intel.o
+obj-y += vpmu.o
+obj-$(CONFIG_AMD) += vpmu_amd.o
+obj-$(CONFIG_INTEL) += vpmu_intel.o
diff --git a/xen/arch/x86/include/asm/vpmu.h b/xen/arch/x86/include/asm/vpmu.h
index dae9b43dac..e7a8f211f8 100644
--- a/xen/arch/x86/include/asm/vpmu.h
+++ b/xen/arch/x86/include/asm/vpmu.h
@@ -11,6 +11,7 @@
 #define __ASM_X86_HVM_VPMU_H_
 
 #include 
+#include 
 
 #define vcpu_vpmu(vcpu)   (&(vcpu)->arch.vpmu)
 #define vpmu_vcpu(vpmu)   container_of((vpmu), struct vcpu, arch.vpmu)
@@ -42,9 +43,27 @@ struct arch_vpmu_ops {
 #endif
 };
 
+#ifdef CONFIG_INTEL
 const struct arch_vpmu_ops *core2_vpmu_init(void);
+#else
+static inline const struct arch_vpmu_ops *core2_vpmu_init(void)
+{
+return ERR_PTR(-ENODEV);
+}
+#endif
+#ifdef CONFIG_AMD
 const struct arch_vpmu_ops *amd_vpmu_init(void);
 const struct arch_vpmu_ops *hygon_vpmu_init(void);
+#else
+static inline const struct arch_vpmu_ops *amd_vpmu_init(void)
+{
+return ERR_PTR(-ENODEV);
+}
+static inline const struct arch_vpmu_ops *hygon_vpmu_init(void)
+{
+return ERR_PTR(-ENODEV);
+}
+#endif
 
 struct vpmu_struct {
 u32 flags;
-- 
2.25.1