All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rik van Riel <riel@redhat.com>
To: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Hansen <dave.hansen@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	X86 ML <x86@kernel.org>, Thomas Gleixner <tglx@linutronix.de>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Ingo Molnar <mingo@redhat.com>,
	Andrew Lutomirski <luto@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>, Borislav Petkov <bp@suse.de>
Subject: Re: [PATCH RFC 2/5] x86,fpu: delay FPU register loading until switch to userspace
Date: Mon, 03 Oct 2016 22:47:53 -0400	[thread overview]
Message-ID: <1475549273.21644.51.camel@redhat.com> (raw)
In-Reply-To: <CALCETrVQetdSzd9t1njEXpSiX-4xoGjG-fTfQKwfb=x4Jxpx4A@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2075 bytes --]

On Mon, 2016-10-03 at 19:09 -0700, Andy Lutomirski wrote:

> > Having two separate status booleans for "registers valid"
> > and "memory valid" may make more sense.
> 
> I have no problem with the concept of "owner_ctx", and I think it's a
> perfectly reasonable data structure.  My problem with it is that it's
> subtle and knowledge of it is spread all over the place.  Just going
> with "registers valid" in a variable won't work, I think, because
> there's nowhere to put it.  We need to be able to delete a struct fpu
> while that struct fpu might have a valid copy in a different cpu's
> registers.
> 
> Anyway, feel free to tell me that I'm making this too difficult :)

How about we rename fpu_want_lazy_restore to
fpu_registers_valid()?  Problem solved :)

Then we can rename __cpu_disable_lazy_restore
to fpu_invalidate_registers(), and call that
before we modify any in-memory FPU state.

> > We can get rid of fpu.counter, since nobody uses it
> > any more.
> 
> We should definitely do this.
> 
> Maybe getting in some cleanups first (my lazy fpu deletion,
> fpu.counter removal, etc) first is the way to go.

Sounds good.  I will keep my patch 1/4 as part of the
cleanup series, and will not move on to the harder
stuff until after the cleanups.

Any other stuff I should clean up while we're there?

> > > > > 
> > You are right, read_pkru() and write_pkru() can only deal with
> > the pkru state being present in registers. Is this because of an
> > assumption in the code, or because of a hardware requirement?

read_pkru and write_pkru would be candidates for using
fpu_registers_valid, and potentially a fpu_make_registers_valid,
which restores the contents of the fpu registers from memory,
if fpu_registers_valid is not true.

Likewise, we can have an fpu_make_memory_valid to ensure the
in kernel memory copy of the FPU registers is valid, potentially
a _read and _write version that do exactly what the pstate code
wants today.

Would that make sense as an API?

-- 
All Rights Reversed.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

  reply	other threads:[~2016-10-04  2:47 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-01 20:31 [PATCH RFC 0/5] x86,fpu: make FPU context switching much lazier riel
2016-10-01 20:31 ` [PATCH RFC 1/5] x86,fpu: split prev/next task fpu state handling riel
2016-10-01 23:26   ` Andy Lutomirski
2016-10-02  0:02     ` Rik van Riel
2016-10-01 20:31 ` [PATCH RFC 2/5] x86,fpu: delay FPU register loading until switch to userspace riel
2016-10-01 23:44   ` Andy Lutomirski
2016-10-02  0:08     ` Rik van Riel
2016-10-03 20:54       ` Andy Lutomirski
2016-10-03 21:21         ` Rik van Riel
2016-10-03 21:36           ` Andy Lutomirski
2016-10-04  1:29             ` Rik van Riel
2016-10-04  2:09               ` Andy Lutomirski
2016-10-04  2:47                 ` Rik van Riel [this message]
2016-10-04  3:02                   ` Andy Lutomirski
2016-10-04  6:35                 ` Ingo Molnar
2016-10-04 12:48                   ` Rik van Riel
2016-10-04  2:11             ` Rik van Riel
2016-10-04  3:02               ` Andy Lutomirski
2016-10-02  0:42     ` Rik van Riel
2016-10-03 16:23       ` Dave Hansen
2016-10-01 20:31 ` [PATCH RFC 3/5] x86,fpu: add kernel fpu argument to __kernel_fpu_begin riel
2016-10-01 20:31 ` [PATCH RFC 4/5] x86,fpu: lazily skip FPU restore when still loaded riel
2016-10-03 20:04   ` Dave Hansen
2016-10-03 20:22     ` Rik van Riel
2016-10-03 20:49       ` Dave Hansen
2016-10-03 21:02         ` Rik van Riel
2016-10-01 20:31 ` [PATCH RFC 5/5] x86,fpu: kinda sorta fix up signal path riel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1475549273.21644.51.camel@redhat.com \
    --to=riel@redhat.com \
    --cc=bp@suse.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.