Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit?

[H. Peter Anvin]

On December 17, 2015 9:29:21 PM PST, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote:
>On Dec 17, 2015 6:53 PM, "Dave Hansen" <dave.hansen@xxxxxxxxxxxxxxx>
>wrote:
>>
>> On 12/17/2015 06:32 PM, Andy Lutomirski wrote:
>> > On Thu, Dec 17, 2015 at 6:13 PM, Dave Hansen
><dave.hansen@xxxxxxxxxxxxxxx> wrote:
>> >> 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.
>> >
>> > I think we leave the extended regs alone.  Don't we?
>> >
>> > I think that, for most signals, we want to leave PKRU as is,
>> > especially for things that aren't SIGSEGV.  For SIGSEGV, maybe we
>want
>> > an option to reset PKRU on delivery (and then set the flag to
>restore
>> > on return?).
>>
>> Is there some precedent for doing the state differently for different
>> signals?
>
>Yes, to a very limited extent: SA_ONSTACK.
>
>>
>> >> Well, the signal handler isn't necessarily going to clobber it,
>but the
>> >> delivery code already clobbers it when going to the init state.
>> >
>> > Can you point to that code?
>>
>> handle_signal() -> fpu__clear()
>>
>> The comment around it says:
>>
>> "Ensure the signal handler starts with the new fpu state."
>>
>
>You win this round :)
>
>So maybe we should have a mask of xfeatures that aren't cleared on
>signal delivery (e.g. PKRU, perhaps) and that are, by default,
>preserved across signal returns.
>
>> >>> 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?).
>> >
>> > I thought they were everywhere.  fpu/signal.c looks that way to me.
> I
>> > could be missing something -- this code isn't the most
>straightforward
>> > in the world.
>>
>> I think there's some extra space on the ia32 frame for this stuff,
>but
>> some clarity from someone who knows the history would be appreciated.
>>
>> >> Not a huge deal, but something we want to think about, especially
>as it
>> >> pertains to the init/modified optimizations.
>> >
>> > Fair point.  FWIW, I don't think that sigreturn performance matters
>> > all that much, so if we inadvertently lose some of the
>optimizations,
>> > it may not be the end of the world.
>>
>> Once we lose the init optimization, we're sunk for good.  We never
>get
>> it back until we restore the init state again.  Having it in the init
>> state can save writing the state at XSAVE time, which can now save up
>to
>> ~2k of writes at each context switch.
>>
>> I'm sure we can preserve it, we just need to be _careful_.
>
>Right.
>
>How much does XSAVEOPT help here?  IOW if we're careful to save to the
>same place we restored from and we don't modify the state in the mean
>time, how much of the time do we save?  In the best case, I guess we
>save the memory writes but not the reads?
>
>--Andy

I find the notion of PKRU not being initialized at the entry to a signal handler a bit odd.  Saving/restoring it seems like the right thing to me.

What am I missing?
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/