On 2019-03-11 12:06:05 [+0100], To Dave Hansen wrote: > On 2019-03-08 11:01:25 [-0800], Dave Hansen wrote: > > On 3/8/19 10:08 AM, Sebastian Andrzej Siewior wrote: > > > On 2019-02-25 10:16:24 [-0800], Dave Hansen wrote: > > >>> + if (!cpu_feature_enabled(X86_FEATURE_OSPKE)) > > >>> + return; > > >>> + > > >>> + if (current->mm) { > > >>> + pk = get_xsave_addr(&new_fpu->state.xsave, XFEATURE_PKRU); > > >>> + WARN_ON_ONCE(!pk); > … > > Nothing will break, but the warning will trigger, which isn't nice. > > the warning should trigger if something goes south, I was not expecting > it to happen. > > > > My understanding is that the in-kernel XSAVE will always save everything > > > so we should never "lose" the XFEATURE_PKRU no matter what user space > > > does. > > > > > > So as test case you want > > > xsave (-1 & ~XFEATURE_PKRU) > > > xrestore (-1 & ~XFEATURE_PKRU) > > > > > > in userland and then a context switch to see if the warning above > > > triggers? > > > > I think you need an XRSTOR with RFBM=-1 (or at least with the PKRU bit > > set) and the PKRU bit in the XFEATURES field in the XSAVE buffer set to 0. > > let me check that, write a test case in userland and I come back with > the results. I can remove that warning but I wasn't expecting it to > trigger so let me verify that first. so I made dis: https://breakpoint.cc/tc-xsave.c and it doesn't trigger. XSAVE saves what is specified and enabled. XRSTOR restores what is specified, available in the header and enabled. Which means even if userland disables a bit in the header, it is still available during context switch by kernel's XSAVE as long as it is set in XSTATE_BV. The user can't use XSETBV (can only query via XGETBV) which means that XFEATURE_PKRU can't be removed by the user. But you got me thinking: During signal delivery we save tasks' FPU content on the signal stack. If the signal handler removes XFEATURE_PKRU bit then __fpu__restore_sig() will load the "missing state" from init_fpstate. Which means that the protection key will be set to zero. Not sure we want this or not but this the case. A XRSTOR in userland without XFEATURE_PKRU would leave the PKRU state unchanged. The test case from above does this if you want to double check. Sebastian