Hello Cédric, Sorry I missed this mail earlier.
@@ -1846,7 +1863,11 @@ static void pnv_chip_power10_quad_realize(Pnv10Chip *chip10, Error **errp)for (i = 0; i < chip10->nr_quads; i++) { PnvQuad *eq = &chip10->quads[i]; - pnv_chip_quad_realize_one(chip, eq, chip->cores[i * 4], + if (chip_class->chip_type == PNV_TYPE_POWER11) + pnv_chip_quad_realize_one(chip, eq, chip->cores[i * 4], + PNV_QUAD_TYPE_NAME("power11")); + else + pnv_chip_quad_realize_one(chip, eq, chip->cores[i * 4], PNV_QUAD_TYPE_NAME("power10"));Please change introduce a new pnv_chip_quad_realize() routine taking an extra type_name argument.Sure, can do it. But as it's called from 'pnv_chip_power10_realize', might require a 'pnv_chip_power11_realize' function also, so it can pass type_name as "power11" vs "power10".Do the Power11 and Power10 processors have the same XSCOM and MMIO address spaces ?
Yes. Hence using the same base and sizes. Thanks, Aditya Gupta
Will do it.pnv_xscom_add_subregion(chip, PNV10_XSCOM_EQ_BASE(eq->quad_id),@@ -2116,6 +2137,35 @@ static void pnv_chip_power10_class_init(ObjectClass *klass, void *data)&k->parent_realize); }+static void pnv_chip_power11_class_init(ObjectClass *klass, void *data)+{ + DeviceClass *dc = DEVICE_CLASS(klass); + PnvChipClass *k = PNV_CHIP_CLASS(klass); ++ static const int i2c_ports_per_engine[PNV10_CHIP_MAX_I2C] = {14, 14, 2, 16};+ + k->chip_cfam_id = 0x120da04900008000ull; /* P11 (with NX) */ + k->chip_type = PNV_TYPE_POWER11; + k->cores_mask = POWER11_CORE_MASK; + k->chip_pir = pnv_chip_pir_p10; + k->intc_create = pnv_chip_power10_intc_create; + k->intc_reset = pnv_chip_power10_intc_reset; + k->intc_destroy = pnv_chip_power10_intc_destroy; + k->intc_print_info = pnv_chip_power10_intc_print_info; + k->isa_create = pnv_chip_power10_isa_create; + k->dt_populate = pnv_chip_power10_dt_populate; + k->pic_print_info = pnv_chip_power10_pic_print_info; + k->xscom_core_base = pnv_chip_power10_xscom_core_base; + k->xscom_pcba = pnv_chip_power10_xscom_pcba; + dc->desc = "PowerNV Chip POWER11"; + k->num_pecs = PNV10_CHIP_MAX_PEC; + k->i2c_num_engines = PNV10_CHIP_MAX_I2C; + k->i2c_ports_per_engine = i2c_ports_per_engine; + + device_class_set_parent_realize(dc, pnv_chip_power10_realize, + &k->parent_realize); +} + static void pnv_chip_core_sanitize(PnvChip *chip, Error **errp) { PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);@@ -2510,6 +2560,22 @@ static void pnv_machine_p10_rainier_class_init(ObjectClass *oc, void *data)pmc->i2c_init = pnv_rainier_i2c_init; }+static void pnv_machine_power11_class_init(ObjectClass *oc, void *data)+{ + MachineClass *mc = MACHINE_CLASS(oc); + PnvMachineClass *pmc = PNV_MACHINE_CLASS(oc); + static const char compat[] = "qemu,powernv11\0ibm,powernv"; + + /* do power10_class_init as p11 core is same as p10 */ + pnv_machine_p10_common_class_init(oc, data); + + mc->desc = "IBM PowerNV (Non-Virtualized) POWER11"; + mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power11");This should be using a versionned CPU "power11_vx.y".Okay.+ + pmc->compat = compat; + pmc->compat_size = sizeof(compat); +} + static bool pnv_machine_get_hb(Object *obj, Error **errp) { PnvMachineState *pnv = PNV_MACHINE(obj);@@ -2613,7 +2679,23 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data).parent = TYPE_PNV10_CHIP, \ } +#define DEFINE_PNV11_CHIP_TYPE(type, class_initfn) \ + { \ + .name = type, \ + .class_init = class_initfn, \ + .parent = TYPE_PNV11_CHIP, \ + } + static const TypeInfo types[] = { + { + .name = MACHINE_TYPE_NAME("powernv11"), + .parent = TYPE_PNV_MACHINE, + .class_init = pnv_machine_power11_class_init, + .interfaces = (InterfaceInfo[]) { + { TYPE_XIVE_FABRIC }, + { }, + }, + }, { .name = MACHINE_TYPE_NAME("powernv10-rainier"), .parent = MACHINE_TYPE_NAME("powernv10"), @@ -2668,6 +2750,16 @@ static const TypeInfo types[] = { .abstract = true, }, + /* + * P11 chip and variants + */ + { + .name = TYPE_PNV11_CHIP, + .parent = TYPE_PNV10_CHIP, + .instance_size = sizeof(Pnv11Chip), + },+ DEFINE_PNV11_CHIP_TYPE(TYPE_PNV_CHIP_POWER11, pnv_chip_power11_class_init),+ /* * P10 chip and variants */ diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c index f40ab721d6fc..4522655793da 100644 --- a/hw/ppc/pnv_core.c +++ b/hw/ppc/pnv_core.c @@ -402,7 +402,16 @@ static const TypeInfo pnv_core_infos[] = { DEFINE_PNV_CORE_TYPE(power8, "power8_v2.0"), DEFINE_PNV_CORE_TYPE(power8, "power8nvl_v1.0"), DEFINE_PNV_CORE_TYPE(power9, "power9_v2.2"), - DEFINE_PNV_CORE_TYPE(power10, "power10_v2.0"), + { + .parent = TYPE_PNV_CORE, + .name = PNV_CORE_TYPE_NAME("power10_v2.0"), + .class_init = pnv_core_power10_class_init, + .class_base_init = pnv_core_power10_class_init, + },hmm, what is this change ?I expanded the macro, to add '.class_base_init', as .class_base_init will be run by QEMU during initialisation of any child classes also. And I wanted to let QEMU call 'pnv_core_power10_class_init' when it initialises 'power11' type.+ { + .parent = PNV_CORE_TYPE_NAME("power10_v2.0"), + .name = PNV_CORE_TYPE_NAME("power11"), + }nope. please use DEFINE_PNV_CORE_TYPE()Okay, then, should I have a 'pnv_core_power11_class_init' call 'pnv_core_power10_class_init' ?yes.}; DEFINE_TYPES(pnv_core_infos) @@ -633,6 +642,11 @@ static const TypeInfo pnv_quad_infos[] = { .parent = TYPE_PNV_QUAD, .name = PNV_QUAD_TYPE_NAME("power10"), .class_init = pnv_quad_power10_class_init, + .class_base_init = pnv_quad_power10_class_init, + }, + { + .parent = PNV_QUAD_TYPE_NAME("power10"), + .name = PNV_QUAD_TYPE_NAME("power11"),hmm, I'd rather introduce pnv_quad_power11_class_init.Same, in that case, I can have a 'pnv_quad_power11_class_init' which just calls 'pnv_quad_power10_class_init'.yes. Thanks, C.Any comments ?<...snip...> +struct Pnv11Chip { + /*< private >*/ + Pnv10Chip parent_obj;Can't you use a typedef instead ?Yes, makes sense. Thanks Cédric. - Aditya GuptaThanks, C.+}; + struct PnvChipClass { /*< private >*/ SysBusDeviceClass parent_class;
