[PATCH] hw/arm/virt enable support for virtio-mem

Jonathan Cameron posted 1 patch 3 years, 5 months ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20201105171144.560612-1-Jonathan.Cameron@huawei.com
Maintainers: Peter Maydell <peter.maydell@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, David Hildenbrand <david@redhat.com>
There is a newer version of this series
default-configs/devices/aarch64-softmmu.mak |  1 +
hw/arm/Kconfig                              |  1 +
hw/arm/virt.c                               | 64 ++++++++++++++++++++-
hw/virtio/Kconfig                           |  4 ++
hw/virtio/virtio-mem.c                      |  2 +
5 files changed, 71 insertions(+), 1 deletion(-)
[PATCH] hw/arm/virt enable support for virtio-mem
Posted by Jonathan Cameron 3 years, 5 months ago
Basically a cut and paste job from the x86 support with the exception of
needing a larger block size as the Memory Block Size (MIN_SECTION_SIZE)
on ARM64 in Linux is 1G.

Tested:
* In full emulation and with KVM on an arm64 server.
* cold and hotplug for the virtio-mem-pci device.
* Wide range of memory sizes, added at creation and later.
* Fairly basic memory usage of memory added.  Seems to function as normal.
* NUMA setup with virtio-mem-pci devices on each node.
* Simple migration test.

Related kernel patch just enables the Kconfig item for ARM64 as an
alternative to x86 in drivers/virtio/Kconfig

The original patches from David Hildenbrand stated that he thought it should
work for ARM64 but it wasn't enabled in the kernel [1]
It appears he was correct and everything 'just works'.

The build system related stuff is intended to ensure virtio-mem support is
not built for arm32 (build will fail due no defined block size).
If there is a more elegant way to do this, please point me in the right
direction.

[1] https://lore.kernel.org/linux-mm/20191212171137.13872-1-david@redhat.com/

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 default-configs/devices/aarch64-softmmu.mak |  1 +
 hw/arm/Kconfig                              |  1 +
 hw/arm/virt.c                               | 64 ++++++++++++++++++++-
 hw/virtio/Kconfig                           |  4 ++
 hw/virtio/virtio-mem.c                      |  2 +
 5 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/default-configs/devices/aarch64-softmmu.mak b/default-configs/devices/aarch64-softmmu.mak
index 958b1e08e4..31d6128a29 100644
--- a/default-configs/devices/aarch64-softmmu.mak
+++ b/default-configs/devices/aarch64-softmmu.mak
@@ -6,3 +6,4 @@ include arm-softmmu.mak
 CONFIG_XLNX_ZYNQMP_ARM=y
 CONFIG_XLNX_VERSAL=y
 CONFIG_SBSA_REF=y
+CONFIG_ARCH_VIRTIO_MEM_SUPPORTED=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index fdf4464b94..eeae77eee9 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -20,6 +20,7 @@ config ARM_VIRT
     select PLATFORM_BUS
     select SMBIOS
     select VIRTIO_MMIO
+    select VIRTIO_MEM_SUPPORTED if ARCH_VIRTIO_MEM_SUPPORTED
     select ACPI_PCI
     select MEM_DEVICE
     select DIMM
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 8abb385d4e..a9f927ce83 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -73,9 +73,11 @@
 #include "hw/acpi/acpi.h"
 #include "target/arm/internals.h"
 #include "hw/mem/pc-dimm.h"
+#include "hw/mem/memory-device.h"
 #include "hw/mem/nvdimm.h"
 #include "hw/acpi/generic_event_device.h"
 #include "hw/virtio/virtio-iommu.h"
+#include "hw/virtio/virtio-mem-pci.h"
 #include "hw/char/pl011.h"
 #include "qemu/guest-random.h"
 
@@ -2286,6 +2288,34 @@ static void virt_memory_plug(HotplugHandler *hotplug_dev,
                          dev, &error_abort);
 }
 
+static void virt_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;
+
+    if (!hotplug_dev2 && dev->hotplugged) {
+        /*
+         * Without a bus hotplug handler, we cannot control the plug/unplug
+         * order. We should never reach this point when hotplugging,
+         * however, better add a safety net.
+         */
+        error_setg(errp, "hotplug of virtio-mem based memory devices not supported"
+                   " on this bus.");
+        return;
+    }
+    /*
+     * First, see if we can plug this memory device at all. If that
+     * succeeds, branch of to the actual hotplug handler.
+     */
+    memory_device_pre_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev), NULL,
+                           &local_err);
+    if (!local_err && hotplug_dev2) {
+        hotplug_handler_pre_plug(hotplug_dev2, dev, &local_err);
+    }
+    error_propagate(errp, local_err);
+}
+
 static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
                                             DeviceState *dev, Error **errp)
 {
@@ -2293,6 +2323,8 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
 
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
         virt_memory_pre_plug(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
+        virt_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);        
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
         hwaddr db_start = 0, db_end = 0;
         char *resv_prop_str;
@@ -2322,6 +2354,27 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
     }
 }
 
