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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.12.16 at 15:15, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 09/12/16 13:28, Jan Beulich wrote:
>>>>> On 09.12.16 at 14:00, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 09/12/16 11:54, Jan Beulich wrote:
>>>> Commit dc88221c97 ("x86: rename XMM* features to SSE*") pointlessly
>>>> added them - these features are always available on 64-bit CPUs. (Let's
>>>> not assume this for MMX though in at least the insn emulator.)
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> This isn't necessarily true when compiled for 32bit in the userspace
>>> harness.
>> In the test harness vcpu_has_* == cpu_has_*, as also
>> demonstrated by
>>
>> #define host_and_vcpu_must_have(feat) vcpu_must_have(feat)
>>
>> . The change (as its title is trying to say) really only affects the
>> hypervisor.
> 
> Right, but this change is still contrary to the written requirement.

I don't understand.

> /*                                                                           
>  * Note the difference between vcpu_must_have_<feature>() and                 
>                                                          
>  * host_and_vcpu_must_have(<feature>): The latter needs to be used when       
>                                                                   
>  * emulation code is using the same instruction class for carrying out        
>                                                                   
>  * the actual operation.                                                      
>              
>  */

This comment sits inside #ifdef __XEN__.

> We are using SSE and SSE2 instructions for carrying out that emulation,
> so should still be using the host_and_vcpu check.

Xen only running on x86-64, and x86-64 taking SSE and SSE2 as
given (all math operations other than 80-bit ones are using these
instead of the x87), there's no need to look at any flags here
(and as you can see from the patch, the flags also already haven't
been used anywhere outside the insn emulator, despite us using
SSE2 insns, e.g. MOVNTI in the page copying and clearing code).

> 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.

Jan


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