Re: [Xen-devel] [PATCH 2/2] x86: drop cpu_has_sse{,2}

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.12.16 at 15:56, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/include/asm-x86/cpufeature.h
> +++ b/xen/include/asm-x86/cpufeature.h
> @@ -59,8 +59,8 @@ XEN_CPUFEATURE(XEN_SMAP,        (FSCAPINTS+0)*32+ 11)
> /* SMAP gets used by Xen i
>  #define cpu_has_sep            boot_cpu_has(X86_FEATURE_SEP)
>  #define cpu_has_mtrr           1
>  #define cpu_has_mmx            1
> -#define cpu_has_sse            boot_cpu_has(X86_FEATURE_SSE)
> -#define cpu_has_sse2           boot_cpu_has(X86_FEATURE_SSE2)
> +#define cpu_has_sse            1
> +#define cpu_has_sse2           1

Such defines are a complete waste of space and compiler parsing
bandwidth. I'd really prefer not having to introduce them; I did
propose in the past to make e.g. cpu_has_fpu and cpu_has_mmx
actual checks again, as there certainly are signs of either or both
not having a permanent place in the ISA. Stuff like cpu_has_mtrr
would imo also better either go away (if we mean to tie ourselves
to present MTRRs) or be an actual check.

>>> Swapping the hypervisor defines to being 1 will cause the
>>> generate_exception_if() clause to become dead and get dropped, turning
>>> host_and_vcpu_must_have() into just vcpu_must_have_##feat
>> Which is fine for the hypervisor. And for the test harness the
>> vcpu_must_have() is sufficient, as explained before.
> 
> It is a layering violation to remove the host_has_* part of the check
> from the emulator.

This is from a very, very abstract perspective. The fact that
host_and_vcpu_*() and vcpu_*() are the same in the harness
should allow for them to be used interchangeably when only the
harness is affected.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel