On 11/09/2019 06:04, David Gibson wrote: > We've made several changes in the past to the machine reset order to fix > specific problems. However, we've ended up with an order that doesn't make > a lot of logical sense. This is an attempt to rectify this.
There are some more problems though. See below. > > First we reset global CAS options, since that should depend on nothing > else. Then we reset the CPUs, which shouldn't depend on external devices. > Then the irq subsystem, then the bulk of devices (which might rely on > irqs). Finally we set up the entry state ready for boot, which could > potentially rely on a bunch of other things. > > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > --- > hw/ppc/spapr.c | 47 +++++++++++++++++++++++++---------------------- > 1 file changed, 25 insertions(+), 22 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 5a919a6cc1..1560a11738 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1724,6 +1724,28 @@ static void spapr_machine_reset(MachineState *machine) > void *fdt; > int rc; > > + /* > + * If this reset wasn't generated by CAS, we should reset our > + * negotiated options and start from scratch > + */ > + if (!spapr->cas_reboot) { > + spapr_ovec_cleanup(spapr->ov5_cas); > + spapr->ov5_cas = spapr_ovec_new(); > + > + ppc_set_compat_all(spapr->max_compat_pvr, &error_fatal); > + } > + > + /* > + * There is no CAS under qtest. Simulate one to please the code that > + * depends on spapr->ov5_cas. This is especially needed to test device > + * unplug, so we do that before resetting the DRCs. > + */ > + if (qtest_enabled()) { > + spapr_ovec_cleanup(spapr->ov5_cas); > + spapr->ov5_cas = spapr_ovec_clone(spapr->ov5); > + } > + > + /* Reset the CPUs */ > spapr_caps_apply(spapr); > > first_ppc_cpu = POWERPC_CPU(first_cpu); > @@ -1741,34 +1763,15 @@ static void spapr_machine_reset(MachineState *machine) > spapr_setup_hpt_and_vrma(spapr); > } > > - qemu_devices_reset(); > - > - /* > - * If this reset wasn't generated by CAS, we should reset our > - * negotiated options and start from scratch > - */ > - if (!spapr->cas_reboot) { > - spapr_ovec_cleanup(spapr->ov5_cas); > - spapr->ov5_cas = spapr_ovec_new(); > - > - ppc_set_compat_all(spapr->max_compat_pvr, &error_fatal); > - } > - > + /* Reset IRQ subsystem */ > /* > * This is fixing some of the default configuration of the XIVE > * devices. To be called after the reset of the machine devices. > */ > spapr_irq_reset(spapr, &error_fatal); spapr_irq_reset() is now called before qemu_devices_reset(). So it will break the XIVE emulated model. KVM P8 guests are also broken : qemu-system-ppc64: kernel_irqchip requested but unavailable: Unable to restore KVM interrupt controller state (0x0) for CPU 0: Invalid argument Something wrong in kvmppc_xics_set_icp(). I need to look closer. and TCG P9 guests still do the reset after CAS. C. > > - /* > - * There is no CAS under qtest. Simulate one to please the code that > - * depends on spapr->ov5_cas. This is especially needed to test device > - * unplug, so we do that before resetting the DRCs. > - */ > - if (qtest_enabled()) { > - spapr_ovec_cleanup(spapr->ov5_cas); > - spapr->ov5_cas = spapr_ovec_clone(spapr->ov5); > - } > + /* Reset other devices */ > + qemu_devices_reset(); > > /* DRC reset may cause a device to be unplugged. This will cause troubles > * if this device is used by another device (eg, a running vhost backend >