| Message ID | 20240903072614.291048-1-Sergiy_Kibrik@epam.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [XEN,v4] x86/intel: optional build of PSR support | expand |
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
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;
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(-)