On Tue, 2022-11-29 at 18:42 +0100, Pierre Morel wrote:
> We will need a Topology device to transfer the topology
> during migration and to implement machine reset.
>
> The device creation is fenced by s390_has_topology().
>
> Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
> ---
> include/hw/s390x/cpu-topology.h | 44 +++++++++++++++
> include/hw/s390x/s390-virtio-ccw.h | 1 +
> hw/s390x/cpu-topology.c | 87 ++++++++++++++++++++++++++++++
> hw/s390x/s390-virtio-ccw.c | 25 +++++++++
> hw/s390x/meson.build | 1 +
> 5 files changed, 158 insertions(+)
> create mode 100644 include/hw/s390x/cpu-topology.h
> create mode 100644 hw/s390x/cpu-topology.c
>
[...]
> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
> index 9bba21a916..47ce0aa6fa 100644
> --- a/include/hw/s390x/s390-virtio-ccw.h
> +++ b/include/hw/s390x/s390-virtio-ccw.h
> @@ -28,6 +28,7 @@ struct S390CcwMachineState {
> bool dea_key_wrap;
> bool pv;
> uint8_t loadparm[8];
> + DeviceState *topology;
Why is this a DeviceState, not S390Topology?
It *has* to be a S390Topology, right? Since you cast it to one in patch 2.
> };
>
> struct S390CcwMachineClass {
> diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c
> new file mode 100644
> index 0000000000..bbf97cd66a
> --- /dev/null
> +++ b/hw/s390x/cpu-topology.c
>
[...]
>
> +static DeviceState *s390_init_topology(MachineState *machine, Error **errp)
> +{
> + DeviceState *dev;
> +
> + dev = qdev_new(TYPE_S390_CPU_TOPOLOGY);
> +
> + object_property_add_child(&machine->parent_obj,
> + TYPE_S390_CPU_TOPOLOGY, OBJECT(dev));
Why set this property, and why on the machine parent?
> + object_property_set_int(OBJECT(dev), "num-cores",
> + machine->smp.cores * machine->smp.threads, errp);
> + object_property_set_int(OBJECT(dev), "num-sockets",
> + machine->smp.sockets, errp);
> +
> + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), errp);
I must admit that I haven't fully grokked qemu's memory management yet.
Is the topology devices now owned by the sysbus?
If so, is it fine to have a pointer to it S390CcwMachineState?
> +
> + return dev;
> +}
> +
[...]
