Re: [PATCH 5/7] x86/fpu: Change fpu->fpregs_active users to fpu->fpstate_active

From: Rik van Riel <riel@redhat.com>
To: Andy Lutomirski <luto@amacapital.net>, Ingo Molnar <mingo@kernel.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Borislav Petkov <bp@alien8.de>,
	pbonzini@redhat.com, Dave Hansen <dave.hansen@linux.intel.com>,
	Fenghua Yu <fenghua.yu@intel.com>,
	"H . Peter Anvin" <hpa@zytor.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Yu-cheng Yu <yu-cheng.yu@intel.com>
Subject: Re: [PATCH 5/7] x86/fpu: Change fpu->fpregs_active users to fpu->fpstate_active
Date: Thu, 26 Jan 2017 13:04:32 -0500	[thread overview]
Message-ID: <1485453872.15964.58.camel@redhat.com> (raw)
In-Reply-To: <CALCETrUo7moqG=wAcoiQSKHM2DDhkqYRC84js+yx3xG0jkhr5Q@mail.gmail.com>

On Thu, 2017-01-26 at 09:00 -0800, Andy Lutomirski wrote:
> On Thu, Jan 26, 2017 at 7:53 AM, Ingo Molnar <mingo@kernel.org>
> wrote:
> > 
> > * Rik van Riel <riel@redhat.com> wrote:
> > 
> > > Let me go totally reimplement this whole project in a different
> > > way...
> > 
> > Note that I can still be convinced about complicating the FPU state
> > machine as
> > well if that ends up being the best approach for KVM - but it
> > appears to me (from
> > a very superficial look) that turning vCPU threads into no-FPU
> > kthreads or
> > representing the guest FPU state directly with the host FPU context
> > would be even
> > more beneficial, from the simplicity and KVM performance POV?
> 
> I may be misunderstanding you, but I don't see how this would work
> without getting either messy or slow.
> 
> But I think that your series may still be a good base for Rik's work.
> With your series applied, there are three possible FPU states: regs
> active (regs are in the CPU), regs inactive (in memory), and regs
> cached (in memory *and* regs).  What Rik's series does doesn't really
> complicate the state machine -- there are still just these three
> states.  The difference is that it's possible for the regs to be
> inactive or cached even for the current task so long as we're not in
> user mode.  The point being that the user vCPU thread can enter the
> kernel, get its FPU state inactivated, enter the guest, and reenter
> the kernel without reactivating its regs.
> 
> Rik, if you think about it that way, does your work map cleanly onto
> Ingo's patches?

It does, but the discussion with Ingo also led me to reconsider
an approach I looked at before.

A task could have multiple FPU structures associated with it.
In kvm_vcpu_ioctl(KVM_RUN) we could save the userspace context,
and load the guest FPU context.

Once we are about ready to return to userspace, we can save the
guest FPU context, and load the userspace FPU context.

The only complication is that signal handling and ptrace need
to access the _userspace_ FPU context, even if it is not the
currently used one for the task.

That means we cannot just swap out the contents of
current->thread.fpu, but we need to keep a pointer to the
currently used FPU in current->thread, and have the signal
and ptrace code always work on the userspace FPU data,
which means the in-register data if it is loaded, or the
memory data if it isn't.

On the KVM side, we should be able to drop kernel_fpu_begin
and kernel_fpu_end from entering/leaving the guest. All we
need to swap out in that spot will be the PKRU keys.

The "is the FPU still loaded?" stuff at context switch time
would ensure that guest FPU state loading can be skipped if
all that was run between guest exit and re-entry is kernel
threads.

I suspect this could be slightly lower complexity than the
approach I had been working on, for essentially the same
performance benefit.