All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@linux.intel.com>
To: Andy Lutomirski <luto@kernel.org>, Ingo Molnar <mingo@kernel.org>,
	Borislav Petkov <bp@alien8.de>, "H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Brian Gerst <brgerst@gmail.com>, Oleg Nesterov <oleg@redhat.com>
Subject: Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit?
Date: Thu, 17 Dec 2015 18:13:37 -0800	[thread overview]
Message-ID: <56736BD1.5080700@linux.intel.com> (raw)
In-Reply-To: <CALCETrVw2V+Ny7=D=_E2m0Qa_v7Bp8GUjhyDXx_bWC35R2ohjg@mail.gmail.com>

On 12/17/2015 05:48 PM, Andy Lutomirski wrote:
> I think that, for PKRU in particular, we want the default signal
> handling behavior to be a bit unusual.
> 
> When a signal is delivered, I think we should save the entire xstate
> including PKRU.  I see no reason to do anything other than that.

Yep, agreed.

But what about the register state when delivering a signal?  Don't we
set the registers to the init state?  Do we need to preserve PKRU state
instead of init'ing it?  The init state _is_ nice here because it is
permissive allows us to do something useful no matter what PKRU gets set to.

But, if we leave the init state in place when entering a delivering a
signal, we _can't_ decide to (by default at least) preserve the
in-signal state.

> When a signal returns (sigreturn is called), though, I think we should
> *not* restore PKRU.  To me, PKRU seems like a global per-thread state,
> not something that signal handlers are likely to clobber and should
> therefore have restored.  It's also unusual in that it doesn't fit
> into the usual xstate feature model of being a bunch of registers that
> are mostly caller-saved.
> 
> Does this make sense?  Should we do this?

Well, the signal handler isn't necessarily going to clobber it, but the
delivery code already clobbers it when going to the init state.

> We have _fpx_sw_bytes.xfeatures and _xstate._header.xfeatures.  They
> appear to do more or less the same thing.

I thought the _fpx_sw_bytes were only for 32-bit (or FXSAVE/FXRSTOR?).

> Could we say that, for
> certain new features (e.g. PKRU), if they're not in
> _fpx_sw_bytes.xfeatures, then sigreturn will preserve the old content
> rather than copying it?  User code that wants to change it on
> sigreturn would manually or the feature in to xfeatures, which would
> cause the feature to go to its init state if it's not in
> _header.xfeatures or to go into the saved state if it is in
> _header.xfeatures?

I think we first need to decide on the state upon signal delivery.

A practial problem at the moment is that we always call XRSTOR (aka
copy_user_to_xregs()) with RFBM (aka 'mask') with all of the supported
xfeatures.  So RFBM[i]=1 for each state, effectively.  A state with
XSTATE_BV[i]=0 (aka header.xfeatures) and RFBM[i]=1 will init the state.

We'd need to rig up the copy_user_to_xregs() to first read in
header.xfeatures and then or RFBM with it.

Not a huge deal, but something we want to think about, especially as it
pertains to the init/modified optimizations.

  reply	other threads:[~2015-12-18  2:13 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-18  1:48 Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Andy Lutomirski
2015-12-18  2:13 ` Dave Hansen [this message]
2015-12-18  2:32   ` Andy Lutomirski
2015-12-18  2:52     ` Dave Hansen
2015-12-18  5:29       ` Andy Lutomirski
2015-12-18  6:43         ` H. Peter Anvin
2015-12-18 16:04           ` Andy Lutomirski
2015-12-18 16:56             ` Dave Hansen
2015-12-18 18:42             ` Dave Hansen
2015-12-18 19:21               ` Andy Lutomirski
2015-12-18 20:07                 ` Dave Hansen
2015-12-18 20:28                   ` Andy Lutomirski
2015-12-18 20:37                   ` Linus Torvalds
2015-12-18 20:49                     ` Andy Lutomirski
2015-12-18 20:58                       ` H. Peter Anvin
2015-12-18 21:02                         ` Andy Lutomirski
2015-12-18 21:08                           ` Dave Hansen
2015-12-18 21:04                       ` Linus Torvalds
2015-12-18 21:09                         ` Linus Torvalds
2015-12-18 21:12                         ` Dave Hansen
2015-12-18 21:45                           ` Linus Torvalds
2015-12-18 22:28                             ` Andy Lutomirski
2015-12-18 23:08                               ` Linus Torvalds
2015-12-18 23:16                                 ` Andy Lutomirski
2015-12-18 23:20                                   ` Linus Torvalds
2015-12-21 17:04                                   ` Dave Hansen
2015-12-21 22:52                                     ` Andy Lutomirski
2015-12-21 23:00                                       ` Dave Hansen
2015-12-21 23:02                                         ` Andy Lutomirski
2015-12-21 23:05                                           ` Dave Hansen
2015-12-21 23:04                               ` Dave Hansen
2015-12-21 23:07                                 ` Andy Lutomirski
2016-06-30 17:36                                   ` Andy Lutomirski
2016-06-30 21:25                                     ` Dave Hansen
2016-07-01 16:30                                       ` Andy Lutomirski
2015-12-29 23:48                             ` Dave Hansen
2015-12-18  8:32         ` Ingo Molnar
2015-12-18  8:59 ` Christoph Hellwig
2015-12-18 12:57   ` Borislav Petkov
2016-01-12 13:38     ` Ingo Molnar
2016-01-12 13:42       ` Christoph Hellwig
2016-01-13 10:48         ` Ingo Molnar

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=56736BD1.5080700@linux.intel.com \
    --to=dave.hansen@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=tglx@linutronix.de \
    /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.