From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48153) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bViJ3-0003ds-Jl for qemu-devel@nongnu.org; Fri, 05 Aug 2016 12:49:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bViJ0-0000lI-C0 for qemu-devel@nongnu.org; Fri, 05 Aug 2016 12:49:09 -0400 Received: from 5.mo69.mail-out.ovh.net ([46.105.43.105]:49318) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bViJ0-0000k7-1m for qemu-devel@nongnu.org; Fri, 05 Aug 2016 12:49:06 -0400 Received: from player696.ha.ovh.net (b7.ovh.net [213.186.33.57]) by mo69.mail-out.ovh.net (Postfix) with ESMTP id A6DB2FFB48E for ; Fri, 5 Aug 2016 18:49:04 +0200 (CEST) References: <1470388537-2908-1-git-send-email-clg@kaod.org> <1470388537-2908-3-git-send-email-clg@kaod.org> <1470390251.12584.157.camel@kernel.crashing.org> From: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= Message-ID: Date: Fri, 5 Aug 2016 18:48:59 +0200 MIME-Version: 1.0 In-Reply-To: <1470390251.12584.157.camel@kernel.crashing.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/3] ppc/pnv: add a PnvChip object List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Benjamin Herrenschmidt , qemu-ppc@nongnu.org Cc: David Gibson , Alexander Graf , qemu-devel@nongnu.org On 08/05/2016 11:44 AM, Benjamin Herrenschmidt wrote: > On Fri, 2016-08-05 at 11:15 +0200, C=C3=A9dric Le Goater wrote: >> This is is an abstraction of a P8 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. >> >> We start with an empty PnvChip which we will grow in the subsequent >> patches with controllers required to run the system.. >=20 > We should create a subclass PnvChipP8 which we instanciate for now > since P9 is around the corner and will be a bit different Yes. I gave it a try on a new 2.8 branch, check it here :=20 https://github.com/legoater/qemu/commit/60a6206c2b31d897d1af2ea3641870c2= cc0e8c41 It defines a PnvChip base class with a realize routine in which=20 we can handle specific attributes of child classes like the=20 PnvChipPower8. I will send it in v2 for review when David has=20 taken a look at v1. All the controllers are still under the base class, so all looks good. But when the PnvChipPower9 comes in play, we will need to=20 redispatch and see how it fits the purpose of supporting multiple Chip models. The core initialization should be ok but building the device=20 tree might be a bit of a burden if we have to 'cast' in the chip=20 type we need. We will see. So what would be the big differences with what we have today ? =20 Thanks, C. =20 > Cheers, > Ben. >=20 >> Signed-off-by: C=C3=A9dric Le Goater >> --- >> hw/ppc/pnv.c | 47 >> +++++++++++++++++++++++++++++++++++++++++++++++ >> include/hw/ppc/pnv.h | 15 +++++++++++++++ >> 2 files changed, 62 insertions(+) >> >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >> index 3bb6a240c25b..a680780e9dea 100644 >> --- a/hw/ppc/pnv.c >> +++ b/hw/ppc/pnv.c >> @@ -185,6 +185,7 @@ static void ppc_powernv_init(MachineState >> *machine) >> sPowerNVMachineState *pnv =3D POWERNV_MACHINE(machine); >> long fw_size; >> char *filename; >> + int i; >> =20 >> if (ram_size < (1 * G_BYTE)) { >> error_report("Warning: skiboot may not work with < 1GB of >> RAM"); >> @@ -236,6 +237,23 @@ static void ppc_powernv_init(MachineState >> *machine) >> pnv->initrd_base =3D 0; >> pnv->initrd_size =3D 0; >> } >> + >> + /* Create PowerNV chips >> + * >> + * FIXME: We should decide how many chips to create based on >> + * #cores and Venice vs. Murano vs. Naples chip type etc..., for >> + * now, just create one chip, with all the cores. >> + */ >> + pnv->num_chips =3D 1; >> + >> + pnv->chips =3D g_new0(PnvChip, pnv->num_chips); >> + for (i =3D 0; i < pnv->num_chips; i++) { >> + PnvChip *chip =3D &pnv->chips[i]; >> + >> + object_initialize(chip, sizeof(*chip), TYPE_PNV_CHIP); >> + object_property_set_int(OBJECT(chip), i, "chip-id", >> &error_abort); >> + object_property_set_bool(OBJECT(chip), true, "realized", >> &error_abort); >> + } >> } >> =20 >> static void powernv_machine_class_init(ObjectClass *oc, void *data) >> @@ -274,10 +292,39 @@ static const TypeInfo powernv_machine_2_8_info >> =3D { >> .class_init =3D powernv_machine_2_8_class_init, >> }; >> =20 >> + >> +static void pnv_chip_realize(DeviceState *dev, Error **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, >> + .instance_size =3D sizeof(PnvChip), >> + .class_init =3D pnv_chip_class_init, >> +}; >> + >> + >> static void powernv_machine_register_types(void) >> { >> type_register_static(&powernv_machine_info); >> type_register_static(&powernv_machine_2_8_info); >> + type_register_static(&pnv_chip_info); >> } >> =20 >> type_init(powernv_machine_register_types) >> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h >> index 2990f691672d..6907dc9e5c3d 100644 >> --- a/include/hw/ppc/pnv.h >> +++ b/include/hw/ppc/pnv.h >> @@ -20,6 +20,18 @@ >> #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) >> + >> +typedef struct PnvChip { >> + /*< private >*/ >> + SysBusDevice parent_obj; >> + >> + /*< public >*/ >> + uint32_t chip_id; >> +} PnvChip; >> =20 >> #define TYPE_POWERNV_MACHINE "powernv-machine" >> #define POWERNV_MACHINE(obj) \ >> @@ -31,6 +43,9 @@ typedef struct sPowerNVMachineState { >> =20 >> uint32_t initrd_base; >> long initrd_size; >> + >> + uint32_t num_chips; >> + PnvChip *chips; >> } sPowerNVMachineState; >> =20 >> #endif /* _PPC_PNV_H */