On 14/07/2017 06:06, Junkang Fu wrote: > Host pkru is restored right after vcpu exit (commit 1be0e61) so > host pkru will be saved on migration, fix this by using guest_pkru > explicitly in fill_save. > > Reviewed-by: Yang Zhang <zy107165@xxxxxxxxxxxxxxx > <mailto:zy107165@xxxxxxxxxxxxxxx>> > Signed-off-by: Tianyi <junkang.fjk@xxxxxxxxxxxxxxx > <mailto:junkang.fjk@xxxxxxxxxxxxxxx>> > Signed-off-by: Quan Xu <wutu.xq@xxxxxxxxxxxxxxx > <mailto:wutu.xq@xxxxxxxxxxxxxxx>> > --- > v2: > - replace Yoda conditions > v3 > - load guest pkru later to avoid using guest pkru on the host > > @@ -3274,7 +3280,20 @@ static void load_xsave(struct kvm_vcpu *vcpu, u8 > *src) > u32 size, offset, ecx, edx; > cpuid_count(XSTATE_CPUID, index, > &size, &offset, &ecx, &edx); > - memcpy(dest, src + offset, size); > + if (feature == XFEATURE_MASK_PKRU) { > + /* > + * host pkru was initialized in vcpu_load already > + * it's safe to use kvm_x86_ops->get_pkru to get it > + */ > + host_pkru = kvm_x86_ops->get_pkru(vcpu, false); Just use read_pkru? > + memcpy(&guest_pkru, src + offset, 4); > + if (host_pkru != guest_pkru) No need for the if. > + kvm_x86_ops->set_pkru(vcpu, guest_pkru); > + > + /* It is safer to restore guest_pkru later */ > + memcpy(dest, &host_pkru, 4); This would give: u32 host_pkru = read_pkru(); u32 guest_pkru; memcpy(&guest_pkru, src + offset, sizeof(guest_pkru)); kvm_x86_ops->set_pkru(vcpu, guest_pkru); memcpy(dest, &host_pkru, sizeof(host_pkru)); but alternatively, let's store the PKRU here in load_xsave, without affecting the extended state buffer: u32 guest_pkru; memcpy(&guest_pkru, src + offset, sizeof(guest_pkru)); kvm_x86_ops->set_pkru(vcpu, guest_pkru); Then you can mask out PKRU in kvm_load_guest_fpu: diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h index 255645f60ca2..554cdb205d17 100644 --- a/arch/x86/include/asm/fpu/internal.h +++ b/arch/x86/include/asm/fpu/internal.h @@ -450,10 +450,10 @@ static inline int copy_fpregs_to_fpstate(struct fpu *fpu) return 0; } -static inline void __copy_kernel_to_fpregs(union fpregs_state *fpstate) +static inline void __copy_kernel_to_fpregs(union fpregs_state *fpstate, u64 mask) { if (use_xsave()) { - copy_kernel_to_xregs(&fpstate->xsave, -1); + copy_kernel_to_xregs(&fpstate->xsave, mask); } else { if (use_fxsr()) copy_kernel_to_fxregs(&fpstate->fxsave); @@ -477,7 +477,7 @@ static inline void copy_kernel_to_fpregs(union fpregs_state *fpstate) : : [addr] "m" (fpstate)); } - __copy_kernel_to_fpregs(fpstate); + __copy_kernel_to_fpregs(fpstate, -1); } extern int copy_fpstate_to_sigframe(void __user *buf, void __user *fp, int size); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 52314a5b97c2..7fc49cbe6891 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -7653,7 +7653,8 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu) */ vcpu->guest_fpu_loaded = 1; __kernel_fpu_begin(); - __copy_kernel_to_fpregs(&vcpu->arch.guest_fpu.state); + /* PKRU is separately restored in kvm_x86_ops->run. */ + __copy_kernel_to_fpregs(&vcpu->arch.guest_fpu.state, + ~XFEATURE_MASK_PKRU); trace_kvm_fpu(1); }
