On 05/02/2018 03:59 AM, Paolo Bonzini wrote:
> 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?
Yes, I can take a look at it.
>
> 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);
>>
>
