qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/1] hw/arm/virt: Support for virtio-mem-pci


From: Gavin Shan
Subject: Re: [PATCH 1/1] hw/arm/virt: Support for virtio-mem-pci
Date: Wed, 1 Dec 2021 16:09:43 +1100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.0

On 11/30/21 8:37 PM, David Hildenbrand wrote:
On 30.11.21 01:33, Gavin Shan wrote:
This supports virtio-mem-pci device on "virt" platform, by simply
following the implementation on x86.

Thanks for picking this up!


Thanks, David.


    * The patch was written by David Hildenbrand <david@redhat.com>
      modified by Jonathan Cameron <Jonathan.Cameron@huawei.com>

Maybe replace this section by

Co-developed-by: David Hildenbrand <david@redhat.com>
Co-developed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>


Yes, it will be included into v2.


    * This implements the hotplug handlers to support virtio-mem-pci
      device hot-add, while the hot-remove isn't supported as we have
      on x86.

    * The block size is 1GB on ARM64 instead of 128MB on x86.

See below, isn't it actually 512 MiB nowadays?


I think so.


    * It has been passing the tests with various combinations like 64KB
      and 4KB page sizes on host and guest, different memory device
      backends like normal, transparent huge page and HugeTLB, plus
      migration.

Perfect. A note that hugetlbfs isn't fully supported/safe to use until
we have preallocation support in QEMU (WIP).


Yes, there is some warnings raised to enlarge 'request-size' on
host with 64KB page size. Note that the memory backends I used
in the testings always have "prealloc=on" property though.


Signed-off-by: Gavin Shan <gshan@redhat.com>
---
  hw/arm/Kconfig         |  1 +
  hw/arm/virt.c          | 68 +++++++++++++++++++++++++++++++++++++++++-
  hw/virtio/virtio-mem.c |  2 ++
  3 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 2d37d29f02..15aff8efb8 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -27,6 +27,7 @@ config ARM_VIRT
      select DIMM
      select ACPI_HW_REDUCED
      select ACPI_APEI
+    select VIRTIO_MEM_SUPPORTED
config CHEETAH
      bool
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 369552ad45..f4599a5ef0 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -71,9 +71,11 @@
  #include "hw/arm/smmuv3.h"
  #include "hw/acpi/acpi.h"
  #include "target/arm/internals.h"
+#include "hw/mem/memory-device.h"
  #include "hw/mem/pc-dimm.h"
  #include "hw/mem/nvdimm.h"
  #include "hw/acpi/generic_event_device.h"
+#include "hw/virtio/virtio-mem-pci.h"
  #include "hw/virtio/virtio-iommu.h"
  #include "hw/char/pl011.h"
  #include "qemu/guest-random.h"
@@ -2480,6 +2482,63 @@ 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 on x86,
+         * however, better add a safety net.
+         */
+        error_setg(errp, "hotplug of virtio 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_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_virtio_md_pci_unplug_request(HotplugHandler *hotplug_dev,
+                                              DeviceState *dev, Error **errp)
+{
+    /* We don't support hot unplug of virtio based memory devices */
+    error_setg(errp, "virtio based memory devices cannot be unplugged.");
+}
+
+
  static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
                                              DeviceState *dev, Error **errp)
  {
@@ -2513,6 +2572,8 @@ static void 
virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
          qdev_prop_set_uint32(dev, "len-reserved-regions", 1);
          qdev_prop_set_string(dev, "reserved-regions[0]", resv_prop_str);
          g_free(resv_prop_str);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
+        virt_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
      }
  }
@@ -2538,6 +2599,8 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
          vms->iommu = VIRT_IOMMU_VIRTIO;
          vms->virtio_iommu_bdf = pci_get_bdf(pdev);
          create_virtio_iommu_dt_bindings(vms);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
+        virt_virtio_md_pci_plug(hotplug_dev, dev, errp);
      }
  }
@@ -2588,6 +2651,8 @@ static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
  {
      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
          virt_dimm_unplug_request(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
+        virt_virtio_md_pci_unplug_request(hotplug_dev, dev, errp);
      } else {
          error_setg(errp, "device unplug request for unsupported device"
                     " type: %s", object_get_typename(OBJECT(dev)));
@@ -2611,7 +2676,8 @@ static HotplugHandler 
*virt_machine_get_hotplug_handler(MachineState *machine,
      MachineClass *mc = MACHINE_GET_CLASS(machine);
if (device_is_dynamic_sysbus(mc, dev) ||
-       (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/virtio-mem.c b/hw/virtio/virtio-mem.c
index d5a578142b..3033692a83 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -126,6 +126,8 @@ static uint64_t virtio_mem_default_block_size(RAMBlock *rb)
   */
  #if defined(TARGET_X86_64) || defined(TARGET_I386)
  #define VIRTIO_MEM_USABLE_EXTENT (2 * (128 * MiB))
+#elif defined(TARGET_ARM)
+#define VIRTIO_MEM_USABLE_EXTENT (2 * (1024 * MiB))


Can we make this 512 MiB ?

arch/arm64/include/asm/sparsemem.h

/*
  * Section size must be at least 512MB for 64K base
  * page size config. Otherwise it will be less than
  * (MAX_ORDER - 1) and the build process will fail.
  */
#ifdef CONFIG_ARM64_64K_PAGES
#define SECTION_SIZE_BITS 29

#else

/*
  * Section size must be at least 128MB for 4K base
  * page size config. Otherwise PMD based huge page
  * entries could not be created for vmemmap mappings.
  * 16K follows 4K for simplicity.
  */
#define SECTION_SIZE_BITS 27
#endif /* CONFIG_ARM64_64K_PAGES */


Apart from that, LGTM -- thanks!


Indeed. The scetion size has been changed by the following
linux commit. v2 will include the changes.

f0b13ee2324184 arm64/sparsemem: reduce SECTION_SIZE_BITS
(Wed Jan 20 21:29:13 2021 Sudarshan Rajagopalan <sudaraja@codeaurora.org>)

Thanks,
Gavin





reply via email to

[Prev in Thread] Current Thread [Next in Thread]