| Message ID | 20230914032334.75212-3-weijiang.yang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce CET supervisor xstate support | expand |
On Wed, Sep 13, 2023, Yang Weijiang wrote: > Fix guest xsave area allocation size from fpu_user_cfg.default_size to > fpu_kernel_cfg.default_size so that the xsave area size is consistent > with fpstate->size set in __fpstate_reset(). > > With the fix, guest fpstate size is sufficient for KVM supported guest > xfeatures. > > Fixes: 69f6ed1d14c6 ("x86/fpu: Provide infrastructure for KVM FPU cleanup"); > Cc: Thomas Gleixner <tglx@linutronix.de> > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> > --- > arch/x86/kernel/fpu/core.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c > index a86d37052a64..a42d8ad26ce6 100644 > --- a/arch/x86/kernel/fpu/core.c > +++ b/arch/x86/kernel/fpu/core.c > @@ -220,7 +220,9 @@ bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu) > struct fpstate *fpstate; > unsigned int size; > > - size = fpu_user_cfg.default_size + ALIGN(offsetof(struct fpstate, regs), 64); > + size = fpu_kernel_cfg.default_size + > + ALIGN(offsetof(struct fpstate, regs), 64); This looks sketchy and incomplete. I haven't looked at the gory details of fpu_user_cfg vs. fpu_kernel_cfg, but the rest of this function uses fpu_user_cfg, including a check on fpu_user_cfg.default_size. That makes me think that changing just the allocation size isn't quite right. /* 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; /* * KVM sets the FP+SSE bits in the XSAVE header when copying FPU state * to userspace, even when XSAVE is unsupported, so that restoring FPU * state on a different CPU that does support XSAVE can cleanly load * the incoming state using its natural XSAVE. In other words, KVM's * uABI size may be larger than this host's default size. Conversely, * the default size should never be larger than KVM's base uABI size; * all features that can expand the uABI size must be opt-in. */ gfpu->uabi_size = sizeof(struct kvm_xsave); if (WARN_ON_ONCE(fpu_user_cfg.default_size > gfpu->uabi_size)) gfpu->uabi_size = fpu_user_cfg.default_size;
On Fri, Oct 20, 2023, Sean Christopherson wrote: > On Wed, Sep 13, 2023, Yang Weijiang wrote: > > Fix guest xsave area allocation size from fpu_user_cfg.default_size to > > fpu_kernel_cfg.default_size so that the xsave area size is consistent > > with fpstate->size set in __fpstate_reset(). > > > > With the fix, guest fpstate size is sufficient for KVM supported guest > > xfeatures. > > > > Fixes: 69f6ed1d14c6 ("x86/fpu: Provide infrastructure for KVM FPU cleanup"); > > Cc: Thomas Gleixner <tglx@linutronix.de> > > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> > > --- > > arch/x86/kernel/fpu/core.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c > > index a86d37052a64..a42d8ad26ce6 100644 > > --- a/arch/x86/kernel/fpu/core.c > > +++ b/arch/x86/kernel/fpu/core.c > > @@ -220,7 +220,9 @@ bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu) > > struct fpstate *fpstate; > > unsigned int size; > > > > - size = fpu_user_cfg.default_size + ALIGN(offsetof(struct fpstate, regs), 64); > > + size = fpu_kernel_cfg.default_size + > > + ALIGN(offsetof(struct fpstate, regs), 64); > > This looks sketchy and incomplete. I haven't looked at the gory details of > fpu_user_cfg vs. fpu_kernel_cfg, but the rest of this function uses fpu_user_cfg, > including a check on fpu_user_cfg.default_size. That makes me think that changing > just the allocation size isn't quite right. Shoot, I didn't realize the CET virtualization series included this a day later. I'll respond there.
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c index a86d37052a64..a42d8ad26ce6 100644 --- a/arch/x86/kernel/fpu/core.c +++ b/arch/x86/kernel/fpu/core.c @@ -220,7 +220,9 @@ bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu) struct fpstate *fpstate; unsigned int size; - size = fpu_user_cfg.default_size + ALIGN(offsetof(struct fpstate, regs), 64); + size = fpu_kernel_cfg.default_size + + ALIGN(offsetof(struct fpstate, regs), 64); + fpstate = vzalloc(size); if (!fpstate) return false;
Fix guest xsave area allocation size from fpu_user_cfg.default_size to fpu_kernel_cfg.default_size so that the xsave area size is consistent with fpstate->size set in __fpstate_reset(). With the fix, guest fpstate size is sufficient for KVM supported guest xfeatures. Fixes: 69f6ed1d14c6 ("x86/fpu: Provide infrastructure for KVM FPU cleanup"); Cc: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> --- arch/x86/kernel/fpu/core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)