Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-27 Thread Ira Weiny
On Fri, Jul 24, 2020 at 11:24:58PM +0200, Thomas Gleixner wrote: > Ira, > > Thomas Gleixner writes: > > Ira Weiny writes: > >> On Thu, Jul 23, 2020 at 09:53:20PM +0200, Thomas Gleixner wrote: > >> I think, after fixing my code (see below), using idtentry_state could still > >> work. If the

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-24 Thread Andy Lutomirski
On Fri, Jul 24, 2020 at 2:25 PM Thomas Gleixner wrote: > > Ira, > > Thomas Gleixner writes: > > Ira Weiny writes: > >> On Thu, Jul 23, 2020 at 09:53:20PM +0200, Thomas Gleixner wrote: > >> I think, after fixing my code (see below), using idtentry_state could still > >> work. If the per-cpu

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-24 Thread Thomas Gleixner
Thomas Gleixner writes: > static __always_inline idtentry_state_t idtentry_nmi_enter(void) > { > idtentry_state_t state = {}; > > nmi_enter(); > instrumentation_begin(); > state.key = save_and_clear_key(); > instrumentation_end(); Clearly lacks a

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-24 Thread Thomas Gleixner
Ira, Thomas Gleixner writes: > Ira Weiny writes: >> On Thu, Jul 23, 2020 at 09:53:20PM +0200, Thomas Gleixner wrote: >> I think, after fixing my code (see below), using idtentry_state could still >> work. If the per-cpu cache and the MSR is updated in idtentry_exit() that >> should carry the

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-24 Thread Ira Weiny
On Fri, Jul 24, 2020 at 10:29:23AM -0700, Andy Lutomirski wrote: > > > On Jul 24, 2020, at 10:23 AM, Ira Weiny wrote: > > > > On Thu, Jul 23, 2020 at 10:15:17PM +0200, Thomas Gleixner wrote: > >> Thomas Gleixner writes: > >> > >>> Ira Weiny writes: > On Fri, Jul 17, 2020 at 12:06:10PM

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-24 Thread Andy Lutomirski
> On Jul 24, 2020, at 10:23 AM, Ira Weiny wrote: > > On Thu, Jul 23, 2020 at 10:15:17PM +0200, Thomas Gleixner wrote: >> Thomas Gleixner writes: >> >>> Ira Weiny writes: On Fri, Jul 17, 2020 at 12:06:10PM +0200, Peter Zijlstra wrote: >> On Fri, Jul 17, 2020 at 12:20:56AM -0700,

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-24 Thread Ira Weiny
On Thu, Jul 23, 2020 at 10:15:17PM +0200, Thomas Gleixner wrote: > Thomas Gleixner writes: > > > Ira Weiny writes: > >> On Fri, Jul 17, 2020 at 12:06:10PM +0200, Peter Zijlstra wrote: > >>> On Fri, Jul 17, 2020 at 12:20:56AM -0700, ira.we...@intel.com wrote: > >> I've been really digging into

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-23 Thread Thomas Gleixner
Ira, Ira Weiny writes: > On Thu, Jul 23, 2020 at 09:53:20PM +0200, Thomas Gleixner wrote: > I think, after fixing my code (see below), using idtentry_state could still > work. If the per-cpu cache and the MSR is updated in idtentry_exit() that > should carry the state to the new cpu, correct?

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-23 Thread Thomas Gleixner
Andy Lutomirski writes: >> On Jul 23, 2020, at 1:22 PM, Thomas Gleixner wrote: >> Andy Lutomirski writes: >>> My suggestion is to enlarge pt_regs. The save and restore logic can >>> probably be in C, but pt_regs is the logical place to put a register >>> that is saved and restored across all

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-23 Thread Ira Weiny
On Thu, Jul 23, 2020 at 09:53:20PM +0200, Thomas Gleixner wrote: > Ira, > > ira.we...@intel.com writes: > > > ... > > // ref == 0 > > dev_access_enable() // ref += 1 ==> disable protection > > irq() > > // enable protection > > // ref

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-23 Thread Andy Lutomirski
> On Jul 23, 2020, at 1:22 PM, Thomas Gleixner wrote: > > Andy Lutomirski writes: > >> Suppose some kernel code (a syscall or kernel thread) changes PKRS >> then takes a page fault. The page fault handler needs a fresh >> PKRS. Then the page fault handler (say a VMA’s .fault handler) changes >>

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-23 Thread Thomas Gleixner
Dave Hansen writes: > On 7/23/20 10:08 AM, Andy Lutomirski wrote: >> Suppose some kernel code (a syscall or kernel thread) changes PKRS >> then takes a page fault. The page fault handler needs a fresh PKRS. >> Then the page fault handler (say a VMA’s .fault handler) changes >> PKRS. The we get

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-23 Thread Thomas Gleixner
Andy Lutomirski writes: > Suppose some kernel code (a syscall or kernel thread) changes PKRS > then takes a page fault. The page fault handler needs a fresh > PKRS. Then the page fault handler (say a VMA’s .fault handler) changes > PKRS. The we get an interrupt. The interrupt *also* needs a

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-23 Thread Thomas Gleixner
Thomas Gleixner writes: > Ira Weiny writes: >> On Fri, Jul 17, 2020 at 12:06:10PM +0200, Peter Zijlstra wrote: >>> On Fri, Jul 17, 2020 at 12:20:56AM -0700, ira.we...@intel.com wrote: >> I've been really digging into this today and I'm very concerned that I'm >> completely missing something WRT

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-23 Thread Thomas Gleixner
Ira Weiny writes: > On Fri, Jul 17, 2020 at 12:06:10PM +0200, Peter Zijlstra wrote: >> On Fri, Jul 17, 2020 at 12:20:56AM -0700, ira.we...@intel.com wrote: > I've been really digging into this today and I'm very concerned that I'm > completely missing something WRT idtentry_enter() and

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-23 Thread Thomas Gleixner
Ira, ira.we...@intel.com writes: > ... > // ref == 0 > dev_access_enable() // ref += 1 ==> disable protection > irq() > // enable protection > // ref = 0 > _handler() >

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-23 Thread Dave Hansen
On 7/23/20 10:08 AM, Andy Lutomirski wrote: > Suppose some kernel code (a syscall or kernel thread) changes PKRS > then takes a page fault. The page fault handler needs a fresh PKRS. > Then the page fault handler (say a VMA’s .fault handler) changes > PKRS. The we get an interrupt. The interrupt

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-23 Thread Andy Lutomirski
> On Jul 23, 2020, at 9:52 AM, Fenghua Yu wrote: > > Hi, Dave, > >> On Thu, Jul 23, 2020 at 09:23:13AM -0700, Dave Hansen wrote: >>> On 7/23/20 9:18 AM, Fenghua Yu wrote: >>> The PKRS MSR has been preserved in thread_info during kernel entry. We >>> don't need to preserve it in another place

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-23 Thread Fenghua Yu
Hi, Dave, On Thu, Jul 23, 2020 at 09:23:13AM -0700, Dave Hansen wrote: > On 7/23/20 9:18 AM, Fenghua Yu wrote: > > The PKRS MSR has been preserved in thread_info during kernel entry. We > > don't need to preserve it in another place (i.e. idtentry_state). > > I'm missing how the PKRS MSR gets

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-23 Thread Dave Hansen
On 7/23/20 9:18 AM, Fenghua Yu wrote: > The PKRS MSR has been preserved in thread_info during kernel entry. We > don't need to preserve it in another place (i.e. idtentry_state). I'm missing how the PKRS MSR gets preserved in thread_info. Could you explain the mechanism by which this happens and

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-23 Thread Fenghua Yu
On Wed, Jul 22, 2020 at 09:21:43AM -0700, Andy Lutomirski wrote: > On Fri, Jul 17, 2020 at 12:21 AM wrote: > > > > From: Ira Weiny > > > > The PKRS MSR is not managed by XSAVE. It is already preserved through a > > context switch but this support leaves exception handling code open to > >

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-22 Thread Ira Weiny
On Wed, Jul 22, 2020 at 11:48:53AM +0200, Peter Zijlstra wrote: > On Tue, Jul 21, 2020 at 10:27:09PM -0700, Ira Weiny wrote: > > > I've been really digging into this today and I'm very concerned that I'm > > completely missing something WRT idtentry_enter() and idtentry_exit(). > > > > I've

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-22 Thread Andy Lutomirski
On Fri, Jul 17, 2020 at 12:21 AM wrote: > > From: Ira Weiny > > The PKRS MSR is not managed by XSAVE. It is already preserved through a > context switch but this support leaves exception handling code open to > memory accesses which the interrupted process has allowed. > > Close this hole by

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-22 Thread Peter Zijlstra
On Tue, Jul 21, 2020 at 10:27:09PM -0700, Ira Weiny wrote: > I've been really digging into this today and I'm very concerned that I'm > completely missing something WRT idtentry_enter() and idtentry_exit(). > > I've instrumented idt_{save,restore}_pkrs(), and __dev_access_{en,dis}able() > with

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-21 Thread Ira Weiny
On Fri, Jul 17, 2020 at 12:06:10PM +0200, Peter Zijlstra wrote: > On Fri, Jul 17, 2020 at 12:20:56AM -0700, ira.we...@intel.com wrote: > > First I'm not sure if adding this state to idtentry_state and having > > that state copied is the right way to go. It seems like we should start > > passing

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-21 Thread Peter Zijlstra
On Tue, Jul 21, 2020 at 11:01:34AM -0700, Ira Weiny wrote: > On Fri, Jul 17, 2020 at 11:30:41AM +0200, Peter Zijlstra wrote: > > On Fri, Jul 17, 2020 at 12:20:56AM -0700, ira.we...@intel.com wrote: > > > +static void noinstr idt_save_pkrs(idtentry_state_t state) > > > > noinstr goes in the same

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-21 Thread Ira Weiny
On Fri, Jul 17, 2020 at 11:30:41AM +0200, Peter Zijlstra wrote: > On Fri, Jul 17, 2020 at 12:20:56AM -0700, ira.we...@intel.com wrote: > > +static void noinstr idt_save_pkrs(idtentry_state_t state) > > noinstr goes in the same place you would normally put inline, that's > before the return type,

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-17 Thread Peter Zijlstra
On Fri, Jul 17, 2020 at 12:20:56AM -0700, ira.we...@intel.com wrote: > First I'm not sure if adding this state to idtentry_state and having > that state copied is the right way to go. It seems like we should start > passing this by reference instead of value. But for now this works as > an RFC.

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-17 Thread Peter Zijlstra
On Fri, Jul 17, 2020 at 12:20:56AM -0700, ira.we...@intel.com wrote: > +/* Define as macros to prevent conflict of inline and noinstr */ > +#define idt_save_pkrs(state) > +#define idt_restore_pkrs(state) Use __always_inline

Re: [PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-17 Thread Peter Zijlstra
On Fri, Jul 17, 2020 at 12:20:56AM -0700, ira.we...@intel.com wrote: > +static void noinstr idt_save_pkrs(idtentry_state_t state) noinstr goes in the same place you would normally put inline, that's before the return type, not after it.

[PATCH RFC V2 17/17] x86/entry: Preserve PKRS MSR across exceptions

2020-07-17 Thread ira . weiny
From: Ira Weiny The PKRS MSR is not managed by XSAVE. It is already preserved through a context switch but this support leaves exception handling code open to memory accesses which the interrupted process has allowed. Close this hole by preserve the current task's PKRS MSR, reset the PKRS MSR