[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v2 2/7] ppc/pnv: add a PnvChip object
From: |
Greg Kurz |
Subject: |
Re: [Qemu-ppc] [PATCH v2 2/7] ppc/pnv: add a PnvChip object |
Date: |
Thu, 1 Sep 2016 19:21:53 +0200 |
On Wed, 31 Aug 2016 18:34:10 +0200
Cédric Le Goater <address@hidden> 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 <address@hidden>
> ---
>
> 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 = 0x120d304980000000ull;
> + 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 = 0x220ea04980000000ull;
> + 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 = 0x221ef04980000000ull;
> + 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_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,
> + .class_init = pnv_chip_class_init,
> + .class_size = sizeof(PnvChipClass),
> + .abstract = 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 **errp)
> +{
> + PnvMachineState *pnv = POWERNV_MACHINE(obj);
> + int num_chips;
> +
> + if (sscanf(value, "%d", &num_chips) != 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 = num_chips;
> +}
> +
> +static void powernv_machine_initfn(Object *obj)
> +{
> + PnvMachineState *pnv = POWERNV_MACHINE(obj);
> + pnv->num_chips = 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
> chips",
> + NULL);
> }
>
> static void powernv_machine_class_init(ObjectClass *oc, void *data)
> @@ -233,12 +382,17 @@ static const TypeInfo powernv_machine_info = {
> .name = TYPE_POWERNV_MACHINE,
> .parent = TYPE_MACHINE,
> .instance_size = sizeof(PnvMachineState),
> + .instance_init = powernv_machine_initfn,
> .class_init = powernv_machine_class_init,
> };
>
> 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);
> }
>
> 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
>
> #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))
>
> #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;
>
> #endif /* _PPC_PNV_H */