From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57673) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V7NYQ-0004RK-Dn for qemu-devel@nongnu.org; Thu, 08 Aug 2013 06:34:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V7NYI-00078z-NU for qemu-devel@nongnu.org; Thu, 08 Aug 2013 06:34:50 -0400 Message-ID: <5203743D.6060009@suse.de> Date: Thu, 08 Aug 2013 12:34:37 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <20130808125106.6b29a78c@zephyr> <20130808125629.4e5f435d@zephyr> In-Reply-To: <20130808125629.4e5f435d@zephyr> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/2] [v3] target-ppc: Enhance CPU nodes of device tree to be PAPR compliant. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Prerna Saxena Cc: qemu-ppc , Alexey Kardashevisky , QEMU , Anthony Liguori , Alexander Graf Am 08.08.2013 09:26, schrieb Prerna Saxena: >=20 > From: Prerna Saxena > Date: Thu, 8 Aug 2013 06:38:03 +0530 > Subject: [PATCH 2/2] Enhance CPU nodes of device tree to be PAPR compli= ant. >=20 > This is based on patch from Andreas which enables the default CPU with = KVM > to show up as "-cpu ", such as "POWER7_V2.3@0" >=20 > While this is definitely, more descriptive, PAPR mandates the device tr= ee CPU > node names to be of the form : "PowerPC," where should not= have > underscores. > Hence replacing the CPU model (which has underscores) with CPU alias. >=20 > With this patch, the CPU nodes of device tree show up as : > /proc/device-tree/cpus/PowerPC,POWER7@0/... > /proc/device-tree/cpus/PowerPC,POWER7@4/... >=20 > Signed-off-by: Prerna Saxena Not yet happy... > --- > hw/ppc/spapr.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 59e2fea..8efd84e 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -43,6 +43,7 @@ > #include "hw/pci-host/spapr.h" > #include "hw/ppc/xics.h" > #include "hw/pci/msi.h" > +#include "cpu-models.h" > =20 > #include "hw/pci/pci.h" > =20 > @@ -80,6 +81,8 @@ > =20 > #define HTAB_SIZE(spapr) (1ULL << ((spapr)->htab_shift)) > =20 > +#define PPC_DEVTREE_STR "PowerPC," > + > sPAPREnvironment *spapr; > =20 > int spapr_allocate_irq(int hint, bool lsi) > @@ -322,9 +325,16 @@ static void *spapr_create_fdt_skel(const char *cpu= _model, > _FDT((fdt_property_cell(fdt, "#address-cells", 0x1))); > _FDT((fdt_property_cell(fdt, "#size-cells", 0x0))); > =20 > - modelname =3D g_strdup(cpu_model); > + /* > + * PAPR convention mandates that > + * Device tree nodes must be named as: > + * PowerPC,CPU-NAME@... > + * Also, CPU-NAME must not have underscores.(hence use of CPU-ALIA= S) > + */ > + > + modelname =3D g_strdup_printf(PPC_DEVTREE_STR "%s", cpu_model); > =20 > - for (i =3D 0; i < strlen(modelname); i++) { > + for (i =3D strlen(PPC_DEVTREE_STR); i < strlen(modelname); i++) { > modelname[i] =3D toupper(modelname[i]); > } > =20 One of your colleagues had brought up that "PowerPC," prefix were not mandatory - is it *required* by the PAPR spec now, or is it just that the IBM CPUs used with PAPR happen to have such a name? > @@ -1315,6 +1325,14 @@ static void ppc_spapr_init(QEMUMachineInitArgs *= args) > =20 > cpu_model =3D g_strndup(parent_name, > strlen(parent_name) - strlen("-" TYPE_POWERPC_CPU)); > + > + for (i =3D 0; ppc_cpu_aliases[i].model !=3D NULL; i++) { > + if (strcmp(ppc_cpu_aliases[i].model, cpu_model) =3D=3D 0) = { > + g_free(cpu_model); > + cpu_model =3D g_strndup(ppc_cpu_aliases[i].alias, > + strlen(ppc_cpu_aliases[i].alias)); > + } > + } > } > =20 > /* Prepare the device tree */ This is still fixing up the name in the wrong place: -cpu POWER7_v2.3 will not get fixed, only -cpu host or KVM's default. The solution I had discussed with Alex is the following: When devices need to expose their name to firmware in a special way, we have the DeviceClass::fw_name field. All we have to do is assign it and use it instead of cpu_model if non-NULL, just like we assign DeviceClass::desc. The way to do it would be to extend the family of POWERPC_DEF* macros to specify the additional field on the relevant CPU models. Therefore my above question: Would it be sufficient to explicitly name POWER7_v2.3 PowerPC,POWER7 etc. and to drop the upper-casing? Or would we also need to name a CPU such as MPC8572E (random Freescale CPU where I don't know the expected fw_name and that is unlikely to occur/work in sPAPR) "PowerPC,MPC8572E" if someone specified it with -cpu MPC8572E? 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