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 14:57:02 +0100
Message-ID: <561E5F2E.9020604@citrix.com>
References: <561E734D02000078000AAEC5@prv-mh.provo.novell.com>
	<561E74CC02000078000AAEE0@prv-mh.provo.novell.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.bemta3.messagelabs.com ([195.245.230.39])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <prvs=7227d41b3=Andrew.Cooper3@citrix.com>)
	id 1ZmMYG-0000uA-Ll
	for xen-devel@lists.xenproject.org; Wed, 14 Oct 2015 13:57:08 +0000
In-Reply-To: <561E74CC02000078000AAEE0@prv-mh.provo.novell.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: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