From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45095) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f1GCa-0008H3-9y for qemu-devel@nongnu.org; Wed, 28 Mar 2018 14:53:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1f1GCV-0003nO-E9 for qemu-devel@nongnu.org; Wed, 28 Mar 2018 14:53:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59198) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1f1GCV-0003nE-4W for qemu-devel@nongnu.org; Wed, 28 Mar 2018 14:53:35 -0400 Date: Wed, 28 Mar 2018 15:53:31 -0300 From: Eduardo Habkost Message-ID: <20180328185331.GN5046@localhost.localdomain> References: <20180328153024.23039-1-rkagan@virtuozzo.com> <20180328153024.23039-3-rkagan@virtuozzo.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180328153024.23039-3-rkagan@virtuozzo.com> Subject: Re: [Qemu-devel] [PATCH for-2.12 v2 2/2] i386/hyperv: error out if features requested but unsupported List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roman Kagan Cc: qemu-devel@nongnu.org, Paolo Bonzini , Vitaly Kuznetsov On Wed, Mar 28, 2018 at 06:30:24PM +0300, Roman Kagan wrote: > In order to guarantee compatibility on migration, QEMU should have > complete control over the features it announces to the guest via CPUID. > > However, for a number of Hyper-V-related cpu properties, if the > corresponding feature is not supported by the underlying KVM, the > propery is silently ignored and the feature is not announced to the > guest. > > Refuse to start with an error instead. > > Signed-off-by: Roman Kagan Something I didn't consider before: Will this block migration before it even starts, or will crash the VM only after all migration data was sent to the destination? I didn't test it, but kvm_arch_init_vcpu() seems to be too late to block an invalid/unsupport configuration. Maybe we can simply call hyperv_handle_properties() earlier, inside x86_cpu_realizefn()? (I know it's very late for this kind of intrusive change in v2.12, but I still think it's a good idea to fix this as soon as possible.) > --- > v1 -> v2: > - indicate what flag requested the feature that can't be enabled in the > error message > - fix a typo in the error message for VP_RUNTIME > > target/i386/kvm.c | 32 ++++++++++++++++++++++++++++---- > 1 file changed, 28 insertions(+), 4 deletions(-) > > diff --git a/target/i386/kvm.c b/target/i386/kvm.c > index b35623ae24..113926aff2 100644 > --- a/target/i386/kvm.c > +++ b/target/i386/kvm.c > @@ -659,17 +659,41 @@ static int hyperv_handle_properties(CPUState *cs) > env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS; > env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE; > } > - if (cpu->hyperv_crash && has_msr_hv_crash) { > + if (cpu->hyperv_crash) { > + if (!has_msr_hv_crash) { > + fprintf(stderr, "Hyper-V crash MSRs " > + "(requested by 'hv-crash' cpu flag) " > + "are not supported by kernel\n"); > + return -ENOSYS; > + } > env->features[FEAT_HYPERV_EDX] |= HV_GUEST_CRASH_MSR_AVAILABLE; > } > env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE; > - if (cpu->hyperv_reset && has_msr_hv_reset) { > + if (cpu->hyperv_reset) { > + if (!has_msr_hv_reset) { > + fprintf(stderr, "Hyper-V reset MSR " > + "(requested by 'hv-reset' cpu flag) " > + "is not supported by kernel\n"); > + return -ENOSYS; > + } > env->features[FEAT_HYPERV_EAX] |= HV_RESET_AVAILABLE; > } > - if (cpu->hyperv_vpindex && has_msr_hv_vpindex) { > + if (cpu->hyperv_vpindex) { > + if (!has_msr_hv_vpindex) { > + fprintf(stderr, "Hyper-V VP_INDEX MSR " > + "(requested by 'hv-vpindex' cpu flag) " > + "is not supported by kernel\n"); > + return -ENOSYS; > + } > env->features[FEAT_HYPERV_EAX] |= HV_VP_INDEX_AVAILABLE; > } > - if (cpu->hyperv_runtime && has_msr_hv_runtime) { > + if (cpu->hyperv_runtime) { > + if (!has_msr_hv_runtime) { > + fprintf(stderr, "Hyper-V VP_RUNTIME MSR " > + "(requested by 'hv-runtime' cpu flag) " > + "is not supported by kernel\n"); > + return -ENOSYS; > + } > env->features[FEAT_HYPERV_EAX] |= HV_VP_RUNTIME_AVAILABLE; > } > if (cpu->hyperv_synic) { > -- > 2.14.3 > -- Eduardo