[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c179a5f4fba9086f9d35e56ec16423558cea5b8b.camel@intel.com>
Date: Tue, 30 Jan 2024 01:38:45 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "Yang, Weijiang" <weijiang.yang@...el.com>, "Hansen, Dave"
<dave.hansen@...el.com>, "seanjc@...gle.com" <seanjc@...gle.com>,
"x86@...nel.org" <x86@...nel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "yuan.yao@...ux.intel.com"
<yuan.yao@...ux.intel.com>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"pbonzini@...hat.com" <pbonzini@...hat.com>
CC: "john.allen@....com" <john.allen@....com>, "peterz@...radead.org"
<peterz@...radead.org>, "Gao, Chao" <chao.gao@...el.com>,
"mlevitsk@...hat.com" <mlevitsk@...hat.com>
Subject: Re: [PATCH v9 06/27] x86/fpu/xstate: Create guest fpstate with guest
specific config
On Tue, 2024-01-23 at 18:41 -0800, Yang Weijiang wrote:
> -bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu)
> +static struct fpstate *__fpu_alloc_init_guest_fpstate(struct
> fpu_guest *gfpu)
> {
> struct fpstate *fpstate;
> unsigned int size;
>
> - size = fpu_user_cfg.default_size + ALIGN(offsetof(struct
> fpstate, regs), 64);
> + /*
> + * fpu_guest_cfg.default_size is initialized to hold 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.
> + */
> + size = fpu_guest_cfg.default_size +
> + ALIGN(offsetof(struct fpstate, regs), 64);
Minor, I'm not sure the extra char warrants changing it to a wrapped
line, but that's just my personal opinion.
> +
> 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 = fpu_guest_cfg.default_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);
The above two statements could be just one line and still even fit
under 80 chars.
All the same,
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@...el.com>
> +
> + if (!fpstate)
> + return false;
>
Powered by blists - more mailing lists