From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47114) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TxCad-0000W2-P0 for qemu-devel@nongnu.org; Mon, 21 Jan 2013 03:18:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TxCab-0005z9-Q2 for qemu-devel@nongnu.org; Mon, 21 Jan 2013 03:18:47 -0500 Received: from cantor2.suse.de ([195.135.220.15]:52240 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TxCab-0005z0-G6 for qemu-devel@nongnu.org; Mon, 21 Jan 2013 03:18:45 -0500 Message-ID: <50FCF9E0.7040407@suse.de> Date: Mon, 21 Jan 2013 09:18:40 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1358435794-8406-1-git-send-email-imammedo@redhat.com> <1358435794-8406-3-git-send-email-imammedo@redhat.com> In-Reply-To: <1358435794-8406-3-git-send-email-imammedo@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/5] target-i386: replace uint32_t vendor fields by vendor string in x86_def_t List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: qemu-devel@nongnu.org, ehabkost@redhat.com Am 17.01.2013 16:16, schrieb Igor Mammedov: > Vendor property setter takes string as vendor value but cpudefs > use uint32_t vendor[123] fields to define vendor value. It makes it > difficult to unify and use property setter for values from cpudefs. >=20 > Simplify code by using vendor property setter, vendor[123] fields > are converted into vendor[13] array to keep its value. And vendor > property setter is used to access/set value on CPU. >=20 > - Make for() cycle reusable for the next patch by adding > x86_cpu_vendor_words2str() >=20 > Intel's CPUID spec[1] says: > " > 5.1.1 ... > These registers contain the ASCII string: GenuineIntel > ... > " >=20 > List[2] of known vendor values shows that they all are 12 ASCII > characters long, padded where necessary with space >=20 > Current supported values are all ASCII characters packed in > ebx, edx, ecx. So lets state that qemu supports 12 ASCII characters > packed in ebx, edx, ecx registers for cpuid(0) instruction. >=20 > *1 - http://www.intel.com/Assets/PDF/appnote/241618.pdf > *2 - http://en.wikipedia.org/wiki/CPUID#EAX.3D0:_Get_vendor_ID >=20 > Signed-off-by: Igor Mammedov So, hearing that my suggestion of a union to give us the best of both worlds did not work out well due to endianness conversions, I would still like to drop the vendor[0] assertion. And I spot no documentation for char vendor[...] in this patch, only in the commit message; we could spare that if we change char vendor[...] array to char *vendor, what do you think? Erroring out (or padding) could be done when setting it via "vendor" property onto X86CPU (maybe I'll try to cook up something for demonstration). Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg