Messages in this thread | | | Date | Wed, 18 Aug 2021 13:33:27 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH v9 10/26] x86/fpu/xstate: Update the XSTATE buffer address finder to support dynamic states |
| |
On Fri, Jul 30, 2021 at 07:59:41AM -0700, Chang S. Bae wrote: > __raw_xsave_addr() returns the requested component's pointer in an XSTATE > buffer, by simply looking up the offset table. The offset used to be fixed, > but, with dynamic user states, it becomes variable. > > get_xstate_size() has a routine to find an offset at runtime. Refactor to > use it for the address finder. > > Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com> > Reviewed-by: Len Brown <len.brown@intel.com> > Cc: x86@kernel.org > Cc: linux-kernel@vger.kernel.org > --- > Changes from v5: > * Updated for future proofed __raw_xsave_addr(). > > Changes from v3: > * Added the function description in the kernel-doc style. (Borislav Petkov) > * Removed 'no functional change' in the changelog. (Borislav Petkov) > --- > arch/x86/kernel/fpu/xstate.c | 78 ++++++++++++++++++++++++------------ > 1 file changed, 53 insertions(+), 25 deletions(-) > > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c > index 26f6d5e0f1ed..98ab10e4da3b 100644 > --- a/arch/x86/kernel/fpu/xstate.c > +++ b/arch/x86/kernel/fpu/xstate.c > @@ -182,6 +182,38 @@ static bool xfeature_is_supervisor(int xfeature_nr) > return ecx & 1; > } > > +/** > + * get_xstate_comp_offset - Find the feature's offset in the compacted > + * format. > + * @mask: This bitmap tells which components reserved in the format.
There's that "reserved" confusion thing. Rewrite pls.
> + * @feature_nr: The feature number > + * > + * Returns: The offset value > + */ > +static unsigned int get_xstate_comp_offset(u64 mask, int feature_nr) > +{ > + u64 xmask = BIT_ULL(feature_nr + 1) - 1; > + unsigned int next_offset, offset = 0; > + int i; > + > + if ((xfeatures_mask_all & xmask) == (mask & xmask)) > + return xstate_comp_offsets[feature_nr]; > + > + /* > + * With the given mask, no relevant size is found. Calculate it by > + * summing up each state size. > + */ > + for (next_offset = FXSAVE_SIZE + XSAVE_HDR_SIZE, i = FIRST_EXTENDED_XFEATURE; > + i <= feature_nr; i++) { > + if (!(mask & BIT_ULL(i))) > + continue; > + > + offset = xstate_aligns[i] ? ALIGN(next_offset, 64) : next_offset; > + next_offset += xstate_sizes[i];
Why is this more complex than it has to be?
IOW, why can't you simply do:
offset = FXSAVE_SIZE + XSAVE_HDR_SIZE; for (i = FIRST_EXTENDED_XFEATURE; i <= feature_nr; i++) { if (!(mask & BIT_ULL(i))) continue;
if (xstate_aligns[i]) offset = ALIGN(offset, 64);
offset += xstate_sizes[i]; } return offset;
like it was before?
-- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |