Re: [PATCH] x86: reduce magic number usage in XSAVE code (RE: [PATCH][HVM] fix XSAVE leaf 0 EBX size calculation)

[Wei Huang <wei.huang2@amd.com>]

Acked-by: Wei Huang <wei.huang2@amd.com>


On 02/08/2011 11:32 PM, Wei, Gang wrote:
> Wei Huang wrote on 2011-02-05:
>> Fixes a size calculation bug when enabled bits in XFEATURE_MASK (xcr0)
>> aren't contiguous.
>>
>> Current for_loop will stop when xcr0 feature bit is 0. But in reality,
>> the bits can be non-contiguous. One example is that LWP is bit 62 on
>> AMD platform. This patch iterates through all bits to calculate the size for enabled features.
>>
>> Signed-off-by: Wei Huang<wei.huang2@amd.com>
> ACK on the patch despite it was checked in already. BTW, I want to add a trivial patch to limit the magic number usage in XSAVE code.
>
> x86: reduce magic number usage in XSAVE code
>
> Signed-off-by: Wei Gang<gang.wei@intel.com>
>
> diff -r aeda4adecaf8 xen/arch/x86/hvm/hvm.c
> --- a/xen/arch/x86/hvm/hvm.c	Tue Feb 08 16:35:35 2011 +0000
> +++ b/xen/arch/x86/hvm/hvm.c	Thu Feb 10 19:12:29 2011 +0800
> @@ -2223,7 +2223,7 @@ void hvm_cpuid(unsigned int input, unsig
>           if ( count == 0&&  v->arch.xcr0 )
>           {
>               /* reset EBX to default value first */
> -            *ebx = 576;
> +            *ebx = XSAVE_AREA_MIN_SIZE;
>               for ( sub_leaf = 2; sub_leaf<  64; sub_leaf++ )
>               {
>                   if ( !(v->arch.xcr0&  (1ULL<<  sub_leaf)) )
> diff -r aeda4adecaf8 xen/arch/x86/i387.c
> --- a/xen/arch/x86/i387.c	Tue Feb 08 16:35:35 2011 +0000
> +++ b/xen/arch/x86/i387.c	Thu Feb 10 19:14:02 2011 +0800
> @@ -221,7 +221,6 @@ static void restore_fpu(struct vcpu *v)
>   }
>
>   #define XSTATE_CPUID 0xd
> -#define XSAVE_AREA_MIN_SIZE (512 + 64) /* FP/SSE + XSAVE.HEADER */
>
>   /*
>    * Maximum size (in byte) of the XSAVE/XRSTOR save area required by all
> diff -r aeda4adecaf8 xen/include/asm-x86/i387.h
> --- a/xen/include/asm-x86/i387.h	Tue Feb 08 16:35:35 2011 +0000
> +++ b/xen/include/asm-x86/i387.h	Thu Feb 10 19:11:37 2011 +0800
> @@ -21,13 +21,14 @@ int xsave_alloc_save_area(struct vcpu *v
>   int xsave_alloc_save_area(struct vcpu *v);
>   void xsave_free_save_area(struct vcpu *v);
>
> +#define XSAVE_AREA_MIN_SIZE (512 + 64) /* FP/SSE + XSAVE.HEADER */
>   #define XSTATE_FP       (1ULL<<  0)
>   #define XSTATE_SSE      (1ULL<<  1)
>   #define XSTATE_YMM      (1ULL<<  2)
>   #define XSTATE_LWP      (1ULL<<  62) /* AMD lightweight profiling */
>   #define XSTATE_FP_SSE   (XSTATE_FP | XSTATE_SSE)
>   #define XCNTXT_MASK     (XSTATE_FP | XSTATE_SSE | XSTATE_YMM | XSTATE_LWP)
> -#define XSTATE_YMM_OFFSET  (512 + 64)
> +#define XSTATE_YMM_OFFSET  XSAVE_AREA_MIN_SIZE
>   #define XSTATE_YMM_SIZE    256
>   #define XSAVEOPT        (1<<  0)
>
>
> Jimmy