From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [PATCH v6 05/16] target-i386: Use Hypervisor level in -machine pc,accel=kvm. Date: Wed, 10 Oct 2012 09:03:20 -0400 Message-ID: <50757218.6080503@CloudSwitch.Com> References: <1348497138-2516-1-git-send-email-Don@CloudSwitch.com> <1348497138-2516-6-git-send-email-Don@CloudSwitch.com> <20121009171805.GA26004@amt.cnet> <20121009182726.GA27536@amt.cnet> <20121009184753.GA1403@amt.cnet> <5074765D.8000208@CloudSwitch.Com> <20121009191136.GD2927@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Cc: , , , , , , , To: Marcelo Tosatti Return-path: Received: from hub021-nj-1.exch021.serverdata.net ([206.225.164.216]:43318 "EHLO hub021-nj-1.exch021.serverdata.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755907Ab2JJNDW (ORCPT ); Wed, 10 Oct 2012 09:03:22 -0400 In-Reply-To: <20121009191136.GD2927@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: On 10/09/12 15:11, Marcelo Tosatti wrote: > On Tue, Oct 09, 2012 at 03:09:17PM -0400, Don Slutz wrote: >> On 10/09/12 14:47, Marcelo Tosatti wrote: >>> On Tue, Oct 09, 2012 at 03:27:26PM -0300, Marcelo Tosatti wrote: >>>> On Tue, Oct 09, 2012 at 02:18:05PM -0300, Marcelo Tosatti wrote: >>>>> On Mon, Sep 24, 2012 at 10:32:07AM -0400, Don Slutz wrote: >>>>>> Also known as Paravirtualization level. >>>>>> >>>>>> This change is based on: >>>>>> >>>>>> Microsoft Hypervisor CPUID Leaves: >>>>>> http://msdn.microsoft.com/en-us/library/windows/hardware/ff542428%28v=vs.85%29.aspx >>>>>> >>>>>> Linux kernel change starts with: >>>>>> http://fixunix.com/kernel/538707-use-cpuid-communicate-hypervisor.html >>>>>> Also: >>>>>> http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html >>>>>> >>>>>> VMware documention on CPUIDs (Mechanisms to determine if software is >>>>>> running in a VMware virtual machine): >>>>>> http://kb.vmware.com/selfservice/microsites/search.do?language=en_US&cmd=displayKC&externalId=1009458 >>>>>> >>>>>> QEMU knows this is KVM_CPUID_SIGNATURE (0x40000000). >>>>>> >>>>>> Signed-off-by: Don Slutz >>>>>> --- >>>>>> target-i386/kvm.c | 4 ++-- >>>>>> 1 files changed, 2 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c >>>>>> index 895d848..8462c75 100644 >>>>>> --- a/target-i386/kvm.c >>>>>> +++ b/target-i386/kvm.c >>>>>> @@ -389,12 +389,12 @@ int kvm_arch_init_vcpu(CPUX86State *env) >>>>>> c = &cpuid_data.entries[cpuid_i++]; >>>>>> memset(c, 0, sizeof(*c)); >>>>>> c->function = KVM_CPUID_SIGNATURE >>>>>> - if (!hyperv_enabled()) { >>>>>> + if (!env->cpuid_hv_level_set) { >>>>>> memcpy(signature, "KVMKVMKVM\0\0\0", 12); >>>>>> c->eax = 0; >>>>>> } else { >>>>>> memcpy(signature, "Microsoft Hv", 12); >>>>>> - c->eax = HYPERV_CPUID_MIN; >>>>>> + c->eax = env->cpuid_hv_level; >>>>> This breaks hyperv_enabled() checks. >>>>> >>>>> Don, are you certain it is worthwhile to make this configurable? >>>>> Can you explain why, under your scenario, it is worthwhile? >>>>> >>>>> Because these are separate problems: >>>>> >>>>> - "Fake" VMWare hypervisor (which seems to be your main goal). >>>>> - Make CPUID HV leafs configurable via command line. >>>> Err, meant via properties. Point is, why have VMWare CPUID >>>> configuration as data, if there are reasons to believe code >>>> is a better fit (code as in current Hyper-V implementation). >>> Nevermind, its the right thing to do. Just separate the patchset >>> please: >>> >>> 1) Create object properties. >>> 2) Export VMWare CPUID via properties. >>> 3) Convert Hyper-V. >>> >>> Be careful to make sure Hyper-V's current options are functional >>> in 3). >>> >> Did you mean 3 patch sets (or more)? Or just a different order? >> -Don Slutz > Different order. Patches should be logically related (think of what > information the reviewer needs). Please write changelogs for > every patch. > > Using this order causes support for Hyper-V to stop working in the middle of the patch set. How about this order: 1) Create object properties. 2) Convert Hyper-V to set the new properties. 3) Change kvm.c to use the new properties. 4) Export VMWare CPUID via properties. 5) Change accel=tcg to use the new properties. -Don Slutz