linux-kernel - Re: [PATCH v8 05/26] x86/fpu/xstate: Introduce fpu_guest

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <961bb6f9-1d82-44ff-b63a-d7a07e3f6bd2@intel.com>
Date: Wed, 3 Jan 2024 17:17:18 +0800
From: "Yang, Weijiang" <weijiang.yang@...el.com>
To: Maxim Levitsky <mlevitsk@...hat.com>
CC: <linux-kernel@...r.kernel.org>, <kvm@...r.kernel.org>,
	<dave.hansen@...el.com>, <pbonzini@...hat.com>, <seanjc@...gle.com>,
	<peterz@...radead.org>, <chao.gao@...el.com>, <rick.p.edgecombe@...el.com>,
	<john.allen@....com>
Subject: Re: [PATCH v8 05/26] x86/fpu/xstate: Introduce fpu_guest_cfg for
 guest FPU configuration

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@...el.com>
>> Reviewed-by: Maxim Levitsky <mlevitsk@...hat.com>
>> ---
>>   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.