Re: [PATCH 2/3] hw/ppc/e500plat: Fix modifying QOM class internal state from instance

[Alexander Graf]

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 <shen...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <phi...@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";