[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 1/4] x86: remove unused x87 remnants of 32-bit days



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@xxxxxxxx>

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, 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

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.