Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752409AbcKEAJf (ORCPT ); Fri, 4 Nov 2016 20:09:35 -0400 Received: from mail-qt0-f171.google.com ([209.85.216.171]:34928 "EHLO mail-qt0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750935AbcKEAJe (ORCPT ); Fri, 4 Nov 2016 20:09:34 -0400 MIME-Version: 1.0 In-Reply-To: References: <20161101013749.4003-1-khuey@kylehuey.com> <20161101013749.4003-8-khuey@kylehuey.com> From: David Matlack Date: Fri, 4 Nov 2016 17:09:02 -0700 Message-ID: Subject: Re: [PATCH v8 7/7] KVM: x86: virtualize cpuid faulting To: Paolo Bonzini Cc: Kyle Huey , "Robert O'Callahan" , Thomas Gleixner , Andy Lutomirski , Ingo Molnar , "H. Peter Anvin" , X86 ML , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Jeff Dike , Richard Weinberger , Alexander Viro , Shuah Khan , Dave Hansen , Borislav Petkov , Peter Zijlstra , Boris Ostrovsky , Len Brown , "Rafael J. Wysocki" , Dmitry Safonov , "linux-kernel@vger.kernel.org" , Eduardo Habkost Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3163 Lines: 81 On Fri, Nov 4, 2016 at 2:57 PM, Paolo Bonzini wrote: > > On 04/11/2016 21:34, David Matlack wrote: >> On Mon, Oct 31, 2016 at 6:37 PM, Kyle Huey 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 >>