Certain fallback code can be made subject to DCE this way. Note that
CX16 has no compiler provided manifest constant, so CONFIG_* are used
there instead. Note also that we don't have cpu_has_movbe nor
cpu_has_lzcnt (aka cpu_has_abm).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Of course we could use IS_ENABLED(CONFIG_X86_64_V<n>) everywhere, but as
CX16 shows this isn't necessarily better than the #if/#else approach
based on compiler-provided manifest constants. While not really intended
to be used that way, it looks as if we could also use
IS_ENABLED(__POPCNT__) and alike.

We could go further and also short-circuit SSE*, AVX and alike, which we
don't use outside of the emulator.

--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -76,13 +76,19 @@ static inline bool boot_cpu_has(unsigned
 #define cpu_has_eist            boot_cpu_has(X86_FEATURE_EIST)
 #define cpu_has_ssse3           boot_cpu_has(X86_FEATURE_SSSE3)
 #define cpu_has_fma             boot_cpu_has(X86_FEATURE_FMA)
-#define cpu_has_cx16            boot_cpu_has(X86_FEATURE_CX16)
+#define cpu_has_cx16            (IS_ENABLED(CONFIG_X86_64_V2) || \
+                                 IS_ENABLED(CONFIG_X86_64_V3) || \
+                                 boot_cpu_has(X86_FEATURE_CX16))
 #define cpu_has_pdcm            boot_cpu_has(X86_FEATURE_PDCM)
 #define cpu_has_pcid            boot_cpu_has(X86_FEATURE_PCID)
 #define cpu_has_sse4_1          boot_cpu_has(X86_FEATURE_SSE4_1)
 #define cpu_has_sse4_2          boot_cpu_has(X86_FEATURE_SSE4_2)
 #define cpu_has_x2apic          boot_cpu_has(X86_FEATURE_X2APIC)
+#ifdef __POPCNT__
+#define cpu_has_popcnt          true
+#else
 #define cpu_has_popcnt          boot_cpu_has(X86_FEATURE_POPCNT)
+#endif
 #define cpu_has_aesni           boot_cpu_has(X86_FEATURE_AESNI)
 #define cpu_has_xsave           boot_cpu_has(X86_FEATURE_XSAVE)
 #define cpu_has_avx             boot_cpu_has(X86_FEATURE_AVX)
@@ -114,11 +120,19 @@ static inline bool boot_cpu_has(unsigned
 #define cpu_has_xsaves          boot_cpu_has(X86_FEATURE_XSAVES)
 
 /* CPUID level 0x00000007:0.ebx */
+#ifdef __BMI__
+#define cpu_has_bmi1            true
+#else
 #define cpu_has_bmi1            boot_cpu_has(X86_FEATURE_BMI1)
+#endif
 #define cpu_has_hle             boot_cpu_has(X86_FEATURE_HLE)
 #define cpu_has_avx2            boot_cpu_has(X86_FEATURE_AVX2)
 #define cpu_has_smep            boot_cpu_has(X86_FEATURE_SMEP)
+#ifdef __BMI2__
+#define cpu_has_bmi2            true
+#else
 #define cpu_has_bmi2            boot_cpu_has(X86_FEATURE_BMI2)
+#endif
 #define cpu_has_invpcid         boot_cpu_has(X86_FEATURE_INVPCID)
 #define cpu_has_rtm             boot_cpu_has(X86_FEATURE_RTM)
 #define cpu_has_pqe             boot_cpu_has(X86_FEATURE_PQE)

On Wed, Jul 12, 2023 at 8:36 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> Certain fallback code can be made subject to DCE this way. Note that
> CX16 has no compiler provided manifest constant, so CONFIG_* are used
> there instead. Note also that we don't have cpu_has_movbe nor
> cpu_has_lzcnt (aka cpu_has_abm).
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

One thought below.

> ---
> Of course we could use IS_ENABLED(CONFIG_X86_64_V<n>) everywhere, but as
> CX16 shows this isn't necessarily better than the #if/#else approach
> based on compiler-provided manifest constants. While not really intended
> to be used that way, it looks as if we could also use
> IS_ENABLED(__POPCNT__) and alike.
>
> We could go further and also short-circuit SSE*, AVX and alike, which we
> don't use outside of the emulator.
>
> --- a/xen/arch/x86/include/asm/cpufeature.h
> +++ b/xen/arch/x86/include/asm/cpufeature.h
> @@ -76,13 +76,19 @@ static inline bool boot_cpu_has(unsigned
>  #define cpu_has_eist            boot_cpu_has(X86_FEATURE_EIST)
>  #define cpu_has_ssse3           boot_cpu_has(X86_FEATURE_SSSE3)
>  #define cpu_has_fma             boot_cpu_has(X86_FEATURE_FMA)
> -#define cpu_has_cx16            boot_cpu_has(X86_FEATURE_CX16)
> +#define cpu_has_cx16            (IS_ENABLED(CONFIG_X86_64_V2) || \
> +                                 IS_ENABLED(CONFIG_X86_64_V3) || \
> +                                 boot_cpu_has(X86_FEATURE_CX16))

If you think there may be more ABI selections in the future, it might
be better to express the "V$N" numerically and check >= 2.  Or you can
add a Kconfig CONFIG_X86_64_CX16 and select that as appropriate.  But
if there aren't going to be more of these, then this is fine.

Regards,
Jason

On 17.07.2023 14:35, Jason Andryuk wrote:
> On Wed, Jul 12, 2023 at 8:36 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> Certain fallback code can be made subject to DCE this way. Note that
>> CX16 has no compiler provided manifest constant, so CONFIG_* are used
>> there instead. Note also that we don't have cpu_has_movbe nor
>> cpu_has_lzcnt (aka cpu_has_abm).
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

Thanks.

>> --- a/xen/arch/x86/include/asm/cpufeature.h
>> +++ b/xen/arch/x86/include/asm/cpufeature.h
>> @@ -76,13 +76,19 @@ static inline bool boot_cpu_has(unsigned
>>  #define cpu_has_eist            boot_cpu_has(X86_FEATURE_EIST)
>>  #define cpu_has_ssse3           boot_cpu_has(X86_FEATURE_SSSE3)
>>  #define cpu_has_fma             boot_cpu_has(X86_FEATURE_FMA)
>> -#define cpu_has_cx16            boot_cpu_has(X86_FEATURE_CX16)
>> +#define cpu_has_cx16            (IS_ENABLED(CONFIG_X86_64_V2) || \
>> +                                 IS_ENABLED(CONFIG_X86_64_V3) || \
>> +                                 boot_cpu_has(X86_FEATURE_CX16))
> 
> If you think there may be more ABI selections in the future, it might
> be better to express the "V$N" numerically and check >= 2.  Or you can
> add a Kconfig CONFIG_X86_64_CX16 and select that as appropriate.  But
> if there aren't going to be more of these, then this is fine.

I was thinking this same way: If more appear (which aren't SIMD-only,
like v4 is), we can use a numeric CONFIG_*, but for now it's good
enough (and slightly simpler) this way.

Jan