Re: [PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch

2020-12-18 Thread Thomas Gleixner
On Fri, Dec 18 2020 at 13:58, Dan Williams wrote: > On Fri, Dec 18, 2020 at 1:06 PM Thomas Gleixner wrote: >> kmap_local() is fine. That can work automatically because it's strict >> local to the context which does the mapping. >> >> kmap() is dubious because it's a 'global' mapping as dictated

Re: [PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch

2020-12-18 Thread Dan Williams
On Fri, Dec 18, 2020 at 1:06 PM Thomas Gleixner wrote: > > On Fri, Dec 18 2020 at 11:20, Dan Williams wrote: > > On Fri, Dec 18, 2020 at 5:58 AM Thomas Gleixner wrote: > > [..] > >> 5) The DAX case which you made "work" with dev_access_enable() and > >> dev_access_disable(), i.e. with yet

Re: [PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch

2020-12-18 Thread Thomas Gleixner
On Fri, Dec 18 2020 at 11:42, Ira Weiny wrote: > On Fri, Dec 18, 2020 at 02:57:51PM +0100, Thomas Gleixner wrote: >> 2) Modify kmap() so that it marks the to be mapped page as 'globaly >> unprotected' instead of doing this global unprotect PKS dance. >> kunmap() undoes that. That

Re: [PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch

2020-12-18 Thread Thomas Gleixner
On Fri, Dec 18 2020 at 11:20, Dan Williams wrote: > On Fri, Dec 18, 2020 at 5:58 AM Thomas Gleixner wrote: > [..] >> 5) The DAX case which you made "work" with dev_access_enable() and >> dev_access_disable(), i.e. with yet another lazy approach of >> avoiding to change a handful of

Re: [PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch

2020-12-18 Thread Dave Hansen
On 12/18/20 11:42 AM, Ira Weiny wrote: > Another problem would be if the kmap and kunmap happened in different > contexts... :-/ I don't think that is done either but I don't know for > certain. It would be really nice to put together some surveillance patches to help become more certain about

Re: [PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch

2020-12-18 Thread Ira Weiny
On Fri, Dec 18, 2020 at 02:57:51PM +0100, Thomas Gleixner wrote: > On Thu, Dec 17 2020 at 23:43, Thomas Gleixner wrote: > > The only use case for this in your tree is: kmap() and the possible > > usage of that mapping outside of the thread context which sets it up. > > > > The only hint for doing

Re: [PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch

2020-12-18 Thread Dan Williams
On Fri, Dec 18, 2020 at 5:58 AM Thomas Gleixner wrote: [..] > 5) The DAX case which you made "work" with dev_access_enable() and > dev_access_disable(), i.e. with yet another lazy approach of > avoiding to change a handful of usage sites. > > The use cases are strictly context

Re: [NEEDS-REVIEW] [PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch

2020-12-18 Thread Dave Hansen
On 12/17/20 8:10 PM, Ira Weiny wrote: > On Thu, Dec 17, 2020 at 12:41:50PM -0800, Dave Hansen wrote: >> On 11/6/20 3:29 PM, ira.we...@intel.com wrote: >>> void disable_TSC(void) >>> @@ -644,6 +668,8 @@ void __switch_to_xtra(struct task_struct *prev_p, >>> struct task_struct *next_p) >>> >>>

Re: [PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch

2020-12-18 Thread Thomas Gleixner
On Thu, Dec 17 2020 at 23:43, Thomas Gleixner wrote: > The only use case for this in your tree is: kmap() and the possible > usage of that mapping outside of the thread context which sets it up. > > The only hint for doing this at all is: > > Some users, such as kmap(), sometimes requires PKS

Re: [NEEDS-REVIEW] [PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch

2020-12-17 Thread Ira Weiny
On Thu, Dec 17, 2020 at 12:41:50PM -0800, Dave Hansen wrote: > On 11/6/20 3:29 PM, ira.we...@intel.com wrote: > > void disable_TSC(void) > > @@ -644,6 +668,8 @@ void __switch_to_xtra(struct task_struct *prev_p, > > struct task_struct *next_p) > > > > if ((tifp ^ tifn) & _TIF_SLD) > >

Re: [PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch

2020-12-17 Thread Ira Weiny
On Thu, Dec 17, 2020 at 03:50:55PM +0100, Thomas Gleixner wrote: > On Fri, Nov 06 2020 at 15:29, ira weiny wrote: > > --- a/arch/x86/kernel/process.c > > +++ b/arch/x86/kernel/process.c > > @@ -43,6 +43,7 @@ > > #include > > #include > > #include > > +#include > > > > #include

Re: [PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch

2020-12-17 Thread Thomas Gleixner
On Thu, Dec 17 2020 at 15:50, Thomas Gleixner wrote: > On Fri, Nov 06 2020 at 15:29, ira weiny wrote: > >> +void write_pkrs(u32 new_pkrs) >> +{ >> +u32 *pkrs; >> + >> +if (!static_cpu_has(X86_FEATURE_PKS)) >> +return; >> + >> +pkrs = get_cpu_ptr(_cache); > > So this is

Re: [NEEDS-REVIEW] [PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch

2020-12-17 Thread Dave Hansen
On 11/6/20 3:29 PM, ira.we...@intel.com wrote: > void disable_TSC(void) > @@ -644,6 +668,8 @@ void __switch_to_xtra(struct task_struct *prev_p, struct > task_struct *next_p) > > if ((tifp ^ tifn) & _TIF_SLD) > switch_to_sld(tifn); > + > + pks_sched_in(); > } Does the

Re: [PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch

2020-12-17 Thread Thomas Gleixner
On Fri, Nov 06 2020 at 15:29, ira weiny wrote: > --- a/arch/x86/kernel/process.c > +++ b/arch/x86/kernel/process.c > @@ -43,6 +43,7 @@ > #include > #include > #include > +#include > > #include "process.h" > > @@ -187,6 +188,27 @@ int copy_thread(unsigned long clone_flags, unsigned long

[PATCH V3 04/10] x86/pks: Preserve the PKRS MSR on context switch

2020-11-06 Thread ira . weiny
From: Ira Weiny The PKRS MSR is defined as a per-logical-processor register. This isolates memory access by logical CPU. Unfortunately, the MSR is not managed by XSAVE. Therefore, tasks must save/restore the MSR value on context switch. Define a saved PKRS value in the task struct, as well