lkml.org 
[lkml]   [2020]   [Jul]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] x86/fpu/xstate: Fix an xstate size check warning
From
Date


On 7/20/2020 1:33 PM, Cyrill Gorcunov wrote:
> On Mon, Jul 20, 2020 at 06:50:51AM -0700, kan.liang@linux.intel.com wrote:
> ...
>> static unsigned int __init get_xsave_size(void)
>> {
>> unsigned int eax, ebx, ecx, edx;
>> @@ -710,7 +741,7 @@ static int __init init_xstate_size(void)
>> xsave_size = get_xsave_size();
>>
>> if (boot_cpu_has(X86_FEATURE_XSAVES))
>> - possible_xstate_size = get_xsaves_size();
>> + possible_xstate_size = get_xsaves_size_no_dynamic();
>> else
>> possible_xstate_size = xsave_size;
>
> Hi! Maybe we could enhance get_xsaves_size instead ? The get_xsaves_size is
> static and __init function (thus not a hot path) used once as far as I see.
> Say
>
> static unsigned int __init get_xsaves_size(void)
> {
> u64 mask = xfeatures_mask_dynamic();
> unsigned int eax, ebx, ecx, edx;
>
> /*
> * In case if dynamic features are present make
> * sure they are not accounted in the result since
> * the buffer should be allocated separately from
> * task->fpu.
> */
> if (mask)
> wrmsrl(MSR_IA32_XSS, xfeatures_mask_supervisor());
>
> /*
> * - CPUID function 0DH, sub-function 1:
> * EBX enumerates the size (in bytes) required by
> * the XSAVES instruction for an XSAVE area
> * containing all the state components
> * corresponding to bits currently set in
> * XCR0 | IA32_XSS.
> */
> cpuid_count(XSTATE_CPUID, 1, &eax, &ebx, &ecx, &edx);
>
> if (mask)
> wrmsrl(MSR_IA32_XSS, xfeatures_mask_supervisor() | mask);
>
> return ebx;
> }
>
> but if you expect more use of get_xsaves_size_no_dynamic() and
> get_xsaves_size() in future then sure, we need a separate function.
>

For now, I don't have more use of
get_xsaves_size_no_dynamic()/get_xsaves_size(). I don't know if anyone
else will use them later.

> The benefit from such extension is that when you read get_xsaves_size
> you'll notice the dependency on dynamic features immediaely.
>
> Though I'm fine with current patch as well, up to you. Thanks for the patch!
>

Personally, I prefer to keep the current patch because I like the name
get_xsaves_size_no_dynamic(), which explicitly tells the dynamic
features are excluded.

> Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>
>

Thanks for the review.

Kan

\
 
 \ /
  Last update: 2020-07-21 20:29    [W:0.071 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site