On 01/05/2018 16:49, Wei Huang wrote:
> The CPUID bits of OSXSAVE (function=0x1) and OSPKE (func=0x7, leaf=0x0)
> allows user apps to detect if OS has set CR4.OSXSAVE or CR4.PKE. KVM is
> supposed to update these CPUID bits when CR4 is updated. Current KVM
> code doesn't handle some special cases when updates come from emulator.
> Here is one example:
>
> Step 1: guest boots
> Step 2: guest OS enables XSAVE ==> CR4.OSXSAVE=1 and CPUID.OSXSAVE=1
> Step 3: guest hot reboot ==> QEMU reset CR4 to 0, but CPUID.OSXAVE==1
> Step 4: guest os checks CPUID.OSXAVE, detects 1, then executes xgetbv
>
> Step 4 above will cause an #UD and guest crash because guest OS hasn't
> turned on OSXAVE yet. This patch solves the problem by comparing the the
> old_cr4 with cr4. If the related bits have been changed,
> kvm_update_cpuid() needs to be called.
It seems possible to write a testcase for this. You could use S3
instead of hot reboot, or you could use the new framework in
tools/testing/selftests (execute mov to CR4 in guest, trigger vmexit, do
KVM_SET_SREGS in host, execute CPUID in guest).
Would you like to give it a shot?
Paolo
> Signed-off-by: Wei Huang <wei@xxxxxxxxxx>
> ---
> arch/x86/kvm/x86.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 51ecd381793b..daeb5e2176c6 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7979,6 +7979,7 @@ static int __set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
> {
> struct msr_data apic_base_msr;
> int mmu_reset_needed = 0;
> + int cpuid_update_needed = 0;
> int pending_vec, max_bits, idx;
> struct desc_ptr dt;
> int ret = -EINVAL;
> @@ -8017,8 +8018,10 @@ static int __set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
> vcpu->arch.cr0 = sregs->cr0;
>
> mmu_reset_needed |= kvm_read_cr4(vcpu) != sregs->cr4;
> + cpuid_update_needed |= ((kvm_read_cr4(vcpu) ^ sregs->cr4) &
> + (X86_CR4_OSXSAVE | X86_CR4_PKE));
> kvm_x86_ops->set_cr4(vcpu, sregs->cr4);
> - if (sregs->cr4 & (X86_CR4_OSXSAVE | X86_CR4_PKE))
> + if (cpuid_update_needed)
> kvm_update_cpuid(vcpu);
>
> idx = srcu_read_lock(&vcpu->kvm->srcu);
>
