| Message ID | 20250127142824.494644-3-david@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | s390x: support virtio-mem-pci | expand |
On 27/01/2025 15.28, David Hildenbrand wrote: > Let's just wire it up, unlocking virtio-mem-pci support on s390x. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > hw/s390x/s390-virtio-ccw.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) Thanks for tackling this! Reviewed-by: Thomas Huth <thuth@redhat.com>
On 1/27/25 15:28, David Hildenbrand wrote: > Let's just wire it up, unlocking virtio-mem-pci support on s390x. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > hw/s390x/s390-virtio-ccw.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index 3af613d4e9..71f3443a53 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -554,8 +554,7 @@ static void s390_machine_device_pre_plug(HotplugHandler *hotplug_dev, > if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW)) { > virtio_ccw_md_pre_plug(VIRTIO_MD_CCW(dev), MACHINE(hotplug_dev), errp); > } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) { > - error_setg(errp, > - "PCI-attached virtio based memory devices not supported"); > + virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp); > } > } > > @@ -566,7 +565,8 @@ static void s390_machine_device_plug(HotplugHandler *hotplug_dev, > > if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { > s390_cpu_plug(hotplug_dev, dev, errp); > - } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW)) { > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW) || > + object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) { > /* > * At this point, the device is realized and set all memdevs mapped, so > * qemu_maxrampagesize() will pick up the page sizes of these memdevs > @@ -580,7 +580,11 @@ static void s390_machine_device_plug(HotplugHandler *hotplug_dev, > " initial memory"); > return; > } > - virtio_ccw_md_plug(VIRTIO_MD_CCW(dev), MACHINE(hotplug_dev), errp); > + if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW)) { > + virtio_ccw_md_plug(VIRTIO_MD_CCW(dev), MACHINE(hotplug_dev), errp); > + } else { > + virtio_md_pci_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp); > + } > } > } > > @@ -589,10 +593,12 @@ static void s390_machine_device_unplug_request(HotplugHandler *hotplug_dev, > { > if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { > error_setg(errp, "CPU hot unplug not supported on this machine"); > - return; This looks suspicious. Do we really want to continue executing the rest of the function in this case (though there's nothing to execute currently)? OTOH, the statement can be put back should it be ever needed. > } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW)) { > virtio_ccw_md_unplug_request(VIRTIO_MD_CCW(dev), MACHINE(hotplug_dev), > errp); > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) { > + virtio_md_pci_unplug_request(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), > + errp); > } > } Michal
>> >> @@ -589,10 +593,12 @@ static void s390_machine_device_unplug_request(HotplugHandler *hotplug_dev, >> { >> if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { >> error_setg(errp, "CPU hot unplug not supported on this machine"); >> - return; > > This looks suspicious. Do we really want to continue executing the rest > of the function in this case (though there's nothing to execute > currently)? These functions are usually a big switch-case statement, so the "return" is actually superfluous. Note that this is already what we do with TYPE_CPU in s390_machine_device_plug(). Thanks!
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 3af613d4e9..71f3443a53 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -554,8 +554,7 @@ static void s390_machine_device_pre_plug(HotplugHandler *hotplug_dev, if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW)) { virtio_ccw_md_pre_plug(VIRTIO_MD_CCW(dev), MACHINE(hotplug_dev), errp); } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) { - error_setg(errp, - "PCI-attached virtio based memory devices not supported"); + virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp); } } @@ -566,7 +565,8 @@ static void s390_machine_device_plug(HotplugHandler *hotplug_dev, if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { s390_cpu_plug(hotplug_dev, dev, errp); - } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW)) { + } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW) || + object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) { /* * At this point, the device is realized and set all memdevs mapped, so * qemu_maxrampagesize() will pick up the page sizes of these memdevs @@ -580,7 +580,11 @@ static void s390_machine_device_plug(HotplugHandler *hotplug_dev, " initial memory"); return; } - virtio_ccw_md_plug(VIRTIO_MD_CCW(dev), MACHINE(hotplug_dev), errp); + if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW)) { + virtio_ccw_md_plug(VIRTIO_MD_CCW(dev), MACHINE(hotplug_dev), errp); + } else { + virtio_md_pci_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp); + } } } @@ -589,10 +593,12 @@ static void s390_machine_device_unplug_request(HotplugHandler *hotplug_dev, { if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { error_setg(errp, "CPU hot unplug not supported on this machine"); - return; } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW)) { virtio_ccw_md_unplug_request(VIRTIO_MD_CCW(dev), MACHINE(hotplug_dev), errp); + } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) { + virtio_md_pci_unplug_request(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), + errp); } } @@ -601,7 +607,9 @@ static void s390_machine_device_unplug(HotplugHandler *hotplug_dev, { if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW)) { virtio_ccw_md_unplug(VIRTIO_MD_CCW(dev), MACHINE(hotplug_dev), errp); - } + } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) { + virtio_md_pci_unplug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp); + } } static CpuInstanceProperties s390_cpu_index_to_props(MachineState *ms,
Let's just wire it up, unlocking virtio-mem-pci support on s390x. Signed-off-by: David Hildenbrand <david@redhat.com> --- hw/s390x/s390-virtio-ccw.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)