lkml.org 
[lkml]   [2016]   [Nov]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v8 7/7] KVM: x86: virtualize cpuid faulting
On Fri, Nov 4, 2016 at 5:09 PM, David Matlack <dmatlack@google.com> wrote:
> On Fri, Nov 4, 2016 at 2:57 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 04/11/2016 21:34, David Matlack wrote:
>>> On Mon, Oct 31, 2016 at 6:37 PM, Kyle Huey <me@kylehuey.com> wrote:
>>>> + case MSR_PLATFORM_INFO:
>>>> + /* cpuid faulting is supported */
>>>> + msr_info->data = PLATINFO_CPUID_FAULT;
>>>> + break;
>>>
>>> This could break save/restore, if for example, a VM is migrated to a
>>> version of KVM without MSR_PLATFORM_INFO support. I think the way to
>>> handle this is to make MSR_PLATFORM_INFO writeable (but only from
>>> userspace) so that hypervisors can defend themselves (by setting this
>>> MSR to 0).
>>
>> Right---and with my QEMU hat on, this feature will have to be enabled
>> manually on the command line because of the way QEMU supports running
>> with old kernels. :( This however does not impact the KVM patch.
>>
>> We may decide that, because CPUID faulting doesn't have a CPUID bit and
>> is relatively a "fringe" feature, we are okay if the kernel enables this
>> unconditionally and then userspace can arrange to block migration (in
>> QEMU this would use a subsection). David, Eduardo, opinions?
>
> Sounds reasonable. Accurate CPU virtualization might be another reason
> to disable this feature from userspace.
>
> My worry is a kernel rollback, where migrating to an older kernel
> version is unavoidable.
>
>>
>>>
>>>> + case MSR_MISC_FEATURES_ENABLES:
>>>> + msr_info->data = 0;
>>>> + if (vcpu->arch.cpuid_fault)
>>>> + msr_info->data |= CPUID_FAULT_ENABLE;
>>>> + break;
>>>
>>> MSR_MISC_FEATURES_ENABLES should be added to emulated_msrs[] so that
>>> the hypervisor will maintain the value of CPUID_FAULT_ENABLE across a
>>> save/restore.
>>
>> This is definitely necessary. Thanks David.
>>
>> Paolo
>>
>>>> default:
>>>> if (kvm_pmu_is_valid_msr(vcpu, msr_info->index))
>>>> return kvm_pmu_get_msr(vcpu, msr_info->index, &msr_info->data);
>>>> if (!ignore_msrs) {
>>>> vcpu_unimpl(vcpu, "unhandled rdmsr: 0x%x\n", msr_info->index);
>>>> return 1;
>>>> } else {
>>>> vcpu_unimpl(vcpu, "ignored rdmsr: 0x%x\n", msr_info->index);
>>>> @@ -7493,16 +7507,18 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>>>> kvm_update_dr0123(vcpu);
>>>> vcpu->arch.dr6 = DR6_INIT;
>>>> kvm_update_dr6(vcpu);
>>>> vcpu->arch.dr7 = DR7_FIXED_1;
>>>> kvm_update_dr7(vcpu);
>>>>
>>>> vcpu->arch.cr2 = 0;
>>>>
>>>> + vcpu->arch.cpuid_fault = false;
>>>> +
>>>> kvm_make_request(KVM_REQ_EVENT, vcpu);
>>>> vcpu->arch.apf.msr_val = 0;
>>>> vcpu->arch.st.msr_val = 0;
>>>>
>>>> kvmclock_reset(vcpu);
>>>>
>>>> kvm_clear_async_pf_completion_queue(vcpu);
>>>> kvm_async_pf_hash_reset(vcpu);
>>>> --
>>>> 2.10.2
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>

Thanks for the feedback. A new patch series is coming up.

- Kyle

\
 
 \ /
  Last update: 2016-11-06 21:54    [W:0.083 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site