From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44938) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VGRtO-00015V-Cq for qemu-devel@nongnu.org; Mon, 02 Sep 2013 07:02:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VGRtJ-0005Kw-CS for qemu-devel@nongnu.org; Mon, 02 Sep 2013 07:01:58 -0400 Received: from cantor2.suse.de ([195.135.220.15]:46681 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VGRtJ-0005Ko-3P for qemu-devel@nongnu.org; Mon, 02 Sep 2013 07:01:53 -0400 Message-ID: <5224701C.6070108@suse.de> Date: Mon, 02 Sep 2013 13:01:48 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1377617077-25174-1-git-send-email-ehabkost@redhat.com> <5220B168.8020703@suse.de> <20130830194952.GC3041@otherpad.lan.raisama.net> In-Reply-To: <20130830194952.GC3041@otherpad.lan.raisama.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2] i386: Use #defines instead of magic numbers for CPUID cache information List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: =?ISO-8859-1?Q?Beno=EEt_Canet?= , Igor Mammedov , qemu-devel@nongnu.org, liguang Am 30.08.2013 21:49, schrieb Eduardo Habkost: > On Fri, Aug 30, 2013 at 04:51:20PM +0200, Andreas F=E4rber wrote: >> Am 27.08.2013 17:24, schrieb Eduardo Habkost: >>> This is an attempt to make the CPUID cache topology code clearer, by >>> replacing the magic numbers in the code with #defines, and moving all >>> the cache information to the same place in the file. >>> >>> I took care of comparing the assembly output of compiling >>> target-i386/cpu.c before and after applying this change, to make sure >>> not a single bit was changed on cpu_x86_cpuid() before and after >>> applying this patch (unfortunately I had to manually check existing >>> differences, because of __LINE__ expansions on >>> object_class_dynamic_cast_assert() calls). >>> >>> This even keeps the code bug-compatible with the previous version: to= day >>> the cache information returned on AMD cache information leaves (CPUID >>> 0x80000005 & 0x80000006) do not match the information returned on CPU= ID >>> leaves 2 and 4. The L2 cache information on CPUID leaf 2 also doesn't >>> match the information on CPUID leaf 2. The new constants should make = it >>> easier to eventually fix those inconsistencies. All inconsistencies I >>> have found are documented in code comments. >>> >>> Signed-off-by: Eduardo Habkost >>> Reviewed-by: liguang >>> --- >>> Changes v1 -> v2: >>> * s/leafs/leaves/ on code comments >>> --- >>> target-i386/cpu.c | 184 ++++++++++++++++++++++++++++++++++++++++++++= +++------- >>> 1 file changed, 162 insertions(+), 22 deletions(-) >> >> I think this is a good idea and the code looked sane, but it is unclea= r >> to me from v1 whether Li Guang has verified as part of his review that >> all the bits match the original ones or just that Coding Style and >> general idea is okay? >> >> I'm therefore holding off applying this one for today's pull, waiting >> until either someone confirms Eduardo's results or I find the time to = do >> so myself, the former being appreciated. :) >=20 > In case anybody wants to verify it: compile it before/after applying th= e > patch, with: > make CFLAGS=3D'-save-temps -DNDEBUG > and save x86_64-softmmu/cpu.s file from each run. >=20 > You are going to see some differences between both files due to __LINE_= _ > being used as argument to object*_dynamic_cast_assert(), but nothing > else. So the only difference I see is movl lines changing slightly before object_dynamic_cast_assert() calls, so I'm applying it to qom-cpu: https://github.com/afaerber/qemu-cpu/commits/qom-cpu (Since for backporting commits I consider it more important to have consistent prefixes, I have shortened the end of the subject instead.) Thanks, 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