From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59171) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLyON-0008Gw-DG for qemu-devel@nongnu.org; Wed, 10 Oct 2012 11:40:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TLyOJ-0006vc-Bu for qemu-devel@nongnu.org; Wed, 10 Oct 2012 11:40:14 -0400 Received: from cantor2.suse.de ([195.135.220.15]:45055 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLyOJ-0006qa-1s for qemu-devel@nongnu.org; Wed, 10 Oct 2012 11:40:11 -0400 Message-ID: <507596D2.2000505@suse.de> Date: Wed, 10 Oct 2012 17:40:02 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1348497138-2516-1-git-send-email-Don@CloudSwitch.com> <1348497138-2516-4-git-send-email-Don@CloudSwitch.com> <20121009162520.GB12330@amt.cnet> <5074773F.6070503@CloudSwitch.Com> <507592C0.6030107@CloudSwitch.Com> In-Reply-To: <507592C0.6030107@CloudSwitch.Com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v6 03/16] target-i386: Add cpu object access routines for Hypervisor level. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Don Slutz , Igor Mammedov Cc: peter.maydell@linaro.org, ehabkost@redhat.com, kvm@vger.kernel.org, Marcelo Tosatti , qemu-devel@nongnu.org, avi@redhat.com, anthony@codemonkey.ws Am 10.10.2012 17:22, schrieb Don Slutz: > On 10/09/12 15:13, Don Slutz wrote: >> On 10/09/12 12:25, Marcelo Tosatti wrote: >>> On Mon, Sep 24, 2012 at 10:32:05AM -0400, Don Slutz wrote: >>>> +static void x86_cpuid_set_hv_level(Object *obj, Visitor *v, void >>>> *opaque, >>>> + const char *name, Error **errp) >>>> +{ >>>> + X86CPU *cpu =3D X86_CPU(obj); >>>> + uint32_t value; >>>> + >>>> + visit_type_uint32(v, &value, name, errp); >>>> + if (error_is_set(errp)) { >>>> + return; >>>> + } >>>> + >>>> + if (value !=3D 0 && value < 0x40000000) { >>>> + value +=3D 0x40000000; >>>> + } >>> Whats the purpose of this? Adds ambiguity. [...] > This is direct copy with adjustment from x86_cpuid_set_xlevel(): >=20 > if (value < 0x80000000) { > value +=3D 0x80000000; > } >=20 > (Pending patch: > http://comments.gmane.org/gmane.comp.emulators.qemu/172703 adds this) (Any pending patch can be changed ;)) > The adjustment is that 0 is a legal value. See > http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html >=20 > This does mean that just like xlevel=3D1 and xlevel=3D0x80000001 are th= e > same; hypervisor-level=3D1 and hypervisor-level=3D0x4000001 are the sam= e.=20 > If this is not wanted, I have no issue with removing it. I have no strong opinion either way, but if there's only one call site, I'd prefer to apply these fixups to user input before setting the property and to have the property setter error out on invalid values. I consider that cleaner than silently fixing up values inside the setter. Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg