Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 2/7] ppc/pnv: add a PnvChip object
On 09/01/2016 07:21 PM, Greg Kurz wrote:
> On Wed, 31 Aug 2016 18:34:10 +0200
> Cédric 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.
>>
>> 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.
>>
>> Signed-off-by: Cédric Le Goater
>> ---
>>
>> Changes since v1:
>>
>> - 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.
>>
>> Maybe this object deserves its own file hw/ppc/pnv_chip.c ?
>>
>> hw/ppc/pnv.c | 154
>> +++
>> include/hw/ppc/pnv.h | 71
>> 2 files changed, 225 insertions(+)
>>
>> 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;
>>
>> /* 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 = 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 = g_new0(PnvChip *, pnv->num_chips);
>> +for (i = 0; i < pnv->num_chips; i++) {
>> +Object *chip = object_new(chip_typename);
>
> ... this call to object_new() will abort.
>
>> +object_property_set_int(chip, CHIP_HWID(i), "chip-id",
>> &error_abort);
>> +object_property_set_bool(chip, true, "realized", &error_abort);
>> +pnv->chips[i] = 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 = DEVICE_CLASS(klass);
>> +PnvChipClass *k = PNV_CHIP_CLASS(klass);
>> +
>> +k->realize = pnv_chip_power8nvl_realize;
>> +k->cpu_model = "POWER8NVL";
>> +k->chip_type = PNV_CHIP_P8NVL;
>> +k->chip_f000f = 0x120d30498000ull;
>> +dc->desc = "PowerNV Chip POWER8NVL";
>> +}
>> +
>> +static const TypeInfo pnv_chip_power8nvl_info = {
>> +.name = TYPE_PNV_CHIP_POWER8NVL,
>> +.parent= TYPE_PNV_CHIP,
>> +.instance_size = sizeof(PnvChipPower8NVL),
>> +.class_init= 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 = DEVICE_CLASS(klass);
>> +PnvChipClass *k = PNV_CHIP_CLASS(klass);
>> +
>> +k->realize = pnv_chip_power8_realize;
>> +k->cpu_model = "POWER8";
>> +k->chip_type = PNV_CHIP_P8;
>> +k->chip_f000f = 0x220ea0498000ull;
>> +dc->desc = "PowerNV Chip POWER8";
>> +}
>> +
>> +static const TypeInfo pnv_chip_power8_info = {
>> +.name = TYPE_PNV_CHIP_POWER8,
>> +.parent= TYPE_PNV_CHIP,
>> +.instance_size = sizeof(PnvChipPower8),
>> +.class_init= 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 = DEVICE_CLASS(klass);
>> +PnvChipClass *k = PNV_CHIP_CLASS(klass);
>> +
>> +k->realize = pnv_chip_power8e_realize;
>> +k->cpu_model = "POWER8E";
>> +k->chip_type = PNV_CHIP_P8E;
>> +k->chip_f000f = 0x221ef0498000ull;
>> +dc->desc = "PowerNV Chip POWER8E";
>> +}
>> +
>> +static const TypeInfo pnv_chip_power8e_info = {
>> +.name = TYPE_PNV_CHIP_POWER8E,
>> +.parent= TYPE_PNV_CHIP,
>> +.instance_size = sizeof(PnvChipPower8e),
>> +.class_init= pnv_chip_power8e_class_init,
>> +};
>> +
>> +static void pnv_chip_realize(DeviceState *dev, Error **errp)
>> +{
>> +PnvChip *chip = PNV_CHIP(dev);
>> +PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
>> +
>> +pcc-
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 2/7] ppc/pnv: add a PnvChip object
On Wed, 31 Aug 2016 18:34:10 +0200
Cédric 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.
>
> 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.
>
> Signed-off-by: Cédric Le Goater
> ---
>
> Changes since v1:
>
> - 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.
>
> Maybe this object deserves its own file hw/ppc/pnv_chip.c ?
>
> hw/ppc/pnv.c | 154
> +++
> include/hw/ppc/pnv.h | 71
> 2 files changed, 225 insertions(+)
>
> 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;
>
> /* 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 = 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 = g_new0(PnvChip *, pnv->num_chips);
> +for (i = 0; i < pnv->num_chips; i++) {
> +Object *chip = object_new(chip_typename);
... this call to object_new() will abort.
> +object_property_set_int(chip, CHIP_HWID(i), "chip-id", &error_abort);
> +object_property_set_bool(chip, true, "realized", &error_abort);
> +pnv->chips[i] = 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 = DEVICE_CLASS(klass);
> +PnvChipClass *k = PNV_CHIP_CLASS(klass);
> +
> +k->realize = pnv_chip_power8nvl_realize;
> +k->cpu_model = "POWER8NVL";
> +k->chip_type = PNV_CHIP_P8NVL;
> +k->chip_f000f = 0x120d30498000ull;
> +dc->desc = "PowerNV Chip POWER8NVL";
> +}
> +
> +static const TypeInfo pnv_chip_power8nvl_info = {
> +.name = TYPE_PNV_CHIP_POWER8NVL,
> +.parent= TYPE_PNV_CHIP,
> +.instance_size = sizeof(PnvChipPower8NVL),
> +.class_init= 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 = DEVICE_CLASS(klass);
> +PnvChipClass *k = PNV_CHIP_CLASS(klass);
> +
> +k->realize = pnv_chip_power8_realize;
> +k->cpu_model = "POWER8";
> +k->chip_type = PNV_CHIP_P8;
> +k->chip_f000f = 0x220ea0498000ull;
> +dc->desc = "PowerNV Chip POWER8";
> +}
> +
> +static const TypeInfo pnv_chip_power8_info = {
> +.name = TYPE_PNV_CHIP_POWER8,
> +.parent= TYPE_PNV_CHIP,
> +.instance_size = sizeof(PnvChipPower8),
> +.class_init= 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 = DEVICE_CLASS(klass);
> +PnvChipClass *k = PNV_CHIP_CLASS(klass);
> +
> +k->realize = pnv_chip_power8e_realize;
> +k->cpu_model = "POWER8E";
> +k->chip_type = PNV_CHIP_P8E;
> +k->chip_f000f = 0x221ef0498000ull;
> +dc->desc = "PowerNV Chip POWER8E";
> +}
> +
> +static const TypeInfo pnv_chip_power8e_info = {
> +.name = TYPE_PNV_CHIP_POWER8E,
> +.parent= TYPE_PNV_CHIP,
> +.instance_size = sizeof(PnvChipPower8e),
> +.class_init= pnv_chip_power8e_class_init,
> +};
> +
> +static void pnv_chip_realize(DeviceState *dev, Error **errp)
> +{
> +PnvChip *chip = PNV_CHIP(dev);
> +PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> +
> +pcc->realize(chip, errp);
> +}
> +
> +static Property pnv_chip_properties[] = {
> +DEFINE_PROP_UINT32("chip-id", PnvChip, chip_id, 0),
> +DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void pnv_chi
