On 12/9/20 6:42 PM, Greg Kurz wrote: > On Wed, 9 Dec 2020 18:34:31 +0100 > Philippe Mathieu-Daudé <phi...@redhat.com> wrote: > >> On 12/9/20 6:00 PM, Greg Kurz wrote: >>> The sPAPR CPU core device can only work with pseries machine types. >>> This is currently checked in the realize function with a dynamic >>> cast of qdev_get_machine(). Some other places also need to reach >>> out to the machine using qdev_get_machine(). >>> >>> Make this dependency explicit by introducing an "spapr" link >>> property which officialy points to the machine. This link is >>> set by pseries machine types only in the pre-plug handler. This >>> allows to drop some users of qdev_get_machine(). >>> >>> Signed-off-by: Greg Kurz <gr...@kaod.org> >>> --- >>> include/hw/ppc/spapr_cpu_core.h | 2 ++ >>> hw/ppc/spapr.c | 4 ++++ >>> hw/ppc/spapr_cpu_core.c | 17 +++++++---------- >>> 3 files changed, 13 insertions(+), 10 deletions(-) >>> >>> diff --git a/include/hw/ppc/spapr_cpu_core.h >>> b/include/hw/ppc/spapr_cpu_core.h >>> index dab3dfc76c0a..0969b29fd96c 100644 >>> --- a/include/hw/ppc/spapr_cpu_core.h >>> +++ b/include/hw/ppc/spapr_cpu_core.h >>> @@ -10,6 +10,7 @@ >>> #define HW_SPAPR_CPU_CORE_H >>> >>> #include "hw/cpu/core.h" >>> +#include "hw/ppc/spapr.h" >>> #include "hw/qdev-core.h" >>> #include "target/ppc/cpu-qom.h" >>> #include "target/ppc/cpu.h" >>> @@ -24,6 +25,7 @@ OBJECT_DECLARE_TYPE(SpaprCpuCore, SpaprCpuCoreClass, >>> struct SpaprCpuCore { >>> /*< private >*/ >>> CPUCore parent_obj; >>> + SpaprMachineState *spapr; >>> >>> /*< public >*/ >>> PowerPCCPU **threads; >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>> index d1dcf3ab2c94..4cc51723c62e 100644 >>> --- a/hw/ppc/spapr.c >>> +++ b/hw/ppc/spapr.c >>> @@ -3816,6 +3816,10 @@ static void spapr_core_pre_plug(HotplugHandler >>> *hotplug_dev, DeviceState *dev, >>> int index; >>> unsigned int smp_threads = machine->smp.threads; >>> >>> + /* Required by spapr_cpu_core_realize() */ >>> + object_property_set_link(OBJECT(dev), "spapr", OBJECT(hotplug_dev), >>> + &error_abort); >>> + >>> if (dev->hotplugged && !mc->has_hotpluggable_cpus) { >>> error_setg(errp, "CPU hotplug not supported for this machine"); >>> return; >>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c >>> index 2f7dc3c23ded..dec09367e4a0 100644 >>> --- a/hw/ppc/spapr_cpu_core.c >>> +++ b/hw/ppc/spapr_cpu_core.c >>> @@ -25,14 +25,13 @@ >>> #include "sysemu/hw_accel.h" >>> #include "qemu/error-report.h" >>> >>> -static void spapr_reset_vcpu(PowerPCCPU *cpu) >>> +static void spapr_reset_vcpu(PowerPCCPU *cpu, SpaprMachineState *spapr) >>> { >>> CPUState *cs = CPU(cpu); >>> CPUPPCState *env = &cpu->env; >>> PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); >>> SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); >>> target_ulong lpcr; >>> - SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine()); >>> >>> cpu_reset(cs); >>> >>> @@ -186,7 +185,7 @@ static void spapr_unrealize_vcpu(PowerPCCPU *cpu, >>> SpaprCpuCore *sc) >>> if (!sc->pre_3_0_migration) { >>> vmstate_unregister(NULL, &vmstate_spapr_cpu_state, >>> cpu->machine_data); >>> } >>> - spapr_irq_cpu_intc_destroy(SPAPR_MACHINE(qdev_get_machine()), cpu); >>> + spapr_irq_cpu_intc_destroy(sc->spapr, cpu); >>> qdev_unrealize(DEVICE(cpu)); >>> } >>> >>> @@ -200,7 +199,7 @@ static void spapr_cpu_core_reset(DeviceState *dev) >>> int i; >>> >>> for (i = 0; i < cc->nr_threads; i++) { >>> - spapr_reset_vcpu(sc->threads[i]); >>> + spapr_reset_vcpu(sc->threads[i], sc->spapr); >> >> Why reset() needs access to the machine state, don't >> you have it in realize()? >> > > This is for the vCPU threads of the sPAPR CPU core. They don't have the > link to the machine state.
They are created by spapr_create_vcpu() + spapr_realize_vcpu() in spapr_cpu_core_realize(), which has sc->spapr set... Am I missing something? > >>> } >>> } >>> >>> @@ -314,16 +313,12 @@ err: >>> >>> static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) >>> { >>> - /* We don't use SPAPR_MACHINE() in order to exit gracefully if the user >>> - * tries to add a sPAPR CPU core to a non-pseries machine. >>> - */ >>> - SpaprMachineState *spapr = >>> - (SpaprMachineState *) object_dynamic_cast(qdev_get_machine(), >>> - TYPE_SPAPR_MACHINE); >>> SpaprCpuCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); >>> + SpaprMachineState *spapr = sc->spapr; >>> CPUCore *cc = CPU_CORE(OBJECT(dev)); >>> int i; >>> >>> + /* Set in spapr_core_pre_plug() */ >>> if (!spapr) { >>> error_setg(errp, TYPE_SPAPR_CPU_CORE " needs a pseries machine"); >>> return; >>> @@ -345,6 +340,8 @@ static Property spapr_cpu_core_properties[] = { >>> DEFINE_PROP_INT32("node-id", SpaprCpuCore, node_id, >>> CPU_UNSET_NUMA_NODE_ID), >>> DEFINE_PROP_BOOL("pre-3.0-migration", SpaprCpuCore, pre_3_0_migration, >>> false), >>> + DEFINE_PROP_LINK("spapr", SpaprCpuCore, spapr, TYPE_SPAPR_MACHINE, >>> + SpaprMachineState *), >>> DEFINE_PROP_END_OF_LIST() >>> }; >>> >>> >> >