Re: [PATCH v4 06/39] x86/fpu: Add helper for modifying xstate

Edgecombe, Rick P
2022-12-21T00:06:39Z

On Tue, 2022-12-20 at 13:04 +0100, Borislav Petkov wrote:
> On Fri, Dec 02, 2022 at 04:35:33PM -0800, Rick Edgecombe wrote:
> > +void fpregs_lock_and_load(void)
>
> Fun naming :)

Yea. Suggested by Thomas.

>
> > +{
> > +       /*
> > +        * fpregs_lock() only disables preemption (mostly). So
> > modifing state
>
> Unknown word [modifing] in comment.
> Suggestions: ['modifying',...

Sorry about this and the others. I get spelling errors in checkpatch,
so I must have dictionary issues or something.

>
> > +        * in an interrupt could screw up some in progress fpregs
> > operation,
> > +        * but appear to work. Warn about it.
> > +        */
> > +       WARN_ON_ONCE(!irq_fpu_usable());
> > +       WARN_ON_ONCE(current->flags & PF_KTHREAD);
> > +
> > +       fpregs_lock();
>
> So it locks them here...
>
> /me goes further into the patchset
> aha, and the counterpart of this function is fpregs_unlock() so
> everything gets sandwitched between the two.
>
> Ok, I guess.
>
> > +EXPORT_SYMBOL_GPL(fpregs_lock_and_load);
>
> Exported for KVM?
>
Yes, the KVM series needed it. Part of the reasoning here was to
provide some helpers to avoid mistakes in modifying xstate, so the
general idea was that it should be available. I suppose that series
could add the export though?