diff mbox series

[XEN,v2,1/5] x86/Kconfig: introduce CENTAUR, HYGON & SHANGHAI config options

Message ID 2a217c9602e92f92050cb4894bb9a42ee99a84ea.1723806405.git.Sergiy_Kibrik@epam.com (mailing list archive)
State New
Headers show
Series x86/CPU: optional build of Intel/AMD CPUs support | expand

Commit Message

Sergiy Kibrik Aug. 16, 2024, 11:10 a.m. UTC
These options aim to represent what's currently supported by Xen, and later
to allow tuning for specific platform(s) only.

HYGON and SHANGHAI options depend on AMD and INTEL as there're build
dependencies on support code for AMD and Intel CPUs respectively.

Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: Alejandro Vallejo <alejandro.vallejo@cloud.com>
CC: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/Kconfig.cpu  | 29 +++++++++++++++++++++++++++++
 xen/arch/x86/cpu/Makefile |  6 +++---
 xen/arch/x86/cpu/common.c |  6 ++++++
 3 files changed, 38 insertions(+), 3 deletions(-)

Comments

Alejandro Vallejo Aug. 16, 2024, 1:16 p.m. UTC | #1
On Fri Aug 16, 2024 at 12:10 PM BST, Sergiy Kibrik wrote:
> These options aim to represent what's currently supported by Xen, and later
> to allow tuning for specific platform(s) only.
>
> HYGON and SHANGHAI options depend on AMD and INTEL as there're build
> dependencies on support code for AMD and Intel CPUs respectively.
>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> CC: Alejandro Vallejo <alejandro.vallejo@cloud.com>
> CC: Jan Beulich <jbeulich@suse.com>
> ---
>  xen/arch/x86/Kconfig.cpu  | 29 +++++++++++++++++++++++++++++
>  xen/arch/x86/cpu/Makefile |  6 +++---
>  xen/arch/x86/cpu/common.c |  6 ++++++
>  3 files changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/Kconfig.cpu b/xen/arch/x86/Kconfig.cpu
> index 5fb18db1aa..ac8f41d464 100644
> --- a/xen/arch/x86/Kconfig.cpu
> +++ b/xen/arch/x86/Kconfig.cpu
> @@ -10,6 +10,25 @@ config AMD
>  	  May be turned off in builds targetting other vendors.  Otherwise,
>  	  must be enabled for Xen to work suitably on AMD platforms.
>  
> +config CENTAUR
> +	bool "Support Centaur CPUs"
> +	default y
> +	help
> +	  Detection, tunings and quirks for VIA platforms.
> +
> +	  May be turned off in builds targeting other vendors. Otherwise, must
> +          be enabled for Xen to work suitably on VIA platforms.
> +
> +config HYGON
> +	bool "Support Hygon CPUs"
> +	depends on AMD
> +	default y
> +	help
> +	  Detection, tunings and quirks for Hygon platforms.
> +
> +	  May be turned off in builds targeting other vendors. Otherwise, must
> +          be enabled for Xen to work suitably on Hygon platforms.
> +
>  config INTEL
>  	bool "Support Intel CPUs"
>  	default y
> @@ -19,4 +38,14 @@ config INTEL
>  	  May be turned off in builds targetting other vendors.  Otherwise,
>  	  must be enabled for Xen to work suitably on Intel platforms.
>  
> +config SHANGHAI
> +	bool "Support Shanghai CPUs"
> +	depends on INTEL
> +	default y
> +	help
> +	  Detection, tunings and quirks for Zhaoxin platforms.
> +
> +	  May be turned off in builds targeting other vendors. Otherwise, must
> +          be enabled for Xen to work suitably on Zhaoxin platforms.
> +
>  endmenu
> diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile
> index eafce5f204..80739d0256 100644
> --- a/xen/arch/x86/cpu/Makefile
> +++ b/xen/arch/x86/cpu/Makefile
> @@ -3,13 +3,13 @@ obj-y += microcode/
>  obj-y += mtrr/
>  
>  obj-y += amd.o
> -obj-y += centaur.o
> +obj-$(CONFIG_CENTAUR) += centaur.o
>  obj-y += common.o
> -obj-y += hygon.o
> +obj-$(CONFIG_HYGON) += hygon.o
>  obj-y += intel.o
>  obj-y += intel_cacheinfo.o
>  obj-y += mwait-idle.o
> -obj-y += shanghai.o
> +obj-$(CONFIG_SHANGHAI) += shanghai.o
>  obj-y += vpmu.o
>  obj-$(CONFIG_AMD) += vpmu_amd.o
>  obj-$(CONFIG_INTEL) += vpmu_intel.o
> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
> index ff4cd22897..dcc2753212 100644
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -339,9 +339,15 @@ void __init early_cpu_init(bool verbose)
>  	case X86_VENDOR_INTEL:    intel_unlock_cpuid_leaves(c);
>  				  actual_cpu = intel_cpu_dev;    break;
>  	case X86_VENDOR_AMD:      actual_cpu = amd_cpu_dev;      break;
> +#ifdef CONFIG_CENTAUR
>  	case X86_VENDOR_CENTAUR:  actual_cpu = centaur_cpu_dev;  break;
> +#endif
> +#ifdef CONFIG_SHANGHAI
>  	case X86_VENDOR_SHANGHAI: actual_cpu = shanghai_cpu_dev; break;
> +#endif
> +#ifdef CONFIG_HYGON
>  	case X86_VENDOR_HYGON:    actual_cpu = hygon_cpu_dev;    break;
> +#endif
>  	default:
>  		actual_cpu = default_cpu;
>  		if (!verbose)

Reviewed-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>

Cheers,
Alejandro
Jan Beulich Aug. 19, 2024, 8:53 a.m. UTC | #2
On 16.08.2024 13:10, Sergiy Kibrik wrote:
> --- a/xen/arch/x86/Kconfig.cpu
> +++ b/xen/arch/x86/Kconfig.cpu
> @@ -10,6 +10,25 @@ config AMD
>  	  May be turned off in builds targetting other vendors.  Otherwise,
>  	  must be enabled for Xen to work suitably on AMD platforms.
>  
> +config CENTAUR
> +	bool "Support Centaur CPUs"
> +	default y
> +	help
> +	  Detection, tunings and quirks for VIA platforms.
> +
> +	  May be turned off in builds targeting other vendors. Otherwise, must
> +          be enabled for Xen to work suitably on VIA platforms.
> +
> +config HYGON
> +	bool "Support Hygon CPUs"
> +	depends on AMD
> +	default y
> +	help
> +	  Detection, tunings and quirks for Hygon platforms.
> +
> +	  May be turned off in builds targeting other vendors. Otherwise, must
> +          be enabled for Xen to work suitably on Hygon platforms.
> +
>  config INTEL
>  	bool "Support Intel CPUs"
>  	default y
> @@ -19,4 +38,14 @@ config INTEL
>  	  May be turned off in builds targetting other vendors.  Otherwise,
>  	  must be enabled for Xen to work suitably on Intel platforms.
>  
> +config SHANGHAI
> +	bool "Support Shanghai CPUs"
> +	depends on INTEL
> +	default y
> +	help
> +	  Detection, tunings and quirks for Zhaoxin platforms.
> +
> +	  May be turned off in builds targeting other vendors. Otherwise, must
> +          be enabled for Xen to work suitably on Zhaoxin platforms.
> +
>  endmenu

Imo this re-raises the question of whether it is a good idea to leave out
"CPU" from the names: The more names there are, the more likely it'll become
that going forward we'll run into a naming collision. Andrew, iirc you
were the main proponent for omitting "CPU" - may I ask that you re-consider?

Furthermore I wonder whether "depends on" is appropriate here. This way one
won't be offered e.g. SHANGHAI if earlier on one de-selected INTEL.

Plus it is mere luck that the alphabetic ordering ends up with the dependents
after their dependencies (things would be somewhat odd the other way around).

Finally please check indentation of help text - there is one inconsistency
repeated for all three entries being added.

Jan
Sergiy Kibrik Aug. 29, 2024, 9:29 a.m. UTC | #3
19.08.24 11:53, Jan Beulich:
> On 16.08.2024 13:10, Sergiy Kibrik wrote:
>> --- a/xen/arch/x86/Kconfig.cpu
>> +++ b/xen/arch/x86/Kconfig.cpu
>> @@ -10,6 +10,25 @@ config AMD
>>   	  May be turned off in builds targetting other vendors.  Otherwise,
>>   	  must be enabled for Xen to work suitably on AMD platforms.
>>   
>> +config CENTAUR
>> +	bool "Support Centaur CPUs"
>> +	default y
>> +	help
>> +	  Detection, tunings and quirks for VIA platforms.
>> +
>> +	  May be turned off in builds targeting other vendors. Otherwise, must
>> +          be enabled for Xen to work suitably on VIA platforms.
>> +
>> +config HYGON
>> +	bool "Support Hygon CPUs"
>> +	depends on AMD
>> +	default y
>> +	help
>> +	  Detection, tunings and quirks for Hygon platforms.
>> +
>> +	  May be turned off in builds targeting other vendors. Otherwise, must
>> +          be enabled for Xen to work suitably on Hygon platforms.
>> +
>>   config INTEL
>>   	bool "Support Intel CPUs"
>>   	default y
>> @@ -19,4 +38,14 @@ config INTEL
>>   	  May be turned off in builds targetting other vendors.  Otherwise,
>>   	  must be enabled for Xen to work suitably on Intel platforms.
>>   
>> +config SHANGHAI
>> +	bool "Support Shanghai CPUs"
>> +	depends on INTEL
>> +	default y
>> +	help
>> +	  Detection, tunings and quirks for Zhaoxin platforms.
>> +
>> +	  May be turned off in builds targeting other vendors. Otherwise, must
>> +          be enabled for Xen to work suitably on Zhaoxin platforms.
>> +
>>   endmenu
> Imo this re-raises the question of whether it is a good idea to leave out
> "CPU" from the names: The more names there are, the more likely it'll become
> that going forward we'll run into a naming collision. Andrew, iirc you
> were the main proponent for omitting "CPU" - may I ask that you re-consider?
> 
> Furthermore I wonder whether "depends on" is appropriate here. This way one
> won't be offered e.g. SHANGHAI if earlier on one de-selected INTEL.

