Messages in this thread | | | Date | Wed, 9 Mar 2022 16:24:59 -0800 | From | Dave Hansen <> | Subject | Re: [PATCH v2 1/2] x86/fpu: Add a helper to prepare AMX state for low-power CPU idle |
| |
On 3/9/22 15:12, Chang S. Bae wrote: > On 3/9/2022 2:46 PM, Dave Hansen wrote: >> On 3/9/22 14:34, Chang S. Bae wrote: >>> +/* >>> + * Initialize register state that may prevent from entering >>> low-power idle. >>> + * This function will be invoked from the cpuidle driver only when >>> needed. >>> + */ >>> +void fpu_idle_fpregs(void) >>> +{ >>> + if (!fpu_state_size_dynamic()) >>> + return; >> >> Is this check just an optimization? > > No. 0day reported the splat [3] with the earlier code in v1 [1]: > > if (fpu_state_size_dynamic() && (xfeatures_in_use() & > XFEATURE_MASK_XTILE)) { ... } > > It looks like GCC-9 reordered to hit XGETBV without checking > fpu_state_size_dynamic(). So this line was separated to avoid that.
I assume that splat is because 0day found a CPU which doesn't support XGETBV1. Since fpu_state_size_dynamic() only ever returns true on XGETBV1 systems so it works as a proxy for checking XGETBV1 support.
Right?
If so, then fpu_state_size_dynamic() is a *bit* of an oblique way to check for XGETBV1 support.
Why don't we do a good old:
cpu_feature_enabled(X86_FEATURE_XGETBV1)
check?
Also, did we get the asm constraints wrong on xgetbv()? Surely we shouldn't be allowing the compiler to reorder it. Do we need a "memory" constraint?
| |