Re: [PATCH v7 06/26] x86/fpu/xstate: Create guest fpstate with guest specific config

[Yang, Weijiang]

On 12/1/2023 1:36 AM, Maxim Levitsky wrote:

[...]

> > +	fpstate->user_size	= fpu_user_cfg.default_size;
> > +	fpstate->user_xfeatures	= fpu_user_cfg.default_features;
> The whole thing makes my head spin like the good old CD/DVD writers used to ....
>
> So just to summarize this is what we have:
>
>
> KERNEL FPU CONFIG
>
> /*
>     all known and CPU supported user and supervisor features except
>     - "dynamic" kernel features" (CET_S)
>     - "independent" kernel features (XFEATURE_LBR)
> */
> fpu_kernel_cfg.max_features;
>
> /*
>     all known and CPU supported user and supervisor features except
>      - "dynamic" kernel features" (CET_S)
>      - "independent" kernel features (arch LBRs)
>      - "dynamic" userspace features (AMX state)
> */
> fpu_kernel_cfg.default_features;
>
>
> // size of compacted buffer with 'fpu_kernel_cfg.max_features'
> fpu_kernel_cfg.max_size;
>
>
> // size of compacted buffer with 'fpu_kernel_cfg.default_features'
> fpu_kernel_cfg.default_size;
>
>
> USER FPU CONFIG
>
> /*
>     all known and CPU supported user features
> */
> fpu_user_cfg.max_features;
>
> /*
>     all known and CPU supported user features except
>     - "dynamic" userspace features (AMX state)
> */
> fpu_user_cfg.default_features;
>
> // size of non compacted buffer with 'fpu_user_cfg.max_features'
> fpu_user_cfg.max_size;
>
> // size of non compacted buffer with 'fpu_user_cfg.default_features'
> fpu_user_cfg.default_size;
>
>
> GUEST FPU CONFIG
> /*
>     all known and CPU supported user and supervisor features except
>     - "independent" kernel features (XFEATURE_LBR)
> */
> fpu_guest_cfg.max_features;
>
> /*
>     all known and CPU supported user and supervisor features except
>      - "independent" kernel features (arch LBRs)
>      - "dynamic" userspace features (AMX state)
> */
> fpu_guest_cfg.default_features;
>
> // size of compacted buffer with 'fpu_guest_cfg.max_features'
> fpu_guest_cfg.max_size;
>
> // size of compacted buffer with 'fpu_guest_cfg.default_features'
> fpu_guest_cfg.default_size;

Good suggestion! Thanks!
how about adding them in patch 5 to make the summaries manifested?

> ---
>
>
> So in essence, guest FPU config is guest kernel fpu config and that is why
> 'fpu_user_cfg.default_size' had to be used above.
>
> How about that we have fpu_guest_kernel_config and fpu_guest_user_config instead
> to make the whole horrible thing maybe even more complicated but at least a bit more orthogonal?

I think it becomes necessary when there were more guest user/kernel xfeaures requiring
special handling like CET-S MSRs, then it looks much reasonable to split guest config into two,
but now we only have one single outstanding xfeature for guest. IMHO, existing definitions still
work with a few comments.

But I really like your ideas of making things clean and tidy :-)