Re: [kvm PATCH v6 2/2] kvm: x86: Dynamically allocate guest_fpu

[Marc Orr <marcorr@google.com>]

> On 10/31/18 4:49 PM, Marc Orr wrote:
> > +     if (!boot_cpu_has(X86_FEATURE_FPU) || !boot_cpu_has(X86_FEATURE_FXSR)) {
> > +             printk(KERN_ERR "kvm: inadequate fpu\n");
> > +             r = -EOPNOTSUPP;
> > +             goto out;
> > +     }
>
> It would be nice to have a comment about _why_ this is inadequate.

Ack. I'll uptdate the patch.

> >       r = -ENOMEM;
> > +     x86_fpu_cache = kmem_cache_create_usercopy(
> > +                             "x86_fpu",
>
> For now, this should probably be kvm_x86_fpu since it's not used as a
> generic x86 thing, yet.
>
> Also, why is this a "usercopy"?  "fpu_kernel_xstate_size" includes (or
> will soon include) supervisor state which can never be copied to
> userspace.  If this structure is going out to userspace, that tells me
> we might instead want fpu_user_xstate_size, *or* we want the
> non-usercopy variant.

Good question. Configuring the usercopy kmem cache to restrict access
beyond fpu_user_xstate_size bytes (rather than fpu_kernel_xstate_size
bytes) from the beginning of the state field seems intuitive to me,
but I'm honestly not familiar with what user space expects KVM to
return through the ioctls. Can someone familiar with this suggest what
to do? Otherwise, I can update the patch to use the non-usercopy
variant.