Re: [PATCH v3 08/21] x86/fpu/xstate: Define the scope of the initial xstate data

[Borislav Petkov]

On Mon, Feb 08, 2021 at 06:53:23PM +0000, Bae, Chang Seok wrote:
> Maybe, drop ‘for convenience’ from this sentence, since the buffer’s usage is
> not much relevant in this changelog.

Yes, "init_fpstate" is kinda clear what it is, from the name.

> It does functional change, but it is conditional to AMX enabling.
>
> It includes all the initial states when AMX states not enabled. But it will
> exclude the AMX state (with 8KB zeros) with the change.

Those sentences "no functional change" are supposed to mean that
the patch doesn't change anything and is only an equivalent code
transformation.

Yours does. So drop it from this one and from all the other patches as
it is causing more confusion than it is trying to dispel.

> I think they are in a different context.
> 
> The helper indicates the mask for the ‘init_fpstate’ buffer. The rest is the
> initial mask value for the per-task xstate buffer.

Wait, what?

Are you trying to tell me that that helper will return different masks
depending on xfeatures_mask_user_dynamic, which changes in its lifetime?

Then drop that helper altogether - that is more confusion and the xstate
code is already confusing enough.

> Since you suggested to introduce get_xstate_buffer_attr(), how about replacing
> what you found with something like:
> 
> get_xstate_buffer_attr(XSTATE_INIT_MASK)

I'd prefer no helper at all but only comments above the usage site.

Thx.

-- 
Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg