Platform Shared Resource feature is available for certain Intel's CPUs only,

Xen's implementation of PSR only supports Intel CPUs right now, hence it can be
made dependant on CONFIG_INTEL build option.
Since platform implementation is not limited to single vendor, intermediate
option CONFIG_PSR introduced, which selected by CONFIG_INTEL.

When !PSR 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>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
---
v3 patch here:
https://lore.kernel.org/xen-devel/20240829090559.149249-1-Sergiy_Kibrik@epam.com/

changes in v4:
 - introduced CONFIG_PSR
 - changed description
 - changes to psr stubs
changes in v3:
 - drop stubs for psr_domain_{init,free} & psr_ctxt_switch_to() and guard these
   routines at call sites
 - add stub for psr_cmt_enabled()
 - drop some of #ifdef-s from arch_do_{domctl,sysctl}
---
 xen/arch/x86/Kconfig           |  3 +++
 xen/arch/x86/Kconfig.cpu       |  1 +
 xen/arch/x86/Makefile          |  2 +-
 xen/arch/x86/domctl.c          |  3 +++
 xen/arch/x86/include/asm/psr.h | 10 ++++++++--
 xen/arch/x86/sysctl.c          |  4 +++-
 6 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 7ef5c8bc48..1fcb7b3a26 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -284,6 +284,9 @@ endchoice
 config GUEST
 	bool
 
+config PSR
+	bool
+
 config XEN_GUEST
 	bool "Xen Guest"
 	select GUEST
diff --git a/xen/arch/x86/Kconfig.cpu b/xen/arch/x86/Kconfig.cpu
index 5fb18db1aa..7c649a478b 100644
--- a/xen/arch/x86/Kconfig.cpu
+++ b/xen/arch/x86/Kconfig.cpu
@@ -13,6 +13,7 @@ config AMD
 config INTEL
 	bool "Support Intel CPUs"
 	default y
