From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40347) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cTsoV-00088L-D0 for qemu-devel@nongnu.org; Wed, 18 Jan 2017 11:10:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cTsoP-0002OQ-Gd for qemu-devel@nongnu.org; Wed, 18 Jan 2017 11:10:19 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56612) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cTsoP-0002NG-BS for qemu-devel@nongnu.org; Wed, 18 Jan 2017 11:10:13 -0500 References: <1484749457-87117-1-git-send-email-phil@philjordan.eu> From: Paolo Bonzini Message-ID: Date: Wed, 18 Jan 2017 17:10:09 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] x86-KVM: Supply TSC and APIC clock rates to guest like VMWare List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Phil Dennis-Jordan Cc: qemu-devel@nongnu.org, Richard Henderson , Eduardo Habkost , Marcelo Tosatti , kvm@vger.kernel.org On 18/01/2017 17:02, Phil Dennis-Jordan wrote: > > One issue is that the TSC frequency can change, for example on > migration. Telling the guest about the TSC frequency makes little sense > if it can change. > > That makes sense. Darwin can't handle changing TSC frequencies in any > case, regardless of cpuid leaf 0x40000010. Do I deduce correctly from > the following code (lines 967~977) that this bit inhibits migration > intrinsically, so other than depending on it, I don't need to > specifically disable migration for this option? Correct. > So the leaf should be conditional on the INVTSC feature > (CPUID[0x80000007].EDX bit 8). You can enable this unconditionally for > new machine types (i.e. making it true here, and turning it off in > include/hw/i386/pc.h's PC_COMPAT_2_8 macro), but only expose it if that > bit is also set. > > Sorry, you've lost me here. Would you mind explaining in a little more > detail? What would I be enabling unconditionally? (I'm getting lost on > what the various 'this'/'that'/'it' are referring to.) You enable vmware-cpuid-freq unconditionally. But then you actually publish 0x40000010 only if INVTSC is set. > > > + if (cpu->vmware_clock_rates) { > > ^^ Here is where you should also check invtsc. > > > + if (cpu->expose_kvm > > I think this should not depend on cpu->expose_kvm. This is not a KVM > leaf, it's a vmware leaf; if it were a KVM leaf, it would obey kvm_base. > Of course checking kvm_base is still a good idea, to avoid stomping on > Hyper-V's CPUID space. > > Hmm, my thinking here is that leaf 0x40000000 only is published if kvm > or Hyper-V is exposed. Without 0x40000000, Darwin won't find 0x40000010. Of course you're right, but please add a comment like this: /* Guests depend on 0x40000000 to detect this, so do not expose * it unless that leaf is present. */ Paolo