From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH 1/4] x86: remove unused x87 remnants of
 32-bit days
Date: Wed, 14 Oct 2015 15:05:18 +0100
Message-ID: <561E611E.9020106@citrix.com>
References: <561E734D02000078000AAEC5@prv-mh.provo.novell.com>
	<561E74CC02000078000AAEE0@prv-mh.provo.novell.com>
	<561E5F2E.9020604@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta5.messagelabs.com ([195.245.231.135])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <prvs=7227d41b3=Andrew.Cooper3@citrix.com>)
	id 1ZmMgT-0001pd-TO
	for xen-devel@lists.xenproject.org; Wed, 14 Oct 2015 14:05:38 +0000
In-Reply-To: <561E5F2E.9020604@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>, xen-devel <xen-devel@lists.xenproject.org>
Cc: Keir Fraser <keir@xen.org>
List-Id: xen-devel@lists.xenproject.org

On 14/10/15 14:57, Andrew Cooper wrote:
> On 14/10/15 14:29, Jan Beulich wrote:
>> x86-64 requires FXSR, XMM, and XMM2, so there's no point in hiding
>> respective code behind conditionals.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, with two
> suggestions.
>
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -1295,10 +1295,8 @@ void __init noreturn __start_xen(unsigne
>>  
>>      identify_cpu(&boot_cpu_data);
>>  
>> -    if ( cpu_has_fxsr )
>> -        set_in_cr4(X86_CR4_OSFXSR);
>> -    if ( cpu_has_xmm )
>> -        set_in_cr4(X86_CR4_OSXMMEXCPT);
>> +    set_in_cr4(X86_CR4_OSFXSR);
>> +    set_in_cr4(X86_CR4_OSXMMEXCPT);
> set_in_cr4() takes a mask, so could take both of these at once, to be
> slightly more efficient.
>
> If the bsp path gained an unconditional write_cr4(mmu_cr4_features) then
> it might be better to fold them into the initialiser for mmu_cr4_features.
>
>> --- a/xen/include/asm-x86/page.h
>> +++ b/xen/include/asm-x86/page.h
>> @@ -206,13 +206,10 @@ typedef struct { u64 pfn; } pagetable_t;
>>  #define pagetable_null()        pagetable_from_pfn(0)
>>  
>>  void clear_page_sse2(void *);
>> -#define clear_page(_p)      (cpu_has_xmm2 ?                             \
>> -                             clear_page_sse2((void *)(_p)) :            \
>> -                             (void)memset((void *)(_p), 0, PAGE_SIZE))
>>  void copy_page_sse2(void *, const void *);
>> -#define copy_page(_t,_f)    (cpu_has_xmm2 ?                             \
>> -                             copy_page_sse2(_t, _f) :                   \
>> -                             (void)memcpy(_t, _f, PAGE_SIZE))
>> +
>> +#define clear_page(_p)      clear_page_sse2(_p)
>> +#define copy_page(_t,_f)    copy_page_sse2(_t, _f)
> As you are changing this, an extra space following the comma.
>
> ~Andrew

A third suggestion to delete the trailing whitespace before fninit in
the following hunk.

--- a/xen/arch/x86/i387.c
+++ b/xen/arch/x86/i387.c
@@ -19,15 +19,12 @@
 
 static void fpu_init(void)
 {
-    unsigned long val;
+    uint32_t val = MXCSR_DEFAULT;
     
     asm volatile ( "fninit" );
-    if ( cpu_has_xmm )
-    {
-        /* load default value into MXCSR control/status register */
-        val = MXCSR_DEFAULT;
-        asm volatile ( "ldmxcsr %0" : : "m" (val) );
-    }
+
+    /* load default value into MXCSR control/status register */
+    asm volatile ( "ldmxcsr %0" : : "m" (val) );
 }
 

~Andrew