Re: [PATCH v8 05/26] x86/fpu/xstate: Introduce fpu_guest_cfg for guest FPU configuration

[Yang, Weijiang]

On 1/3/2024 6:32 AM, Maxim Levitsky wrote:
> On Thu, 2023-12-21 at 09:02 -0500, Yang Weijiang wrote:
> > Define new fpu_guest_cfg to hold all guest FPU settings so that it can
> > differ from generic kernel FPU settings, e.g., enabling CET supervisor
> > xstate by default for guest fpstate while it's remained disabled in
> > kernel FPU config.
> >
> > The kernel dynamic xfeatures are specifically used by guest fpstate now,
> > add the mask for guest fpstate so that guest_perm.__state_permit ==
> > (fpu_kernel_cfg.default_xfeature | XFEATURE_MASK_KERNEL_DYNAMIC). And
> > if guest fpstate is re-allocated to hold user dynamic xfeatures, the
> > resulting permissions are consumed before calculate new guest fpstate.
> >
> > Signed-off-by: Yang Weijiang <weijiang.yang@xxxxxxxxx>
> > Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
> > ---
> >   arch/x86/include/asm/fpu/types.h |  2 +-
> >   arch/x86/kernel/fpu/core.c       | 70 ++++++++++++++++++++++++++++++--
> >   arch/x86/kernel/fpu/xstate.c     | 10 +++++
> >   3 files changed, 78 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
> > index c6fd13a17205..306825ad6bc0 100644
> > --- a/arch/x86/include/asm/fpu/types.h
> > +++ b/arch/x86/include/asm/fpu/types.h
> > @@ -602,6 +602,6 @@ struct fpu_state_config {
> >   };
> >   
> >   /* FPU state configuration information */
> > -extern struct fpu_state_config fpu_kernel_cfg, fpu_user_cfg;
> > +extern struct fpu_state_config fpu_kernel_cfg, fpu_user_cfg, fpu_guest_cfg;
> >   
> >   #endif /* _ASM_X86_FPU_H */
> > diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
> > index a21a4d0ecc34..976f519721e2 100644
> > --- a/arch/x86/kernel/fpu/core.c
> > +++ b/arch/x86/kernel/fpu/core.c
> > @@ -33,10 +33,67 @@ DEFINE_STATIC_KEY_FALSE(__fpu_state_size_dynamic);
> >   DEFINE_PER_CPU(u64, xfd_state);
> >   #endif
> >   
> > -/* The FPU state configuration data for kernel and user space */
> > +/* The FPU state configuration data for kernel, user space and guest. */
> > +/*
> > + * kernel FPU config:
> > + *
> > + * all known and CPU supported user and supervisor features except
> > + *  - 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 (XFEATURE_LBR)
> > + *  - 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;
> > + */
> >   struct fpu_state_config	fpu_kernel_cfg __ro_after_init;
> > +
> > +/*
> > + * 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;
> > + */
> >   struct fpu_state_config fpu_user_cfg __ro_after_init;
> >   
> > +/*
> > + * 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 (XFEATURE_LBR)
> > + *  - 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;
> > + */
>
> IMHO this comment is too verbose. I didn't intend it to be copied verbatim,
> to the kernel, but rather to explain the meaning of the fpu context fields
> to both of us (I also keep on forgetting what each combination means...).
>
> At least this comment should not include examples because xfeatures
> are subject to change.

Yeah, I cannot find a better place to put these annotations, but feel putting them here
is not too bad :-). How about putting them in commit log?

the examples inlined are just to make it clearer for audiences how the fields are used, surely
will remove them later.