Re: [PATCH] x86: add more x86-64 micro-architecture levels

[Dave Hansen]

On 9/15/24 04:05, John wrote:
> +config MAMD_CPU_V2
> +	bool "AMD x86-64-v2"
> +	depends on (CC_IS_GCC && GCC_VERSION > 110000) || (CC_IS_CLANG && CLANG_VERSION >= 120000)
> +	depends on X86_64
> +	help
> +	  AMD x86-64 CPU with v2 instructions.
> +	  Run equally well on all AMD x86-64 CPUs with min support of -march=x86-64-v2.

If these are going to be exposed to end users, we need *some* kind of
help text that helps end users select among these options and what the
pitfalls are.

I actually don't have the foggiest idea what an "AMD x86-64 CPU with v2
instructions" even is.  Even saying "AMD x86-64 CPU" isn't super helpful
because "AMD x86_64" is kinda a generic way to refer to all the 64-bit
x86 CPUs, Intel included.

I assume that the compilers have grouped the CPUs into epochs that have
some similarity.  That's great and all, but we need to tell users what
those are.

Why are there v4's for both AMD and Intel that do the exact same thing?

+        cflags-$(CONFIG_MAMD_CPU_V4)	+= -march=x86-64-v4
...
+        cflags-$(CONFIG_MINTEL_CPU_V4)	+= -march=x86-64-v4

Why is this copied and pasted six times?

+	depends on (CC_IS_GCC && GCC_VERSION > 110000)...

I'm also _kinda_ surprised we don't have some kind of Kconfig option to
just pass random flags into the compiler.  That would be another way to
do this.  That would also be a, maybe, 10-line patch.

Alternatively, anyone wanting to do this could just hack their makefile
or (I assume) pass CFLAGS= into the build command-line.  Why is
something like that insufficient.

In the *WORST* case, we shouldn't be doing this with bools.  Do this:

config X86_MARCH_VER
	int "Compiler Micro-Architecture Level"
	range 2 4
	depends on (CC_IS_GCC   && GCC_VERSION   >  110000) ||
                   (CC_IS_CLANG && CLANG_VERSION >= 120000)
	depends on EXPERT
	depends on X86_64
	help
	  Specify a specific compiler "micro-architecture" version.
	  You might want to do this when...
	  You can find the best version for your CPU here...
	  The pitfalls of this option are...

Then you can do fun like:

 config X86_L1_CACHE_SHIFT
 	int
	default "7" if MPENTIUM4 || MPSC
+	default "6" if MK7 || MK8 || MPENTIUMM || MCORE2 || ...
+		       X86_MARCH_VER >= 2

which has the added advantage of never needing to be touched when v5
gets added.

Oh, and this:

>  config X86_HAVE_PAE
>  	def_bool y
> -	depends on MCRUSOE || MEFFICEON || MCYRIXIII || MPENTIUM4 || MPENTIUMM || MPENTIUMIII || MPENTIUMII || M686 || MK8 || MVIAC7 || MCORE2 || MATOM || X86_64
> +	depends on MCRUSOE || MEFFICEON || MCYRIXIII || MPENTIUM4 || MPENTIUMM || MPENTIUMIII || MPENTIUMII || M686 || MK8 || MVIAC7 || MCORE2 || MATOM || X86_64 || MAMD_CPU_V2 || MAMD_CPU_V3 || MAMD_CPU_V4 || MINTEL_CPU_V2 || MINTEL_CPU_V3 || MINTEL_CPU_V4

is rather silly when M*_CPU_V* all:

	depends on X86_64

right?

So, taking a step back: Please convince us that this is something we
want to expose to end users in the first place, as opposed to having
them hack makefiles or just allowing users a string instead of using the
existing CONFIG_M* Kconfig options.

Then, we can discuss the structure of these options.  Should these
"versions" be new "Processor family" options?  Or, should they be
_instead_ of selecting a "Processor family"

Then, should the new Kconfig options be a series of bools, or an int?

Last, how do we deal with multiple vendors?  Or do we need it at all?
I'm not actually sure at all why this has the AMD versus Intel
distinction at all.