+	select PSR
 	help
 	  Detection, tunings and quirks for Intel platforms.
 
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 286c003ec3..4db3c214b0 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_PSR) += 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 68b5b46d1a..182f5fb11b 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1195,6 +1195,7 @@ long arch_do_domctl(
     case XEN_DOMCTL_psr_alloc:
         switch ( domctl->u.psr_alloc.cmd )
         {
+#ifdef CONFIG_PSR
         case XEN_DOMCTL_PSR_SET_L3_CBM:
             ret = psr_set_val(d, domctl->u.psr_alloc.target,
                               domctl->u.psr_alloc.data,
@@ -1257,6 +1258,8 @@ long arch_do_domctl(
 
 #undef domctl_psr_get_val
 
+#endif /* CONFIG_PSR */
+
         default:
             ret = -EOPNOTSUPP;
             break;
diff --git a/xen/arch/x86/include/asm/psr.h b/xen/arch/x86/include/asm/psr.h
index 51df78794c..7fa51f0cb3 100644
--- a/xen/arch/x86/include/asm/psr.h
+++ b/xen/arch/x86/include/asm/psr.h
@@ -69,12 +69,11 @@ extern struct psr_cmt *psr_cmt;
 
 static inline bool psr_cmt_enabled(void)
 {
-    return !!psr_cmt;
+    return IS_ENABLED(CONFIG_PSR) ? !!psr_cmt : false;
 }
 
 int psr_alloc_rmid(struct domain *d);
 void psr_free_rmid(struct domain *d);
-void psr_ctxt_switch_to(struct domain *d);
 
 int psr_get_info(unsigned int socket, enum psr_type type,
                  uint32_t data[], unsigned int array_len);
@@ -83,8 +82,15 @@ int psr_get_val(struct domain *d, unsigned int socket,
 int psr_set_val(struct domain *d, unsigned int socket,
                 uint64_t new_val, enum psr_type type);
 
+#ifdef CONFIG_PSR
+void psr_ctxt_switch_to(struct domain *d);
 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
 
 #endif /* __ASM_PSR_H__ */
 
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 1d40d82c5a..fedb533ce5 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -225,10 +225,11 @@ long arch_do_sysctl(
 
     case XEN_SYSCTL_psr_alloc:
     {
-        uint32_t data[PSR_INFO_ARRAY_SIZE] = { };
+        uint32_t __maybe_unused data[PSR_INFO_ARRAY_SIZE] = { };
 
         switch ( sysctl->u.psr_alloc.cmd )
         {
+#ifdef CONFIG_PSR
         case XEN_SYSCTL_PSR_get_l3_info:
             ret = psr_get_info(sysctl->u.psr_alloc.target,
                                PSR_TYPE_L3_CBM, data, ARRAY_SIZE(data));
@@ -279,6 +280,7 @@ long arch_do_sysctl(
             if ( __copy_field_to_guest(u_sysctl, sysctl, u.psr_alloc) )
                 ret = -EFAULT;
             break;
+#endif /* CONFIG_PSR */
 
         default:
             ret = -EOPNOTSUPP;
-- 
2.25.1

On 03.09.2024 09:26, Sergiy Kibrik wrote:
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -284,6 +284,9 @@ endchoice
>  config GUEST
>  	bool
>  
> +config PSR
> +	bool
> +
>  config XEN_GUEST
>  	bool "Xen Guest"
>  	select GUEST

Inserting in the middle of guest related setting is a little odd.

> --- a/xen/arch/x86/Kconfig.cpu
> +++ b/xen/arch/x86/Kconfig.cpu
> @@ -13,6 +13,7 @@ config AMD
>  config INTEL
>  	bool "Support Intel CPUs"
>  	default y
> +	select PSR

I realize Andrew suggested it like this, so the question goes to him as
much as to you: If already we can isolate this code, is there a reason
not to make this a user visible option (with a "depends on" rather than a
"select") right away?

> --- a/xen/arch/x86/include/asm/psr.h
> +++ b/xen/arch/x86/include/asm/psr.h
> @@ -69,12 +69,11 @@ extern struct psr_cmt *psr_cmt;
>  
>  static inline bool psr_cmt_enabled(void)
>  {
> -    return !!psr_cmt;
> +    return IS_ENABLED(CONFIG_PSR) ? !!psr_cmt : false;

Perhaps just

    return IS_ENABLED(CONFIG_PSR) && psr_cmt;

?

Jan

09.09.24 17:24, Jan Beulich:
> On 03.09.2024 09:26, Sergiy Kibrik wrote:
>> --- a/xen/arch/x86/Kconfig
>> +++ b/xen/arch/x86/Kconfig
>> @@ -284,6 +284,9 @@ endchoice
>>   config GUEST
>>   	bool
>>   
>> +config PSR
>> +	bool
>> +
>>   config XEN_GUEST
>>   	bool "Xen Guest"
>>   	select GUEST
> 
> Inserting in the middle of guest related setting is a little odd.
> 

you're right, I'll try to find a nicer place

>> --- a/xen/arch/x86/Kconfig.cpu
>> +++ b/xen/arch/x86/Kconfig.cpu
>> @@ -13,6 +13,7 @@ config AMD
>>   config INTEL
>>   	bool "Support Intel CPUs"
>>   	default y
>> +	select PSR
> 
> I realize Andrew suggested it like this, so the question goes to him as
> much as to you: If already we can isolate this code, is there a reason
> not to make this a user visible option (with a "depends on" rather than a
> "select") right away?
> 

The reason is I didn't want to complicate configuration without a 
usecase -- would someone want to disable PSR while keeping the rest of 
Intel support enabled ?

>> --- a/xen/arch/x86/include/asm/psr.h
>> +++ b/xen/arch/x86/include/asm/psr.h
>> @@ -69,12 +69,11 @@ extern struct psr_cmt *psr_cmt;
>>   
>>   static inline bool psr_cmt_enabled(void)
>>   {
>> -    return !!psr_cmt;
>> +    return IS_ENABLED(CONFIG_PSR) ? !!psr_cmt : false;
> 
> Perhaps just
> 
>      return IS_ENABLED(CONFIG_PSR) && psr_cmt;
> 
> ?

sure, why not

   -Sergiy