From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51408) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W6OV9-00033F-Al for qemu-devel@nongnu.org; Thu, 23 Jan 2014 12:55:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W6OV3-0006yB-5a for qemu-devel@nongnu.org; Thu, 23 Jan 2014 12:55:39 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60780) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W6OV2-0006y6-T7 for qemu-devel@nongnu.org; Thu, 23 Jan 2014 12:55:33 -0500 Message-ID: <52E1578D.4090104@redhat.com> Date: Thu, 23 Jan 2014 18:55:25 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1390484449-20974-1-git-send-email-vrozenfe@redhat.com> In-Reply-To: <1390484449-20974-1-git-send-email-vrozenfe@redhat.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 0/7] Hyper-V parameters update List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vadim Rozenfeld , qemu-devel@nongnu.org Cc: KY Srinivasan , mtosatti@redhat.com Il 23/01/2014 14:40, Vadim Rozenfeld ha scritto: > This series consists of several clean-ups, hyper-v MSRs migration > fixes, and adding support for new "hv-time" parameter, which designed > for activating hyper-v timers feature. Hi Vadim! I think patches 1-4 have some problems: (1) patches 1 and 4: the "KVMKVMKVM" and "Microsoft Hv" signatures are used by Linux to understand the format of the leaves starting at 0x40000001. These are of course different between KVM and Hyper-V. Microsoft suggests that guests detect Hyper-V by checking if eax=0x31237648 at CPUID[0x40000001]. Linux should be corrected to do this check (and KVM probably should reserve some bit, e.g. bit 16, to avoid that its own feature mask ever is 0x31237648), but anyway we cannot change the vendor signature as that conflicts with the detection scheme for KVM's own leaves. (2) patches 2 and 3 should not be applied without some command-line option or versioning scheme, because they would cause CPUID to change across migration. (3) patches 4, in addition, misses the point of KVM_CPUID_SIGNATURE_NEXT, which is to signal that the KVM_CPUID_FEATURES leaf is not at 0x40000001 but rather at 0x40000101 (KVM_CPUID_FEATURES + KVM_CPUID_SIGNATURE_NEXT - KVM_CPUID_SIGNATURE, if you want). This way, Linux can use the KVM features even if Hyper-V enlightenments enabled. Unfortunately, the logic is broken because KVM_CPUID_FEATURES is not to 0x40000101 if hyperv_enabled(). I include an untested patch to fix this at the end of the message. Luckily they are unnecessary, I'll review patches 5-7 tomorrow but at a first glance they seem good. Paolo diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 7522e98..d5cff89 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -454,6 +454,7 @@ int kvm_arch_init_vcpu(CPUState *cs) uint32_t unused; struct kvm_cpuid_entry2 *c; uint32_t signature[3]; + int kvm_base = KVM_CPUID_SIGNATURE; int r; memset(&cpuid_data, 0, sizeof(cpuid_data)); @@ -461,26 +462,22 @@ int kvm_arch_init_vcpu(CPUState *cs) cpuid_i = 0; /* Paravirtualization CPUIDs */ - c = &cpuid_data.entries[cpuid_i++]; - c->function = KVM_CPUID_SIGNATURE; - if (!hyperv_enabled(cpu)) { - memcpy(signature, "KVMKVMKVM\0\0\0", 12); - c->eax = 0; - } else { + if (hyperv_enabled(cpu)) { + c = &cpuid_data.entries[cpuid_i++]; + c->function = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS; memcpy(signature, "Microsoft Hv", 12); c->eax = HYPERV_CPUID_MIN; - } - c->ebx = signature[0]; - c->ecx = signature[1]; - c->edx = signature[2]; + c->ebx = signature[0]; + c->ecx = signature[1]; + c->edx = signature[2]; - c = &cpuid_data.entries[cpuid_i++]; - c->function = KVM_CPUID_FEATURES; - c->eax = env->features[FEAT_KVM]; - - if (hyperv_enabled(cpu)) { + c = &cpuid_data.entries[cpuid_i++]; + c->function = HYPERV_CPUID_INTERFACE; memcpy(signature, "Hv#1\0\0\0\0\0\0\0\0", 12); c->eax = signature[0]; + c->ebx = 0; + c->ecx = 0; + c->edx = 0; c = &cpuid_data.entries[cpuid_i++]; c->function = HYPERV_CPUID_VERSION; @@ -512,15 +509,21 @@ int kvm_arch_init_vcpu(CPUState *cs) c->eax = 0x40; c->ebx = 0x40; - c = &cpuid_data.entries[cpuid_i++]; - c->function = KVM_CPUID_SIGNATURE_NEXT; - memcpy(signature, "KVMKVMKVM\0\0\0", 12); - c->eax = 0; - c->ebx = signature[0]; - c->ecx = signature[1]; - c->edx = signature[2]; + kvm_base = KVM_CPUID_SIGNATURE_NEXT; } + memcpy(signature, "KVMKVMKVM\0\0\0", 12); + c = &cpuid_data.entries[cpuid_i++]; + c->function = KVM_CPUID_SIGNATURE | kvm_base; + c->eax = 0; + c->ebx = signature[0]; + c->ecx = signature[1]; + c->edx = signature[2]; + + c = &cpuid_data.entries[cpuid_i++]; + c->function = KVM_CPUID_FEATURES | kvm_base; + c->eax = env->features[FEAT_KVM]; + has_msr_async_pf_en = c->eax & (1 << KVM_FEATURE_ASYNC_PF); has_msr_pv_eoi_en = c->eax & (1 << KVM_FEATURE_PV_EOI);