lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 21 Jul 2020 14:27:17 -0400 From: "Liang, Kan" <kan.liang@...ux.intel.com> To: Cyrill Gorcunov <gorcunov@...il.com> Cc: peterz@...radead.org, mingo@...hat.com, acme@...nel.org, tglx@...utronix.de, bp@...en8.de, x86@...nel.org, linux-kernel@...r.kernel.org, dave.hansen@...el.com, yu-cheng.yu@...el.com, bigeasy@...utronix.de, hpa@...or.com, eranian@...gle.com, ak@...ux.intel.com, chang.seok.bae@...el.com Subject: Re: [PATCH] x86/fpu/xstate: Fix an xstate size check warning On 7/20/2020 1:33 PM, Cyrill Gorcunov wrote: > On Mon, Jul 20, 2020 at 06:50:51AM -0700, kan.liang@...ux.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@...il.com> > Thanks for the review. Kan
Powered by blists - more mailing lists