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.
> > }
> > }
> >
> > @@ -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()
> > };
> >
> >
>
