Platform Shared Resource feature is available for certain Intel's CPUs only,
hence can be put under CONFIG_INTEL build option.

When !INTEL then PSR-related sysctls XEN_SYSCTL_psr_cmt_op &
XEN_SYSCTL_psr_alloc are off as well.

Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
---
 xen/arch/x86/Makefile          |  2 +-
 xen/arch/x86/domctl.c          |  2 ++
 xen/arch/x86/include/asm/psr.h | 15 +++++++++++++++
 xen/arch/x86/sysctl.c          |  4 ++++
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index d902fb7acc..02218d32c5 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -57,7 +57,7 @@ obj-y += pci.o
 obj-y += percpu.o
 obj-y += physdev.o
 obj-$(CONFIG_COMPAT) += x86_64/physdev.o
-obj-y += psr.o
+obj-$(CONFIG_INTEL) += psr.o
 obj-y += setup.o
 obj-y += shutdown.o
 obj-y += smp.o
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 9a72d57333..cccf71f745 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1160,6 +1160,7 @@ long arch_do_domctl(
         break;
     }
 
+#ifdef CONFIG_INTEL
     case XEN_DOMCTL_psr_cmt_op:
         if ( !psr_cmt_enabled() )
         {
@@ -1262,6 +1263,7 @@ long arch_do_domctl(
         }
 
         break;
+#endif
 
     case XEN_DOMCTL_get_cpu_policy:
         /* Process the CPUID leaves. */
diff --git a/xen/arch/x86/include/asm/psr.h b/xen/arch/x86/include/asm/psr.h
index 51df78794c..14d5d33970 100644
--- a/xen/arch/x86/include/asm/psr.h
+++ b/xen/arch/x86/include/asm/psr.h
@@ -72,6 +72,8 @@ static inline bool psr_cmt_enabled(void)
     return !!psr_cmt;
 }
 
+#ifdef CONFIG_INTEL
+
 int psr_alloc_rmid(struct domain *d);
 void psr_free_rmid(struct domain *d);
 void psr_ctxt_switch_to(struct domain *d);
@@ -86,6 +88,19 @@ int psr_set_val(struct domain *d, unsigned int socket,
 void psr_domain_init(struct domain *d);
 void psr_domain_free(struct domain *d);
 
+#else
+
+static inline void psr_ctxt_switch_to(struct domain *d)
+{
+}
+static inline void psr_domain_init(struct domain *d)
+{
+}
+static inline void psr_domain_free(struct domain *d)
+{
+}
+#endif /* CONFIG_INTEL */
+
 #endif /* __ASM_PSR_H__ */
 
 /*
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 1d40d82c5a..947c954b42 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -32,6 +32,7 @@
 #include <asm/psr.h>
 #include <asm/cpu-policy.h>
 
+#ifdef CONFIG_INTEL
 struct l3_cache_info {
     int ret;
     unsigned long size;
@@ -46,6 +47,7 @@ static void cf_check l3_cache_get(void *arg)
     if ( !l3_info->ret )
         l3_info->size = info.size / 1024; /* in KB unit */
 }
+#endif
 
 static long cf_check smt_up_down_helper(void *data)
 {
@@ -169,6 +171,7 @@ long arch_do_sysctl(
     }
     break;
 
+#ifdef CONFIG_INTEL
     case XEN_SYSCTL_psr_cmt_op:
         if ( !psr_cmt_enabled() )
             return -ENODEV;
@@ -286,6 +289,7 @@ long arch_do_sysctl(
         }
         break;
     }
+#endif
 
     case XEN_SYSCTL_get_cpu_levelling_caps:
         sysctl->u.cpu_levelling_caps.caps = levelling_caps;
-- 
2.25.1

On 06.06.2024 10:39, Sergiy Kibrik wrote:
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1160,6 +1160,7 @@ long arch_do_domctl(
>          break;
>      }
>  
> +#ifdef CONFIG_INTEL
>      case XEN_DOMCTL_psr_cmt_op:
>          if ( !psr_cmt_enabled() )
>          {
> @@ -1262,6 +1263,7 @@ long arch_do_domctl(
>          }
>  
>          break;
> +#endif

Imo the result thereof shouldn't be -ENOSYS, but -EOPNOTSUPP (at least
for XEN_DOMCTL_psr_alloc; for XEN_DOMCTL_psr_cmt_op it shouldn't change,
even if I consider this wrong, but that's a separate topic). Wouldn't it
be possible here to reduce the #ifdef scope a little to just most of the
inner switch()es (i.e. requiring it to be split into two regions), whose
"default" case is already (kind of) doing what we want?

> --- a/xen/arch/x86/include/asm/psr.h
> +++ b/xen/arch/x86/include/asm/psr.h
> @@ -72,6 +72,8 @@ static inline bool psr_cmt_enabled(void)
>      return !!psr_cmt;
>  }
>  
> +#ifdef CONFIG_INTEL
> +
>  int psr_alloc_rmid(struct domain *d);
>  void psr_free_rmid(struct domain *d);
>  void psr_ctxt_switch_to(struct domain *d);
> @@ -86,6 +88,19 @@ int psr_set_val(struct domain *d, unsigned int socket,
>  void psr_domain_init(struct domain *d);
>  void psr_domain_free(struct domain *d);
>  
> +#else
> +
> +static inline void psr_ctxt_switch_to(struct domain *d)
> +{
> +}
> +static inline void psr_domain_init(struct domain *d)
> +{
> +}
> +static inline void psr_domain_free(struct domain *d)
> +{
> +}
> +#endif /* CONFIG_INTEL */

As I think I did mention elsewhere, such stubs can have the braces on the
same line the the function specifier.

> @@ -169,6 +171,7 @@ long arch_do_sysctl(
>      }
>      break;
>  
> +#ifdef CONFIG_INTEL
>      case XEN_SYSCTL_psr_cmt_op:
>          if ( !psr_cmt_enabled() )
>              return -ENODEV;
> @@ -286,6 +289,7 @@ long arch_do_sysctl(
>          }
>          break;
>      }
> +#endif

Like for domctl I think you want to reduce the #ifdef scope some, even if
for XEN_SYSCTL_psr_cmt_op that'll still result in -ENOSYS. At least we'll
then be consistent in clearing sysctl->u.psr_cmt_op.u.data there.

Jan