From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42289) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bfVgt-0007Ox-H0 for qemu-devel@nongnu.org; Thu, 01 Sep 2016 13:22:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bfVgi-0000yZ-0t for qemu-devel@nongnu.org; Thu, 01 Sep 2016 13:22:14 -0400 Received: from 8.mo179.mail-out.ovh.net ([46.105.75.26]:41376) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bfVgh-0000yE-ON for qemu-devel@nongnu.org; Thu, 01 Sep 2016 13:22:03 -0400 Received: from player698.ha.ovh.net (b7.ovh.net [213.186.33.57]) by mo179.mail-out.ovh.net (Postfix) with ESMTP id DAAB0100C2A4 for ; Thu, 1 Sep 2016 19:22:02 +0200 (CEST) Date: Thu, 1 Sep 2016 19:21:53 +0200 From: Greg Kurz Message-ID: <20160901192153.482b9a3e@bahia.lan> In-Reply-To: <1472661255-20160-3-git-send-email-clg@kaod.org> References: <1472661255-20160-1-git-send-email-clg@kaod.org> <1472661255-20160-3-git-send-email-clg@kaod.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 2/7] ppc/pnv: add a PnvChip object List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?Q8OpZHJpYw==?= Le Goater Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, David Gibson On Wed, 31 Aug 2016 18:34:10 +0200 C=C3=A9dric Le Goater wrote: > This is is an abstraction of a POWER8 chip which is a set of cores > plus other 'units', like the pervasive unit, the interrupt controller, > the memory controller, the on-chip microcontroller, etc. The whole can > be seen as a socket. It depends on a cpu model and its characteristics, > max cores, specific init are defined in a PnvChipClass. >=20 > We start with an near empty PnvChip with only a few cpu constants > which we will grow in the subsequent patches with the controllers > required to run the system. >=20 > Signed-off-by: C=C3=A9dric Le Goater > --- >=20 > Changes since v1: > =20 > - introduced a PnvChipClass depending on the cpu model. It also > provides some chip constants used by devices, like the cpu model hw > id (f000f), a enum type (not sure this is useful yet), a custom > realize ops for customization. > - the num-chips property can be configured on the command line. > =20 > Maybe this object deserves its own file hw/ppc/pnv_chip.c ?=20 >=20 > hw/ppc/pnv.c | 154 +++++++++++++++++++++++++++++++++++++++++++++= ++++++ > include/hw/ppc/pnv.h | 71 ++++++++++++++++++++++++ > 2 files changed, 225 insertions(+) >=20 > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index 70413e3c5740..06051268e200 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -168,6 +168,8 @@ static void ppc_powernv_init(MachineState *machine) > char *fw_filename; > long fw_size; > long kernel_size; > + int i; > + char *chip_typename; > =20 > /* allocate RAM */ > if (ram_size < (1 * G_BYTE)) { > @@ -212,6 +214,153 @@ static void ppc_powernv_init(MachineState *machine) > exit(1); > } > } > + > + /* Create the processor chips */ > + chip_typename =3D g_strdup_printf(TYPE_PNV_CHIP "-%s", machine->cpu_= model); > + You should check that the CPU model is supported by this machine (with object_class_by_name()) and error out if it is not, or... > + pnv->chips =3D g_new0(PnvChip *, pnv->num_chips); > + for (i =3D 0; i < pnv->num_chips; i++) { > + Object *chip =3D object_new(chip_typename); ... this call to object_new() will abort. > + object_property_set_int(chip, CHIP_HWID(i), "chip-id", &error_ab= ort); > + object_property_set_bool(chip, true, "realized", &error_abort); > + pnv->chips[i] =3D PNV_CHIP(chip); > + } > + g_free(chip_typename); > +} > + > +static void pnv_chip_power8nvl_realize(PnvChip *chip, Error **errp) > +{ > + ; > +} > + > +static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc =3D DEVICE_CLASS(klass); > + PnvChipClass *k =3D PNV_CHIP_CLASS(klass); > + > + k->realize =3D pnv_chip_power8nvl_realize; > + k->cpu_model =3D "POWER8NVL"; > + k->chip_type =3D PNV_CHIP_P8NVL; > + k->chip_f000f =3D 0x120d304980000000ull; > + dc->desc =3D "PowerNV Chip POWER8NVL"; > +} > + > +static const TypeInfo pnv_chip_power8nvl_info =3D { > + .name =3D TYPE_PNV_CHIP_POWER8NVL, > + .parent =3D TYPE_PNV_CHIP, > + .instance_size =3D sizeof(PnvChipPower8NVL), > + .class_init =3D pnv_chip_power8nvl_class_init, > +}; > + > +static void pnv_chip_power8_realize(PnvChip *chip, Error **errp) > +{ > + ; > +} > + > +static void pnv_chip_power8_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc =3D DEVICE_CLASS(klass); > + PnvChipClass *k =3D PNV_CHIP_CLASS(klass); > + > + k->realize =3D pnv_chip_power8_realize; > + k->cpu_model =3D "POWER8"; > + k->chip_type =3D PNV_CHIP_P8; > + k->chip_f000f =3D 0x220ea04980000000ull; > + dc->desc =3D "PowerNV Chip POWER8"; > +} > + > +static const TypeInfo pnv_chip_power8_info =3D { > + .name =3D TYPE_PNV_CHIP_POWER8, > + .parent =3D TYPE_PNV_CHIP, > + .instance_size =3D sizeof(PnvChipPower8), > + .class_init =3D pnv_chip_power8_class_init, > +}; > + > +static void pnv_chip_power8e_realize(PnvChip *chip, Error **errp) > +{ > + ; > +} > + > +static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc =3D DEVICE_CLASS(klass); > + PnvChipClass *k =3D PNV_CHIP_CLASS(klass); > + > + k->realize =3D pnv_chip_power8e_realize; > + k->cpu_model =3D "POWER8E"; > + k->chip_type =3D PNV_CHIP_P8E; > + k->chip_f000f =3D 0x221ef04980000000ull; > + dc->desc =3D "PowerNV Chip POWER8E"; > +} > + > +static const TypeInfo pnv_chip_power8e_info =3D { > + .name =3D TYPE_PNV_CHIP_POWER8E, > + .parent =3D TYPE_PNV_CHIP, > + .instance_size =3D sizeof(PnvChipPower8e), > + .class_init =3D pnv_chip_power8e_class_init, > +}; > + > +static void pnv_chip_realize(DeviceState *dev, Error **errp) > +{ > + PnvChip *chip =3D PNV_CHIP(dev); > + PnvChipClass *pcc =3D PNV_CHIP_GET_CLASS(chip); > + > + pcc->realize(chip, errp); > +} > + > +static Property pnv_chip_properties[] =3D { > + DEFINE_PROP_UINT32("chip-id", PnvChip, chip_id, 0), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void pnv_chip_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc =3D DEVICE_CLASS(klass); > + > + dc->realize =3D pnv_chip_realize; > + dc->props =3D pnv_chip_properties; > + dc->desc =3D "PowerNV Chip"; > + } > + > +static const TypeInfo pnv_chip_info =3D { > + .name =3D TYPE_PNV_CHIP, > + .parent =3D TYPE_SYS_BUS_DEVICE, > + .class_init =3D pnv_chip_class_init, > + .class_size =3D sizeof(PnvChipClass), > + .abstract =3D true, > +}; > + > +static char *pnv_get_num_chips(Object *obj, Error **errp) > +{ > + return g_strdup_printf("%d", POWERNV_MACHINE(obj)->num_chips); > +} > + > +static void pnv_set_num_chips(Object *obj, const char *value, Error **er= rp) > +{ > + PnvMachineState *pnv =3D POWERNV_MACHINE(obj); > + int num_chips; > + > + if (sscanf(value, "%d", &num_chips) !=3D 1) { > + error_setg(errp, "invalid num_chips property: '%s'", value); > + } > + > + /* > + * FIXME: should we decide on how many chips we can create based > + * on #cores and Venice vs. Murano vs. Naples chip type etc..., > + */ > + pnv->num_chips =3D num_chips; > +} > + > +static void powernv_machine_initfn(Object *obj) > +{ > + PnvMachineState *pnv =3D POWERNV_MACHINE(obj); > + pnv->num_chips =3D 1; > + > + object_property_add_str(obj, "num-chips", pnv_get_num_chips, > + pnv_set_num_chips, NULL); > + object_property_set_description(obj, "num-chips", > + "Specifies the number of processor c= hips", > + NULL); > } > =20 > static void powernv_machine_class_init(ObjectClass *oc, void *data) > @@ -233,12 +382,17 @@ static const TypeInfo powernv_machine_info =3D { > .name =3D TYPE_POWERNV_MACHINE, > .parent =3D TYPE_MACHINE, > .instance_size =3D sizeof(PnvMachineState), > + .instance_init =3D powernv_machine_initfn, > .class_init =3D powernv_machine_class_init, > }; > =20 > static void powernv_machine_register_types(void) > { > type_register_static(&powernv_machine_info); > + type_register_static(&pnv_chip_info); > + type_register_static(&pnv_chip_power8e_info); > + type_register_static(&pnv_chip_power8_info); > + type_register_static(&pnv_chip_power8nvl_info); > } > =20 > type_init(powernv_machine_register_types) > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h > index 31a57ed7f465..1f32573dedff 100644 > --- a/include/hw/ppc/pnv.h > +++ b/include/hw/ppc/pnv.h > @@ -20,6 +20,74 @@ > #define _PPC_PNV_H > =20 > #include "hw/boards.h" > +#include "hw/sysbus.h" > + > +#define TYPE_PNV_CHIP "powernv-chip" > +#define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP) > +#define PNV_CHIP_CLASS(klass) \ > + OBJECT_CLASS_CHECK(PnvChipClass, (klass), TYPE_PNV_CHIP) > +#define PNV_CHIP_GET_CLASS(obj) \ > + OBJECT_GET_CLASS(PnvChipClass, (obj), TYPE_PNV_CHIP) > + > +typedef enum PnvChipType { > + PNV_CHIP_P8E, /* AKA Murano (default) */ > + PNV_CHIP_P8, /* AKA Venice */ > + PNV_CHIP_P8NVL, /* AKA Naples */ > +} PnvChipType; > + > +typedef struct PnvChip { > + /*< private >*/ > + SysBusDevice parent_obj; > + > + /*< public >*/ > + uint32_t chip_id; > +} PnvChip; > + > +typedef struct PnvChipClass { > + /*< private >*/ > + SysBusDeviceClass parent_class; > + /*< public >*/ > + const char *cpu_model; > + PnvChipType chip_type; > + uint64_t chip_f000f; > + > + void (*realize)(PnvChip *dev, Error **errp); > +} PnvChipClass; > + > +#define TYPE_PNV_CHIP "powernv-chip" > + This macro is already defined above. > +#define TYPE_PNV_CHIP_POWER8E "powernv-chip-POWER8E" What about this ? #define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-POWER8E" > +#define PNV_CHIP_POWER8E(obj) \ > + OBJECT_CHECK(PnvChipPower8e, (obj), TYPE_PNV_CHIP_POWER8E) > + > +typedef struct PnvChipPower8e { > + PnvChip pnv_chip; > +} PnvChipPower8e; Why Power8e here and... > + > +#define TYPE_PNV_CHIP_POWER8 "powernv-chip-POWER8" > +#define PNV_CHIP_POWER8(obj) \ > + OBJECT_CHECK(PnvChipPower8, (obj), TYPE_PNV_CHIP_POWER8) > + > +typedef struct PnvChipPower8 { > + PnvChip pnv_chip; > +} PnvChipPower8; > + > +#define TYPE_PNV_CHIP_POWER8NVL "powernv-chip-POWER8NVL" > +#define PNV_CHIP_POWER8NVL(obj) \ > + OBJECT_CHECK(PnvChipPower8NVL, (obj), TYPE_PNV_CHIP_POWER8NVL) > + > +typedef struct PnvChipPower8NVL { > + PnvChip pnv_chip; > +} PnvChipPower8NVL; ... Power8NVL there ? > + > +/* > + * This generates a HW chip id depending on an index: > + * > + * 0x0, 0x1, 0x10, 0x11, 0x20, 0x21, ... > + * > + * Is this correct ? > + */ > +#define CHIP_HWID(i) ((((i) & 0x3e) << 3) | ((i) & 0x1)) > =20 > #define TYPE_POWERNV_MACHINE MACHINE_TYPE_NAME("powernv") > #define POWERNV_MACHINE(obj) \ > @@ -32,6 +100,9 @@ typedef struct PnvMachineState { > uint32_t initrd_base; > long initrd_size; > hwaddr fdt_addr; > + > + uint32_t num_chips; > + PnvChip **chips; > } PnvMachineState; > =20 > #endif /* _PPC_PNV_H */