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

From: Marc Orr <marcorr@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org, Jim Mattson <jmattson@google.com>,
	David Rientjes <rientjes@google.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	linux-mm@kvack.org, akpm@linux-foundation.org,
	rkrcmar@redhat.com, willy@infradead.org,
	sean.j.christopherson@intel.com, dave.hansen@linux.intel.com,
	Wanpeng Li <kernellwp@gmail.com>,
	Dave Hansen <dave.hansen@intel.com>
Subject: Re: [kvm PATCH v7 2/2] kvm: x86: Dynamically allocate guest_fpu
Date: Fri, 21 Dec 2018 09:30:16 -0800	[thread overview]
Message-ID: <CAA03e5FpxXXho-2XQUDbJ48a6j4-tpRqDkKPO0-QvvhCJZurdw@mail.gmail.com> (raw)
In-Reply-To: <fe4cff79-f24e-4eb0-a28c-ca770e3186df@redhat.com>

On Fri, Dec 21, 2018 at 2:28 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 06/11/18 23:20, Marc Orr wrote:
> > +     x86_fpu_cache = kmem_cache_create_usercopy(
> > +                             "x86_fpu",
> > +                             fpu_kernel_xstate_size,
>
> This unfortunately is wrong because there are other members in struct
> fpu before the fpregs_state union.  It's enough to run a guest and then
> rmmod kvm to see slub errors which are actually caused by memory
> corruption.
>
> The right way to size it is shown in fpu__init_task_struct_size but for
> now I'll revert it to sizeof(struct fpu).  I have plans to move
> fsave/fxsave/xsave directly in KVM, without using the kernel FPU
> helpers, and actually this guest_fpu thing will come in handy for that.
> :)  Once it's done, the size of the object in the cache will be
> something like kvm_xstate_size.
>
> Paolo
>
>
> > +                             __alignof__(struct fpu),
> > +                             SLAB_ACCOUNT,
> > +                             offsetof(struct fpu, state),
> > +                             fpu_kernel_xstate_size,
> > +                             NULL);
>

Oops. Thanks for debugging, explaining and fixing!