Re: [OPTIONAL/RFC v2 36/39] x86/fpu: Add helper for initing features

[Edgecombe, Rick P]

On Mon, 2022-10-03 at 12:07 -0700, Chang S. Bae wrote:
> > +/*
> > + * Given the xsave area and a state inside, this function
> > + * initializes an xfeature in the buffer.
> 
> But, this function sets XSTATE_BV bits in the buffer. That does not 
> *initialize* the state, right?

No, it doesn't actually write out the init state to the buffer.

> 
> > + *
> > + * get_xsave_addr() will return NULL if the feature bit is
> > + * not present in the header. This function will make it so
> > + * the xfeature buffer address is ready to be retrieved by
> > + * get_xsave_addr().
> 
> Looks like this is used in the next patch to help ptracer().
> 
> We have the state copy function -- copy_uabi_to_xstate() that
> retrieves 
> the address using __raw_xsave_addr() instead of get_xsave_addr(),
> copies 
> the state, and then updates XSTATE_BV.
> 
> __raw_xsave_addr() also ensures whether the state is in the
> compacted 
> format or not. I think you can use it.
> 
> Also, I'm curious about the reason why you want to update XSTATE_BV 
> first with this new helper.
> 
> Overall, I'm not sure these new helpers are necessary.

Thomas had experimented with this optimization where init state
features weren't saved:
https://lore.kernel.org/lkml/20220404103741.809025935@xxxxxxxxxxxxx/

It made me think non-fpu code should not assume things about the state
of the buffer, as FPU code might have to move things when initing them.
So the operation is worth centralizing in a helper. I think you are
right, today it is not doing much and could be open coded. I guess the
question is, should it be open coded or centralized? I'm fine either
way.