From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56805) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQ56l-0006YT-Ms for qemu-devel@nongnu.org; Sun, 31 Jan 2016 22:24:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aQ56j-0001pH-AZ for qemu-devel@nongnu.org; Sun, 31 Jan 2016 22:24:55 -0500 Date: Mon, 1 Feb 2016 13:39:20 +1100 From: David Gibson Message-ID: <20160201023920.GE23043@voom.redhat.com> References: <1453960195-15181-1-git-send-email-bharata@linux.vnet.ibm.com> <1453960195-15181-9-git-send-email-bharata@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="djTb5EnuS5qMpC9h" Content-Disposition: inline In-Reply-To: <1453960195-15181-9-git-send-email-bharata@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v7 08/13] target-ppc: Introduce PowerPC specific CPU core device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: mjrosato@linux.vnet.ibm.com, mdroth@linux.vnet.ibm.com, aik@ozlabs.ru, agraf@suse.de, qemu-devel@nongnu.org, pbonzini@redhat.com, qemu-ppc@nongnu.org, tyreld@linux.vnet.ibm.com, nfont@linux.vnet.ibm.com, imammedo@redhat.com, afaerber@suse.de, ehabkost@redhat.com --djTb5EnuS5qMpC9h Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jan 28, 2016 at 11:19:50AM +0530, Bharata B Rao wrote: > CPU core device is a container of CPU thread devices. CPU hotplug is > performed at the granularity of CPU core device. When hotplugged, CPU core > creates CPU thread devices. >=20 > Signed-off-by: Bharata B Rao The basic logic here seems fine. However, I'm not really convinced that a "powerpc generic" device makes sense here. Given the diversity of powerpc platforms, it's pretty plausible they could have very different restrictions about how hotplug needs to operate. So, I think it would make more sense to go for more specific devices that either represent real hardware chips / modules, or something else that makes sense for a specific platform. So, in the first instance a "PAPR virtual module" device. It would make sense to have that derive from a more-or-less generic base class with essentially the logic below, since even if we have significantly different variants they're likely to share the basic logic. But I don't think that base type should be directly instantiable. Of course, once that's the case, there's really nothing ppc specific about the base device either. > --- > hw/ppc/Makefile.objs | 1 + > hw/ppc/cpu-core.c | 75 +++++++++++++++++++++++++++++++++++++++++= ++++++ > include/hw/ppc/cpu-core.h | 33 +++++++++++++++++++++ > 3 files changed, 109 insertions(+) > create mode 100644 hw/ppc/cpu-core.c > create mode 100644 include/hw/ppc/cpu-core.h >=20 > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > index c1ffc77..a6b7cfb 100644 > --- a/hw/ppc/Makefile.objs > +++ b/hw/ppc/Makefile.objs > @@ -21,3 +21,4 @@ obj-$(CONFIG_E500) +=3D e500.o mpc8544ds.o e500plat.o > obj-$(CONFIG_E500) +=3D mpc8544_guts.o ppce500_spin.o > # PowerPC 440 Xilinx ML507 reference board. > obj-$(CONFIG_XILINX) +=3D virtex_ml507.o > +obj-y +=3D cpu-core.o > diff --git a/hw/ppc/cpu-core.c b/hw/ppc/cpu-core.c > new file mode 100644 > index 0000000..aa96e79 > --- /dev/null > +++ b/hw/ppc/cpu-core.c > @@ -0,0 +1,75 @@ > +/* > + * PowerPC CPU core device, acts as container of CPU thread devices. > + * > + * Copyright (C) 2016 Bharata B Rao > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or la= ter. > + * See the COPYING file in the top-level directory. > + */ > +#include "hw/ppc/cpu-core.h" > +#include "hw/boards.h" > +#include > +#include "qemu/error-report.h" > + > +static int ppc_cpu_core_realize_child(Object *child, void *opaque) > +{ > + Error **errp =3D opaque; > + > + object_property_set_bool(child, true, "realized", errp); > + if (*errp) { > + return 1; > + } > + > + return 0; > +} > + > +static void ppc_cpu_core_realize(DeviceState *dev, Error **errp) > +{ > + object_child_foreach(OBJECT(dev), ppc_cpu_core_realize_child, errp); > +} > + > +static void ppc_cpu_core_class_init(ObjectClass *oc, void *data) > +{ > + DeviceClass *dc =3D DEVICE_CLASS(oc); > + > + dc->realize =3D ppc_cpu_core_realize; > + dc->desc =3D "PowerPC CPU core"; > +} > + > +static void ppc_cpu_core_instance_init(Object *obj) > +{ > + int i; > + CPUState *cpu; > + MachineState *machine =3D MACHINE(qdev_get_machine()); > + PowerPCCPUCore *core =3D POWERPC_CPU_CORE(obj); > + > + /* Create as many CPU threads as specified in the topology */ > + for (i =3D 0; i < smp_threads; i++) { > + cpu =3D cpu_generic_init(TYPE_POWERPC_CPU, machine->cpu_model); > + if (!cpu) { > + error_report("Unable to find CPU definition: %s", > + machine->cpu_model); > + exit(EXIT_FAILURE); > + } > + object_property_add_child(obj, "thread[*]", OBJECT(cpu), &error_= abort); > + object_unref(OBJECT(cpu)); > + if (!i) { > + core->thread0 =3D POWERPC_CPU(cpu); > + } > + } > +} > + > +static const TypeInfo ppc_cpu_core_type_info =3D { > + .name =3D TYPE_POWERPC_CPU_CORE, > + .parent =3D TYPE_DEVICE, > + .class_init =3D ppc_cpu_core_class_init, > + .instance_init =3D ppc_cpu_core_instance_init, > + .instance_size =3D sizeof(PowerPCCPUCore), > +}; > + > +static void cpu_core_register_types(void) > +{ > + type_register_static(&ppc_cpu_core_type_info); > +} > + > +type_init(cpu_core_register_types) > diff --git a/include/hw/ppc/cpu-core.h b/include/hw/ppc/cpu-core.h > new file mode 100644 > index 0000000..ff2ebc2 > --- /dev/null > +++ b/include/hw/ppc/cpu-core.h > @@ -0,0 +1,33 @@ > +/* > + * CPU core device. > + * > + * Copyright (C) 2016 Bharata B Rao > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or la= ter. > + * See the COPYING file in the top-level directory. > + */ > +#ifndef HW_PPC_CPU_CORE_H > +#define HW_PPC_CPU_CORE_H > + > +#include "hw/qdev.h" > + > +#ifdef TARGET_PPC64 > +#define TYPE_POWERPC_CPU_CORE "powerpc64-cpu-core" > +#elif defined(TARGET_PPCEMB) > +#define TYPE_POWERPC_CPU_CORE "embedded-powerpc-cpu-core" > +#else > +#define TYPE_POWERPC_CPU_CORE "powerpc-cpu-core" > +#endif > + > +#define POWERPC_CPU_CORE(obj) \ > + OBJECT_CHECK(PowerPCCPUCore, (obj), TYPE_POWERPC_CPU_CORE) > + > +typedef struct PowerPCCPUCore { > + /*< private >*/ > + DeviceState parent_obj; > + /*< public >*/ > + > + PowerPCCPU *thread0; > +} PowerPCCPUCore; > + > +#endif --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --djTb5EnuS5qMpC9h Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWrsVYAAoJEGw4ysog2bOSStcQAMZ9fcvapftPUnZpREOT+KNa LjW0RpOxffZ+ysKzj/MYp4u3XqZA8ezbE1IhTsABV716bdyfquRiK6sk3TU2bmUM DSKofpIcGDcKGIvcuARtxi9VP8Qol5NWTbMsz4pgRnTVALw1dc+Qpz8y8easiHIR 3gfS9mwib+jIaA9IKmeIWf1KbW8o4y+IqhsY32xWZTfAcNuR0oUEhklPQgmvj7ya MsqH7YXy+ZjVu4BwCSz6OkJWffBS0RK3jCzaYkcAJhCJG1H48ZQLJ0TmW9j7KoW3 XxomjRATGLFvHpavFZZHjsrG6zjIkiPolEvl6zMjxcj4LizhiCAJBI6VgvGlx4HU kW+XnSwJHiRaMiIYHkJo085ntcN6O053w+KadaSmzI+SZU37++P16v3Mp5lUlNa6 6Ct5WH3eJ/nrqGy8bCm9cDvNN5h64TzNLpvJMCu6xEEsHZO9TxX0euej3Iw5OUbH xjVnzyLIGZHQeqz+aoLShwSsjgPSfcldfSycYynnIZ371bm0P0xg3pl4R2RNiFZu dNTF1nr17XWV+v0L1mFK1rhVoOKlHRWSnZqO6kL0wnKcDhMkClQJ4hH3ZyJXerVl V/jAk4yw+hbB7Jh0jVRP+fLFghzCW5hJ4GYHOGHJITltLPObT+NX5QUvRGVfHSaA vXHjIDulkCvSfJNPfwcy =uN/E -----END PGP SIGNATURE----- --djTb5EnuS5qMpC9h--