On Mon, 15 May 2017 13:40:04 +0200
Greg Kurz <gr...@kaod.org> wrote:
> Commit 5bc8d26de20c ("spapr: allocate the ICPState object from under
> sPAPRCPUCore") moved ICP objects from the machine to CPU cores. This
> is an improvement since we no longer allocate ICP objects that will
> never be used. But it has the side-effect of breaking migration of
> older machine types from older QEMU versions.
>
> This patch introduces a compat flag in the sPAPR machine class so
> that all pseries machine up to 2.9 go on with the previous behavior
> of pre-allocating ICP objects.
>
> While here, we also ensure that object_property_add_child() errors cause
> QEMU to abort for newer machines.
>
Looking at the code again...
> Signed-off-by: Greg Kurz <gr...@kaod.org>
> ---
> hw/ppc/spapr.c | 36 ++++++++++++++++++++++++++++++++++++
> hw/ppc/spapr_cpu_core.c | 28 ++++++++++++++++++++--------
> include/hw/ppc/spapr.h | 2 ++
> 3 files changed, 58 insertions(+), 8 deletions(-)
[...]
> @@ -142,12 +145,19 @@ static void spapr_cpu_core_realize_child(Object *child,
> Error **errp)
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> Object *obj;
>
> - obj = object_new(spapr->icp_type);
> - object_property_add_child(OBJECT(cpu), "icp", obj, NULL);
... I now realize that we're lacking
object_unref(obj);
and the ref count of the ICP object can never go down to zero...
I'll fix that in a preparatory patch for v2.
> - object_property_add_const_link(obj, "xics", OBJECT(spapr), &error_abort);
> - object_property_set_bool(obj, true, "realized", &local_err);
> - if (local_err) {
> - goto error;
> + if (spapr->legacy_icps) {
> + int index = cpu->parent_obj.cpu_index;
> +
> + obj = OBJECT(&spapr->legacy_icps[index]);
> + } else {
> + obj = object_new(spapr->icp_type);
> + object_property_add_child(OBJECT(cpu), "icp", obj, &error_abort);
> + object_property_add_const_link(obj, "xics", OBJECT(spapr),
> + &error_abort);
> + object_property_set_bool(obj, true, "realized", &local_err);
> + if (local_err) {
> + goto error;
> + }
> }
>
> object_property_set_bool(child, true, "realized", &local_err);
> @@ -164,7 +174,9 @@ static void spapr_cpu_core_realize_child(Object *child,
> Error **errp)
> return;
>
> error:
> - object_unparent(obj);
> + if (!spapr->legacy_icps) {
> + object_unparent(obj);
> + }
> error_propagate(errp, local_err);
> }
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 5802f888c39d..72cd5af2679b 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -53,6 +53,7 @@ struct sPAPRMachineClass {
> bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */
> bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */
> const char *tcg_default_cpu; /* which (TCG) CPU to simulate by default */
> + bool must_pre_allocate_icps; /* only for pseries-2.9 and older */
> void (*phb_placement)(sPAPRMachineState *spapr, uint32_t index,
> uint64_t *buid, hwaddr *pio,
> hwaddr *mmio32, hwaddr *mmio64,
> @@ -109,6 +110,7 @@ struct sPAPRMachineState {
> MemoryHotplugState hotplug_memory;
>
> const char *icp_type;
> + ICPState *legacy_icps;
> };
>
> #define H_SUCCESS 0
>
>
pgpQ5HmhlSA72.pgp
Description: OpenPGP digital signature