I see, then probably I'll use 'select INTEL' here.
And also 'select AMD' for HYGON.

   -Sergiy
diff mbox series

Patch

diff --git a/xen/arch/x86/Kconfig.cpu b/xen/arch/x86/Kconfig.cpu
index 5fb18db1aa..ac8f41d464 100644
--- a/xen/arch/x86/Kconfig.cpu
+++ b/xen/arch/x86/Kconfig.cpu
@@ -10,6 +10,25 @@  config AMD
 	  May be turned off in builds targetting other vendors.  Otherwise,
 	  must be enabled for Xen to work suitably on AMD platforms.
 
+config CENTAUR
+	bool "Support Centaur CPUs"
+	default y
+	help
+	  Detection, tunings and quirks for VIA platforms.
+
+	  May be turned off in builds targeting other vendors. Otherwise, must
+          be enabled for Xen to work suitably on VIA platforms.
+
+config HYGON
+	bool "Support Hygon CPUs"
+	depends on AMD
+	default y
+	help
+	  Detection, tunings and quirks for Hygon platforms.
+
+	  May be turned off in builds targeting other vendors. Otherwise, must
+          be enabled for Xen to work suitably on Hygon platforms.
+
 config INTEL
 	bool "Support Intel CPUs"
 	default y
@@ -19,4 +38,14 @@  config INTEL
 	  May be turned off in builds targetting other vendors.  Otherwise,
 	  must be enabled for Xen to work suitably on Intel platforms.
 
+config SHANGHAI
+	bool "Support Shanghai CPUs"
+	depends on INTEL
+	default y
+	help
+	  Detection, tunings and quirks for Zhaoxin platforms.
+
+	  May be turned off in builds targeting other vendors. Otherwise, must
+          be enabled for Xen to work suitably on Zhaoxin platforms.
+
 endmenu
diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile
index eafce5f204..80739d0256 100644
--- a/xen/arch/x86/cpu/Makefile
+++ b/xen/arch/x86/cpu/Makefile
@@ -3,13 +3,13 @@  obj-y += microcode/
 obj-y += mtrr/
 
 obj-y += amd.o
-obj-y += centaur.o
+obj-$(CONFIG_CENTAUR) += centaur.o
 obj-y += common.o
-obj-y += hygon.o
+obj-$(CONFIG_HYGON) += hygon.o
 obj-y += intel.o
 obj-y += intel_cacheinfo.o
 obj-y += mwait-idle.o
-obj-y += shanghai.o
+obj-$(CONFIG_SHANGHAI) += shanghai.o
 obj-y += vpmu.o
 obj-$(CONFIG_AMD) += vpmu_amd.o
 obj-$(CONFIG_INTEL) += vpmu_intel.o
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index ff4cd22897..dcc2753212 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -339,9 +339,15 @@  void __init early_cpu_init(bool verbose)
 	case X86_VENDOR_INTEL:    intel_unlock_cpuid_leaves(c);
 				  actual_cpu = intel_cpu_dev;    break;
 	case X86_VENDOR_AMD:      actual_cpu = amd_cpu_dev;      break;
+#ifdef CONFIG_CENTAUR
 	case X86_VENDOR_CENTAUR:  actual_cpu = centaur_cpu_dev;  break;
+#endif
+#ifdef CONFIG_SHANGHAI
 	case X86_VENDOR_SHANGHAI: actual_cpu = shanghai_cpu_dev; break;
+#endif
+#ifdef CONFIG_HYGON
 	case X86_VENDOR_HYGON:    actual_cpu = hygon_cpu_dev;    break;
+#endif
 	default:
 		actual_cpu = default_cpu;
 		if (!verbose)