On 11/26/24 02:17, Chao Gao wrote:
> From: Yang Weijiang <weijiang.yang@xxxxxxxxx>
>
> 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_perm ==
> (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.
This kinda restates what the code does, but I don't think it matches the
code.
> With new guest FPU config added, there're 3 categories of FPU configs in
> kernel, the usages and key fields are recapped as below.
This changelog is pretty rough. It's got a lot of words but not much
substance.
> kernel FPU config:
> @fpu_kernel_cfg.max_features
> - all known and CPU supported user and supervisor features except
> independent kernel features
>
> @fpu_kernel_cfg.default_features
> - all known and CPU supported user and supervisor features except
> dynamic kernel features, independent kernel features and dynamic
> userspace features.
>
> @fpu_kernel_cfg.max_size
> - size of compacted buffer with 'fpu_kernel_cfg.max_features'
>
> @fpu_kernel_cfg.default_size
> - size of compacted buffer with 'fpu_kernel_cfg.default_features'
>
> user FPU config:
> @fpu_user_cfg.max_features
> - all known and CPU supported user features
>
> @fpu_user_cfg.default_features
> - all known and CPU supported user features except dynamic userspace
> features.
>
> @fpu_user_cfg.max_size
> - size of non-compacted buffer with 'fpu_user_cfg.max_features'
>
> @fpu_user_cfg.default_size
> - size of non-compacted buffer with 'fpu_user_cfg.default_features'
>
> guest FPU config:
> @fpu_guest_cfg.max_features
> - all known and CPU supported user and supervisor features except
> independent kernel features.
>
> @fpu_guest_cfg.default_features
> - all known and CPU supported user and supervisor features except
> independent kernel features and dynamic userspace features.
>
> @fpu_guest_cfg.max_size
> - size of compacted buffer with 'fpu_guest_cfg.max_features'
>
> @fpu_guest_cfg.default_size
> - size of compacted buffer with 'fpu_guest_cfg.default_features'
This looks like kerneldoc, not changelog. Are you sure you want it _here_?
> diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
> index b49e65120d34..da6583a1c0a2 100644
> --- a/arch/x86/include/asm/fpu/types.h
> +++ b/arch/x86/include/asm/fpu/types.h
> @@ -611,6 +611,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_TYPES_H */
> diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
> index 1209c7aebb21..9e2e5c46cf28 100644
> --- a/arch/x86/kernel/fpu/core.c
> +++ b/arch/x86/kernel/fpu/core.c
> @@ -33,9 +33,10 @@ 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. */
> struct fpu_state_config fpu_kernel_cfg __ro_after_init;
> struct fpu_state_config fpu_user_cfg __ro_after_init;
> +struct fpu_state_config fpu_guest_cfg __ro_after_init;
>
> /*
> * Represents the initial FPU state. It's mostly (but not completely) zeroes,
> @@ -536,8 +537,15 @@ void fpstate_reset(struct fpu *fpu)
> fpu->perm.__state_perm = fpu_kernel_cfg.default_features;
> fpu->perm.__state_size = fpu_kernel_cfg.default_size;
> fpu->perm.__user_state_size = fpu_user_cfg.default_size;
> - /* Same defaults for guests */
> - fpu->guest_perm = fpu->perm;
> +
> + /* Guest permission settings */
> + fpu->guest_perm.__state_perm = fpu_guest_cfg.default_features;
> + fpu->guest_perm.__state_size = fpu_guest_cfg.default_size;
> + /*
> + * Set guest's __user_state_size to fpu_user_cfg.default_size so that
> + * existing uAPIs can still work.
> + */
> + fpu->guest_perm.__user_state_size = fpu_user_cfg.default_size;
> }
>
> static inline void fpu_inherit_perms(struct fpu *dst_fpu)
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index c38e477e3e45..17c3255dfa41 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -686,6 +686,7 @@ static int __init init_xstate_size(void)
> {
> /* Recompute the context size for enabled features: */
> unsigned int user_size, kernel_size, kernel_default_size;
> + unsigned int guest_default_size;
> bool compacted = cpu_feature_enabled(X86_FEATURE_XCOMPACTED);
>
> /* Uncompacted user space size */
> @@ -707,13 +708,18 @@ static int __init init_xstate_size(void)
> kernel_default_size =
> xstate_calculate_size(fpu_kernel_cfg.default_features, compacted);
>
> + guest_default_size =
> + xstate_calculate_size(fpu_guest_cfg.default_features, compacted);
> +
> if (!paranoid_xstate_size_valid(kernel_size))
> return -EINVAL;
>
> fpu_kernel_cfg.max_size = kernel_size;
> fpu_user_cfg.max_size = user_size;
> + fpu_guest_cfg.max_size = kernel_size;
>
> fpu_kernel_cfg.default_size = kernel_default_size;
> + fpu_guest_cfg.default_size = guest_default_size;
> fpu_user_cfg.default_size =
> xstate_calculate_size(fpu_user_cfg.default_features, false);
>
> @@ -829,6 +835,10 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
> fpu_user_cfg.default_features = fpu_user_cfg.max_features;
> fpu_user_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC;
>
> + fpu_guest_cfg.max_features = fpu_kernel_cfg.max_features;
> + fpu_guest_cfg.default_features = fpu_guest_cfg.max_features;
> + fpu_guest_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC;
I thought this was saying above that it was _setting_ dynamic features.
Why _not_ set them by default?
