[meta-intel] [PATCH] tune-skylake.inc: Improve handling the newly introduced skylake tune.

[Mittal, Anuj]

Hi Alejandro

Thank you for the patch. I have a couple of comments:

On Thu, 2019-08-22 at 01:47 -0700, Alejandro Enedino Hernandez
Samaniego wrote:
> The skylake tune introduced on 04510bfa
> 
> Currently disables qemu-usermode by default, due to the lack
> of support from QEMU to several of the Intel instruction set
> extensions introduced within the few latest generations of
> CPU architectures (e.g. QEMU does not support avx2)
> 
> While there is a good reason to perform the removal of qemu-usermode
> from MACHINE_FEATURES, there are several components within the
> build system that rely on it for proper compilation and behavior,
> for example anything that uses gobject data introspection or even
> the components like the chromium web browser require to run a QEMU
> for the target architecture to build successfully.
> 
> There is no reason why we can't have those components built without
> sacrificing the cpu (most) optimizations.
> 
> The process I followed on meta-chromebook to enable an optimized
> build
> and whats being upstreamed by this patch is that by doing some
> reverse engineering, I was able to figure out which instruction
> set extensions are not compatible with QEMU Skylake-Client, by
> performing a bit gcc magic from inside QEMU (target) to get the
> available optimizations for the native architecture (which is
> actually our target in this case).
> 
> These are all (not surprisingly) the avx2 extensions, listed as
> follows:
> -mno-avx
> -mno-avx2
> -mno-avx512f
> -mno-avx512er
> -mno-avx512cd
> -mno-avx512pf
> -mno-avx512dq
> -mno-avx512bw
> -mno-avx512vl
> -mno-avx512ifma
> -mno-avx512vbmi
> -mno-avx512vbmi2
> -mno-avx512vnni
> -mno-avx512bitalg
> 
> Specifically disabling these manually (for now), allows us to build
> an
> optimized system for the skylake/skylake based architectures (e.g.
> KabyLake)
> while keeping the capability of using qemu-usermode, as a side note
> GCC
> does show more unavailable optimization tunes, (hence why there might
> be
> some warnings), but getting rid of these specifically seems enough to
> make
> it run happily in qemu-usermode.
> 
> Basically the MACHINE_FEATURES variable is able to dictate how we
> will tune
> the build for our device, if qemu-usermode is present, TUNE_CCARGS
> will
> expand as follows:
> 
> TUNE_CCARGS=" -m64 -march=skylake  -mtune=skylake  -mno-avx -mno-avx2
> -mno-avx512f -mno-avx512er -mno-avx512cd -mno-avx512pf -mno-avx512dq
> -mno-avx512bw -mno-avx512vl -mno-avx512ifma -mno-avx512vbmi -mno-
> avx512vbmi2
> -mno-avx512vnni -mno-avx512bitalg  -mfpmath=sse"
> 
> Whats this means is that the build will be tuned for skylake
> architectures,
> enabling all possible extensions, (MOVBE, MMX, SSE, SSE2, SSE3,
> SSSE3, SSE4.1,
> SSE4.2, POPCNT, AES, PCLMUL, FSGSBASE, RDRND, FMA, BMI, BMI2, F16C,
> RDSEED,
> ADCX, PREFETCHW, CLFLUSHOPT, XSAVEC and XSAVES) minus the ones QEMU
> is unable
> to run, which will result in an illegal instruction error, notice the
> tune is
> kept as skylake.
> 
> Whilst, if qemu-usermode is not found on MACHINE_FEATURES,
> TUNE_CCARGS will
> expand to:
> TUNE_CCARGS=" -m64 -march=skylake -mtune=generic -mavx2 -mfpmath=sse"
> 
> Which is exactly what its set to with the current tune, so this patch
> should
> be harmless, and only extend current functionality.
> 
> As the GCC manual states, we should really try to avoid using
> mtune=generic
> when possible, and this patch allows us to do so [1].

-march also implies -mtune. So it's not necessary if it's going to be
set to Skylake.

> 
> This patch also addresses a problem on which the current skylake tune
> includes
> tune-core2.inc instead of tune-corei7.inc to get the list of
> AVAILTUNES
> and PACKAGE_EXTRA_ARCHS.
> 
> Right now, AVAILTUNES are set as follows:
> AVAILTUNES=" x86 x86-64 x86-64-x32 i586 i686 core2-32 core2-64 core2-
> 64-x32
> skylake-64"
> 
> Where the proper set should be (after this patch):
> AVAILTUNES=" x86 x86-64 x86-64-x32 i586 i686 core2-32 core2-64 core2-
> 64-x32
> corei7-32 corei7-64 corei7-64-x32 skylake-64"
> 
> When (if), QEMU gains support for the AVX2 instruction set extensions
> these
> can be easily removed to provide full support for qemu-usermode.
> 
> [1] https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/x86-Options.html
> 
> Signed-off-by: Alejandro Enedino Hernandez Samaniego <
> aehs29 at gmail.com>
> ---
>  conf/machine/include/tune-skylake.inc | 26 ++++++++++++++++++++++---
> -
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/conf/machine/include/tune-skylake.inc
> b/conf/machine/include/tune-skylake.inc
> index 78645de..1b1e224 100644
> --- a/conf/machine/include/tune-skylake.inc
> +++ b/conf/machine/include/tune-skylake.inc
> @@ -8,11 +8,13 @@
>  DEFAULTTUNE ?= "skylake-64"
>  
>  # Include the previous tune to pull in PACKAGE_EXTRA_ARCHS
> -require conf/machine/include/tune-core2.inc
> +require conf/machine/include/tune-corei7.inc
> +
> +SKYLAKE_TUNE .= "${@bb.utils.contains('MACHINE_FEATURES', 'qemu-
> usermode', ' -mtune=skylake ${QEMU_UNAVAILABLE_ISA}', '-mtune=generic 
> -mavx2', d)}"
>  
>  # Extra tune features
>  TUNEVALID[skylake] = "Enable skylake specific processor
> optimizations"
> -TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'skylake', '
> -march=skylake -mtune=generic -mfpmath=sse -mavx2', '', d)}"
> +TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'skylake', '
> -march=skylake ${SKYLAKE_TUNE} -mfpmath=sse', '', d)}"
>  
>  # Extra tune selections
>  
> @@ -23,6 +25,22 @@ TUNE_PKGARCH_tune-skylake-64 = "skylake-64"
>  PACKAGE_EXTRA_ARCHS_tune-skylake-64 = "${PACKAGE_EXTRA_ARCHS_tune-
> core2-64} skylake-64"
>  QEMU_EXTRAOPTIONS_skylake-64 = " -cpu Skylake-Client"
>  
> -# Disable QEMU user
> -MACHINE_FEATURES_remove = "qemu-usermode"
> +# Disable QEMU user (get avx2 but lose qemu-user capability)
> +#MACHINE_FEATURES_remove = "qemu-usermode"

Most people just use the default options and if that's working, don't
actually care about anything else.

So, I'd prefer to remove the qemu-usermode support and let this be
built with the AVX instruction support by default to get the
performance benefit that this MACHINE is supposed to provide.

Thanks,

Anuj