[XEN,v5] x86/intel: optional build of PSR support

Commit Message

Sergiy Kibrik Sept. 18, 2024, 9:15 a.m. UTC

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 <[e-mail]>
CC: Jan Beulich <[e-mail]>
CC: Andrew Cooper <[e-mail]>
---
v4 patch here:
https://lore.kernel.org/xen-devel/[e-mail]/

changes in v5:
 - simplify psr_cmt_enabled()
 - move PSR config option and add description
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(-)

Comments

Jan Beulich Sept. 23, 2024, 3:01 p.m. UTC | #1

On 18.09.2024 11:15, Sergiy Kibrik wrote:
> changes in v5:
>  - simplify psr_cmt_enabled()
>  - move PSR config option and add description

What you did is not so much add a description, but ...

> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -231,6 +231,9 @@ config TBOOT
>  
>  	  If unsure, stay with the default.
>  
> +config PSR
> +	bool "Platform Shared Resource support"

... add a prompt. With a prompt, it also wants to have help text. And with
a prompt the question then is why ...

> --- 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

... INTEL=y uniformly forces it on, while for INTEL=n (where it's useless)
it can be manually set to on. Imo, if you permit user configurability (as
was asked for), then it wants to be

config PSR
	bool "Platform Shared Resource support"
	default INTEL

Further: For an acronym like PSR I consider it reasonably likely that
something similarly abbreviated may appear in common code. I wonder if we
weren't better off naming this X86_PSR right away. (Likely this would
extend to various other settings as well that we gained more or less
recently.)

Jan

Patch

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 62f0b5e0f4..75d4d0bf7d 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -231,6 +231,9 @@  config TBOOT
 
 	  If unsure, stay with the default.
 
+config PSR
+	bool "Platform Shared Resource support"
+
 choice
 	prompt "Alignment of Xen image"
 	default XEN_ALIGN_2M if PV_SHIM_EXCLUSIVE
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..c36ce992f4 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;
 }
 
 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;