| Message ID | 20230523064408.57941-3-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw: Fix abuse of QOM class internals modified by their instances | expand |
Hi Philippe, On 23.05.23 08:44, Philippe Mathieu-Daudé wrote: > QOM object instance should not modify its class state (because > all other objects instanciated from this class get affected). > > Instead of modifying the PPCE500MachineClass 'mpic_version' field > in the instance machine_init() handler, set it in the machine > class init handler (e500plat_machine_class_init). > > Inspired-by: Bernhard Beschow <shentey@gmail.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > hw/ppc/e500plat.c | 25 +++++++++++-------------- > 1 file changed, 11 insertions(+), 14 deletions(-) > > diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c > index 3032bd3f6d..c3b0ed01cf 100644 > --- a/hw/ppc/e500plat.c > +++ b/hw/ppc/e500plat.c > @@ -30,18 +30,6 @@ static void e500plat_fixup_devtree(void *fdt) > sizeof(compatible)); > } > > -static void e500plat_init(MachineState *machine) > -{ > - PPCE500MachineClass *pmc = PPCE500_MACHINE_GET_CLASS(machine); > - /* Older KVM versions don't support EPR which breaks guests when we announce > - MPIC variants that support EPR. Revert to an older one for those */ > - if (kvm_enabled() && !kvmppc_has_cap_epr()) { > - pmc->mpic_version = OPENPIC_MODEL_FSL_MPIC_20; > - } > - > - ppce500_init(machine); Won't this drop the call to ppce500_init(machine)? > -} > - > static void e500plat_machine_device_plug_cb(HotplugHandler *hotplug_dev, > DeviceState *dev, Error **errp) > { > @@ -81,7 +69,6 @@ static void e500plat_machine_class_init(ObjectClass *oc, void *data) > pmc->pci_first_slot = 0x1; > pmc->pci_nr_slots = PCI_SLOT_MAX - 1; > pmc->fixup_devtree = e500plat_fixup_devtree; > - pmc->mpic_version = OPENPIC_MODEL_FSL_MPIC_42; > pmc->has_mpc8xxx_gpio = true; > pmc->has_esdhc = true; > pmc->platform_bus_base = 0xf00000000ULL; > @@ -94,8 +81,18 @@ static void e500plat_machine_class_init(ObjectClass *oc, void *data) > pmc->pci_mmio_bus_base = 0xE0000000ULL; > pmc->spin_base = 0xFEF000000ULL; > > + if (kvm_enabled() && !kvmppc_has_cap_epr()) { > + /* > + * Older KVM versions don't support EPR which breaks guests when > + * we announce MPIC variants that support EPR. Revert to an older > + * one for those. > + */ > + pmc->mpic_version = OPENPIC_MODEL_FSL_MPIC_20; > + } else { > + pmc->mpic_version = OPENPIC_MODEL_FSL_MPIC_42; > + } > + > mc->desc = "generic paravirt e500 platform"; > - mc->init = e500plat_init; I suppose best would be to just put it in here instead of e500plat_init? Alex > mc->max_cpus = 32; > mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("e500v2_v30"); > mc->default_ram_id = "mpc8544ds.ram";
On 5/22/23 23:44, Philippe Mathieu-Daudé wrote: > QOM object instance should not modify its class state (because > all other objects instanciated from this class get affected). > > Instead of modifying the PPCE500MachineClass 'mpic_version' field > in the instance machine_init() handler, set it in the machine > class init handler (e500plat_machine_class_init). > > Inspired-by: Bernhard Beschow<shentey@gmail.com> > Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org> > --- > hw/ppc/e500plat.c | 25 +++++++++++-------------- > 1 file changed, 11 insertions(+), 14 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c index 3032bd3f6d..c3b0ed01cf 100644 --- a/hw/ppc/e500plat.c +++ b/hw/ppc/e500plat.c @@ -30,18 +30,6 @@ static void e500plat_fixup_devtree(void *fdt) sizeof(compatible)); } -static void e500plat_init(MachineState *machine) -{ - PPCE500MachineClass *pmc = PPCE500_MACHINE_GET_CLASS(machine); - /* Older KVM versions don't support EPR which breaks guests when we announce - MPIC variants that support EPR. Revert to an older one for those */ - if (kvm_enabled() && !kvmppc_has_cap_epr()) { - pmc->mpic_version = OPENPIC_MODEL_FSL_MPIC_20; - } - - ppce500_init(machine); -} - static void e500plat_machine_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { @@ -81,7 +69,6 @@ static void e500plat_machine_class_init(ObjectClass *oc, void *data) pmc->pci_first_slot = 0x1; pmc->pci_nr_slots = PCI_SLOT_MAX - 1; pmc->fixup_devtree = e500plat_fixup_devtree; - pmc->mpic_version = OPENPIC_MODEL_FSL_MPIC_42; pmc->has_mpc8xxx_gpio = true; pmc->has_esdhc = true; pmc->platform_bus_base = 0xf00000000ULL; @@ -94,8 +81,18 @@ static void e500plat_machine_class_init(ObjectClass *oc, void *data) pmc->pci_mmio_bus_base = 0xE0000000ULL; pmc->spin_base = 0xFEF000000ULL; + if (kvm_enabled() && !kvmppc_has_cap_epr()) { + /* + * Older KVM versions don't support EPR which breaks guests when + * we announce MPIC variants that support EPR. Revert to an older + * one for those. + */ + pmc->mpic_version = OPENPIC_MODEL_FSL_MPIC_20; + } else { + pmc->mpic_version = OPENPIC_MODEL_FSL_MPIC_42; + } + mc->desc = "generic paravirt e500 platform"; - mc->init = e500plat_init; mc->max_cpus = 32; mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("e500v2_v30"); mc->default_ram_id = "mpc8544ds.ram";
QOM object instance should not modify its class state (because all other objects instanciated from this class get affected). Instead of modifying the PPCE500MachineClass 'mpic_version' field in the instance machine_init() handler, set it in the machine class init handler (e500plat_machine_class_init). Inspired-by: Bernhard Beschow <shentey@gmail.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/ppc/e500plat.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-)