+static void virt_virtio_md_pci_plug(HotplugHandler *hotplug_dev,
+                                  DeviceState *dev, Error **errp)
+{
+    HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
+    Error *local_err = NULL;
+
+    /*
+     * Plug the memory device first and then branch off to the actual
+     * hotplug handler. If that one fails, we can easily undo the memory
+     * device bits.
+     */
+    memory_device_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
+    if (hotplug_dev2) {
+        hotplug_handler_plug(hotplug_dev2, dev, &local_err);
+        if (local_err) {
+            memory_device_unplug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
+        }
+    }
+    error_propagate(errp, local_err);
+}
+
 static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
                                         DeviceState *dev, Error **errp)
 {
@@ -2336,6 +2389,9 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
         virt_memory_plug(hotplug_dev, dev, errp);
     }
+    if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
+        virt_virtio_md_pci_plug(hotplug_dev, dev, errp);
+    }
     if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
         PCIDevice *pdev = PCI_DEVICE(dev);
 
@@ -2363,6 +2419,11 @@ static void virt_dimm_unplug_request(HotplugHandler *hotplug_dev,
         goto out;
     }
 
+    if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
+        error_setg(&local_err,
+                   "virtio-mem based memory devices cannot be unplugged.");
+        goto out;
+    }
     hotplug_handler_unplug_request(HOTPLUG_HANDLER(vms->acpi_dev), dev,
                                    &local_err);
 out:
@@ -2413,7 +2474,8 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
                                                         DeviceState *dev)
 {
     if (object_dynamic_cast(OBJECT(dev), TYPE_SYS_BUS_DEVICE) ||
-       (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM))) {
+        object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
+        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)){
         return HOTPLUG_HANDLER(machine);
     }
     if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index 0eda25c4e1..00dbf2939e 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -48,6 +48,10 @@ config VIRTIO_PMEM
     depends on VIRTIO_PMEM_SUPPORTED
     select MEM_DEVICE
 
+config ARCH_VIRTIO_MEM_SUPPORTED
+    bool
+    default n
+
 config VIRTIO_MEM_SUPPORTED
     bool
 
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 7c8ca9f28b..c28f12547a 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -53,6 +53,8 @@
  */
 #if defined(TARGET_X86_64) || defined(TARGET_I386)
 #define VIRTIO_MEM_USABLE_EXTENT (2 * (128 * MiB))
+#elif defined (TARGET_AARCH64)
+#define VIRTIO_MEM_USABLE_EXTENT (2 * (1024 * MiB))
 #else
 #error VIRTIO_MEM_USABLE_EXTENT not defined
 #endif
-- 
2.19.1


Re: [PATCH] hw/arm/virt enable support for virtio-mem
Posted by no-reply@patchew.org 3 years, 5 months ago
Patchew URL: https://patchew.org/QEMU/20201105171144.560612-1-Jonathan.Cameron@huawei.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20201105171144.560612-1-Jonathan.Cameron@huawei.com
Subject: [PATCH] hw/arm/virt enable support for virtio-mem

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20201105171144.560612-1-Jonathan.Cameron@huawei.com -> patchew/20201105171144.560612-1-Jonathan.Cameron@huawei.com
Switched to a new branch 'test'
03ce0eb hw/arm/virt enable support for virtio-mem

=== OUTPUT BEGIN ===
WARNING: line over 80 characters
#89: FILE: hw/arm/virt.c:2289:
+        error_setg(errp, "hotplug of virtio-mem based memory devices not supported"

ERROR: trailing whitespace
#113: FILE: hw/arm/virt.c:2313:
+        virt_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);        $

ERROR: space required before the open brace '{'
#173: FILE: hw/arm/virt.c:2464:
+        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)){

ERROR: space prohibited between function name and open parenthesis '('
#200: FILE: hw/virtio/virtio-mem.c:56:
+#elif defined (TARGET_AARCH64)

total: 3 errors, 1 warnings, 138 lines checked

Commit 03ce0eb928fb (hw/arm/virt enable support for virtio-mem) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20201105171144.560612-1-Jonathan.Cameron@huawei.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com