From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58417) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V6DaJ-000612-5L for qemu-devel@nongnu.org; Mon, 05 Aug 2013 01:44:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V6DaE-0002su-4b for qemu-devel@nongnu.org; Mon, 05 Aug 2013 01:43:59 -0400 Date: Mon, 5 Aug 2013 15:43:41 +1000 From: Paul Mackerras Message-ID: <20130805054341.GE19254@iris.ozlabs.ibm.com> References: <1375412374-24701-1-git-send-email-aik@ozlabs.ru> <51FBDAF6.10302@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <51FBDAF6.10302@suse.de> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] target-ppc: Add POWER7+ CPU model List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: Alexey Kardashevskiy , Anthony Liguori , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Alexander Graf On Fri, Aug 02, 2013 at 06:14:46PM +0200, Andreas F=E4rber wrote: > Am 02.08.2013 04:59, schrieb Alexey Kardashevskiy: > > This patch adds CPU PVR definition for POWER7+. > >=20 > > Signed-off-by: Alexey Kardashevskiy > > --- > > target-ppc/cpu-models.c | 2 ++ > > target-ppc/cpu-models.h | 1 + > > 2 files changed, 3 insertions(+) > >=20 > > diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c > > index 9578ed8..c97c183 100644 > > --- a/target-ppc/cpu-models.c > > +++ b/target-ppc/cpu-models.c > > @@ -1143,6 +1143,8 @@ > > "POWER7 v2.1") > > POWERPC_DEF("POWER7_v2.3", CPU_POWERPC_POWER7_v23, = POWER7, > > "POWER7 v2.3") > > + POWERPC_DEF("POWER7P", CPU_POWERPC_POWER7P, = POWER7, > > + "POWER7P") >=20 > Subject says POWER7+ rather than POWER7P. Since this is a string there'= s > nothing wrong with +. How should it show up in SLOF? Do you mean in the device tree? The children of the /cpus node should be named like "PowerPC,POWER7+@xxx". > See also below. >=20 > > POWERPC_DEF("POWER8_v1.0", CPU_POWERPC_POWER8_v10, = POWER8, > > "POWER8 v1.0") > > POWERPC_DEF("970", CPU_POWERPC_970, = 970, > > diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h > > index 01e488f..c3c78d1 100644 > > --- a/target-ppc/cpu-models.h > > +++ b/target-ppc/cpu-models.h > > @@ -556,6 +556,7 @@ enum { > > CPU_POWERPC_POWER7_v20 =3D 0x003F0200, > > CPU_POWERPC_POWER7_v21 =3D 0x003F0201, > > CPU_POWERPC_POWER7_v23 =3D 0x003F0203, > > + CPU_POWERPC_POWER7P =3D 0x004A0201, >=20 > Shouldn't this be ..._POWER7P_v21 to align with the surrounding models? > Ditto for the model name then, POWER7+ being an alias to the latest > version only. Does the fact that all these minor revisions are enumerated imply that QEMU is always matching all the bits of the PVR value? If so then that seems very fragile to me, given that QEMU looks at the host's PVR value when using KVM. All it takes is for IBM to release a new minor revision of a CPU to break existing compiled versions of qemu (e.g. from a distro). Wouldn't it be better to be able to match only the top 16 bits, at least for the situations where there is no architectural or significant behavioural change between the versions? Paul.