On Mon, Jul 20, 2020 at 06:50:51AM -0700, kan.li...@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.
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!
Reviewed-by: Cyrill Gorcunov <gorcu...@gmail.com>
