Re: [PATCH v1 15/17] pc: Support for virtio-mem-pci

Pankaj Gupta
2020-05-06T12:19:20Z

> Let's wire it up similar to virtio-pmem. Also disallow unplug, so it's
> harder for users to shoot themselves into the foot.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> hw/i386/Kconfig | 1 +
> hw/i386/pc.c | 49 ++++++++++++++++++++++++++++---------------------
> 2 files changed, 29 insertions(+), 21 deletions(-)
>
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index c93f32f657..03e347b207 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -35,6 +35,7 @@ config PC
> select ACPI_PCI
> select ACPI_VMGENID
> select VIRTIO_PMEM_SUPPORTED
> + select VIRTIO_MEM_SUPPORTED
>
> config PC_PCI
> bool
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index f6b8431c8b..588804f895 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -86,6 +86,7 @@
> #include "hw/net/ne2000-isa.h"
> #include "standard-headers/asm-x86/bootparam.h"
> #include "hw/virtio/virtio-pmem-pci.h"
> +#include "hw/virtio/virtio-mem-pci.h"
> #include "hw/mem/memory-device.h"
> #include "sysemu/replay.h"
> #include "qapi/qmp/qerror.h"
> @@ -1654,8 +1655,8 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
> numa_cpu_pre_plug(cpu_slot, dev, errp);
> }
>
> -static void pc_virtio_pmem_pci_pre_plug(HotplugHandler *hotplug_dev,
> - DeviceState *dev, Error **errp)
> +static void pc_virtio_md_pci_pre_plug(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
> {
> HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
> Error *local_err = NULL;
> @@ -1666,7 +1667,8 @@ static void pc_virtio_pmem_pci_pre_plug(HotplugHandler *hotplug_dev,
> * order. This should never be the case on x86, however better add
> * a safety net.
> */
> - error_setg(errp, "virtio-pmem-pci not supported on this bus.");
> + error_setg(errp,
> + "virtio based memory devices not supported on this bus.");
> return;
> }
> /*
> @@ -1681,8 +1683,8 @@ static void pc_virtio_pmem_pci_pre_plug(HotplugHandler *hotplug_dev,
> error_propagate(errp, local_err);
> }
>
> -static void pc_virtio_pmem_pci_plug(HotplugHandler *hotplug_dev,
> - DeviceState *dev, Error **errp)
> +static void pc_virtio_md_pci_plug(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
> {
> HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
> Error *local_err = NULL;
> @@ -1700,17 +1702,17 @@ static void pc_virtio_pmem_pci_plug(HotplugHandler *hotplug_dev,
> error_propagate(errp, local_err);
> }
>
> -static void pc_virtio_pmem_pci_unplug_request(HotplugHandler *hotplug_dev,
> - DeviceState *dev, Error **errp)
> +static void pc_virtio_md_pci_unplug_request(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
> {
> - /* We don't support virtio pmem hot unplug */
> - error_setg(errp, "virtio pmem device unplug not supported.");
> + /* We don't support hot unplug of virtio based memory devices */
> + error_setg(errp, "virtio based memory devices cannot be unplugged.");
> }
>
> -static void pc_virtio_pmem_pci_unplug(HotplugHandler *hotplug_dev,
> - DeviceState *dev, Error **errp)
> +static void pc_virtio_md_pci_unplug(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
> {
> - /* We don't support virtio pmem hot unplug */
> + /* We don't support hot unplug of virtio based memory devices */
> }
>
> static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> @@ -1720,8 +1722,9 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> pc_memory_pre_plug(hotplug_dev, dev, errp);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> pc_cpu_pre_plug(hotplug_dev, dev, errp);
> - } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI)) {
> - pc_virtio_pmem_pci_pre_plug(hotplug_dev, dev, errp);
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
> + object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
> + pc_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
> }
> }
>
> @@ -1732,8 +1735,9 @@ static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,
> pc_memory_plug(hotplug_dev, dev, errp);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> pc_cpu_plug(hotplug_dev, dev, errp);
> - } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI)) {
> - pc_virtio_pmem_pci_plug(hotplug_dev, dev, errp);
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
> + object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
> + pc_virtio_md_pci_plug(hotplug_dev, dev, errp);
> }
> }
>
> @@ -1744,8 +1748,9 @@ static void pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> pc_memory_unplug_request(hotplug_dev, dev, errp);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> pc_cpu_unplug_request_cb(hotplug_dev, dev, errp);
> - } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI)) {
> - pc_virtio_pmem_pci_unplug_request(hotplug_dev, dev, errp);
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
> + object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
> + pc_virtio_md_pci_unplug_request(hotplug_dev, dev, errp);
> } else {
> error_setg(errp, "acpi: device unplug request for not supported device"
> " type: %s", object_get_typename(OBJECT(dev)));
> @@ -1759,8 +1764,9 @@ static void pc_machine_device_unplug_cb(HotplugHandler *hotplug_dev,
> pc_memory_unplug(hotplug_dev, dev, errp);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> pc_cpu_unplug_cb(hotplug_dev, dev, errp);
> - } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI)) {
> - pc_virtio_pmem_pci_unplug(hotplug_dev, dev, errp);
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
> + object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
> + pc_virtio_md_pci_unplug(hotplug_dev, dev, errp);
> } else {
> error_setg(errp, "acpi: device unplug for not supported device"
> " type: %s", object_get_typename(OBJECT(dev)));
> @@ -1772,7 +1778,8 @@ static HotplugHandler *pc_get_hotplug_handler(MachineState *machine,
> {
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
> object_dynamic_cast(OBJECT(dev), TYPE_CPU) ||
> - object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI)) {
> + object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
> + object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
> return HOTPLUG_HANDLER(machine);
> }
>
> --

Reviewed-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>

> 2.25.3
>
>