| Message ID | 20190109114744.10936-19-bigeasy@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v6] x86: load FPU registers on return to userland | expand |
On 1/9/19 3:47 AM, Sebastian Andrzej Siewior wrote: > + pk = get_xsave_addr(¤t->thread.fpu.state.xsave, XFEATURE_PKRU); > + /* > + * The PKRU value in xstate needs to be in sync with the value that is > + * written to the CPU. The FPU restore on return to userland would > + * otherwise load the previous value again. > + */ > + __fpregs_changes_begin(); > + if (pk) > + pk->pkru = pkru; > + __write_pkru(pkru); > + __fpregs_changes_end(); > } I'm not sure this is right. The "if (pk)" check is basically to see if there was a location for XFEATURE_PKRU in the XSAVE buffer. The only way this can be false in the current code is if the "init optimization" is in play and XFEATURE_PKRU was in the init state (all 0's for PKRU). If it were in the init state, we need to take it *out* of the init state, both in the buffer and in the registers. The __write_pkru() obviously does this for the registers, but "pk->pkru = pkru" is not enough for the XSAVE buffer. xsave->header.xfeatures (aka. XSTATE_BV) also needs to have XFEATURE_PKRU set. Otherwise, two calls to this function in succession would break. pk = get_xsave_addr(...xsave, XFEATURE_PKRU); pk->pkru = pkru; __write_pkru(pkru); pk = get_xsave_addr(...xsave, XFEATURE_PKRU); /* 'pk' is still NULL, won't see 'pkru' set */ I *think* just setting xsave->header.xfeatures |= XFEATURE_MASK_PKRU; will fix this. I thought we did that whole dance somewhere else in the code, but I don't see it right now. Might have been in some other patch.
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h index 40616e8052924..5eed44798ae95 100644 --- a/arch/x86/include/asm/pgtable.h +++ b/arch/x86/include/asm/pgtable.h @@ -23,6 +23,8 @@ #ifndef __ASSEMBLY__ #include <asm/x86_init.h> +#include <asm/fpu/xstate.h> +#include <asm/fpu/api.h> extern pgd_t early_top_pgt[PTRS_PER_PGD]; int __init __early_make_pgtable(unsigned long address, pmdval_t pmd); @@ -133,8 +135,22 @@ static inline u32 read_pkru(void) static inline void write_pkru(u32 pkru) { - if (boot_cpu_has(X86_FEATURE_OSPKE)) - __write_pkru(pkru); + struct pkru_state *pk; + + if (!boot_cpu_has(X86_FEATURE_OSPKE)) + return; + + pk = get_xsave_addr(¤t->thread.fpu.state.xsave, XFEATURE_PKRU); + /* + * The PKRU value in xstate needs to be in sync with the value that is + * written to the CPU. The FPU restore on return to userland would + * otherwise load the previous value again. + */ + __fpregs_changes_begin(); + if (pk) + pk->pkru = pkru; + __write_pkru(pkru); + __fpregs_changes_end(); } static inline int pte_young(pte_t pte)
During the context switch the xstate is loaded which also includes the PKRU value. If xstate is restored on return to userland it is required that the PKRU value in xstate is the same as the one in the CPU. Save the PKRU in xstate during modification. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- arch/x86/include/asm/pgtable.h | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)