* [PATCH v2 0/2] hw/arm/virt: Support for virtio-mem-pci
@ 2021-12-03 3:35 Gavin Shan
2021-12-03 3:35 ` [PATCH v2 1/2] " Gavin Shan
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Gavin Shan @ 2021-12-03 3:35 UTC (permalink / raw)
To: qemu-arm
Cc: peter.maydell, drjones, david, richard.henderson, qemu-devel,
eric.auger, shan.gavin, Jonathan.Cameron, imammedo
This series supports virtio-mem-pci device, by simply following the
implementation on x86. The exception is the block size is 512MB on
ARM64 instead of 128MB on x86, compatible with the memory section
size in linux guest.
The work was done by David Hildenbrand and then Jonathan Cameron. I'm
taking the patch and putting more efforts, which is all about testing
to me at current stage.
Testing
=======
The upstream linux kernel (v5.16.rc3) is used on host/guest during
the testing. The guest kernel includes changes to enable virtio-mem
driver, which is simply to enable CONFIG_VIRTIO_MEM on ARM64.
Mutiple combinations like page sizes on host/guest, memory backend
device etc are covered in the testing. Besides, migration is also
tested. The following command lines are used for VM or virtio-mem-pci
device hot-add. It's notable that virtio-mem-pci device hot-remove
isn't supported, similar to what we have on x86.
host.pgsize guest.pgsize backend hot-add hot-remove migration
---------------------------------------------------------------------
4KB 4KB normal ok ok ok
THP ok ok ok
hugeTLB ok ok ok
4KB 64KB normal ok ok ok
THP ok ok ok
hugeTLB ok ok ok
64KB 4KB normal ok ok ok
THP ok ok ok
hugeTLB ok ok ok
64KB 64KB normal ok ok ok
THP ok ok ok
hugeTLB ok ok ok
The command lines are used for VM. When hugeTLBfs is used, all memory
backend objects are popuated on /dev/hugepages-2048kB or
/dev/hugepages-524288kB, depending on the host page sizes.
/home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
-accel kvm -machine virt,gic-version=host \
-cpu host -smp 4,sockets=2,cores=2,threads=1 \
-m 1024M,slots=16,maxmem=64G \
-object memory-backend-ram,id=mem0,size=512M \
-object memory-backend-ram,id=mem1,size=512M \
-numa node,nodeid=0,cpus=0-1,memdev=mem0 \
-numa node,nodeid=1,cpus=2-3,memdev=mem1 \
:
-kernel /home/gavin/sandbox/linux.guest/arch/arm64/boot/Image \
-initrd /home/gavin/sandbox/images/rootfs.cpio.xz \
-append earlycon=pl011,mmio,0x9000000 \
-device pcie-root-port,bus=pcie.0,chassis=1,id=pcie.1 \
-device pcie-root-port,bus=pcie.0,chassis=2,id=pcie.2 \
-device pcie-root-port,bus=pcie.0,chassis=3,id=pcie.3 \
-object memory-backend-ram,id=vmem0,size=512M \
-device virtio-mem-pci,id=vm0,bus=pcie.1,memdev=vmem0,node=0,requested-size=0 \
-object memory-backend-ram,id=vmem1,size=512M \
-device virtio-mem-pci,id=vm1,bus=pcie.2,memdev=vmem1,node=1,requested-size=0
Command lines used for memory hot-add and hot-remove:
(qemu) qom-set vm1 requested-size 512M
(qemu) qom-set vm1 requested-size 0
(qemu) qom-set vm1 requested-size 512M
Command lines used for virtio-mem-pci device hot-add:
(qemu) object_add memory-backend-ram,id=hp-mem1,size=512M
(qemu) device_add virtio-mem-pci,id=hp-vm1,bus=pcie.3,memdev=hp-mem1,node=1
(qemu) qom-set hp-vm1 requested-size 512M
(qemu) qom-set hp-vm1 requested-size 0
(qemu) qom-set hp-vm1 requested-size 512M
Changelog
=========
v2:
* Include David/Jonathan as co-developers in the commit log (David)
* Decrease VIRTIO_MEM_USABLE_EXTENT to 512MB on ARM64 in PATCH[1/2] (David)
* PATCH[2/2] is added to correct the THP sizes on ARM64 (David)
Gavin Shan (2):
hw/arm/virt: Support for virtio-mem-pci
virtio-mem: Correct default THP size for ARM64
hw/arm/Kconfig | 1 +
hw/arm/virt.c | 68 +++++++++++++++++++++++++++++++++++++++++-
hw/virtio/virtio-mem.c | 36 ++++++++++++++--------
3 files changed, 91 insertions(+), 14 deletions(-)
--
2.23.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] hw/arm/virt: Support for virtio-mem-pci
2021-12-03 3:35 [PATCH v2 0/2] hw/arm/virt: Support for virtio-mem-pci Gavin Shan
@ 2021-12-03 3:35 ` Gavin Shan
2021-12-03 18:18 ` David Hildenbrand
2021-12-03 3:35 ` [PATCH v2 2/2] virtio-mem: Correct default THP size for ARM64 Gavin Shan
2021-12-03 14:10 ` [PATCH v2 0/2] hw/arm/virt: Support for virtio-mem-pci Jonathan Cameron via
2 siblings, 1 reply; 9+ messages in thread
From: Gavin Shan @ 2021-12-03 3:35 UTC (permalink / raw)
To: qemu-arm
Cc: peter.maydell, drjones, david, richard.henderson, qemu-devel,
eric.auger, shan.gavin, Jonathan.Cameron, imammedo
This supports virtio-mem-pci device on "virt" platform, by simply
following the implementation on x86.
* 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 512MB on ARM64 instead of 128MB on x86.
* 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.
Co-developed-by: David Hildenbrand <david@redhat.com>
Co-developed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
hw/arm/Kconfig | 1 +
hw/arm/virt.c | 68 +++++++++++++++++++++++++++++++++++++++++-
hw/virtio/virtio-mem.c | 4 ++-
3 files changed, 71 insertions(+), 2 deletions(-)
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..ac7a40f514 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -117,7 +117,7 @@ static uint64_t virtio_mem_default_block_size(RAMBlock *rb)
* The memory block size corresponds mostly to the section size.
*
* This allows e.g., to add 20MB with a section size of 128MB on x86_64, and
- * a section size of 1GB on arm64 (as long as the start address is properly
+ * a section size of 512MB on arm64 (as long as the start address is properly
* aligned, similar to ordinary DIMMs).
*
* We can change this at any time and maybe even make it configurable if
@@ -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 * (512 * MiB))
#else
#error VIRTIO_MEM_USABLE_EXTENT not defined
#endif
--
2.23.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] virtio-mem: Correct default THP size for ARM64
2021-12-03 3:35 [PATCH v2 0/2] hw/arm/virt: Support for virtio-mem-pci Gavin Shan
2021-12-03 3:35 ` [PATCH v2 1/2] " Gavin Shan
@ 2021-12-03 3:35 ` Gavin Shan
2021-12-03 18:16 ` David Hildenbrand
2021-12-03 14:10 ` [PATCH v2 0/2] hw/arm/virt: Support for virtio-mem-pci Jonathan Cameron via
2 siblings, 1 reply; 9+ messages in thread
From: Gavin Shan @ 2021-12-03 3:35 UTC (permalink / raw)
To: qemu-arm
Cc: peter.maydell, drjones, david, richard.henderson, qemu-devel,
eric.auger, shan.gavin, Jonathan.Cameron, imammedo
The default block size is same as to the THP size, which is either
retrieved from "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size"
or hardcoded to 2MB. There are flaws in both mechanisms and this
intends to fix them up.
* When "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size" is
used to getting the THP size, 32MB and 512MB are valid values
when we have 16KB and 64KB page size on ARM64.
* When the hardcoded THP size is used, 2MB, 32MB and 512MB are
valid values when we have 4KB, 16KB and 64KB page sizes on
ARM64.
Co-developed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
hw/virtio/virtio-mem.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index ac7a40f514..8f3c95300f 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -38,14 +38,25 @@
*/
#define VIRTIO_MEM_MIN_BLOCK_SIZE ((uint32_t)(1 * MiB))
-#if defined(__x86_64__) || defined(__arm__) || defined(__aarch64__) || \
- defined(__powerpc64__)
-#define VIRTIO_MEM_DEFAULT_THP_SIZE ((uint32_t)(2 * MiB))
-#else
- /* fallback to 1 MiB (e.g., the THP size on s390x) */
-#define VIRTIO_MEM_DEFAULT_THP_SIZE VIRTIO_MEM_MIN_BLOCK_SIZE
+static uint32_t virtio_mem_default_thp_size(void)
+{
+ uint32_t default_thp_size = VIRTIO_MEM_MIN_BLOCK_SIZE;
+
+#if defined(__x86_64__) || defined(__arm__) || defined(__powerpc64__)
+ default_thp_size = (uint32_t)(2 * MiB);
+#elif defined(__aarch64__)
+ if (qemu_real_host_page_size == (4 * KiB)) {
+ default_thp_size = (uint32_t)(2 * MiB);
+ } else if (qemu_real_host_page_size == (16 * KiB)) {
+ default_thp_size = (uint32_t)(32 * MiB);
+ } else if (qemu_real_host_page_size == (64 * KiB)) {
+ default_thp_size = (uint32_t)(512 * MiB);
+ }
#endif
+ return default_thp_size;
+}
+
/*
* We want to have a reasonable default block size such that
* 1. We avoid splitting THPs when unplugging memory, which degrades
@@ -78,11 +89,8 @@ static uint32_t virtio_mem_thp_size(void)
if (g_file_get_contents(HPAGE_PMD_SIZE_PATH, &content, NULL, NULL) &&
!qemu_strtou64(content, &endptr, 0, &tmp) &&
(!endptr || *endptr == '\n')) {
- /*
- * Sanity-check the value, if it's too big (e.g., aarch64 with 64k base
- * pages) or weird, fallback to something smaller.
- */
- if (!tmp || !is_power_of_2(tmp) || tmp > 16 * MiB) {
+ /* Sanity-check the value and fallback to something reasonable. */
+ if (!tmp || !is_power_of_2(tmp)) {
warn_report("Read unsupported THP size: %" PRIx64, tmp);
} else {
thp_size = tmp;
@@ -90,7 +98,7 @@ static uint32_t virtio_mem_thp_size(void)
}
if (!thp_size) {
- thp_size = VIRTIO_MEM_DEFAULT_THP_SIZE;
+ thp_size = virtio_mem_default_thp_size();
warn_report("Could not detect THP size, falling back to %" PRIx64
" MiB.", thp_size / MiB);
}
--
2.23.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/2] hw/arm/virt: Support for virtio-mem-pci
2021-12-03 3:35 [PATCH v2 0/2] hw/arm/virt: Support for virtio-mem-pci Gavin Shan
2021-12-03 3:35 ` [PATCH v2 1/2] " Gavin Shan
2021-12-03 3:35 ` [PATCH v2 2/2] virtio-mem: Correct default THP size for ARM64 Gavin Shan
@ 2021-12-03 14:10 ` Jonathan Cameron via
2021-12-03 23:06 ` Gavin Shan
2 siblings, 1 reply; 9+ messages in thread
From: Jonathan Cameron via @ 2021-12-03 14:10 UTC (permalink / raw)
To: Gavin Shan
Cc: qemu-arm, qemu-devel, david, eric.auger, drjones, imammedo,
peter.maydell, richard.henderson, shan.gavin
On Fri, 3 Dec 2021 11:35:20 +0800
Gavin Shan <gshan@redhat.com> wrote:
> This series supports virtio-mem-pci device, by simply following the
> implementation on x86. The exception is the block size is 512MB on
> ARM64 instead of 128MB on x86, compatible with the memory section
> size in linux guest.
>
> The work was done by David Hildenbrand and then Jonathan Cameron. I'm
> taking the patch and putting more efforts, which is all about testing
> to me at current stage.
Hi Gavin,
Thanks for taking this forwards. What you have here looks good to me, but
I've not looked at this for a while, so I'll go with whatever David and
others say :)
Jonathan
>
> Testing
> =======
> The upstream linux kernel (v5.16.rc3) is used on host/guest during
> the testing. The guest kernel includes changes to enable virtio-mem
> driver, which is simply to enable CONFIG_VIRTIO_MEM on ARM64.
>
> Mutiple combinations like page sizes on host/guest, memory backend
> device etc are covered in the testing. Besides, migration is also
> tested. The following command lines are used for VM or virtio-mem-pci
> device hot-add. It's notable that virtio-mem-pci device hot-remove
> isn't supported, similar to what we have on x86.
>
> host.pgsize guest.pgsize backend hot-add hot-remove migration
> ---------------------------------------------------------------------
> 4KB 4KB normal ok ok ok
> THP ok ok ok
> hugeTLB ok ok ok
> 4KB 64KB normal ok ok ok
> THP ok ok ok
> hugeTLB ok ok ok
> 64KB 4KB normal ok ok ok
> THP ok ok ok
> hugeTLB ok ok ok
> 64KB 64KB normal ok ok ok
> THP ok ok ok
> hugeTLB ok ok ok
>
> The command lines are used for VM. When hugeTLBfs is used, all memory
> backend objects are popuated on /dev/hugepages-2048kB or
> /dev/hugepages-524288kB, depending on the host page sizes.
>
> /home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
> -accel kvm -machine virt,gic-version=host \
> -cpu host -smp 4,sockets=2,cores=2,threads=1 \
> -m 1024M,slots=16,maxmem=64G \
> -object memory-backend-ram,id=mem0,size=512M \
> -object memory-backend-ram,id=mem1,size=512M \
> -numa node,nodeid=0,cpus=0-1,memdev=mem0 \
> -numa node,nodeid=1,cpus=2-3,memdev=mem1 \
> :
> -kernel /home/gavin/sandbox/linux.guest/arch/arm64/boot/Image \
> -initrd /home/gavin/sandbox/images/rootfs.cpio.xz \
> -append earlycon=pl011,mmio,0x9000000 \
> -device pcie-root-port,bus=pcie.0,chassis=1,id=pcie.1 \
> -device pcie-root-port,bus=pcie.0,chassis=2,id=pcie.2 \
> -device pcie-root-port,bus=pcie.0,chassis=3,id=pcie.3 \
> -object memory-backend-ram,id=vmem0,size=512M \
> -device virtio-mem-pci,id=vm0,bus=pcie.1,memdev=vmem0,node=0,requested-size=0 \
> -object memory-backend-ram,id=vmem1,size=512M \
> -device virtio-mem-pci,id=vm1,bus=pcie.2,memdev=vmem1,node=1,requested-size=0
>
> Command lines used for memory hot-add and hot-remove:
>
> (qemu) qom-set vm1 requested-size 512M
> (qemu) qom-set vm1 requested-size 0
> (qemu) qom-set vm1 requested-size 512M
>
> Command lines used for virtio-mem-pci device hot-add:
>
> (qemu) object_add memory-backend-ram,id=hp-mem1,size=512M
> (qemu) device_add virtio-mem-pci,id=hp-vm1,bus=pcie.3,memdev=hp-mem1,node=1
> (qemu) qom-set hp-vm1 requested-size 512M
> (qemu) qom-set hp-vm1 requested-size 0
> (qemu) qom-set hp-vm1 requested-size 512M
>
> Changelog
> =========
> v2:
> * Include David/Jonathan as co-developers in the commit log (David)
> * Decrease VIRTIO_MEM_USABLE_EXTENT to 512MB on ARM64 in PATCH[1/2] (David)
> * PATCH[2/2] is added to correct the THP sizes on ARM64 (David)
>
> Gavin Shan (2):
> hw/arm/virt: Support for virtio-mem-pci
> virtio-mem: Correct default THP size for ARM64
>
> hw/arm/Kconfig | 1 +
> hw/arm/virt.c | 68 +++++++++++++++++++++++++++++++++++++++++-
> hw/virtio/virtio-mem.c | 36 ++++++++++++++--------
> 3 files changed, 91 insertions(+), 14 deletions(-)
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] virtio-mem: Correct default THP size for ARM64
2021-12-03 3:35 ` [PATCH v2 2/2] virtio-mem: Correct default THP size for ARM64 Gavin Shan
@ 2021-12-03 18:16 ` David Hildenbrand
2021-12-03 23:25 ` Gavin Shan
0 siblings, 1 reply; 9+ messages in thread
From: David Hildenbrand @ 2021-12-03 18:16 UTC (permalink / raw)
To: Gavin Shan, qemu-arm
Cc: peter.maydell, drjones, richard.henderson, qemu-devel,
eric.auger, shan.gavin, Jonathan.Cameron, imammedo
On 03.12.21 04:35, Gavin Shan wrote:
> The default block size is same as to the THP size, which is either
> retrieved from "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size"
> or hardcoded to 2MB. There are flaws in both mechanisms and this
> intends to fix them up.
>
> * When "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size" is
> used to getting the THP size, 32MB and 512MB are valid values
> when we have 16KB and 64KB page size on ARM64.
Ah, right, there is 16KB as well :)
>
> * When the hardcoded THP size is used, 2MB, 32MB and 512MB are
> valid values when we have 4KB, 16KB and 64KB page sizes on
> ARM64.
>
> Co-developed-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
> hw/virtio/virtio-mem.c | 32 ++++++++++++++++++++------------
> 1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
> index ac7a40f514..8f3c95300f 100644
> --- a/hw/virtio/virtio-mem.c
> +++ b/hw/virtio/virtio-mem.c
> @@ -38,14 +38,25 @@
> */
> #define VIRTIO_MEM_MIN_BLOCK_SIZE ((uint32_t)(1 * MiB))
>
> -#if defined(__x86_64__) || defined(__arm__) || defined(__aarch64__) || \
> - defined(__powerpc64__)
> -#define VIRTIO_MEM_DEFAULT_THP_SIZE ((uint32_t)(2 * MiB))
> -#else
> - /* fallback to 1 MiB (e.g., the THP size on s390x) */
> -#define VIRTIO_MEM_DEFAULT_THP_SIZE VIRTIO_MEM_MIN_BLOCK_SIZE
> +static uint32_t virtio_mem_default_thp_size(void)
> +{
> + uint32_t default_thp_size = VIRTIO_MEM_MIN_BLOCK_SIZE;
> +
> +#if defined(__x86_64__) || defined(__arm__) || defined(__powerpc64__)
> + default_thp_size = (uint32_t)(2 * MiB);
> +#elif defined(__aarch64__)
> + if (qemu_real_host_page_size == (4 * KiB)) {
you can drop the superfluous (), also in the cases below.
> + default_thp_size = (uint32_t)(2 * MiB);
The explicit cast shouldn't be required.
> + } else if (qemu_real_host_page_size == (16 * KiB)) {
> + default_thp_size = (uint32_t)(32 * MiB);
> + } else if (qemu_real_host_page_size == (64 * KiB)) {
> + default_thp_size = (uint32_t)(512 * MiB);
> + }
> #endif
>
> + return default_thp_size;
> +}
> +
> /*
> * We want to have a reasonable default block size such that
> * 1. We avoid splitting THPs when unplugging memory, which degrades
> @@ -78,11 +89,8 @@ static uint32_t virtio_mem_thp_size(void)
> if (g_file_get_contents(HPAGE_PMD_SIZE_PATH, &content, NULL, NULL) &&
> !qemu_strtou64(content, &endptr, 0, &tmp) &&
> (!endptr || *endptr == '\n')) {
> - /*
> - * Sanity-check the value, if it's too big (e.g., aarch64 with 64k base
> - * pages) or weird, fallback to something smaller.
> - */
> - if (!tmp || !is_power_of_2(tmp) || tmp > 16 * MiB) {
> + /* Sanity-check the value and fallback to something reasonable. */
> + if (!tmp || !is_power_of_2(tmp)) {
> warn_report("Read unsupported THP size: %" PRIx64, tmp);
> } else {
> thp_size = tmp;
> @@ -90,7 +98,7 @@ static uint32_t virtio_mem_thp_size(void)
> }
>
> if (!thp_size) {
> - thp_size = VIRTIO_MEM_DEFAULT_THP_SIZE;
> + thp_size = virtio_mem_default_thp_size();
> warn_report("Could not detect THP size, falling back to %" PRIx64
> " MiB.", thp_size / MiB);
> }
>
Apart from that,
Reviewed-by: David Hildenbrand <david@redhat.com>
Thanks!
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] hw/arm/virt: Support for virtio-mem-pci
2021-12-03 3:35 ` [PATCH v2 1/2] " Gavin Shan
@ 2021-12-03 18:18 ` David Hildenbrand
2021-12-03 23:23 ` Gavin Shan
0 siblings, 1 reply; 9+ messages in thread
From: David Hildenbrand @ 2021-12-03 18:18 UTC (permalink / raw)
To: Gavin Shan, qemu-arm
Cc: peter.maydell, drjones, richard.henderson, qemu-devel,
eric.auger, shan.gavin, Jonathan.Cameron, imammedo
On 03.12.21 04:35, Gavin Shan wrote:
> This supports virtio-mem-pci device on "virt" platform, by simply
> following the implementation on x86.
>
> * 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 512MB on ARM64 instead of 128MB on x86.
>
> * 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.
>
I would turn this patch into 2/2, reshuffling both patches.
> Co-developed-by: David Hildenbrand <david@redhat.com>
> Co-developed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Thanks Gavin!
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/2] hw/arm/virt: Support for virtio-mem-pci
2021-12-03 14:10 ` [PATCH v2 0/2] hw/arm/virt: Support for virtio-mem-pci Jonathan Cameron via
@ 2021-12-03 23:06 ` Gavin Shan
0 siblings, 0 replies; 9+ messages in thread
From: Gavin Shan @ 2021-12-03 23:06 UTC (permalink / raw)
To: Jonathan Cameron
Cc: peter.maydell, drjones, david, richard.henderson, qemu-devel,
eric.auger, qemu-arm, shan.gavin, imammedo
Hi Jonathan,
On 12/4/21 1:10 AM, Jonathan Cameron wrote:
> On Fri, 3 Dec 2021 11:35:20 +0800
> Gavin Shan <gshan@redhat.com> wrote:
>
>> This series supports virtio-mem-pci device, by simply following the
>> implementation on x86. The exception is the block size is 512MB on
>> ARM64 instead of 128MB on x86, compatible with the memory section
>> size in linux guest.
>>
>> The work was done by David Hildenbrand and then Jonathan Cameron. I'm
>> taking the patch and putting more efforts, which is all about testing
>> to me at current stage.
>
> Thanks for taking this forwards. What you have here looks good to me, but
> I've not looked at this for a while, so I'll go with whatever David and
> others say :)
>
[...]
I would translate this as your reviewed-by tag, which will be added to v3.
However, it shouldn't stop you from further reviewing :)
Thanks,
Gavin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] hw/arm/virt: Support for virtio-mem-pci
2021-12-03 18:18 ` David Hildenbrand
@ 2021-12-03 23:23 ` Gavin Shan
0 siblings, 0 replies; 9+ messages in thread
From: Gavin Shan @ 2021-12-03 23:23 UTC (permalink / raw)
To: David Hildenbrand, qemu-arm
Cc: peter.maydell, drjones, richard.henderson, qemu-devel,
eric.auger, shan.gavin, Jonathan.Cameron, imammedo
On 12/4/21 5:18 AM, David Hildenbrand wrote:
> On 03.12.21 04:35, Gavin Shan wrote:
>> This supports virtio-mem-pci device on "virt" platform, by simply
>> following the implementation on x86.
>>
>> * 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 512MB on ARM64 instead of 128MB on x86.
>>
>> * 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.
>>
>
> I would turn this patch into 2/2, reshuffling both patches.
>
>> Co-developed-by: David Hildenbrand <david@redhat.com>
>> Co-developed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>
> Reviewed-by: David Hildenbrand <david@redhat.com>
>
> Thanks Gavin!
>
Yup, I thought of it. The fixed issue doesn't exist if virtio-mem
isn't enabled on ARM64 with PATCH[1/2]. That's why I have this
patch as PATCH[2/2]. However, It's also sensible to me to reshuffle
the patches: to eliminate potential issues before enabling virtio-mem
on ARM64. v3 will have the changes :)
Thanks,
Gavin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] virtio-mem: Correct default THP size for ARM64
2021-12-03 18:16 ` David Hildenbrand
@ 2021-12-03 23:25 ` Gavin Shan
0 siblings, 0 replies; 9+ messages in thread
From: Gavin Shan @ 2021-12-03 23:25 UTC (permalink / raw)
To: David Hildenbrand, qemu-arm
Cc: peter.maydell, drjones, richard.henderson, qemu-devel,
eric.auger, shan.gavin, Jonathan.Cameron, imammedo
On 12/4/21 5:16 AM, David Hildenbrand wrote:
> On 03.12.21 04:35, Gavin Shan wrote:
>> The default block size is same as to the THP size, which is either
>> retrieved from "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size"
>> or hardcoded to 2MB. There are flaws in both mechanisms and this
>> intends to fix them up.
>>
>> * When "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size" is
>> used to getting the THP size, 32MB and 512MB are valid values
>> when we have 16KB and 64KB page size on ARM64.
>
> Ah, right, there is 16KB as well :)
>
Yep, even though it's rarely used :)
>>
>> * When the hardcoded THP size is used, 2MB, 32MB and 512MB are
>> valid values when we have 4KB, 16KB and 64KB page sizes on
>> ARM64.
>>
>> Co-developed-by: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>> hw/virtio/virtio-mem.c | 32 ++++++++++++++++++++------------
>> 1 file changed, 20 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
>> index ac7a40f514..8f3c95300f 100644
>> --- a/hw/virtio/virtio-mem.c
>> +++ b/hw/virtio/virtio-mem.c
>> @@ -38,14 +38,25 @@
>> */
>> #define VIRTIO_MEM_MIN_BLOCK_SIZE ((uint32_t)(1 * MiB))
>>
>> -#if defined(__x86_64__) || defined(__arm__) || defined(__aarch64__) || \
>> - defined(__powerpc64__)
>> -#define VIRTIO_MEM_DEFAULT_THP_SIZE ((uint32_t)(2 * MiB))
>> -#else
>> - /* fallback to 1 MiB (e.g., the THP size on s390x) */
>> -#define VIRTIO_MEM_DEFAULT_THP_SIZE VIRTIO_MEM_MIN_BLOCK_SIZE
>> +static uint32_t virtio_mem_default_thp_size(void)
>> +{
>> + uint32_t default_thp_size = VIRTIO_MEM_MIN_BLOCK_SIZE;
>> +
>> +#if defined(__x86_64__) || defined(__arm__) || defined(__powerpc64__)
>> + default_thp_size = (uint32_t)(2 * MiB);
>> +#elif defined(__aarch64__)
>> + if (qemu_real_host_page_size == (4 * KiB)) {
>
> you can drop the superfluous (), also in the cases below.
>
It will be included in v3.
>> + default_thp_size = (uint32_t)(2 * MiB);
>
> The explicit cast shouldn't be required.
>
It's not required, but inherited from the definition
of VIRTIO_MEM_MIN_BLOCK_SIZE. However, it's safe to
drop the cast and it will be included in v3.
>> + } else if (qemu_real_host_page_size == (16 * KiB)) {
>> + default_thp_size = (uint32_t)(32 * MiB);
>> + } else if (qemu_real_host_page_size == (64 * KiB)) {
>> + default_thp_size = (uint32_t)(512 * MiB);
>> + }
>> #endif
>>
>> + return default_thp_size;
>> +}
>> +
>> /*
>> * We want to have a reasonable default block size such that
>> * 1. We avoid splitting THPs when unplugging memory, which degrades
>> @@ -78,11 +89,8 @@ static uint32_t virtio_mem_thp_size(void)
>> if (g_file_get_contents(HPAGE_PMD_SIZE_PATH, &content, NULL, NULL) &&
>> !qemu_strtou64(content, &endptr, 0, &tmp) &&
>> (!endptr || *endptr == '\n')) {
>> - /*
>> - * Sanity-check the value, if it's too big (e.g., aarch64 with 64k base
>> - * pages) or weird, fallback to something smaller.
>> - */
>> - if (!tmp || !is_power_of_2(tmp) || tmp > 16 * MiB) {
>> + /* Sanity-check the value and fallback to something reasonable. */
>> + if (!tmp || !is_power_of_2(tmp)) {
>> warn_report("Read unsupported THP size: %" PRIx64, tmp);
>> } else {
>> thp_size = tmp;
>> @@ -90,7 +98,7 @@ static uint32_t virtio_mem_thp_size(void)
>> }
>>
>> if (!thp_size) {
>> - thp_size = VIRTIO_MEM_DEFAULT_THP_SIZE;
>> + thp_size = virtio_mem_default_thp_size();
>> warn_report("Could not detect THP size, falling back to %" PRIx64
>> " MiB.", thp_size / MiB);
>> }
>>
>
> Apart from that,
>
> Reviewed-by: David Hildenbrand <david@redhat.com>
>
Thanks for your review, David!
Thanks,
Gavin
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-12-03 23:50 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-03 3:35 [PATCH v2 0/2] hw/arm/virt: Support for virtio-mem-pci Gavin Shan
2021-12-03 3:35 ` [PATCH v2 1/2] " Gavin Shan
2021-12-03 18:18 ` David Hildenbrand
2021-12-03 23:23 ` Gavin Shan
2021-12-03 3:35 ` [PATCH v2 2/2] virtio-mem: Correct default THP size for ARM64 Gavin Shan
2021-12-03 18:16 ` David Hildenbrand
2021-12-03 23:25 ` Gavin Shan
2021-12-03 14:10 ` [PATCH v2 0/2] hw/arm/virt: Support for virtio-mem-pci Jonathan Cameron via
2021-12-03 23:06 ` Gavin Shan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.