From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50908) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wsg5M-0007jW-BF for qemu-devel@nongnu.org; Thu, 05 Jun 2014 18:24:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wsg5E-0002AP-Pc for qemu-devel@nongnu.org; Thu, 05 Jun 2014 18:24:36 -0400 Received: from cantor2.suse.de ([195.135.220.15]:53861 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wsg5E-0002AL-FB for qemu-devel@nongnu.org; Thu, 05 Jun 2014 18:24:28 -0400 Message-ID: <5390EE1A.9080503@suse.de> Date: Fri, 06 Jun 2014 00:24:26 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1401984741-26882-1-git-send-email-ehabkost@redhat.com> <539099B6.2090000@suse.de> <53909A41.1060800@redhat.com> <53909D79.1070609@suse.de> <53909E53.9050300@redhat.com> <53909E9C.6080009@suse.de> <5390A06A.9070200@redhat.com> <5390A1A9.40602@suse.de> <20140605174801.GQ17594@otherpad.lan.raisama.net> In-Reply-To: <20140605174801.GQ17594@otherpad.lan.raisama.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with "allow-emulation" option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Michael Mueller , kvm@vger.kernel.org, "Michael S. Tsirkin" , qemu-devel@nongnu.org, Christian Borntraeger , "Gabriel L. Somlo" , Borislav Petkov , "Jason J. Herne" , Paolo Bonzini , =?ISO-8859-1?Q?Andreas_?= =?ISO-8859-1?Q?F=E4rber?= On 05.06.14 19:48, Eduardo Habkost wrote: > On Thu, Jun 05, 2014 at 06:58:17PM +0200, Alexander Graf wrote: >> On 05.06.14 18:52, Paolo Bonzini wrote: >>> Il 05/06/2014 18:45, Alexander Graf ha scritto: >>>>> Only if you were using "-cpu somethingThatHasAVX", though, no? >>>> Yes. The same argument goes the other way around. I want to use AVX >>>> emulation, do "allow-emulation" and suddenly I get MONITOR/MWAIT >>>> emulation. >>> What about: >>> >>> - letting "-cpu foo,+emulatedfeature" just work >>> >>> - adding emulated=yes that blindly enables all emulated features >>> >>> - making "-cpu ...,check" prints a warning for emulated features >>> unless emulated=yes >> So: >> >> -cpu foo,+emulatedFeature just works > The problem with this is: > > * -cpu foo,+emulatedFeature,enforce MUST fail. We don't want to enable > emulated/experimental features by accident, even if they appear in > the command-line expliclty. > * -cpu foo,+emulatedFeature (no "enforce" flag) MUST remove remove the > feature and report it on the "filtered-features" property, because > that's the only API available for management to check if a feature is > not available on GET_SUPPORTED_CPUID. > > That's why I suggest that the only way to enable emulatedFeature be > using "allow-emulation=yes" explicitly on the command-line IN ADDITION > to already having the feature included in the CPU model definition or in > explicitly in the command-line. > > (or "allow-experimental-features", or whatever name we choose) > >> -cpu foo,+notEmulatedFeature still sets the CPUID bit for that feature > That's OK, but (I assume) you mean notEmulatedFeature is on > GET_SUPPORTED_CPUID. > > Detailing the requirements: > > * "-cpu foo,+SomeFeature" MUST NOT enable SomeFeature unless it is > present on GET_SUPPORTED_CPUID. > * "-cpu foo,+SomeFeature,enforce" MUST abort if the feature is not > present on GET_SUPPORTED_CPUID. > * "-cpu foo,+SomeFeature" MUST report SomeFeature on > "filtered-features", so management knows the feature is not set on > GET_SUPPORTED_CPUID. > > The items above are already part of the semantics of "-cpu" and if we > change it, we risk defeating the whole purpose of GET_EMULATED_CPUID in > the first place. > >> -cpu foo,check prints warnings for all cpuid bits not in the >> "allowed" bitmap. It prints different warnings depending on whether >> the bit is in "emulated" or not > That part makes sense. And we may also report GET_EMULATED_CPUID > features on an "emulated-features" property, so management can get that > information in a machine-friendly way. I personally like the feature=force way of specifying forcefully enabled cpuid bits. Whether it's in GET_EMULATED_CPUID doesn't matter really. Only "check" should ever care about that bitmap. But can we drop the EMULATED name somehow? Can we rename [1] the ioctl to say GET_UNSUPPORTED_CPUID or something along those lines? The name is just a really really bad pick. Alex [1] rename obviously means "introduce a new name with the same ioctl number and keep the old name around for legacy compatibility reasons"