From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44770) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V9w2W-0003a0-KJ for qemu-devel@nongnu.org; Thu, 15 Aug 2013 07:48:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V9w2M-0006CG-6Y for qemu-devel@nongnu.org; Thu, 15 Aug 2013 07:48:28 -0400 Message-ID: <520CBFFD.5040100@suse.de> Date: Thu, 15 Aug 2013 13:48:13 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <7AAF2DA2-1FEF-4ECA-B220-366D94EC70C4@suse.de> <1376552735-2953-1-git-send-email-aik@ozlabs.ru> <520C8C11.6080905@ozlabs.ru> <98FAFA62-3B3A-40E9-8973-705C5F569CEE@suse.de> <520CB2EC.8060105@suse.de> <471DDE4C-6C6F-4F07-BCFB-8765AC88FEF4@suse.de> In-Reply-To: <471DDE4C-6C6F-4F07-BCFB-8765AC88FEF4@suse.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Anthony Liguori , Alexey Kardashevskiy , qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Paul Mackerras Am 15.08.2013 13:03, schrieb Alexander Graf: >=20 > On 15.08.2013, at 12:52, Andreas F=E4rber wrote: >=20 >> Am 15.08.2013 10:45, schrieb Alexander Graf: >>> >>> Yes, I think it makes sense to keep the full PVR around when we want = to be specific. What I'm referring to is class specific logic that can as= semble major/minor numbers from the command line. So >>> >>> -cpu POWER7,major=3D2,minor=3D0 >>> >>> would result in a PVR value that is identical to POWER7_v2.0. The ass= embly of this PVR value is class specific, because different classes of C= PUs have different semantics for their major and minor numbers. >>> >>> That way in the future we won't have to add any new version specific = CPU types but instead the user can assemble those himself, making everyon= e's life a lot easier. >>> >>> My point was that if we have that logic, we could at the same place j= ust say "if my major/minor is 0, default to something reasonable". >>> >>> But let's ask Andreas whether he has a better idea here :). >> >> If you read the previous discussion on the initial POWER7+ patch, I >> believe I had proposed major-version / minor-version or so properties = at >> family level, to be able to use different implementations or none at a= ll >> where we don't see a scheme. >=20 > Sounds like a good idea. >=20 >> However if we want to use that from -cpu as >> in your example above, we would have to implement custom parsing code >> for cpu_model, which I would rather avoid, given we want to replace it >> with -device in the future. >=20 > Can't we make this generic QOM property parsing code? >=20 > -cpu POWER7,major-version=3D2,minor-version=3D0 >=20 > would do >=20 > cpu =3D new POWER7(major-version =3D 2, minor_version =3D 0); >=20 > and then the POWER7 class can decide what to do with this additional in= formation? That is "custom parsing code for cpu_model" in target-ppc then. x86 has its own implementation and so does sparc, both not fully QOM'ified yet, so there is no one-size-fits-all. >> But maybe I didn't fully catch the exact question. :) >> >> The custom parenting strikes me as a wrong consequence of us not havin= g >> fully QOM'ified / cleaned up the family classes yet. We had discussed >> two ways: Either have, e.g., POWER7+ inherit from POWER7 (which looks >> like the only reason this is being done here) and/or have, e.g., POWER= 5+ >> copy and modify 970fx values via #defines. >=20 > IIUC the family parenting is orthogonal to this. Here we're looking at = having families as classes at all. Currently we don't - we only have expl= icit versioned implementations as classes. That's simply not true!!! All is hidden by macros as requested by you - sounds as if that was a bad idea after all. :/ We do have the following: "object" +- "device" +- "cpu" +- "powerpc64-cpu" +- "POWER7-family-powerpc64-cpu" -> POWERPC_FAMILY() +- "POWER7_v2.0-powerpc64-cpu" -> POWERPC_DEF_SVR() +- "host-powerpc64-cpu" (depending on host PVR) That's why I was saying: If we need POWER7+-specific family code, we need to have a POWER7P family and not reuse POWER7 as conveniently done today. All is there to implement properties or whatever at that level. And that's also why trying to do the parent tweaking in POWERPC_DEF_FAMILY_MEMBER() is bogus. The existing infrastructure just needs to be used the right way, sigh. And to clean up the aliases business, we should simply move them into the POWER7_v2.0-powerpc64-cpu level class as an array, I think. That would greatly simplify -cpu ?, and the alias-to-type lookup would get faster at the same time since we wouldn't be looking at unavailable models anymore. > Whether we have >=20 > PowerPC > `- POWER7 > `- POWER7+ > `- POWER7+ v1.0 >=20 > or >=20 > PowerPC > `- POWER7+ > `- POWER7+ v1.0 >=20 > is a different question I think. My question is: Why are you guys trying to create yet another type for "POWER7" when we already have one. The only plausible-to-me explanation was that avoidance of separate POWER7P family was the core cause, but apparently the core problem is that no one except me is actually grasping the macro'fied code or at least you lost the overview during your vacation... :( 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