On 08/16/2016 04:21 AM, David Gibson wrote:
> On Fri, Aug 05, 2016 at 11:15:36AM +0200, Cédric 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.
>>
>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
>> ---
>> 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 = POWERNV_MACHINE(machine);
>> long fw_size;
>> char *filename;
>> + int i;
>>
>> 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 = 0;
>> pnv->initrd_size = 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 = 1;
>> +
>> + pnv->chips = g_new0(PnvChip, pnv->num_chips);
>> + for (i = 0; i < pnv->num_chips; i++) {
>> + PnvChip *chip = &pnv->chips[i];
>> +
>> + object_initialize(chip, sizeof(*chip), TYPE_PNV_CHIP);
>
> I think you'd be better off having an array of pointers, each one you
> allocate with object_new() rather than doing an explicit g_new0() for
> the whole array then using object_initialize().
>
> For one thing, if certain chip subtypes need to allocate more space
> for their instance, then this approach will break, whereas
> object_new() will get that right.
ok. I did not know that. This is the approach I have taken in the
new patchset but for another reason. I made the type of the PnvChip
object depend on the cpu_model. It should be useful for other chiplets
which can behave differently.
A first specificity to handle is the support of the LPC interrupts via
the LPC controller for the Power8NVL chip. Ben just worked that out.
I need to see how this comes together with the model I have.
Thanks,
C.
>> + object_property_set_int(OBJECT(chip), i, "chip-id", &error_abort);
>> + object_property_set_bool(OBJECT(chip), true, "realized",
>> &error_abort);
>> + }
>> }
>>
>> static void powernv_machine_class_init(ObjectClass *oc, void *data)
>> @@ -274,10 +292,39 @@ static const TypeInfo powernv_machine_2_8_info = {
>> .class_init = powernv_machine_2_8_class_init,
>> };
>>
>> +
>> +static void pnv_chip_realize(DeviceState *dev, Error **errp)
>> +{
>> + ;
>> +}
>> +
>> +static Property pnv_chip_properties[] = {
>> + 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 = DEVICE_CLASS(klass);
>> +
>> + dc->realize = pnv_chip_realize;
>> + dc->props = pnv_chip_properties;
>> + dc->desc = "PowerNV Chip";
>> + }
>> +
>> +static const TypeInfo pnv_chip_info = {
>> + .name = TYPE_PNV_CHIP,
>> + .parent = TYPE_SYS_BUS_DEVICE,
>> + .instance_size = sizeof(PnvChip),
>> + .class_init = 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);
>> }
>>
>> 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
>>
>> #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;
>>
>> #define TYPE_POWERNV_MACHINE "powernv-machine"
>> #define POWERNV_MACHINE(obj) \
>> @@ -31,6 +43,9 @@ typedef struct sPowerNVMachineState {
>>
>> uint32_t initrd_base;
>> long initrd_size;
>> +
>> + uint32_t num_chips;
>> + PnvChip *chips;
>> } sPowerNVMachineState;
>>
>> #endif /* _PPC_PNV_H */
>
