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.
