Message ID | 20231124055330.138870-7-weijiang.yang@intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce62:0:b0:403:3b70:6f57 with SMTP id o2csp984296vqx; Fri, 24 Nov 2023 00:00:47 -0800 (PST) X-Google-Smtp-Source: AGHT+IHizFyvjA0CNn+MBoViclUmzElQiGRJ4lKTgrWlaiECW72awZb8KiAB62DYQ6mnFXw6ZXQ3 X-Received: by 2002:a05:6870:f78f:b0:1f9:574c:b5ec with SMTP id fs15-20020a056870f78f00b001f9574cb5ecmr2570945oab.27.1700812847757; Fri, 24 Nov 2023 00:00:47 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700812847; cv=none; d=google.com; s=arc-20160816; b=jLMc1/lz7p4iTjd1diV+0eSmR73Y/Z6yjVOxj09FhuO75I+OY8Bti9LKrUketFKjvp aNnL6RQE9tbyb6WTDHIlW9hSvalwRZxaEofk+t/agzB8W8vNe4vOjyqv12G9PzponbWS nl+d8Zpfb3Y5HSvkLDO0GFgkSrIqFejAkCyO1KXYRYaO//Ec0XFVZrvUkKMM73UtoJIa bieEsrDFPIJV3q22aclcVQLBK75SlWFq5OSYKH0lrFpGZfDLk8f60LEe72d/1SNrtMdv KJZg8plR4PFwzSm5/0vFbiu0mVTiasmA3x4NFATbHvVmbWlQOfkbxS5W5YCYUX3ugY+6 bpww== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=Vn7H1cIFu2QJ4soBoSUN2TWXzY+rODFniUGSjnVwhnQ=; fh=uy0EBGgYIm8+MgsxUvKHUXUo3s9z4H9hdLwRv8YoeJU=; b=tDsCptCG6LlxGrEtNJi/qLlj83CeA13Dzob+w91JEtYkfbFhYCvJXGpB44OoD0k5L0 4Bl6Y8Fq0twJAgTYtugodaTc5sJvDlrdPtdQwYbH6b6uNpr3VVB6dEkcb2+35kvkpEOe V0Mrxg6WAF8TpXAKIUcADQA5yZY1KDG2sgyovJkREuJPhp164guFzbTywlWoZd2CSNvS HMGtLFLpjZJfd6r07wcnNP3i35DsvfqotoE0Maq8lbGUXozIUN9mQSvHKQ8v6mNKNuJF UQxWCfeeUDObVnw7lKjncUlfNNG1F3+YzKkNi0cLzQ8UT84OpsZzVOVfrbXAZu35pai6 /G5Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=Ak4Ldskp; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from morse.vger.email (morse.vger.email. [23.128.96.31]) by mx.google.com with ESMTPS id t10-20020a9d590a000000b006d7fd488ed3si923801oth.179.2023.11.24.00.00.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Nov 2023 00:00:47 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=Ak4Ldskp; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by morse.vger.email (Postfix) with ESMTP id EB264809E8BF; Fri, 24 Nov 2023 00:00:04 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at morse.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345163AbjKXH7b (ORCPT <rfc822;jaysivo@gmail.com> + 29 others); Fri, 24 Nov 2023 02:59:31 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46422 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232291AbjKXH6h (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 24 Nov 2023 02:58:37 -0500 Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.100]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 06AA610C8; Thu, 23 Nov 2023 23:58:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1700812723; x=1732348723; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=1vwvfuaBTc7h5kz+PPAFiMP1RdwSkDr6j//+KudefV4=; b=Ak4Ldskpm9w5KOqmB3eQTagizetRVcHNTf7wKyMhhXPrjbizl1G3+CfQ PyboXnzwUbVhcj+Au5vjrYacYIKuocSGvqRj9EVK0MffU/IBr0BX10ZSg ZF0AUmNXKQMA7l+c9npO6UNvIgWllqTAB67eCpVdt1/Mp+IN2aFfpaYxr HIdAT+1Xv1mJ7whZcHeqTCe6Rw7aERDDJePMkmcTH9ZV9ljhpVtJxQRYB cZAGvGR7NvvpC83BFPdHnNd06I8XtUekkFhUgu0uHmdygtJUUmudY7yKB 3OETi3XuqXvc7wPhMBSAP/J4Xdh42SQRutzyInvDy20EKQnhtXsZPvvou A==; X-IronPort-AV: E=McAfee;i="6600,9927,10902"; a="458872298" X-IronPort-AV: E=Sophos;i="6.04,223,1695711600"; d="scan'208";a="458872298" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Nov 2023 23:58:37 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10902"; a="833629807" X-IronPort-AV: E=Sophos;i="6.04,223,1695711600"; d="scan'208";a="833629807" Received: from unknown (HELO embargo.jf.intel.com) ([10.165.9.183]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Nov 2023 23:58:36 -0800 From: Yang Weijiang <weijiang.yang@intel.com> To: seanjc@google.com, pbonzini@redhat.com, dave.hansen@intel.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org Cc: peterz@infradead.org, chao.gao@intel.com, rick.p.edgecombe@intel.com, mlevitsk@redhat.com, john.allen@amd.com, weijiang.yang@intel.com Subject: [PATCH v7 06/26] x86/fpu/xstate: Create guest fpstate with guest specific config Date: Fri, 24 Nov 2023 00:53:10 -0500 Message-Id: <20231124055330.138870-7-weijiang.yang@intel.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20231124055330.138870-1-weijiang.yang@intel.com> References: <20231124055330.138870-1-weijiang.yang@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Fri, 24 Nov 2023 00:00:06 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783431532614068563 X-GMAIL-MSGID: 1783431532614068563 |
Series |
Enable CET Virtualization
|
|
Commit Message
Yang, Weijiang
Nov. 24, 2023, 5:53 a.m. UTC
Use fpu_guest_cfg to calculate guest fpstate settings, open code for
__fpstate_reset() to avoid using kernel FPU config.
Below configuration steps are currently enforced to get guest fpstate:
1) Kernel sets up guest FPU settings in fpu__init_system_xstate().
2) User space sets vCPU thread group xstate permits via arch_prctl().
3) User space creates guest fpstate via __fpu_alloc_init_guest_fpstate()
for vcpu thread.
4) User space enables guest dynamic xfeatures and re-allocate guest
fpstate.
By adding kernel dynamic xfeatures in above #1 and #2, guest xstate area
size is expanded to hold (fpu_kernel_cfg.default_features | kernel dynamic
xfeatures | user dynamic xfeatures), then host xsaves/xrstors can operate
for all guest xfeatures.
The user_* fields remain unchanged for compatibility with KVM uAPIs.
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
arch/x86/kernel/fpu/core.c | 48 ++++++++++++++++++++++++++++--------
arch/x86/kernel/fpu/xstate.c | 2 +-
arch/x86/kernel/fpu/xstate.h | 1 +
3 files changed, 40 insertions(+), 11 deletions(-)
Comments
On Fri, 2023-11-24 at 00:53 -0500, Yang Weijiang wrote: > +static struct fpstate *__fpu_alloc_init_guest_fpstate(struct > fpu_guest *gfpu) > { > + bool compacted = cpu_feature_enabled(X86_FEATURE_XCOMPACTED); > + unsigned int gfpstate_size, size; > struct fpstate *fpstate; > - unsigned int size; > > - size = fpu_user_cfg.default_size + ALIGN(offsetof(struct > fpstate, regs), 64); > + /* > + * fpu_guest_cfg.default_features includes all enabled > xfeatures > + * except the user dynamic xfeatures. If the user dynamic > xfeatures > + * are enabled, the guest fpstate will be re-allocated to > hold all > + * guest enabled xfeatures, so omit user dynamic xfeatures > here. > + */ > + gfpstate_size = > xstate_calculate_size(fpu_guest_cfg.default_features, > + compacted); Why not fpu_guest_cfg.default_size here? > + > + size = gfpstate_size + ALIGN(offsetof(struct fpstate, regs), > 64);
On 11/28/2023 11:19 PM, Edgecombe, Rick P wrote: > On Fri, 2023-11-24 at 00:53 -0500, Yang Weijiang wrote: >> +static struct fpstate *__fpu_alloc_init_guest_fpstate(struct >> fpu_guest *gfpu) >> { >> + bool compacted = cpu_feature_enabled(X86_FEATURE_XCOMPACTED); >> + unsigned int gfpstate_size, size; >> struct fpstate *fpstate; >> - unsigned int size; >> >> - size = fpu_user_cfg.default_size + ALIGN(offsetof(struct >> fpstate, regs), 64); >> + /* >> + * fpu_guest_cfg.default_features includes all enabled >> xfeatures >> + * except the user dynamic xfeatures. If the user dynamic >> xfeatures >> + * are enabled, the guest fpstate will be re-allocated to >> hold all >> + * guest enabled xfeatures, so omit user dynamic xfeatures >> here. >> + */ >> + gfpstate_size = >> xstate_calculate_size(fpu_guest_cfg.default_features, >> + compacted); > Why not fpu_guest_cfg.default_size here? Nice catch! I should use fpu_guest_cfg.default_size directly instead of re-calculating it with the same manner. Thanks! > >> + >> + size = gfpstate_size + ALIGN(offsetof(struct fpstate, regs), >> 64);
On Fri, 2023-11-24 at 00:53 -0500, Yang Weijiang wrote: > Use fpu_guest_cfg to calculate guest fpstate settings, open code for > __fpstate_reset() to avoid using kernel FPU config. > > Below configuration steps are currently enforced to get guest fpstate: > 1) Kernel sets up guest FPU settings in fpu__init_system_xstate(). > 2) User space sets vCPU thread group xstate permits via arch_prctl(). > 3) User space creates guest fpstate via __fpu_alloc_init_guest_fpstate() > for vcpu thread. > 4) User space enables guest dynamic xfeatures and re-allocate guest > fpstate. > > By adding kernel dynamic xfeatures in above #1 and #2, guest xstate area > size is expanded to hold (fpu_kernel_cfg.default_features | kernel dynamic > xfeatures | user dynamic xfeatures), then host xsaves/xrstors can operate > for all guest xfeatures. > > The user_* fields remain unchanged for compatibility with KVM uAPIs. > > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> > --- > arch/x86/kernel/fpu/core.c | 48 ++++++++++++++++++++++++++++-------- > arch/x86/kernel/fpu/xstate.c | 2 +- > arch/x86/kernel/fpu/xstate.h | 1 + > 3 files changed, 40 insertions(+), 11 deletions(-) > > diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c > index 516af626bf6a..985eaf8b55e0 100644 > --- a/arch/x86/kernel/fpu/core.c > +++ b/arch/x86/kernel/fpu/core.c > @@ -194,8 +194,6 @@ void fpu_reset_from_exception_fixup(void) > } > > #if IS_ENABLED(CONFIG_KVM) > -static void __fpstate_reset(struct fpstate *fpstate, u64 xfd); > - > static void fpu_init_guest_permissions(struct fpu_guest *gfpu) > { > struct fpu_state_perm *fpuperm; > @@ -216,25 +214,55 @@ static void fpu_init_guest_permissions(struct fpu_guest *gfpu) > gfpu->perm = perm & ~FPU_GUEST_PERM_LOCKED; > } > > -bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu) > +static struct fpstate *__fpu_alloc_init_guest_fpstate(struct fpu_guest *gfpu) > { > + bool compacted = cpu_feature_enabled(X86_FEATURE_XCOMPACTED); > + unsigned int gfpstate_size, size; > struct fpstate *fpstate; > - unsigned int size; > > - size = fpu_user_cfg.default_size + ALIGN(offsetof(struct fpstate, regs), 64); > + /* > + * fpu_guest_cfg.default_features includes all enabled xfeatures > + * except the user dynamic xfeatures. If the user dynamic xfeatures > + * are enabled, the guest fpstate will be re-allocated to hold all > + * guest enabled xfeatures, so omit user dynamic xfeatures here. > + */ This is a very good comment to have, although I don't think there is any way to ensure that the whole thing is not utterly confusing..... > + gfpstate_size = xstate_calculate_size(fpu_guest_cfg.default_features, > + compacted); > + > + size = gfpstate_size + ALIGN(offsetof(struct fpstate, regs), 64); > + > fpstate = vzalloc(size); > if (!fpstate) > - return false; > + return NULL; > + /* > + * Initialize sizes and feature masks, use fpu_user_cfg.* > + * for user_* settings for compatibility of exiting uAPIs. > + */ > + fpstate->size = gfpstate_size; > + fpstate->xfeatures = fpu_guest_cfg.default_features; > + 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; --- 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? Best regards, Maxim Levitsky > + fpstate->xfd = 0; > > - /* Leave xfd to 0 (the reset value defined by spec) */ > - __fpstate_reset(fpstate, 0); > fpstate_init_user(fpstate); > fpstate->is_valloc = true; > fpstate->is_guest = true; > > gfpu->fpstate = fpstate; > - gfpu->xfeatures = fpu_user_cfg.default_features; > - gfpu->perm = fpu_user_cfg.default_features; > + gfpu->xfeatures = fpu_guest_cfg.default_features; > + gfpu->perm = fpu_guest_cfg.default_features; > + > + return fpstate; > +} > + > +bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu) > +{ > + struct fpstate *fpstate; > + > + fpstate = __fpu_alloc_init_guest_fpstate(gfpu); > + > + if (!fpstate) > + return false; > > /* > * KVM sets the FP+SSE bits in the XSAVE header when copying FPU state > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c > index aa8f8595cd41..253944cb2298 100644 > --- a/arch/x86/kernel/fpu/xstate.c > +++ b/arch/x86/kernel/fpu/xstate.c > @@ -559,7 +559,7 @@ static bool __init check_xstate_against_struct(int nr) > return true; > } > > -static unsigned int xstate_calculate_size(u64 xfeatures, bool compacted) > +unsigned int xstate_calculate_size(u64 xfeatures, bool compacted) > { > unsigned int topmost = fls64(xfeatures) - 1; > unsigned int offset = xstate_offsets[topmost]; > diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h > index 3518fb26d06b..c032acb56306 100644 > --- a/arch/x86/kernel/fpu/xstate.h > +++ b/arch/x86/kernel/fpu/xstate.h > @@ -55,6 +55,7 @@ extern void fpu__init_cpu_xstate(void); > extern void fpu__init_system_xstate(unsigned int legacy_size); > > extern void *get_xsave_addr(struct xregs_state *xsave, int xfeature_nr); > +extern unsigned int xstate_calculate_size(u64 xfeatures, bool compacted); > > static inline u64 xfeatures_mask_supervisor(void) > {
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 :-)
On Fri, 2023-12-01 at 16:36 +0800, Yang, Weijiang wrote: > 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? I don't know if we want to add these comments to the source - I made them up for myself/you to understand the subtle differences between each of these variables. There is some documentation on the struct fields, it is reasonable, but I do think that it will help a lot to add documentation to each of fpu_kernel_cfg, fpu_user_cfg and fpu_guest_cfg. > > > --- > > > > > > 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. It's all up to you to decide. The thing is one big mess, IMHO no comment can really make it understandable without hours of research. However as usual, the more comments the better, comments do help. Best regards, Maxim Levitsky > > But I really like your ideas of making things clean and tidy :-) > >
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c index 516af626bf6a..985eaf8b55e0 100644 --- a/arch/x86/kernel/fpu/core.c +++ b/arch/x86/kernel/fpu/core.c @@ -194,8 +194,6 @@ void fpu_reset_from_exception_fixup(void) } #if IS_ENABLED(CONFIG_KVM) -static void __fpstate_reset(struct fpstate *fpstate, u64 xfd); - static void fpu_init_guest_permissions(struct fpu_guest *gfpu) { struct fpu_state_perm *fpuperm; @@ -216,25 +214,55 @@ static void fpu_init_guest_permissions(struct fpu_guest *gfpu) gfpu->perm = perm & ~FPU_GUEST_PERM_LOCKED; } -bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu) +static struct fpstate *__fpu_alloc_init_guest_fpstate(struct fpu_guest *gfpu) { + bool compacted = cpu_feature_enabled(X86_FEATURE_XCOMPACTED); + unsigned int gfpstate_size, size; struct fpstate *fpstate; - unsigned int size; - size = fpu_user_cfg.default_size + ALIGN(offsetof(struct fpstate, regs), 64); + /* + * fpu_guest_cfg.default_features includes all enabled xfeatures + * except the user dynamic xfeatures. If the user dynamic xfeatures + * are enabled, the guest fpstate will be re-allocated to hold all + * guest enabled xfeatures, so omit user dynamic xfeatures here. + */ + gfpstate_size = xstate_calculate_size(fpu_guest_cfg.default_features, + compacted); + + size = gfpstate_size + ALIGN(offsetof(struct fpstate, regs), 64); + fpstate = vzalloc(size); if (!fpstate) - return false; + return NULL; + /* + * Initialize sizes and feature masks, use fpu_user_cfg.* + * for user_* settings for compatibility of exiting uAPIs. + */ + fpstate->size = gfpstate_size; + fpstate->xfeatures = fpu_guest_cfg.default_features; + fpstate->user_size = fpu_user_cfg.default_size; + fpstate->user_xfeatures = fpu_user_cfg.default_features; + fpstate->xfd = 0; - /* Leave xfd to 0 (the reset value defined by spec) */ - __fpstate_reset(fpstate, 0); fpstate_init_user(fpstate); fpstate->is_valloc = true; fpstate->is_guest = true; gfpu->fpstate = fpstate; - gfpu->xfeatures = fpu_user_cfg.default_features; - gfpu->perm = fpu_user_cfg.default_features; + gfpu->xfeatures = fpu_guest_cfg.default_features; + gfpu->perm = fpu_guest_cfg.default_features; + + return fpstate; +} + +bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu) +{ + struct fpstate *fpstate; + + fpstate = __fpu_alloc_init_guest_fpstate(gfpu); + + if (!fpstate) + return false; /* * KVM sets the FP+SSE bits in the XSAVE header when copying FPU state diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index aa8f8595cd41..253944cb2298 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -559,7 +559,7 @@ static bool __init check_xstate_against_struct(int nr) return true; } -static unsigned int xstate_calculate_size(u64 xfeatures, bool compacted) +unsigned int xstate_calculate_size(u64 xfeatures, bool compacted) { unsigned int topmost = fls64(xfeatures) - 1; unsigned int offset = xstate_offsets[topmost]; diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h index 3518fb26d06b..c032acb56306 100644 --- a/arch/x86/kernel/fpu/xstate.h +++ b/arch/x86/kernel/fpu/xstate.h @@ -55,6 +55,7 @@ extern void fpu__init_cpu_xstate(void); extern void fpu__init_system_xstate(unsigned int legacy_size); extern void *get_xsave_addr(struct xregs_state *xsave, int xfeature_nr); +extern unsigned int xstate_calculate_size(u64 xfeatures, bool compacted); static inline u64 xfeatures_mask_supervisor(void) {