virtio-mem: Paravirtualized memory hot(un)plug
v4 cover virtio-mem: Paravirtualized memory hot(un)plug
This is the very basic, initial version of virtio-mem. More info on virtio-mem in general can be found in the Linux kernel driver v2 posting
[1] and in patch #10. The Linux driver is currently on its way upstream.
This series is based on [3]:
"[PATCH v1] pc: Support coldplugging of virtio-pmem-pci devices on all
buses"
And [4]:
"[PATCH v2] hmp: Make json format optional for qom-set"
The patches can be found at:
https://github.com/davidhildenbrand/qemu.git virtio-mem-v4
"The basic idea of virtio-mem is to provide a flexible, cross-architecture memory hot(un)plug solution that avoids many limitations imposed by existing technologies, architectures, and interfaces."
There are a lot of addons in the works (esp. protection of unplugged memory, better hugepage support (esp. when reading unplugged memory), resizeable memory backends, support for more architectures, ...), this is the very basic version to get the ball rolling.
The first 8 patches make sure we don't have any sudden surprises e.g., if somebody tries to pin all memory in RAM blocks, resulting in a higher memory consumption than desired. The remaining patches add basic virtio-mem along with support for x86-64. The last patch indicates to the guest OS the maximum possible PFN using ACPI SRAT, such that Linux can properly enable the swiotlb when booting only with DMA memory.
[1] https://lkml.kernel.org/r/[email protected]
[2] https://lkml.kernel.org/r/[email protected]
[3] https://lkml.kernel.org/r/[email protected]
[3] https://lkml.kernel.org/r/[email protected]
Based-on: <[email protected]>
Based-on: <[email protected]>
Cc: teawater <[email protected]>
Cc: Pankaj Gupta <[email protected]>
v3 -> v4
- Adapt to virtio-mem config layout change (block size now is 64bit)
- Added "numa: Auto-enable NUMA when any memory devices are possible"
v2 -> v3:
- Rebased on upstream/[3]
- "virtio-mem: Exclude unplugged memory during migration" -- Added
- "virtio-mem: Paravirtualized memory hot(un)plug" -- Simplify bitmap operations, find consecutive areas -- Tweak error messages
-- Reshuffle some checks
-- Minor cleanups
- "accel/kvm: Convert to ram_block_discard_disable()"
- "target/i386: sev: Use ram_block_discard_disable()" -- Keep asserts clean of functional things
v1 -> v2:
- Rebased to object_property_*() changes
- "exec: Introduce ram_block_discard_(disable|require)()"
-- Change the function names and rephrase/add comments
- "virtio-balloon: Rip out qemu_balloon_inhibit()"
-- Add and use "migration_in_incoming_postcopy()"
- "migration/rdma: Use ram_block_discard_disable()"
-- Add a comment regarding pin_all vs. !pin_all
- "virtio-mem: Paravirtualized memory hot(un)plug"
-- Replace virtio_mem_discard_inhibited() by
migration_in_incoming_postcopy()
-- Drop some asserts
-- Drop virtio_mem_bad_request(), use virtio_error() directly, printing
more information
-- Replace "Note: Discarding should never fail ..." comments by
error_report()
-- Replace virtio_stw_p() by cpu_to_le16()
-- Drop migration_addr and migration_block_size
-- Minor cleanups
- "linux-headers: update to contain virtio-mem"
-- Updated to latest v4 in Linux
- General changes
-- Fixup the users of the renamed ram_block_discard_(disable|require) -- Use "X: cannot disable RAM discard"-styled error messages
- Added
-- "virtio-mem: Migration sanity checks"
-- "virtio-mem: Add trace events"
David Hildenbrand (21):
exec: Introduce ram_block_discard_(disable|require)()
vfio: Convert to ram_block_discard_disable() accel/kvm: Convert to ram_block_discard_disable() s390x/pv: Convert to ram_block_discard_disable()
virtio-balloon: Rip out qemu_balloon_inhibit() target/i386: sev: Use ram_block_discard_disable() migration/rdma: Use ram_block_discard_disable() migration/colo: Use ram_block_discard_disable()
linux-headers: update to contain virtio-mem
virtio-mem: Paravirtualized memory hot(un)plug
virtio-pci: Proxy for virtio-mem
MAINTAINERS: Add myself as virtio-mem maintainer
hmp: Handle virtio-mem when printing memory device info
numa: Handle virtio-mem in NUMA stats
pc: Support for virtio-mem-pci
virtio-mem: Allow notifiers for size changes
virtio-pci: Send qapi events when the virtio-mem size changes
virtio-mem: Migration sanity checks
virtio-mem: Add trace events
virtio-mem: Exclude unplugged memory during migration
numa: Auto-enable NUMA when any memory devices are possible
MAINTAINERS | 8 + accel/kvm/kvm-all.c | 4 +- balloon.c | 17 - exec.c | 52 ++ hw/arm/virt.c | 2 + hw/core/numa.c | 17 +- hw/i386/Kconfig | 1 + hw/i386/microvm.c | 1 + hw/i386/pc.c | 50 +- hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c | 1 + hw/s390x/s390-virtio-ccw.c | 22 +- hw/vfio/ap.c | 10 +- hw/vfio/ccw.c | 11 +- hw/vfio/common.c | 53 +- hw/vfio/pci.c | 6 +- hw/virtio/Kconfig | 11 + hw/virtio/Makefile.objs | 2 + hw/virtio/trace-events | 10 + hw/virtio/virtio-balloon.c | 8 +- hw/virtio/virtio-mem-pci.c | 157 ++++ hw/virtio/virtio-mem-pci.h | 34 + hw/virtio/virtio-mem.c | 872 ++++++++++++++++++++ include/exec/memory.h | 41 + include/hw/boards.h | 1 + include/hw/pci/pci.h | 1 + include/hw/vfio/vfio-common.h | 4 +- include/hw/virtio/virtio-mem.h | 86 ++ include/migration/colo.h | 2 +- include/migration/misc.h | 2 + include/standard-headers/linux/virtio_ids.h | 1 + include/standard-headers/linux/virtio_mem.h | 211 +++++ include/sysemu/balloon.h | 2 - migration/migration.c | 15 +- migration/postcopy-ram.c | 23 - migration/rdma.c | 18 +- migration/savevm.c | 11 +- monitor/hmp-cmds.c | 16 + monitor/monitor.c | 1 + qapi/misc.json | 64 +- target/i386/sev.c | 7 + 41 files changed, 1730 insertions(+), 126 deletions(-) create mode 100644 hw/virtio/virtio-mem-pci.c create mode 100644 hw/virtio/virtio-mem-pci.h create mode 100644 hw/virtio/virtio-mem.c create mode 100644 include/hw/virtio/virtio-mem.h create mode 100644 include/standard-headers/linux/virtio_mem.h
Signature · 1 line
2.26.2
PATCH v4 1/21 exec: Introduce ram_block_discard_(disable|require)()
We want to replace qemu_balloon_inhibit() by something more generic. Especially, we want to make sure that technologies that really rely on RAM block discards to work reliably to run mutual exclusive with technologies that effectively break it.
E.g., vfio will usually pin all guest memory, turning the virtio-balloon basically useless and make the VM consume more memory than reported via the balloon. While the balloon is special already (=> no guarantees, same behavior possible afer reboots and with huge pages), this will be different, especially, with virtio-mem.
Let's implement a way such that we can make both types of technology run mutually exclusive. We'll convert existing balloon inhibitors in successive patches and add some new ones. Add the check to qemu_balloon_is_inhibited() for now. We might want to make virtio-balloon an acutal inhibitor in the future - however, that requires more thought to not break existing setups.
--- balloon.c | 3 ++- exec.c | 52 +++++++++++++++++++++++++++++++++++++++++++ include/exec/memory.h | 41 ++++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 1 deletion(-)
balloon.c +2 -1
@@ -40,7 +40,8 @@ static int balloon_inhibit_count;bool qemu_balloon_is_inhibited (void ) { -return atomic_read (&balloon_inhibit_count) >0 ; +return atomic_read (&balloon_inhibit_count) >0 || +ram_block_discard_is_disabled (); }void qemu_balloon_inhibit (bool state)
exec.c +52 -0
56 lines not on this page · raw message
include/exec/memory.h +41 -0
47 lines not on this page · raw message
Signature · 1 line
2.26.2
PATCH v4 2/21 vfio: Convert to ram_block_discard_disable()
In reply to David HildenbrandVFIO is (except devices without a physical IOMMU or some mediated devices) incompatible with discarding of RAM. The kernel will pin basically all VM memory. Let's convert to ram_block_discard_disable(), which can now fail, in contrast to qemu_balloon_inhibit().
Leave "x-balloon-allowed" named as it is for now.
--- hw/vfio/ap.c | 10 +++---- hw/vfio/ccw.c | 11 ++++---- hw/vfio/common.c | 53 +++++++++++++++++++---------------- hw/vfio/pci.c | 6 ++-- include/hw/vfio/vfio-common.h | 4 +-- 5 files changed, 45 insertions(+), 39 deletions(-)
hw/vfio/ap.c +5 -5
@@ -105,12 +105,12 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp) vapdev->vdev.dev = dev;/* -* vfio-ap devices operate in a way compatible with -* memory ballooning, as no pages are pinned in the host. -* This needs to be set before vfio_get_device() for vfio common to -* handle the balloon inhibitor. +* vfio-ap devices operate in a way compatible discarding of memory in +* RAM blocks, as no pages are pinned in the host. This needs to be +* set before vfio_get_device() for vfio common to handle +* ram_block_discard_disable(). */ - vapdev->vdev.balloon_allowed = true; + vapdev->vdev.ram_block_discard_allowed = true; ret =vfio_get_device (vfio_group, mdevid, &vapdev->vdev, errp);if (ret) {
hw/vfio/ccw.c +6 -5
@@ -418,12 +418,13 @@ static void vfio_ccw_get_device(VFIOGroup *group, VFIOCCWDevice *vcdev,/* * All vfio-ccw devices are believed to operate in a way compatible with -* memory ballooning, ie. pages pinned in the host are in the current -* working set of the guest driver and therefore never overlap with pages -* available to the guest balloon driver. This needs to be set before -* vfio_get_device() for vfio common to handle the balloon inhibitor. +* discarding of memory in RAM blocks, ie. pages pinned in the host are +* in the current working set of the guest driver and therefore never +* overlap e.g., with pages available to the guest balloon driver. This +* needs to be set before vfio_get_device() for vfio common to handle +* ram_block_discard_disable(). */ - vcdev->vdev.balloon_allowed = true; + vcdev->vdev.ram_block_discard_allowed = true;if (vfio_get_device (group, vcdev->cdev.mdevid, &vcdev->vdev, errp)) { goto out_err;
hw/vfio/common.c +29 -24
104 lines not on this page · raw message
hw/vfio/pci.c +3 -3
@@ -2796,7 +2796,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp) }/* -* Mediated devices *might* operate compatibly with memory ballooning, but +* Mediated devices *might* operate compatibly with discarding of RAM, but * we cannot know for certain, it depends on whether the mdev vendor driver * stays in sync with the active working set of the guest driver. Prevent * the x-balloon-allowed option unless this is minimally an mdev device.
@@ -2809,7 +2809,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)trace_vfio_mdev (vdev->vbasedev.name, is_mdev); -if (vdev->vbasedev.balloon_allowed && !is_mdev) { +if (vdev->vbasedev.ram_block_discard_allowed && !is_mdev) {error_setg (errp,"x-balloon-allowed only potentially compatible " "with mdev devices" );vfio_put_group (group);
@@ -3163,7 +3163,7 @@ static Property vfio_pci_dev_properties[] = { VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),DEFINE_PROP_BOOL ("x-no-mmap" , VFIOPCIDevice, vbasedev.no_mmap, false),DEFINE_PROP_BOOL ("x-balloon-allowed" , VFIOPCIDevice, - vbasedev.balloon_allowed, false), + vbasedev.ram_block_discard_allowed, false),DEFINE_PROP_BOOL ("x-no-kvm-intx" , VFIOPCIDevice, no_kvm_intx, false),DEFINE_PROP_BOOL ("x-no-kvm-msi" , VFIOPCIDevice, no_kvm_msi, false), DEFINE_PROP_BOOL("x-no-kvm-msix", VFIOPCIDevice, no_kvm_msix, false),
include/hw/vfio/vfio-common.h +2 -2
@@ -108,7 +108,7 @@ typedef struct VFIODevice {bool reset_works;bool needs_reset;bool no_mmap; -bool balloon_allowed; +bool ram_block_discard_allowed;VFIODeviceOps *ops;unsigned int num_irqs;unsigned int num_regions;
@@ -128,7 +128,7 @@ typedef struct VFIOGroup { QLIST_HEAD(, VFIODevice) device_list;QLIST_ENTRY (VFIOGroup) next;QLIST_ENTRY (VFIOGroup) container_next; -bool balloon_allowed; +bool ram_block_discard_allowed; } VFIOGroup; typedef struct VFIODMABuf {
Signature · 1 line
2.26.2
Re: [PATCH v4 02/21] vfio: Convert to ram_block_discard_disable()
Quoted 7 lines · David Hildenbrand
VFIO is (except devices without a physical IOMMU or some mediated devices) incompatible with discarding of RAM. The kernel will pin basically all VM memory. Let's convert to
ram_block_discard_disable(), which can now fail, in contrast toqemu_balloon_inhibit().Leave "x-balloon-allowed" named as it is for now.
Quoted 8 lines
Cc Cornelia Huck <[email protected]>Cc Alex Williamson <[email protected]>Cc Christian Borntraeger <[email protected]>Cc Tony Krowiak <[email protected]>Cc Halil Pasic <[email protected]>Cc Pierre Morel <[email protected]>Cc Eric Farman <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
See my two minor comments, other than that:
Quoted 13 lines
--- hw/vfio/ap.c | 10 +++---- hw/vfio/ccw.c | 11 ++++---- hw/vfio/common.c | 53 +++++++++++++++++++---------------- hw/vfio/pci.c | 6 ++-- include/hw/vfio/vfio-common.h | 4 +-- 5 files changed, 45 insertions(+), 39 deletions(-)hw/vfio/ap.c +0
-0
hw/vfio/ap.c static void vfio_ap_realize(DeviceState *dev, Error **errp) · line 108
Quoted 8 lines
hw/vfio/ap.c +1
-4@@ -105,12 +105,12 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp) vapdev->vdev.dev = dev;/* -* vfio-ap devices operate in a way compatible with -* memory ballooning, as no pages are pinned in the host. -* This needs to be set before vfio_get_device() for vfio common to -* handle the balloon inhibitor. +* vfio-ap devices operate in a way compatible discarding of memory in
s/compatible discarding/compatible with discarding/?
Quoted 159 lines
hw/vfio/ap.c +4
-1+* RAM blocks, as no pages are pinned in the host. This needs to be +* set before vfio_get_device() for vfio common to handle +* ram_block_discard_disable(). */ - vapdev->vdev.balloon_allowed = true; + vapdev->vdev.ram_block_discard_allowed = true; ret =vfio_get_device (vfio_group, mdevid, &vapdev->vdev, errp);if (ret) {hw/vfio/ccw.c +6
-5@@ -418,12 +418,13 @@ static void vfio_ccw_get_device(VFIOGroup *group, VFIOCCWDevice *vcdev,/* * All vfio-ccw devices are believed to operate in a way compatible with -* memory ballooning, ie. pages pinned in the host are in the current -* working set of the guest driver and therefore never overlap with pages -* available to the guest balloon driver. This needs to be set before -* vfio_get_device() for vfio common to handle the balloon inhibitor. +* discarding of memory in RAM blocks, ie. pages pinned in the host are +* in the current working set of the guest driver and therefore never +* overlap e.g., with pages available to the guest balloon driver. This +* needs to be set before vfio_get_device() for vfio common to handle +* ram_block_discard_disable(). */ - vcdev->vdev.balloon_allowed = true; + vcdev->vdev.ram_block_discard_allowed = true;if (vfio_get_device (group, vcdev->cdev.mdevid, &vcdev->vdev, errp)) { goto out_err;hw/vfio/common.c +29
-24@@ -33,7 +33,6 @@#include "qemu/error-report.h" #include "qemu/main-loop.h" #include "qemu/range.h" -#include "sysemu/balloon.h" #include "sysemu/kvm.h" #include "sysemu/reset.h" #include "trace.h" @@ -1215,31 +1214,36 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as, space =vfio_get_address_space (as);/* -* VFIO is currently incompatible with memory ballooning insofar as the +* VFIO is currently incompatible with discarding of RAM insofar as the * madvise to purge (zap) the page from QEMU's address space does not * interact with the memory API and therefore leaves stale virtual to * physical mappings in the IOMMU if the page was previously pinned. We -* therefore add a balloon inhibit for each group added to a container, +* therefore set discarding broken for each group added to a container, * whether the container is used individually or shared. This provides * us with options to allow devices within a group to opt-in and allow -* ballooning, so long as it is done consistently for a group (for instance +* discarding, so long as it is done consistently for a group (for instance * if the device is an mdev device where it is known that the host vendor * driver will never pin pages outside of the working set of the guest -* driver, which would thus not be ballooning candidates). +* driver, which would thus not be discarding candidates). * * The first opportunity to induce pinning occurs here where we attempt to * attach the group to existing containers within the AddressSpace. If any -* pages are already zapped from the virtual address space, such as from a -* previous ballooning opt-in, new pinning will cause valid mappings to be +* pages are already zapped from the virtual address space, such as from +* previous discards, new pinning will cause valid mappings to be * re-established. Likewise, when the overall MemoryListener for a new * container is registered, a replay of mappings within the AddressSpace * will occur, re-establishing any previously zapped pages as well. * -* NB. Balloon inhibiting does not currently block operation of the -* balloon driver or revoke previously pinned pages, it only prevents -* calling madvise to modify the virtual mapping of ballooned pages. +* Especially virtio-balloon is currently only prevented from discarding +* new memory, it will not yet set ram_block_discard_set_required() and +* therefore, neither stops us here or deals with the sudden memory +* consumption of inflated memory. */ -qemu_balloon_inhibit (true); + ret =ram_block_discard_disable (true); +if (ret) { +error_setg_errno (errp, -ret,"Cannot set discarding of RAM broken" ); +return ret; + } QLIST_FOREACH(container, &space->containers, next) {if (!ioctl (group->fd,VFIO_GROUP_SET_CONTAINER , &container->fd)) {@@ -1405,7 +1409,7 @@ close_fd_exit:close (fd); put_space_exit: -qemu_balloon_inhibit (false); +ram_block_discard_disable (false);vfio_put_address_space (space);return ret;@@ -1526,8 +1530,8 @@ void vfio_put_group(VFIOGroup *group)return ; } -if (!group->balloon_allowed) { -qemu_balloon_inhibit (false); +if (!group->ram_block_discard_allowed) { +ram_block_discard_disable (false); }vfio_kvm_device_del_group (group);vfio_disconnect_container (group);@@ -1565,22 +1569,23 @@ int vfio_get_device(VFIOGroup *group, const char *name, }/* -* Clear the balloon inhibitor for this group if the driver knows the -* device operates compatibly with ballooning. Setting must be consistent -* per group, but since compatibility is really only possible with mdev -* currently, we expect singleton groups. +* Set discarding of RAM as not broken for this group if the driver knows +* the device operates compatibly with discarding. Setting must be +* consistent per group, but since compatibility is really only possible +* with mdev currently, we expect singleton groups. */ -if (vbasedev->balloon_allowed != group->balloon_allowed) { +if (vbasedev->ram_block_discard_allowed != + group->ram_block_discard_allowed) {if (!QLIST_EMPTY (&group->device_list)) { -error_setg (errp, -"Inconsistent device balloon setting within group" ); +error_setg (errp,"Inconsistent setting of support for discarding " +"RAM (e.g., balloon) within group" );close (fd);return -1 ; } -if (!group->balloon_allowed) { - group->balloon_allowed = true; -qemu_balloon_inhibit (false); +if (!group->ram_block_discard_allowed) { + group->ram_block_discard_allowed = true; +ram_block_discard_disable (false); } }hw/vfio/pci.c +1
-1@@ -2796,7 +2796,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp) }/* -* Mediated devices *might* operate compatibly with memory ballooning, but +* Mediated devices *might* operate compatibly with discarding of RAM, but * we cannot know for certain, it depends on whether the mdev vendor driver * stays in sync with the active working set of the guest driver. Prevent * the x-balloon-allowed option unless this is minimally an mdev device.
hw/vfio/pci.c static void vfio_realize(PCIDevice *pdev, Error **errp) · line 2814
Quoted 7 lines
hw/vfio/pci.c +1
-1@@ -2809,7 +2809,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)trace_vfio_mdev (vdev->vbasedev.name, is_mdev); -if (vdev->vbasedev.balloon_allowed && !is_mdev) { +if (vdev->vbasedev.ram_block_discard_allowed && !is_mdev) {error_setg (errp,"x-balloon-allowed only potentially compatible " "with mdev devices" );
Should this error message be changed?
Quoted 32 lines
hw/vfio/pci.c +1
-1vfio_put_group (group);@@ -3163,7 +3163,7 @@ static Property vfio_pci_dev_properties[] = { VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),DEFINE_PROP_BOOL ("x-no-mmap" , VFIOPCIDevice, vbasedev.no_mmap, false),DEFINE_PROP_BOOL ("x-balloon-allowed" , VFIOPCIDevice, - vbasedev.balloon_allowed, false), + vbasedev.ram_block_discard_allowed, false),DEFINE_PROP_BOOL ("x-no-kvm-intx" , VFIOPCIDevice, no_kvm_intx, false),DEFINE_PROP_BOOL ("x-no-kvm-msi" , VFIOPCIDevice, no_kvm_msi, false), DEFINE_PROP_BOOL("x-no-kvm-msix", VFIOPCIDevice, no_kvm_msix, false),include/hw/vfio/vfio-common.h +2
-2@@ -108,7 +108,7 @@ typedef struct VFIODevice {bool reset_works;bool needs_reset;bool no_mmap; -bool balloon_allowed; +bool ram_block_discard_allowed;VFIODeviceOps *ops;unsigned int num_irqs;unsigned int num_regions;@@ -128,7 +128,7 @@ typedef struct VFIOGroup { QLIST_HEAD(, VFIODevice) device_list;QLIST_ENTRY (VFIOGroup) next;QLIST_ENTRY (VFIOGroup) container_next; -bool balloon_allowed; +bool ram_block_discard_allowed; } VFIOGroup; typedef struct VFIODMABuf {
Re: [PATCH v4 02/21] vfio: Convert to ram_block_discard_disable()
Quoted 37 lines · Tony Krowiak
Quoted 7 lines · David Hildenbrand
VFIO is (except devices without a physical IOMMU or some mediated devices) incompatible with discarding of RAM. The kernel will pin basically all VM memory. Let's convert to
ram_block_discard_disable(), which can now fail, in contrast toqemu_balloon_inhibit().Leave "x-balloon-allowed" named as it is for now.
Quoted 8 lines
Cc Cornelia Huck <[email protected]>Cc Alex Williamson <[email protected]>Cc Christian Borntraeger <[email protected]>Cc Tony Krowiak <[email protected]>Cc Halil Pasic <[email protected]>Cc Pierre Morel <[email protected]>Cc Eric Farman <[email protected]>Signed-off-by David Hildenbrand <[email protected]>See my two minor comments, other than that:
Reviewed-by Tony Krowiak <[email protected]>Quoted 15 lines
--- hw/vfio/ap.c | 10 +++---- hw/vfio/ccw.c | 11 ++++---- hw/vfio/common.c | 53 +++++++++++++++++++---------------- hw/vfio/pci.c | 6 ++-- include/hw/vfio/vfio-common.h | 4 +-- 5 files changed, 45 insertions(+), 39 deletions(-)hw/vfio/ap.c +0
-0@@ -105,12 +105,12 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp) vapdev->vdev.dev = dev;
Quoted 8 lines
hw/vfio/ap.c static void vfio_ap_realize(DeviceState *dev, Error **errp) · line 108
Quoted 6 lines
hw/vfio/ap.c +1
-4/* -* vfio-ap devices operate in a way compatible with -* memory ballooning, as no pages are pinned in the host. -* This needs to be set before vfio_get_device() for vfio common to -* handle the balloon inhibitor. +* vfio-ap devices operate in a way compatible discarding of memory in s/compatible discarding/compatible with discarding/?
Very right!
[...]
Quoted 15 lines
Quoted 15 lines
hw/vfio/pci.c +1
-1@@ -2796,7 +2796,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp) }/* -* Mediated devices *might* operate compatibly with memory ballooning, but +* Mediated devices *might* operate compatibly with discarding of RAM, but * we cannot know for certain, it depends on whether the mdev vendor driver * stays in sync with the active working set of the guest driver. Prevent * the x-balloon-allowed option unless this is minimally an mdev device. @@ -2809,7 +2809,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
Quoted 8 lines
hw/vfio/pci.c static void vfio_realize(PCIDevice *pdev, Error **errp) · line 2814
Quoted 6 lines
hw/vfio/pci.c +1
-1trace_vfio_mdev (vdev->vbasedev.name, is_mdev); -if (vdev->vbasedev.balloon_allowed && !is_mdev) { +if (vdev->vbasedev.ram_block_discard_allowed && !is_mdev) {error_setg (errp,"x-balloon-allowed only potentially compatible " "with mdev devices" );Should this error message be changed?
I didn't rename the property ("x-balloon-allowed"), so the error message is still correct.
Thanks!
Signature · 3 lines
Thanks, David / dhildenb
Re: [PATCH v4 02/21] vfio: Convert to ram_block_discard_disable()
In reply to David HildenbrandQuoted 15 lines · On Wed, 10 Jun 2020 13:54:00 +0200 David Hildenbrand <[email protected]> wrote:
VFIO is (except devices without a physical IOMMU or some mediated devices) incompatible with discarding of RAM. The kernel will pin basically all VM memory. Let's convert to
ram_block_discard_disable(), which can now fail, in contrast toqemu_balloon_inhibit().Leave "x-balloon-allowed" named as it is for now.
Cc Cornelia Huck <[email protected]>Cc Alex Williamson <[email protected]>Cc Christian Borntraeger <[email protected]>Cc Tony Krowiak <[email protected]>Cc Halil Pasic <[email protected]>Cc Pierre Morel <[email protected]>Cc Eric Farman <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 7 lines
--- hw/vfio/ap.c | 10 +++---- hw/vfio/ccw.c | 11 ++++---- hw/vfio/common.c | 53 +++++++++++++++++++---------------- hw/vfio/pci.c | 6 ++-- include/hw/vfio/vfio-common.h | 4 +-- 5 files changed, 45 insertions(+), 39 deletions(-)
Did not have time to review in detail, but looks sane.
PATCH v4 3/21 accel/kvm: Convert to ram_block_discard_disable()
In reply to David HildenbrandDiscarding memory does not work as expected. At the time this is called, we cannot have anyone active that relies on discards to work properly.
--- accel/kvm/kvm-all.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
accel/kvm/kvm-all.c +2 -2
@@ -40,7 +40,6 @@#include "trace.h" #include "hw/irq.h" #include "sysemu/sev.h" -#include "sysemu/balloon.h" #include "qapi/visitor.h" #include "qapi/qapi-types-common.h" #include "qapi/qapi-visit-common.h"
@@ -2143,7 +2142,8 @@ static int kvm_init(MachineState *ms) s->sync_mmu = !!kvm_vm_check_extension (kvm_state,KVM_CAP_SYNC_MMU );if (!s->sync_mmu) { -qemu_balloon_inhibit (true); + ret =ram_block_discard_disable (true); +assert (!ret); }return 0 ;
Signature · 1 line
2.26.2
PATCH v4 4/21 s390x/pv: Convert to ram_block_discard_disable()
In reply to David HildenbrandDiscarding RAM does not work as expected with protected VMs. Let's switch to ram_block_discard_disable() for now, as we want to get rid of qemu_balloon_inhibit(). Note that it will currently never fail, but might fail in the future with new technologies (e.g., virtio-mem).
--- hw/s390x/s390-virtio-ccw.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)
hw/s390x/s390-virtio-ccw.c +13 -9
@@ -43,7 +43,6 @@#include "hw/qdev-properties.h" #include "hw/s390x/tod.h" #include "sysemu/sysemu.h" -#include "sysemu/balloon.h" #include "hw/s390x/pv.h" #include "migration/blocker.h"
@@ -329,7 +328,7 @@ static void s390_machine_unprotect(S390CcwMachineState *ms) ms->pv = false;migrate_del_blocker (pv_mig_blocker);error_free_or_abort (&pv_mig_blocker); -qemu_balloon_inhibit (false); +ram_block_discard_disable (false); }static int s390_machine_protect (S390CcwMachineState *ms)
@@ -338,17 +337,22 @@ static int s390_machine_protect(S390CcwMachineState *ms)int rc;/* -* Ballooning on protected VMs needs support in the guest for -* sharing and unsharing balloon pages. Block ballooning for -* now, until we have a solution to make at least Linux guests -* either support it or fail gracefully. +* Discarding of memory in RAM blocks does not work as expected with +* protected VMs. Sharing and unsharing pages would be required. Disable +* it for now, until until we have a solution to make at least Linux +* guests either support it (e.g., virtio-balloon) or fail gracefully. */ -qemu_balloon_inhibit (true); + rc =ram_block_discard_disable (true); +if (rc) { +error_report ("protected VMs: cannot disable RAM discard" ); +return rc; + } +error_setg (&pv_mig_blocker,"protected VMs are currently not migrateable." ); rc =migrate_add_blocker (pv_mig_blocker, &local_err);if (rc) { -qemu_balloon_inhibit (false); +ram_block_discard_disable (false);error_report_err (local_err);error_free_or_abort (&pv_mig_blocker);return rc;
@@ -357,7 +361,7 @@ static int s390_machine_protect(S390CcwMachineState *ms)/* Create SE VM */ rc =s390_pv_vm_enable ();if (rc) { -qemu_balloon_inhibit (false); +ram_block_discard_disable (false);migrate_del_blocker (pv_mig_blocker);error_free_or_abort (&pv_mig_blocker);return rc;
Signature · 1 line
2.26.2
Re: [PATCH v4 04/21] s390x/pv: Convert to ram_block_discard_disable()
Quoted 11 lines · On Wed, 10 Jun 2020 13:54:02 +0200 David Hildenbrand <[email protected]> wrote:
Discarding RAM does not work as expected with protected VMs. Let's switch to
ram_block_discard_disable()for now, as we want to get rid ofqemu_balloon_inhibit(). Note that it will currently never fail, but might fail in the future with new technologies (e.g., virtio-mem).Cc Richard Henderson <[email protected]>Cc Cornelia Huck <[email protected]>Cc Halil Pasic <[email protected]>Cc Christian Borntraeger <[email protected]>Cc Janosch Frank <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 3 lines
--- hw/s390x/s390-virtio-ccw.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)
Did not review in detail, but looks sane. Would like to see a sanity check and ack from folks with access to a PV setup.
PATCH v4 5/21 virtio-balloon: Rip out qemu_balloon_inhibit()
In reply to David HildenbrandThe only remaining special case is postcopy. It cannot handle concurrent discards yet, which would result in requesting already sent pages from the source. Special-case it in virtio-balloon instead.
Introduce migration_in_incoming_postcopy(), to find out if incoming postcopy is active.
--- balloon.c | 18 ------------------ hw/virtio/virtio-balloon.c | 8 +++++++- include/migration/misc.h | 2 ++ include/sysemu/balloon.h | 2 -- migration/migration.c | 7 +++++++ migration/postcopy-ram.c | 23 ----------------------- 6 files changed, 16 insertions(+), 44 deletions(-)
balloon.c +0 -18
@@ -36,24 +36,6 @@static QEMUBalloonEvent *balloon_event_fn;static QEMUBalloonStatus *balloon_stat_fn;static void *balloon_opaque; -static int balloon_inhibit_count; - -bool qemu_balloon_is_inhibited (void ) -{ -return atomic_read (&balloon_inhibit_count) >0 || -ram_block_discard_is_disabled (); -} - -void qemu_balloon_inhibit (bool state) -{ -if (state) { -atomic_inc (&balloon_inhibit_count); - }else { -atomic_dec (&balloon_inhibit_count); - } - -assert (atomic_read (&balloon_inhibit_count) >=0 ); -}static bool have_balloon (Error **errp) {
hw/virtio/virtio-balloon.c +7 -1
@@ -63,6 +63,12 @@ static bool virtio_balloon_pbp_matches(PartiallyBalloonedPage *pbp,return pbp->base_gpa == base_gpa; } +static bool virtio_balloon_inhibited (void ) +{ +/* Postcopy cannot deal with concurrent discards, so it's special. */ +return ram_block_discard_is_disabled () ||migration_in_incoming_postcopy (); +} +static void balloon_inflate_page (VirtIOBalloon *balloon,MemoryRegion *mr,hwaddr mr_offset,PartiallyBalloonedPage *pbp)
@@ -360,7 +366,7 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)trace_virtio_balloon_handle_output (memory_region_name (section.mr), pa); -if (!qemu_balloon_is_inhibited ()) { +if (!virtio_balloon_inhibited ()) {if (vq == s->ivq) {balloon_inflate_page (s, section.mr, section.offset_within_region, &pbp);
include/migration/misc.h +2 -0
@@ -69,6 +69,8 @@ bool migration_has_failed(MigrationState *);/* ...and after the device transmission */ bool migration_in_postcopy_after_devices (MigrationState *);void migration_global_dump (Monitor *mon); +/* True if incomming migration entered POSTCOPY_INCOMING_DISCARD */ +bool migration_in_incoming_postcopy (void );/* migration/block-dirty-bitmap.c */ void dirty_bitmap_mig_init (void );
include/sysemu/balloon.h +0 -2
@@ -23,7 +23,5 @@ typedef void (QEMUBalloonStatus)(void *opaque, BalloonInfo *info);int qemu_add_balloon_handler (QEMUBalloonEvent *event_func,QEMUBalloonStatus *stat_func,void *opaque);void qemu_remove_balloon_handler (void *opaque); -bool qemu_balloon_is_inhibited (void ); -void qemu_balloon_inhibit (bool state);#endif
migration/migration.c +7 -0
@@ -1772,6 +1772,13 @@ bool migration_in_postcopy_after_devices(MigrationState *s)return migration_in_postcopy () && s->postcopy_after_devices; } +bool migration_in_incoming_postcopy (void ) +{ +PostcopyState ps =postcopy_state_get (); + +return ps >=POSTCOPY_INCOMING_DISCARD && ps <POSTCOPY_INCOMING_END ; +} +bool migration_is_idle (void ) {MigrationState *s = current_migration;
migration/postcopy-ram.c +0 -23
@@ -27,7 +27,6 @@#include "qemu/notify.h" #include "qemu/rcu.h" #include "sysemu/sysemu.h" -#include "sysemu/balloon.h" #include "qemu/error-report.h" #include "trace.h" #include "hw/boards.h"
@@ -520,20 +519,6 @@ int postcopy_ram_incoming_init(MigrationIncomingState *mis)return 0 ; } -/* -* Manage a single vote to the QEMU balloon inhibitor for all postcopy usage, -* last caller wins. -*/ -static void postcopy_balloon_inhibit (bool state) -{ -static bool cur_state = false; - -if (state != cur_state) { -qemu_balloon_inhibit (state); - cur_state = state; - } -} -/* * At the end of a migration where postcopy_ram_incoming_init was called. */
@@ -565,8 +550,6 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis) mis->have_fault_thread = false; } -postcopy_balloon_inhibit (false); -if (enable_mlock) {if (os_mlock () <0 ) {error_report ("mlock: %s" ,strerror (errno));
@@ -1160,12 +1143,6 @@ int postcopy_ram_incoming_setup(MigrationIncomingState *mis) }memset (mis->postcopy_tmp_zero_page,'\0' , mis->largest_page_size); -/* -* Ballooning can mark pages as absent while we're postcopying -* that would cause false userfaults. -*/ -postcopy_balloon_inhibit (true); -trace_postcopy_ram_enable_notify ();return 0 ;
Signature · 1 line
2.26.2
Re: [PATCH v4 05/21] virtio-balloon: Rip out qemu_balloon_inhibit()
Quoted 11 lines · * David Hildenbrand ([email protected]) wrote:
The only remaining special case is postcopy. It cannot handle concurrent discards yet, which would result in requesting already sent pages from the source. Special-case it in virtio-balloon instead.
Introduce
migration_in_incoming_postcopy(), to find out if incoming postcopy is active.Cc "Michael S. Tsirkin" <[email protected]>Cc Juan Quintela <[email protected]>Cc "Dr. David Alan Gilbert" <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 165 lines
--- balloon.c | 18 ------------------ hw/virtio/virtio-balloon.c | 8 +++++++- include/migration/misc.h | 2 ++ include/sysemu/balloon.h | 2 -- migration/migration.c | 7 +++++++ migration/postcopy-ram.c | 23 ----------------------- 6 files changed, 16 insertions(+), 44 deletions(-)balloon.c +0
-18@@ -36,24 +36,6 @@static QEMUBalloonEvent *balloon_event_fn;static QEMUBalloonStatus *balloon_stat_fn;static void *balloon_opaque; -static int balloon_inhibit_count; - -bool qemu_balloon_is_inhibited (void ) -{ -return atomic_read (&balloon_inhibit_count) >0 || -ram_block_discard_is_disabled (); -} - -void qemu_balloon_inhibit (bool state) -{ -if (state) { -atomic_inc (&balloon_inhibit_count); - }else { -atomic_dec (&balloon_inhibit_count); - } - -assert (atomic_read (&balloon_inhibit_count) >=0 ); -}static bool have_balloon (Error **errp) {hw/virtio/virtio-balloon.c +7
-1@@ -63,6 +63,12 @@ static bool virtio_balloon_pbp_matches(PartiallyBalloonedPage *pbp,return pbp->base_gpa == base_gpa; } +static bool virtio_balloon_inhibited (void ) +{ +/* Postcopy cannot deal with concurrent discards, so it's special. */ +return ram_block_discard_is_disabled () ||migration_in_incoming_postcopy (); +} +static void balloon_inflate_page (VirtIOBalloon *balloon,MemoryRegion *mr,hwaddr mr_offset,PartiallyBalloonedPage *pbp)@@ -360,7 +366,7 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)trace_virtio_balloon_handle_output (memory_region_name (section.mr), pa); -if (!qemu_balloon_is_inhibited ()) { +if (!virtio_balloon_inhibited ()) {if (vq == s->ivq) {balloon_inflate_page (s, section.mr, section.offset_within_region, &pbp);include/migration/misc.h +2
-0@@ -69,6 +69,8 @@ bool migration_has_failed(MigrationState *);/* ...and after the device transmission */ bool migration_in_postcopy_after_devices (MigrationState *);void migration_global_dump (Monitor *mon); +/* True if incomming migration entered POSTCOPY_INCOMING_DISCARD */ +bool migration_in_incoming_postcopy (void );/* migration/block-dirty-bitmap.c */ void dirty_bitmap_mig_init (void );include/sysemu/balloon.h +0
-2@@ -23,7 +23,5 @@ typedef void (QEMUBalloonStatus)(void *opaque, BalloonInfo *info);int qemu_add_balloon_handler (QEMUBalloonEvent *event_func,QEMUBalloonStatus *stat_func,void *opaque);void qemu_remove_balloon_handler (void *opaque); -bool qemu_balloon_is_inhibited (void ); -void qemu_balloon_inhibit (bool state);#endif migration/migration.c +7
-0@@ -1772,6 +1772,13 @@ bool migration_in_postcopy_after_devices(MigrationState *s)return migration_in_postcopy () && s->postcopy_after_devices; } +bool migration_in_incoming_postcopy (void ) +{ +PostcopyState ps =postcopy_state_get (); + +return ps >=POSTCOPY_INCOMING_DISCARD && ps <POSTCOPY_INCOMING_END ; +} +bool migration_is_idle (void ) {MigrationState *s = current_migration;migration/postcopy-ram.c +0
-23@@ -27,7 +27,6 @@#include "qemu/notify.h" #include "qemu/rcu.h" #include "sysemu/sysemu.h" -#include "sysemu/balloon.h" #include "qemu/error-report.h" #include "trace.h" #include "hw/boards.h" @@ -520,20 +519,6 @@ int postcopy_ram_incoming_init(MigrationIncomingState *mis)return 0 ; } -/* -* Manage a single vote to the QEMU balloon inhibitor for all postcopy usage, -* last caller wins. -*/ -static void postcopy_balloon_inhibit (bool state) -{ -static bool cur_state = false; - -if (state != cur_state) { -qemu_balloon_inhibit (state); - cur_state = state; - } -} -/* * At the end of a migration where postcopy_ram_incoming_init was called. */ @@ -565,8 +550,6 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis) mis->have_fault_thread = false; } -postcopy_balloon_inhibit (false); -if (enable_mlock) {if (os_mlock () <0 ) {error_report ("mlock: %s" ,strerror (errno));@@ -1160,12 +1143,6 @@ int postcopy_ram_incoming_setup(MigrationIncomingState *mis) }memset (mis->postcopy_tmp_zero_page,'\0' , mis->largest_page_size); -/* -* Ballooning can mark pages as absent while we're postcopying -* that would cause false userfaults. -*/ -postcopy_balloon_inhibit (true); -trace_postcopy_ram_enable_notify ();return 0 ;Signature · 1 line
2.26.2
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [PATCH v4 05/21] virtio-balloon: Rip out qemu_balloon_inhibit()
In reply to David HildenbrandQuoted 11 lines · David Hildenbrand
The only remaining special case is postcopy. It cannot handle concurrent discards yet, which would result in requesting already sent pages from the source. Special-case it in virtio-balloon instead.
Introduce
migration_in_incoming_postcopy(), to find out if incoming postcopy is active.Cc "Michael S. Tsirkin" <[email protected]>Cc Juan Quintela <[email protected]>Cc "Dr. David Alan Gilbert" <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 164 lines
--- balloon.c | 18 ------------------ hw/virtio/virtio-balloon.c | 8 +++++++- include/migration/misc.h | 2 ++ include/sysemu/balloon.h | 2 -- migration/migration.c | 7 +++++++ migration/postcopy-ram.c | 23 ----------------------- 6 files changed, 16 insertions(+), 44 deletions(-)balloon.c +0
-18@@ -36,24 +36,6 @@static QEMUBalloonEvent *balloon_event_fn;static QEMUBalloonStatus *balloon_stat_fn;static void *balloon_opaque; -static int balloon_inhibit_count; - -bool qemu_balloon_is_inhibited (void ) -{ -return atomic_read (&balloon_inhibit_count) >0 || -ram_block_discard_is_disabled (); -} - -void qemu_balloon_inhibit (bool state) -{ -if (state) { -atomic_inc (&balloon_inhibit_count); - }else { -atomic_dec (&balloon_inhibit_count); - } - -assert (atomic_read (&balloon_inhibit_count) >=0 ); -}static bool have_balloon (Error **errp) {hw/virtio/virtio-balloon.c +7
-1@@ -63,6 +63,12 @@ static bool virtio_balloon_pbp_matches(PartiallyBalloonedPage *pbp,return pbp->base_gpa == base_gpa; } +static bool virtio_balloon_inhibited (void ) +{ +/* Postcopy cannot deal with concurrent discards, so it's special. */ +return ram_block_discard_is_disabled () ||migration_in_incoming_postcopy (); +} +static void balloon_inflate_page (VirtIOBalloon *balloon,MemoryRegion *mr,hwaddr mr_offset,PartiallyBalloonedPage *pbp)@@ -360,7 +366,7 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)trace_virtio_balloon_handle_output (memory_region_name (section.mr), pa); -if (!qemu_balloon_is_inhibited ()) { +if (!virtio_balloon_inhibited ()) {if (vq == s->ivq) {balloon_inflate_page (s, section.mr, section.offset_within_region, &pbp);include/migration/misc.h +2
-0@@ -69,6 +69,8 @@ bool migration_has_failed(MigrationState *);/* ...and after the device transmission */ bool migration_in_postcopy_after_devices (MigrationState *);void migration_global_dump (Monitor *mon); +/* True if incomming migration entered POSTCOPY_INCOMING_DISCARD */ +bool migration_in_incoming_postcopy (void );/* migration/block-dirty-bitmap.c */ void dirty_bitmap_mig_init (void );include/sysemu/balloon.h +0
-2@@ -23,7 +23,5 @@ typedef void (QEMUBalloonStatus)(void *opaque, BalloonInfo *info);int qemu_add_balloon_handler (QEMUBalloonEvent *event_func,QEMUBalloonStatus *stat_func,void *opaque);void qemu_remove_balloon_handler (void *opaque); -bool qemu_balloon_is_inhibited (void ); -void qemu_balloon_inhibit (bool state);#endif migration/migration.c +7
-0@@ -1772,6 +1772,13 @@ bool migration_in_postcopy_after_devices(MigrationState *s)return migration_in_postcopy () && s->postcopy_after_devices; } +bool migration_in_incoming_postcopy (void ) +{ +PostcopyState ps =postcopy_state_get (); + +return ps >=POSTCOPY_INCOMING_DISCARD && ps <POSTCOPY_INCOMING_END ; +} +bool migration_is_idle (void ) {MigrationState *s = current_migration;migration/postcopy-ram.c +0
-23@@ -27,7 +27,6 @@#include "qemu/notify.h" #include "qemu/rcu.h" #include "sysemu/sysemu.h" -#include "sysemu/balloon.h" #include "qemu/error-report.h" #include "trace.h" #include "hw/boards.h" @@ -520,20 +519,6 @@ int postcopy_ram_incoming_init(MigrationIncomingState *mis)return 0 ; } -/* -* Manage a single vote to the QEMU balloon inhibitor for all postcopy usage, -* last caller wins. -*/ -static void postcopy_balloon_inhibit (bool state) -{ -static bool cur_state = false; - -if (state != cur_state) { -qemu_balloon_inhibit (state); - cur_state = state; - } -} -/* * At the end of a migration where postcopy_ram_incoming_init was called. */ @@ -565,8 +550,6 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis) mis->have_fault_thread = false; } -postcopy_balloon_inhibit (false); -if (enable_mlock) {if (os_mlock () <0 ) {error_report ("mlock: %s" ,strerror (errno));@@ -1160,12 +1143,6 @@ int postcopy_ram_incoming_setup(MigrationIncomingState *mis) }memset (mis->postcopy_tmp_zero_page,'\0' , mis->largest_page_size); -/* -* Ballooning can mark pages as absent while we're postcopying -* that would cause false userfaults. -*/ -postcopy_balloon_inhibit (true); -trace_postcopy_ram_enable_notify ();return 0 ;Signature · 1 line
2.26.2
PATCH v4 6/21 target/i386: sev: Use ram_block_discard_disable()
In reply to David HildenbrandAMD SEV will pin all guest memory, mark discarding of RAM broken. At the time this is called, we cannot have anyone active that relies on discards to work properly - let's still implement error handling.
--- target/i386/sev.c | 7 +++++++ 1 file changed, 7 insertions(+)
target/i386/sev.c +7 -0
@@ -649,6 +649,12 @@ sev_guest_init(const char *id)uint32_t host_cbitpos;struct sev_user_data_status status = {}; + ret =ram_block_discard_disable (true); +if (ret) { +error_report ("%s: cannot disable RAM discard" , __func__); +return NULL ; + } + sev_state = s =g_new0 (SEVState,1 ); s->sev_info =lookup_sev_guest_info (id);if (!s->sev_info) {
@@ -724,6 +730,7 @@ sev_guest_init(const char *id) err:g_free (sev_state); sev_state =NULL ; +ram_block_discard_disable (false);return NULL ; }
Signature · 1 line
2.26.2
PATCH v4 7/21 migration/rdma: Use ram_block_discard_disable()
In reply to David HildenbrandRDMA will pin all guest memory (as documented in docs/rdma.txt). We want to disable RAM block discards - however, to keep it simple use ram_block_discard_is_required() instead of inhibiting.
Note: It is not sufficient to limit disabling to pin_all. Even when only conditionally pinning 1 MB chunks, as soon as one page within such a chunk was discarded and one page not, the discarded pages will be pinned as well.
--- migration/rdma.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
migration/rdma.c +16 -2
@@ -29,6 +29,7 @@#include "qemu/sockets.h" #include "qemu/bitmap.h" #include "qemu/coroutine.h" +#include "exec/memory.h" #include <sys/socket.h> #include <netdb.h> #include <arpa/inet.h>
@@ -4017,8 +4018,14 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp)Error *local_err =NULL ;trace_rdma_start_incoming_migration (); - rdma =qemu_rdma_data_init (host_port, &local_err); +/* Avoid ram_block_discard_disable(), cannot change during migration. */ +if (ram_block_discard_is_required ()) { +error_setg (errp,"RDMA: cannot disable RAM discard" ); +return ; + } + + rdma =qemu_rdma_data_init (host_port, &local_err);if (rdma ==NULL ) { goto err; }
@@ -4067,10 +4074,17 @@ void rdma_start_outgoing_migration(void *opaque, const char *host_port, Error **errp) { MigrationState *s = opaque; -RDMAContext *rdma =qemu_rdma_data_init (host_port, errp);RDMAContext *rdma_return_path =NULL ; +RDMAContext *rdma;int ret =0 ; +/* Avoid ram_block_discard_disable(), cannot change during migration. */ +if (ram_block_discard_is_required ()) { +error_setg (errp,"RDMA: cannot disable RAM discard" ); +return ; + } + + rdma =qemu_rdma_data_init (host_port, errp);if (rdma ==NULL ) { goto err; }
Signature · 1 line
2.26.2
PATCH v4 8/21 migration/colo: Use ram_block_discard_disable()
In reply to David HildenbrandCOLO will copy all memory in a RAM block, disable discarding of RAM.
--- include/migration/colo.h | 2 +- migration/migration.c | 8 +++++++- migration/savevm.c | 11 +++++++++-- 3 files changed, 17 insertions(+), 4 deletions(-)
include/migration/colo.h +1 -1
@@ -25,7 +25,7 @@ void migrate_start_colo_process(MigrationState *s);bool migration_in_colo_state (void );/* loadvm */ -void migration_incoming_enable_colo (void ); +int migration_incoming_enable_colo (void );void migration_incoming_disable_colo (void );bool migration_incoming_colo_enabled (void );void *colo_process_incoming_thread (void *opaque);
migration/migration.c +7 -1
@@ -338,12 +338,18 @@ bool migration_incoming_colo_enabled(void)void migration_incoming_disable_colo (void ) { +ram_block_discard_disable (false); migration_colo_enabled = false; } -void migration_incoming_enable_colo (void ) +int migration_incoming_enable_colo (void ) { +if (ram_block_discard_disable (true)) { +error_report ("COLO: cannot disable RAM discard" ); +return -EBUSY ; + } migration_colo_enabled = true; +return 0 ; }void migrate_add_address (SocketAddress *address)
migration/savevm.c +9 -2
@@ -2111,8 +2111,15 @@ static int loadvm_handle_recv_bitmap(MigrationIncomingState *mis,static int loadvm_process_enable_colo (MigrationIncomingState *mis) { -migration_incoming_enable_colo (); -return colo_init_ram_cache (); +int ret =migration_incoming_enable_colo (); + +if (!ret) { + ret =colo_init_ram_cache (); +if (ret) { +migration_incoming_disable_colo (); + } + } +return ret; }/*
Signature · 1 line
2.26.2
PATCH v4 9/21 linux-headers: update to contain virtio-mem
In reply to David HildenbrandTo be merged hopefully soon. Then, we can replace this by a proper header sync.
--- include/standard-headers/linux/virtio_ids.h | 1 + include/standard-headers/linux/virtio_mem.h | 211 ++++++++++++++++++++ 2 files changed, 212 insertions(+) create mode 100644 include/standard-headers/linux/virtio_mem.h
include/standard-headers/linux/virtio_ids.h +1 -0
@@ -44,6 +44,7 @@#define VIRTIO_ID_VSOCK 19/* virtio vsock transport */ #define VIRTIO_ID_CRYPTO 20/* virtio crypto */ #define VIRTIO_ID_IOMMU 23/* virtio IOMMU */ +#define VIRTIO_ID_MEM 24/* virtio mem */ #define VIRTIO_ID_FS 26/* virtio filesystem */ #define VIRTIO_ID_PMEM 27/* virtio pmem */ #define VIRTIO_ID_MAC80211_HWSIM 29/* virtio mac80211-hwsim */
include/standard-headers/linux/virtio_mem.h new file +211 -0
211 lines not on this page · raw message
Signature · 1 line
2.26.2
PATCH v4 10/21 virtio-mem: Paravirtualized memory hot(un)plug
In reply to David HildenbrandThis is the very basic/initial version of virtio-mem. An introduction to virtio-mem can be found in the Linux kernel driver [1]. While it can be used in the current state for hotplug of a smaller amount of memory, it will heavily benefit from resizeable memory regions in the future.
Each virtio-mem device manages a memory region (provided via a memory backend). After requested by the hypervisor ("requested-size"), the guest can try to plug/unplug blocks of memory within that region, in order to reach the requested size. Initially, and after a reboot, all memory is unplugged (except in special cases - reboot during postcopy).
The guest may only try to plug/unplug blocks of memory within the usable region size. The usable region size is a little bigger than the requested size, to give the device driver some flexibility. The usable region size will only grow, except on reboots or when all memory is requested to get unplugged. The guest can never plug more memory than requested. Unplugged memory will get zapped/discarded, similar to in a balloon device.
The block size is variable, however, it is always chosen in a way such that THP splits are avoided (e.g., 2MB). The state of each block (plugged/unplugged) is tracked in a bitmap.
As virtio-mem devices (e.g., virtio-mem-pci) will be memory devices, we now expose "VirtioMEMDeviceInfo" via "query-memory-devices".
--------------------------------------------------------------------------
There are two important follow-up items that are in the works:
1. Resizeable memory regions: Use resizeable allocations/RAM blocks to grow/shrink along with the usable region size. This avoids creating initially very big VMAs, RAM blocks, and KVM slots.
2. Protection of unplugged memory: Make sure the gust cannot actually make use of unplugged memory.
Other follow-up items that are in the works:
1. Exclude unplugged memory during migration (via precopy notifier).
2. Handle remapping of memory.
3. Support for other architectures.
--------------------------------------------------------------------------
Example usage (virtio-mem-pci is introduced in follow-up patches):
Start QEMU with two virtio-mem devices (one per NUMA node):
$ qemu-system-x86_64 -m 4G,maxmem=20G \ -smp sockets=2,cores=2 \ -numa node,nodeid=0,cpus=0-1 -numa node,nodeid=1,cpus=2-3 \ [...] -object memory-backend-ram,id=mem0,size=8G \ -device virtio-mem-pci,id=vm0,memdev=mem0,node=0,requested-size=0M \ -object memory-backend-ram,id=mem1,size=8G \ -device virtio-mem-pci,id=vm1,memdev=mem1,node=1,requested-size=1G
Query the configuration:
(qemu) info memory-devices
Memory device [virtio-mem]: "vm0"
memaddr: 0x140000000
node: 0
requested-size: 0
size: 0
max-size: 8589934592
block-size: 2097152
memdev: /objects/mem0
Memory device [virtio-mem]: "vm1"
memaddr: 0x340000000
node: 1
requested-size: 1073741824
size: 1073741824
max-size: 8589934592
block-size: 2097152
memdev: /objects/mem1
Add some memory to node 0:
(qemu) qom-set vm0 requested-size 500M
Remove some memory from node 1:
(qemu) qom-set vm1 requested-size 200M
Query the configuration again:
(qemu) info memory-devices
Memory device [virtio-mem]: "vm0"
memaddr: 0x140000000
node: 0
requested-size: 524288000
size: 524288000
max-size: 8589934592
block-size: 2097152
memdev: /objects/mem0
Memory device [virtio-mem]: "vm1"
memaddr: 0x340000000
node: 1
requested-size: 209715200
size: 209715200
max-size: 8589934592
block-size: 2097152
memdev: /objects/mem1
[1] https://lkml.kernel.org/r/[email protected]
--- hw/virtio/Kconfig | 11 + hw/virtio/Makefile.objs | 1 + hw/virtio/virtio-mem.c | 724 +++++++++++++++++++++++++++++++++ include/hw/virtio/virtio-mem.h | 78 ++++ qapi/misc.json | 39 +- 5 files changed, 852 insertions(+), 1 deletion(-) create mode 100644 hw/virtio/virtio-mem.c create mode 100644 include/hw/virtio/virtio-mem.h
hw/virtio/Kconfig +11 -0
@@ -47,3 +47,14 @@ config VIRTIO_PMEM depends on VIRTIO depends on VIRTIO_PMEM_SUPPORTED select MEM_DEVICE + +config VIRTIO_MEM_SUPPORTED + bool + +config VIRTIO_MEM + bool + default y + depends on VIRTIO + depends on LINUX + depends on VIRTIO_MEM_SUPPORTED + select MEM_DEVICE
hw/virtio/Makefile.objs +1 -0
@@ -18,6 +18,7 @@ common-obj-$(call land,$(CONFIG_VIRTIO_PMEM),$(CONFIG_VIRTIO_PCI)) += virtio-pme obj-$(call land,$(CONFIG_VHOST_USER_FS),$(CONFIG_VIRTIO_PCI)) += vhost-user-fs-pci.o obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o +obj-$(CONFIG_VIRTIO_MEM) += virtio-mem.o ifeq ($(CONFIG_VIRTIO_PCI),y) obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock-pci.o
hw/virtio/virtio-mem.c new file +724 -0
724 lines not on this page · raw message
include/hw/virtio/virtio-mem.h new file +78 -0
78 lines not on this page · raw message
qapi/misc.json +38 -1
@@ -1354,19 +1354,56 @@ } } +## +# @VirtioMEMDeviceInfo: +# +# VirtioMEMDevice state information +# +# @id: device's ID +# +# @memaddr: physical address in memory, where device is mapped +# +# @requested-size: the user requested size of the device +# +# @size: the (current) size of memory that the device provides +# +# @max-size: the maximum size of memory that the device can provide +# +# @block-size: the block size of memory that the device provides +# +# @node: NUMA node number where device is assigned to +# +# @memdev: memory backend linked with the region +# +# Since: 5.1 +## +{ 'struct': 'VirtioMEMDeviceInfo', + 'data': { '*id': 'str', + 'memaddr': 'size', + 'requested-size': 'size', + 'size': 'size', + 'max-size': 'size', + 'block-size': 'size', + 'node': 'int', + 'memdev': 'str' + } +} + ## # @MemoryDeviceInfo: # # Union containing information about a memory device # # nvdimm is included since 2.12. virtio-pmem is included since 4.1. +# virtio-mem is included since 5.1. # # Since: 2.1 ## { 'union': 'MemoryDeviceInfo', 'data': { 'dimm': 'PCDIMMDeviceInfo', 'nvdimm': 'PCDIMMDeviceInfo', - 'virtio-pmem': 'VirtioPMEMDeviceInfo' + 'virtio-pmem': 'VirtioPMEMDeviceInfo', + 'virtio-mem': 'VirtioMEMDeviceInfo' } }
Signature · 1 line
2.26.2
PATCH v4 11/21 virtio-pci: Proxy for virtio-mem
In reply to David HildenbrandLet's add a proxy for virtio-mem, make it a memory device, and pass-through the properties.
--- hw/virtio/Makefile.objs | 1 + hw/virtio/virtio-mem-pci.c | 129 +++++++++++++++++++++++++++++++++++++ hw/virtio/virtio-mem-pci.h | 33 ++++++++++ include/hw/pci/pci.h | 1 + 4 files changed, 164 insertions(+) create mode 100644 hw/virtio/virtio-mem-pci.c create mode 100644 hw/virtio/virtio-mem-pci.h
hw/virtio/Makefile.objs +1 -0
@@ -19,6 +19,7 @@ obj-$(call land,$(CONFIG_VHOST_USER_FS),$(CONFIG_VIRTIO_PCI)) += vhost-user-fs-p obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o obj-$(CONFIG_VIRTIO_MEM) += virtio-mem.o +common-obj-$(call land,$(CONFIG_VIRTIO_MEM),$(CONFIG_VIRTIO_PCI)) += virtio-mem-pci.o ifeq ($(CONFIG_VIRTIO_PCI),y) obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock-pci.o
hw/virtio/virtio-mem-pci.c new file +129 -0
129 lines not on this page · raw message
hw/virtio/virtio-mem-pci.h new file +33 -0
@@ -0,0 +1,33 @@ +/* +* Virtio MEM PCI device +* +* Copyright (C) 2020 Red Hat, Inc. +* +* Authors: +* David Hildenbrand <[email protected]> +* +* This work is licensed under the terms of the GNU GPL, version 2. +* See the COPYING file in the top-level directory. +*/ + +#ifndef QEMU_VIRTIO_MEM_PCI_H +#define QEMU_VIRTIO_MEM_PCI_H + +#include "hw/virtio/virtio-pci.h" +#include "hw/virtio/virtio-mem.h" + +typedef struct VirtIOMEMPCI VirtIOMEMPCI ; + +/* +* virtio-mem-pci: This extends VirtioPCIProxy. +*/ +#define TYPE_VIRTIO_MEM_PCI "virtio-mem-pci-base" +#define VIRTIO_MEM_PCI (obj) \ + OBJECT_CHECK(VirtIOMEMPCI, (obj), TYPE_VIRTIO_MEM_PCI) + +struct VirtIOMEMPCI { +VirtIOPCIProxy parent_obj; +VirtIOMEM vdev; +}; + +#endif /* QEMU_VIRTIO_MEM_PCI_H */
include/hw/pci/pci.h +1 -0
@@ -87,6 +87,7 @@ extern bool pci_available;#define PCI_DEVICE_ID_VIRTIO_VSOCK 0x1012#define PCI_DEVICE_ID_VIRTIO_PMEM 0x1013#define PCI_DEVICE_ID_VIRTIO_IOMMU 0x1014 +#define PCI_DEVICE_ID_VIRTIO_MEM 0x1015#define PCI_VENDOR_ID_REDHAT 0x1b36#define PCI_DEVICE_ID_REDHAT_BRIDGE 0x0001
Signature · 1 line
2.26.2
PATCH v4 12/21 MAINTAINERS: Add myself as virtio-mem maintainer
In reply to David HildenbrandLet's make sure patches/bug reports find the right person.
--- MAINTAINERS | 8 ++++++++ 1 file changed, 8 insertions(+)
MAINTAINERS +8 -0
@@ -1762,6 +1762,14 @@ F: hw/virtio/virtio-crypto.c F: hw/virtio/virtio-crypto-pci.c F: include/hw/virtio/virtio-crypto.h +virtio-mem +M: David Hildenbrand <[email protected]> +S: Supported +F: hw/virtio/virtio-mem.c +F: hw/virtio/virtio-mem-pci.h +F: hw/virtio/virtio-mem-pci.c +F: include/hw/virtio/virtio-mem.h + nvme M: Keith Busch <[email protected]> L: [email protected]
Signature · 1 line
2.26.2
PATCH v4 13/21 hmp: Handle virtio-mem when printing memory device info
In reply to David HildenbrandPrint the memory device info just like for other memory devices.
--- monitor/hmp-cmds.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
monitor/hmp-cmds.c +16 -0
@@ -1818,6 +1818,7 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)MemoryDeviceInfoList *info_list =qmp_query_memory_devices (&err);MemoryDeviceInfoList *info;VirtioPMEMDeviceInfo *vpi; +VirtioMEMDeviceInfo *vmi;MemoryDeviceInfo *value;PCDIMMDeviceInfo *di;
@@ -1852,6 +1853,21 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)monitor_printf (mon," size: %" PRIu64"\n" , vpi->size);monitor_printf (mon," memdev: %s\n" , vpi->memdev);break ; +case MEMORY_DEVICE_INFO_KIND_VIRTIO_MEM : + vmi = value->u.virtio_mem.data; +monitor_printf (mon,"Memory device [%s]: \"%s\"\n" , +MemoryDeviceInfoKind_str (value->type), + vmi->id ? vmi->id :"" ); +monitor_printf (mon," memaddr: 0x%" PRIx64"\n" , vmi->memaddr); +monitor_printf (mon," node: %" PRId64"\n" , vmi->node); +monitor_printf (mon," requested-size: %" PRIu64"\n" , + vmi->requested_size); +monitor_printf (mon," size: %" PRIu64"\n" , vmi->size); +monitor_printf (mon," max-size: %" PRIu64"\n" , vmi->max_size); +monitor_printf (mon," block-size: %" PRIu64"\n" , + vmi->block_size); +monitor_printf (mon," memdev: %s\n" , vmi->memdev); +break ;default :g_assert_not_reached (); }
Signature · 1 line
2.26.2
Re: [PATCH v4 13/21] hmp: Handle virtio-mem when printing memory device info
Quoted 5 lines · * David Hildenbrand ([email protected]) wrote:
Print the memory device info just like for other memory devices.
Cc "Dr. David Alan Gilbert" <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 41 lines
--- monitor/hmp-cmds.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)monitor/hmp-cmds.c +16
-0@@ -1818,6 +1818,7 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)MemoryDeviceInfoList *info_list =qmp_query_memory_devices (&err);MemoryDeviceInfoList *info;VirtioPMEMDeviceInfo *vpi; +VirtioMEMDeviceInfo *vmi;MemoryDeviceInfo *value;PCDIMMDeviceInfo *di;@@ -1852,6 +1853,21 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)monitor_printf (mon," size: %" PRIu64"\n" , vpi->size);monitor_printf (mon," memdev: %s\n" , vpi->memdev);break ; +case MEMORY_DEVICE_INFO_KIND_VIRTIO_MEM : + vmi = value->u.virtio_mem.data; +monitor_printf (mon,"Memory device [%s]: \"%s\"\n" , +MemoryDeviceInfoKind_str (value->type), + vmi->id ? vmi->id :"" ); +monitor_printf (mon," memaddr: 0x%" PRIx64"\n" , vmi->memaddr); +monitor_printf (mon," node: %" PRId64"\n" , vmi->node); +monitor_printf (mon," requested-size: %" PRIu64"\n" , + vmi->requested_size); +monitor_printf (mon," size: %" PRIu64"\n" , vmi->size); +monitor_printf (mon," max-size: %" PRIu64"\n" , vmi->max_size); +monitor_printf (mon," block-size: %" PRIu64"\n" , + vmi->block_size); +monitor_printf (mon," memdev: %s\n" , vmi->memdev); +break ;default :g_assert_not_reached (); }Signature · 1 line
2.26.2
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [PATCH v4 13/21] hmp: Handle virtio-mem when printing memory device info
In reply to David HildenbrandQuoted 5 lines · * David Hildenbrand ([email protected]) wrote:
Print the memory device info just like for other memory devices.
Cc "Dr. David Alan Gilbert" <[email protected]>Cc "Michael S. Tsirkin" <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 41 lines
--- monitor/hmp-cmds.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)monitor/hmp-cmds.c +16
-0@@ -1818,6 +1818,7 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)MemoryDeviceInfoList *info_list =qmp_query_memory_devices (&err);MemoryDeviceInfoList *info;VirtioPMEMDeviceInfo *vpi; +VirtioMEMDeviceInfo *vmi;MemoryDeviceInfo *value;PCDIMMDeviceInfo *di;@@ -1852,6 +1853,21 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)monitor_printf (mon," size: %" PRIu64"\n" , vpi->size);monitor_printf (mon," memdev: %s\n" , vpi->memdev);break ; +case MEMORY_DEVICE_INFO_KIND_VIRTIO_MEM : + vmi = value->u.virtio_mem.data; +monitor_printf (mon,"Memory device [%s]: \"%s\"\n" , +MemoryDeviceInfoKind_str (value->type), + vmi->id ? vmi->id :"" ); +monitor_printf (mon," memaddr: 0x%" PRIx64"\n" , vmi->memaddr); +monitor_printf (mon," node: %" PRId64"\n" , vmi->node); +monitor_printf (mon," requested-size: %" PRIu64"\n" , + vmi->requested_size); +monitor_printf (mon," size: %" PRIu64"\n" , vmi->size); +monitor_printf (mon," max-size: %" PRIu64"\n" , vmi->max_size); +monitor_printf (mon," block-size: %" PRIu64"\n" , + vmi->block_size); +monitor_printf (mon," memdev: %s\n" , vmi->memdev); +break ;default :g_assert_not_reached (); }Signature · 1 line
2.26.2
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
PATCH v4 14/21 numa: Handle virtio-mem in NUMA stats
In reply to David HildenbrandAccount the memory to the configured nid.
--- hw/core/numa.c | 6 ++++++ 1 file changed, 6 insertions(+)
hw/core/numa.c +6 -0
@@ -812,6 +812,7 @@ static void numa_stat_memory_devices(NumaNodeMem node_mem[])MemoryDeviceInfoList *info;PCDIMMDeviceInfo *pcdimm_info;VirtioPMEMDeviceInfo *vpi; +VirtioMEMDeviceInfo *vmi;for (info = info_list; info; info = info->next) {MemoryDeviceInfo *value = info->value;
@@ -832,6 +833,11 @@ static void numa_stat_memory_devices(NumaNodeMem node_mem[]) node_mem[0 ].node_mem += vpi->size; node_mem[0 ].node_plugged_mem += vpi->size;break ; +case MEMORY_DEVICE_INFO_KIND_VIRTIO_MEM : + vmi = value->u.virtio_mem.data; + node_mem[vmi->node].node_mem += vmi->size; + node_mem[vmi->node].node_plugged_mem += vmi->size; +break ;default :g_assert_not_reached (); }
Signature · 1 line
2.26.2
PATCH v4 15/21 pc: Support for virtio-mem-pci
In reply to David HildenbrandLet's wire it up similar to virtio-pmem. Also disallow unplug, so it's harder for users to shoot themselves into the foot.
--- hw/i386/Kconfig | 1 + hw/i386/pc.c | 49 ++++++++++++++++++++++++++++--------------------- 2 files changed, 29 insertions(+), 21 deletions(-)
hw/i386/Kconfig +1 -0
@@ -35,6 +35,7 @@ config PC select ACPI_PCI select ACPI_VMGENID select VIRTIO_PMEM_SUPPORTED + select VIRTIO_MEM_SUPPORTED config PC_PCI bool
hw/i386/pc.c +28 -21
113 lines not on this page · raw message
Signature · 1 line
2.26.2
PATCH v4 16/21 virtio-mem: Allow notifiers for size changes
In reply to David HildenbrandWe want to send qapi events in case the size of a virtio-mem device changes. This allows upper layers to always know how much memory is actually currently consumed via a virtio-mem device.
Unfortuantely, we have to report the id of our proxy device. Let's provide an easy way for our proxy device to register, so it can send the qapi events. Piggy-backing on the notifier infrastructure (although we'll only ever have one notifier registered) seems to be an easy way.
--- hw/virtio/virtio-mem.c | 21 ++++++++++++++++++++- include/hw/virtio/virtio-mem.h | 5 +++++ 2 files changed, 25 insertions(+), 1 deletion(-)
hw/virtio/virtio-mem.c +20 -1
@@ -184,6 +184,7 @@ static int virtio_mem_state_change_request(VirtIOMEM *vmem, uint64_t gpa, }else { vmem->size -= size; } +notifier_list_notify (&vmem->size_change_notifiers, &vmem->size);return VIRTIO_MEM_RESP_ACK ; }
@@ -242,7 +243,10 @@ static int virtio_mem_unplug_all(VirtIOMEM *vmem)return -EBUSY ; }bitmap_clear (vmem->bitmap,0 , vmem->bitmap_size); - vmem->size =0 ; +if (vmem->size) { + vmem->size =0 ; +notifier_list_notify (&vmem->size_change_notifiers, &vmem->size); + }virtio_mem_resize_usable_region (vmem, vmem->requested_size, true);return 0 ;
@@ -561,6 +565,18 @@ static MemoryRegion *virtio_mem_get_memory_region(VirtIOMEM *vmem, Error **errp)return &vmem->memdev->mr; } +static void virtio_mem_add_size_change_notifier (VirtIOMEM *vmem, +Notifier *notifier) +{ +notifier_list_add (&vmem->size_change_notifiers, notifier); +} + +static void virtio_mem_remove_size_change_notifier (VirtIOMEM *vmem, +Notifier *notifier) +{ +notifier_remove (notifier); +} +static void virtio_mem_get_size (Object *obj,Visitor *v,const char *name,void *opaque,Error **errp) {
@@ -668,6 +684,7 @@ static void virtio_mem_instance_init(Object *obj)VirtIOMEM *vmem =VIRTIO_MEM (obj); vmem->block_size =VIRTIO_MEM_MIN_BLOCK_SIZE ; +notifier_list_init (&vmem->size_change_notifiers);object_property_add (obj,VIRTIO_MEM_SIZE_PROP ,"size" , virtio_mem_get_size,NULL ,NULL ,NULL );
@@ -705,6 +722,8 @@ static void virtio_mem_class_init(ObjectClass *klass, void *data) vmc->fill_device_info = virtio_mem_fill_device_info; vmc->get_memory_region = virtio_mem_get_memory_region; + vmc->add_size_change_notifier = virtio_mem_add_size_change_notifier; + vmc->remove_size_change_notifier = virtio_mem_remove_size_change_notifier; }static const TypeInfo virtio_mem_info = {
include/hw/virtio/virtio-mem.h +5 -0
@@ -64,6 +64,9 @@ typedef struct VirtIOMEM {/* block size and alignment */ uint64_t block_size; + +/* notifiers to notify when "size" changes */ +NotifierList size_change_notifiers; } VirtIOMEM; typedef struct VirtIOMEMClass {
@@ -73,6 +76,8 @@ typedef struct VirtIOMEMClass {/* public */ void (*fill_device_info)(const VirtIOMEM *vmen,VirtioMEMDeviceInfo *vi); MemoryRegion *(*get_memory_region)(VirtIOMEM *vmem, Error **errp); +void (*add_size_change_notifier)(VirtIOMEM *vmem,Notifier *notifier); + void (*remove_size_change_notifier)(VirtIOMEM *vmem, Notifier *notifier); } VirtIOMEMClass;#endif
Signature · 1 line
2.26.2
PATCH v4 17/21 virtio-pci: Send qapi events when the virtio-mem size changes
In reply to David HildenbrandLet's register the notifier and trigger the qapi event with the right device id.
MEMORY_DEVICE_SIZE_CHANGE is similar to BALLOON_CHANGE, however on a memory device level.
Don't unregister the notifier (we neither have finalize() nor unrealize() for VirtIOPCIProxy, so it's not that simple to do it) - both devices are expected to vanish at the same time.
--- hw/virtio/virtio-mem-pci.c | 28 ++++++++++++++++++++++++++++ hw/virtio/virtio-mem-pci.h | 1 + monitor/monitor.c | 1 + qapi/misc.json | 25 +++++++++++++++++++++++++ 4 files changed, 55 insertions(+)
hw/virtio/virtio-mem-pci.c +28 -0
@@ -14,6 +14,7 @@#include "virtio-mem-pci.h" #include "hw/mem/memory-device.h" #include "qapi/error.h" +#include "qapi/qapi-events-misc.h" static void virtio_mem_pci_realize (VirtIOPCIProxy *vpci_dev,Error **errp) {
@@ -74,6 +75,21 @@ static void virtio_mem_pci_fill_device_info(const MemoryDeviceState *md, info->type =MEMORY_DEVICE_INFO_KIND_VIRTIO_MEM ; } +static void virtio_mem_pci_size_change_notify (Notifier *notifier,void *data) +{ +VirtIOMEMPCI *pci_mem =container_of (notifier, VirtIOMEMPCI, + size_change_notifier); +DeviceState *dev =DEVICE (pci_mem); +const uint64_t *const size_p = data; +const char *id =NULL ; + +if (dev->id) { + id =g_strdup (dev->id); + } + +qapi_event_send_memory_device_size_change (!!id, id, *size_p); +} +static void virtio_mem_pci_class_init (ObjectClass *klass,void *data) {DeviceClass *dc =DEVICE_CLASS (klass);
@@ -98,9 +114,21 @@ static void virtio_mem_pci_class_init(ObjectClass *klass, void *data)static void virtio_mem_pci_instance_init (Object *obj) {VirtIOMEMPCI *dev =VIRTIO_MEM_PCI (obj); +VirtIOMEMClass *vmc; +VirtIOMEM *vmem;virtio_instance_init_common (obj, &dev->vdev,sizeof (dev->vdev),TYPE_VIRTIO_MEM ); + + dev->size_change_notifier.notify = virtio_mem_pci_size_change_notify; + vmem =VIRTIO_MEM (&dev->vdev); + vmc =VIRTIO_MEM_GET_CLASS (vmem); +/* +* We never remove the notifier again, as we expect both devices to +* disappear at the same time. +*/ + vmc->add_size_change_notifier (vmem, &dev->size_change_notifier); +object_property_add_alias (obj,VIRTIO_MEM_BLOCK_SIZE_PROP ,OBJECT (&dev->vdev),VIRTIO_MEM_BLOCK_SIZE_PROP ); object_property_add_alias(obj, VIRTIO_MEM_SIZE_PROP, OBJECT(&dev->vdev),
hw/virtio/virtio-mem-pci.h +1 -0
@@ -28,6 +28,7 @@ typedef struct VirtIOMEMPCI VirtIOMEMPCI;struct VirtIOMEMPCI {VirtIOPCIProxy parent_obj;VirtIOMEM vdev; +Notifier size_change_notifier; };#endif /* QEMU_VIRTIO_MEM_PCI_H */
monitor/monitor.c +1 -0
@@ -235,6 +235,7 @@ static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = { [QAPI_EVENT_QUORUM_REPORT_BAD] = { 1000 * SCALE_MS }, [QAPI_EVENT_QUORUM_FAILURE] = { 1000 * SCALE_MS }, [QAPI_EVENT_VSERPORT_CHANGE] = { 1000 * SCALE_MS }, + [QAPI_EVENT_MEMORY_DEVICE_SIZE_CHANGE] = { 1000 * SCALE_MS }, };/*
qapi/misc.json +25 -0
@@ -1432,6 +1432,31 @@ ## { 'command': 'query-memory-devices', 'returns': ['MemoryDeviceInfo'] } +## +# @MEMORY_DEVICE_SIZE_CHANGE: +# +# Emitted when the size of a memory device changes. Only emitted for memory +# devices that can actually change the size (e.g., virtio-mem due to guest +# action). +# +# @id: device's ID +# @size: the new size of memory that the device provides +# +# Note: this event is rate-limited. +# +# Since: 5.1 +# +# Example: +# +# <- { "event": "MEMORY_DEVICE_SIZE_CHANGE", +# "data": { "id": "vm0", "size": 1073741824}, +# "timestamp": { "seconds": 1588168529, "microseconds": 201316 } } +# +## +{ 'event': 'MEMORY_DEVICE_SIZE_CHANGE', + 'data': { '*id': 'str', 'size': 'size' } } + + ## # @MEM_UNPLUG_ERROR: #
Signature · 1 line
2.26.2
PATCH v4 18/21 virtio-mem: Migration sanity checks
In reply to David HildenbrandWe want to make sure that certain properties don't change during migration, especially to catch user errors in a nice way. Let's migrate a temporary structure and validate that the properties didn't change.
--- hw/virtio/virtio-mem.c | 69 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+)
hw/virtio/virtio-mem.c +69 -0
@@ -519,12 +519,81 @@ static int virtio_mem_post_load(void *opaque, int version_id)return virtio_mem_restore_unplugged (VIRTIO_MEM (opaque)); } +typedef struct VirtIOMEMMigSanityChecks { +VirtIOMEM *parent; +uint64_t addr; +uint64_t region_size; +uint64_t block_size; +uint32_t node; +}VirtIOMEMMigSanityChecks ; + +static int virtio_mem_mig_sanity_checks_pre_save (void *opaque) +{ +VirtIOMEMMigSanityChecks *tmp = opaque; +VirtIOMEM *vmem = tmp->parent; + + tmp->addr = vmem->addr; + tmp->region_size =memory_region_size (&vmem->memdev->mr); + tmp->block_size = vmem->block_size; + tmp->node = vmem->node; +return 0 ; +} + +static int virtio_mem_mig_sanity_checks_post_load (void *opaque,int version_id) +{ +VirtIOMEMMigSanityChecks *tmp = opaque; +VirtIOMEM *vmem = tmp->parent; +const uint64_t new_region_size =memory_region_size (&vmem->memdev->mr); + +if (tmp->addr != vmem->addr) { +error_report ("Property '%s' changed from 0x%" PRIx64" to 0x%" PRIx64, +VIRTIO_MEM_ADDR_PROP , tmp->addr, vmem->addr); +return -EINVAL ; + } +/* +* Note: Preparation for resizeable memory regions. The maximum size +* of the memory region must not change during migration. +*/ +if (tmp->region_size != new_region_size) { +error_report ("region size changed from 0x%" PRIx64" to 0x%" PRIx64, + tmp->region_size, new_region_size); +return -EINVAL ; + } +if (tmp->block_size != vmem->block_size) { +error_report ("Property '%s' changed from 0x%" PRIx64" to 0x%" PRIx64, +VIRTIO_MEM_BLOCK_SIZE_PROP , tmp->block_size, + vmem->block_size); +return -EINVAL ; + } +if (tmp->node != vmem->node) { +error_report ("Property '%s' changed from %" PRIu32" to %" PRIu32, +VIRTIO_MEM_NODE_PROP , tmp->node, vmem->node); +return -EINVAL ; + } +return 0 ; +} + +static const VMStateDescription vmstate_virtio_mem_sanity_checks = { + .name ="virtio-mem-device/sanity-checks" , + .pre_save = virtio_mem_mig_sanity_checks_pre_save, + .post_load = virtio_mem_mig_sanity_checks_post_load, + .fields = (VMStateField []) { +VMSTATE_UINT64 (addr, VirtIOMEMMigSanityChecks), +VMSTATE_UINT64 (region_size, VirtIOMEMMigSanityChecks), +VMSTATE_UINT64 (block_size, VirtIOMEMMigSanityChecks), +VMSTATE_UINT32 (node, VirtIOMEMMigSanityChecks), +VMSTATE_END_OF_LIST (), + }, +}; +static const VMStateDescription vmstate_virtio_mem_device = { .name ="virtio-mem-device" , .minimum_version_id =1 , .version_id =1 , .post_load = virtio_mem_post_load, .fields = (VMStateField []) { +VMSTATE_WITH_TMP (VirtIOMEM, VirtIOMEMMigSanityChecks, + vmstate_virtio_mem_sanity_checks),VMSTATE_UINT64 (usable_region_size, VirtIOMEM),VMSTATE_UINT64 (size, VirtIOMEM),VMSTATE_UINT64 (requested_size, VirtIOMEM),
Signature · 1 line
2.26.2
Re: [PATCH v4 18/21] virtio-mem: Migration sanity checks
Quoted 7 lines · * David Hildenbrand ([email protected]) wrote:
We want to make sure that certain properties don't change during migration, especially to catch user errors in a nice way. Let's migrate a temporary structure and validate that the properties didn't change.
Cc "Michael S. Tsirkin" <[email protected]>Cc "Dr. David Alan Gilbert" <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Yep OK, but some comment below
Quoted 33 lines
--- hw/virtio/virtio-mem.c | 69 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+)hw/virtio/virtio-mem.c +21
-0@@ -519,12 +519,81 @@ static int virtio_mem_post_load(void *opaque, int version_id)return virtio_mem_restore_unplugged (VIRTIO_MEM (opaque)); } +typedef struct VirtIOMEMMigSanityChecks { +VirtIOMEM *parent; +uint64_t addr; +uint64_t region_size; +uint64_t block_size; +uint32_t node; +}VirtIOMEMMigSanityChecks ; + +static int virtio_mem_mig_sanity_checks_pre_save (void *opaque) +{ +VirtIOMEMMigSanityChecks *tmp = opaque; +VirtIOMEM *vmem = tmp->parent; + + tmp->addr = vmem->addr; + tmp->region_size =memory_region_size (&vmem->memdev->mr); + tmp->block_size = vmem->block_size; + tmp->node = vmem->node; +return 0 ; +} + +static int virtio_mem_mig_sanity_checks_post_load (void *opaque,int version_id)
hw/virtio/virtio-mem.c static int virtio_mem_post_load(void *opaque, int version_id) · line 552
Quoted 10 lines
hw/virtio/virtio-mem.c +10
-0+{ +VirtIOMEMMigSanityChecks *tmp = opaque; +VirtIOMEM *vmem = tmp->parent; +const uint64_t new_region_size =memory_region_size (&vmem->memdev->mr); + +if (tmp->addr != vmem->addr) { +error_report ("Property '%s' changed from 0x%" PRIx64" to 0x%" PRIx64, +VIRTIO_MEM_ADDR_PROP , tmp->addr, vmem->addr); +return -EINVAL ; + }
It seems weird that you do 'Property ...' and then the string; although you only do it for 3 out of 4.
I was going to ask you to include the device name here, but I'm guessing when it fails the outer migration code will print a 'Failed loading device.....' so at least you know what it is.
I would want it to be obvious when I see a 'region size changed' that I knew it was my virtio-mem device that was screwy.
Dave
Quoted 50 lines
hw/virtio/virtio-mem.c +38
-0+/* +* Note: Preparation for resizeable memory regions. The maximum size +* of the memory region must not change during migration. +*/ +if (tmp->region_size != new_region_size) { +error_report ("region size changed from 0x%" PRIx64" to 0x%" PRIx64, + tmp->region_size, new_region_size); +return -EINVAL ; + } +if (tmp->block_size != vmem->block_size) { +error_report ("Property '%s' changed from 0x%" PRIx64" to 0x%" PRIx64, +VIRTIO_MEM_BLOCK_SIZE_PROP , tmp->block_size, + vmem->block_size); +return -EINVAL ; + } +if (tmp->node != vmem->node) { +error_report ("Property '%s' changed from %" PRIu32" to %" PRIu32, +VIRTIO_MEM_NODE_PROP , tmp->node, vmem->node); +return -EINVAL ; + } +return 0 ; +} + +static const VMStateDescription vmstate_virtio_mem_sanity_checks = { + .name ="virtio-mem-device/sanity-checks" , + .pre_save = virtio_mem_mig_sanity_checks_pre_save, + .post_load = virtio_mem_mig_sanity_checks_post_load, + .fields = (VMStateField []) { +VMSTATE_UINT64 (addr, VirtIOMEMMigSanityChecks), +VMSTATE_UINT64 (region_size, VirtIOMEMMigSanityChecks), +VMSTATE_UINT64 (block_size, VirtIOMEMMigSanityChecks), +VMSTATE_UINT32 (node, VirtIOMEMMigSanityChecks), +VMSTATE_END_OF_LIST (), + }, +}; +static const VMStateDescription vmstate_virtio_mem_device = { .name ="virtio-mem-device" , .minimum_version_id =1 , .version_id =1 , .post_load = virtio_mem_post_load, .fields = (VMStateField []) { +VMSTATE_WITH_TMP (VirtIOMEM, VirtIOMEMMigSanityChecks, + vmstate_virtio_mem_sanity_checks),VMSTATE_UINT64 (usable_region_size, VirtIOMEM),VMSTATE_UINT64 (size, VirtIOMEM),VMSTATE_UINT64 (requested_size, VirtIOMEM),Signature · 1 line
2.26.2
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [PATCH v4 18/21] virtio-mem: Migration sanity checks
Quoted 51 lines · Dr. David Alan Gilbert
Quoted 7 lines · * David Hildenbrand ([email protected]) wrote:
We want to make sure that certain properties don't change during migration, especially to catch user errors in a nice way. Let's migrate a temporary structure and validate that the properties didn't change.
Cc "Michael S. Tsirkin" <[email protected]>Cc "Dr. David Alan Gilbert" <[email protected]>Signed-off-by David Hildenbrand <[email protected]>Yep OK, but some comment below
Reviewed-by Dr. David Alan Gilbert <[email protected]>Quoted 38 lines
--- hw/virtio/virtio-mem.c | 69 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+)hw/virtio/virtio-mem.c +26
-0@@ -519,12 +519,81 @@ static int virtio_mem_post_load(void *opaque, int version_id)return virtio_mem_restore_unplugged (VIRTIO_MEM (opaque)); } +typedef struct VirtIOMEMMigSanityChecks { +VirtIOMEM *parent; +uint64_t addr; +uint64_t region_size; +uint64_t block_size; +uint32_t node; +}VirtIOMEMMigSanityChecks ; + +static int virtio_mem_mig_sanity_checks_pre_save (void *opaque) +{ +VirtIOMEMMigSanityChecks *tmp = opaque; +VirtIOMEM *vmem = tmp->parent; + + tmp->addr = vmem->addr; + tmp->region_size =memory_region_size (&vmem->memdev->mr); + tmp->block_size = vmem->block_size; + tmp->node = vmem->node; +return 0 ; +} + +static int virtio_mem_mig_sanity_checks_post_load (void *opaque,int version_id) +{ +VirtIOMEMMigSanityChecks *tmp = opaque; +VirtIOMEM *vmem = tmp->parent; +const uint64_t new_region_size =memory_region_size (&vmem->memdev->mr); +
Quoted 8 lines
hw/virtio/virtio-mem.c static int virtio_mem_post_load(void *opaque, int version_id) · line 552
Quoted 5 lines
hw/virtio/virtio-mem.c +5
-0+if (tmp->addr != vmem->addr) { +error_report ("Property '%s' changed from 0x%" PRIx64" to 0x%" PRIx64, +VIRTIO_MEM_ADDR_PROP , tmp->addr, vmem->addr); +return -EINVAL ; + }It seems weird that you do 'Property ...' and then the string; although you only do it for 3 out of 4.
Yeah, because it is a derived property of the memdev. I can just make that clearer by referencing the memdev property.
Quoted 5 lines
I was going to ask you to include the device name here, but I'm guessing when it fails the outer migration code will print a 'Failed loading device.....' so at least you know what it is.
I would want it to be obvious when I see a 'region size changed' that I knew it was my virtio-mem device that was screwy.
IIRC that should be the case, yes.
Thanks!
Signature · 3 lines
Thanks, David / dhildenb
PATCH v4 19/21 virtio-mem: Add trace events
In reply to David HildenbrandLet's add some trace events that might come in handy later.
--- hw/virtio/trace-events | 10 ++++++++++ hw/virtio/virtio-mem.c | 10 +++++++++- 2 files changed, 19 insertions(+), 1 deletion(-)
hw/virtio/trace-events +10 -0
@@ -73,3 +73,13 @@ virtio_iommu_get_domain(uint32_t domain_id) "Alloc domain=%d" virtio_iommu_put_domain(uint32_t domain_id) "Free domain=%d" virtio_iommu_translate_out(uint64_t virt_addr, uint64_t phys_addr, uint32_t sid) "0x%"PRIx64" -> 0x%"PRIx64 " for sid=%d" virtio_iommu_report_fault(uint8_t reason, uint32_t flags, uint32_t endpoint, uint64_t addr) "FAULT reason=%d flags=%d endpoint=%d address =0x%"PRIx64 + +# virtio-mem.c +virtio_mem_send_response(uint16_t type) "type=%" PRIu16 +virtio_mem_plug_request(uint64_t addr, uint16_t nb_blocks) "addr=0x%" PRIx64 " nb_blocks=%" PRIu16 +virtio_mem_unplug_request(uint64_t addr, uint16_t nb_blocks) "addr=0x%" PRIx64 " nb_blocks=%" PRIu16 +virtio_mem_unplugged_all(void) "" +virtio_mem_unplug_all_request(void) "" +virtio_mem_resized_usable_region(uint64_t old_size, uint64_t new_size) "old_size=0x%" PRIx64 "new_size=0x%" PRIx64 +virtio_mem_state_request(uint64_t addr, uint16_t nb_blocks) "addr=0x%" PRIx64 " nb_blocks=%" PRIu16 +virtio_mem_state_response(uint16_t state) "state=%" PRIu16
hw/virtio/virtio-mem.c +9 -1
@@ -30,6 +30,7 @@#include "hw/boards.h" #include "hw/qdev-properties.h" #include "config-devices.h" +#include "trace.h" /* * Use QEMU_VMALLOC_ALIGN, so no THP will have to be split when unplugging
@@ -100,6 +101,7 @@ static void virtio_mem_send_response(VirtIOMEM *vmem, VirtQueueElement *elem,VirtIODevice *vdev =VIRTIO_DEVICE (vmem);VirtQueue *vq = vmem->vq; +trace_virtio_mem_send_response (le16_to_cpu (resp->type));iov_from_buf (elem->in_sg, elem->in_num,0 , resp,sizeof (*resp));virtqueue_push (vq, elem,sizeof (*resp));
@@ -195,6 +197,7 @@ static void virtio_mem_plug_request(VirtIOMEM *vmem, VirtQueueElement *elem,const uint16_t nb_blocks =le16_to_cpu (req->u.plug.nb_blocks);uint16_t type; +trace_virtio_mem_plug_request (gpa, nb_blocks); type =virtio_mem_state_change_request (vmem, gpa, nb_blocks, true);virtio_mem_send_response_simple (vmem, elem, type); }
@@ -206,6 +209,7 @@ static void virtio_mem_unplug_request(VirtIOMEM *vmem, VirtQueueElement *elem,const uint16_t nb_blocks =le16_to_cpu (req->u.unplug.nb_blocks);uint16_t type; +trace_virtio_mem_unplug_request (gpa, nb_blocks); type =virtio_mem_state_change_request (vmem, gpa, nb_blocks, false);virtio_mem_send_response_simple (vmem, elem, type); }
@@ -225,6 +229,7 @@ static void virtio_mem_resize_usable_region(VirtIOMEM *vmem,return ; } +trace_virtio_mem_resized_usable_region (vmem->usable_region_size, newsize); vmem->usable_region_size = newsize; }
@@ -247,7 +252,7 @@ static int virtio_mem_unplug_all(VirtIOMEM *vmem) vmem->size =0 ;notifier_list_notify (&vmem->size_change_notifiers, &vmem->size); } - +trace_virtio_mem_unplugged_all ();virtio_mem_resize_usable_region (vmem, vmem->requested_size, true);return 0 ; }
@@ -255,6 +260,7 @@ static int virtio_mem_unplug_all(VirtIOMEM *vmem)static void virtio_mem_unplug_all_request (VirtIOMEM *vmem,VirtQueueElement *elem) { +trace_virtio_mem_unplug_all_request ();if (virtio_mem_unplug_all (vmem)) {virtio_mem_send_response_simple (vmem, elem,VIRTIO_MEM_RESP_BUSY ); }else {
@@ -272,6 +278,7 @@ static void virtio_mem_state_request(VirtIOMEM *vmem, VirtQueueElement *elem, .type = cpu_to_le16(VIRTIO_MEM_RESP_ACK), }; +trace_virtio_mem_state_request (gpa, nb_blocks);if (!virtio_mem_valid_range (vmem, gpa, size)) {virtio_mem_send_response_simple (vmem, elem,VIRTIO_MEM_RESP_ERROR );return ;
@@ -284,6 +291,7 @@ static void virtio_mem_state_request(VirtIOMEM *vmem, VirtQueueElement *elem, }else { resp.u.state.state =cpu_to_le16 (VIRTIO_MEM_STATE_MIXED ); } +trace_virtio_mem_state_response (le16_to_cpu (resp.u.state.state));virtio_mem_send_response (vmem, elem, &resp); }
Signature · 1 line
2.26.2
PATCH v4 20/21 virtio-mem: Exclude unplugged memory during migration
In reply to David HildenbrandThe content of unplugged memory is undefined and should not be migrated, ever. Exclude all unplugged memory during precopy using the precopy notifier infrastructure introduced for free page hinting in virtio-balloon.
Unplugged memory is marked as "not dirty", meaning it won't be considered for migration.
--- hw/virtio/virtio-mem.c | 54 +++++++++++++++++++++++++++++++++- include/hw/virtio/virtio-mem.h | 3 ++ 2 files changed, 56 insertions(+), 1 deletion(-)
hw/virtio/virtio-mem.c +53 -1
@@ -62,8 +62,14 @@ static bool virtio_mem_is_busy(void)/* * Postcopy cannot handle concurrent discards and we don't want to migrate * pages on-demand with stale content when plugging new blocks. +* +* For precopy, we don't want unplugged blocks in our migration stream, and +* when plugging new blocks, the page content might differ between source +* and destination (observable by the guest when not initializing pages +* after plugging them) until we're running on the destination (as we didn't +* migrate these blocks when they were unplugged). */ -return migration_in_incoming_postcopy (); +return migration_in_incoming_postcopy () || !migration_is_idle (); } static bool virtio_mem_test_bitmap(VirtIOMEM *vmem, uint64_t start_gpa,
@@ -475,6 +481,7 @@ static void virtio_mem_device_realize(DeviceState *dev, Error **errp)host_memory_backend_set_mapped (vmem->memdev, true);vmstate_register_ram (&vmem->memdev->mr,DEVICE (vmem));qemu_register_reset (virtio_mem_system_reset, vmem); +precopy_add_notifier (&vmem->precopy_notifier); }static void virtio_mem_device_unrealize (DeviceState *dev)
@@ -482,6 +489,7 @@ static void virtio_mem_device_unrealize(DeviceState *dev)VirtIODevice *vdev =VIRTIO_DEVICE (dev);VirtIOMEM *vmem =VIRTIO_MEM (dev); +precopy_remove_notifier (&vmem->precopy_notifier);qemu_unregister_reset (virtio_mem_system_reset, vmem);vmstate_unregister_ram (&vmem->memdev->mr,DEVICE (vmem));host_memory_backend_set_mapped (vmem->memdev, false);
@@ -756,12 +764,56 @@ static void virtio_mem_set_block_size(Object *obj, Visitor *v, const char *name, vmem->block_size = value; } +static void virtio_mem_precopy_exclude_unplugged (VirtIOMEM *vmem) +{ +void *const host =qemu_ram_get_host_addr (vmem->memdev->mr.ram_block); +unsigned long first_zero_bit, last_zero_bit; +uint64_t offset, length; + +/* +* Find consecutive unplugged blocks and exclude them from migration. +* +* Note: Blocks cannot get (un)plugged during precopy, no locking needed. +*/ + first_zero_bit =find_first_zero_bit (vmem->bitmap, vmem->bitmap_size); +while (first_zero_bit < vmem->bitmap_size) { + offset = first_zero_bit * vmem->block_size; + last_zero_bit =find_next_bit (vmem->bitmap, vmem->bitmap_size, + first_zero_bit +1 ) -1 ; + length = (last_zero_bit - first_zero_bit +1 ) * vmem->block_size; + +qemu_guest_free_page_hint (host + offset, length); + first_zero_bit =find_next_zero_bit (vmem->bitmap, vmem->bitmap_size, + last_zero_bit +2 ); + } +} + +static int virtio_mem_precopy_notify (NotifierWithReturn *n,void *data) +{ +VirtIOMEM *vmem =container_of (n, VirtIOMEM, precopy_notifier); +PrecopyNotifyData *pnd = data; + +switch (pnd->reason) { +case PRECOPY_NOTIFY_SETUP : +precopy_enable_free_page_optimization (); +break ; +case PRECOPY_NOTIFY_AFTER_BITMAP_SYNC : +virtio_mem_precopy_exclude_unplugged (vmem); +break ; +default : +break ; + } + +return 0 ; +} +static void virtio_mem_instance_init (Object *obj) {VirtIOMEM *vmem =VIRTIO_MEM (obj); vmem->block_size =VIRTIO_MEM_MIN_BLOCK_SIZE ;notifier_list_init (&vmem->size_change_notifiers); + vmem->precopy_notifier.notify = virtio_mem_precopy_notify;object_property_add (obj,VIRTIO_MEM_SIZE_PROP ,"size" , virtio_mem_get_size,NULL ,NULL ,NULL );
include/hw/virtio/virtio-mem.h +3 -0
@@ -67,6 +67,9 @@ typedef struct VirtIOMEM {/* notifiers to notify when "size" changes */ NotifierList size_change_notifiers; + +/* don't migrate unplugged memory */ +NotifierWithReturn precopy_notifier; } VirtIOMEM; typedef struct VirtIOMEMClass {
Signature · 1 line
2.26.2
PATCH v4 21/21 numa: Auto-enable NUMA when any memory devices are possible
In reply to David HildenbrandLet's auto-enable it also when maxmem is specified but no slots are defined. This will result in us properly creating ACPI srat tables, indicating the maximum possible PFN to the guest OS. Based on this, e.g., Linux will enable the swiotlb properly.
This avoids having to manually force the switolb on (swiotlb=force) in Linux in case we're booting only using DMA memory (e.g., 2GB on x86-64), and virtio-mem adds memory later on that really needs the swiotlb to be used for DMA.
Let's take care of backwards compatibility if somebody has a setup that specifies "maxram" without "slots".
--- hw/arm/virt.c | 2 ++ hw/core/numa.c | 11 ++++++----- hw/i386/microvm.c | 1 + hw/i386/pc.c | 1 + hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c | 1 + include/hw/boards.h | 1 + 7 files changed, 13 insertions(+), 5 deletions(-)
hw/arm/virt.c +2 -0
@@ -2265,6 +2265,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) mc->numa_mem_supported = true; mc->nvdimm_supported = true; mc->auto_enable_numa_with_memhp = true; + mc->auto_enable_numa_with_memdev = true; mc->default_ram_id ="mach-virt.ram" ; object_class_property_add(oc, "acpi", "OnOffAuto",
@@ -2375,6 +2376,7 @@ DEFINE_VIRT_MACHINE_AS_LATEST(5, 1)static void virt_machine_5_0_options (MachineClass *mc) {virt_machine_5_1_options (mc); + mc->auto_enable_numa_with_memdev = false; }DEFINE_VIRT_MACHINE (5 ,0 )
hw/core/numa.c +6 -5
@@ -681,8 +681,9 @@ void numa_complete_configuration(MachineState *ms)NodeInfo *numa_info = ms->numa_state->nodes;/* -* If memory hotplug is enabled (slots > 0) but without '-numa' -* options explicitly on CLI, guestes will break. +* If memory hotplug is enabled (slot > 0) or memory devices are enabled +* (ms->maxram_size > ram_size) but without '-numa' options explicitly on +* CLI, guests will break. * * Windows: won't enable memory hotplug without SRAT table at all *
@@ -697,9 +698,9 @@ void numa_complete_configuration(MachineState *ms)* assume there is just one node with whole RAM. */ if (ms->numa_state->num_nodes ==0 && - ((ms->ram_slots >0 && - mc->auto_enable_numa_with_memhp) || - mc->auto_enable_numa)) { + ((ms->ram_slots && mc->auto_enable_numa_with_memhp) || + (ms->maxram_size > ms->ram_size && mc->auto_enable_numa_with_memdev) || + mc->auto_enable_numa)) {NumaNodeOptions node = { };parse_numa_node (ms, &node, &error_abort); numa_info[0 ].node_mem = ram_size;
hw/i386/microvm.c +1 -0
@@ -497,6 +497,7 @@ static void microvm_class_init(ObjectClass *oc, void *data) mc->max_cpus =288 ; mc->has_hotpluggable_cpus = false; mc->auto_enable_numa_with_memhp = false; + mc->auto_enable_numa_with_memdev = false; mc->default_cpu_type =TARGET_DEFAULT_CPU_TYPE ; mc->nvdimm_supported = false; mc->default_ram_id ="microvm.ram" ;
hw/i386/pc.c +1 -0
@@ -1955,6 +1955,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) mc->get_default_cpu_node_id = x86_get_default_cpu_node_id; mc->possible_cpu_arch_ids = x86_possible_cpu_arch_ids; mc->auto_enable_numa_with_memhp = true; + mc->auto_enable_numa_with_memdev = true; mc->has_hotpluggable_cpus = true; mc->default_boot_order ="cad" ; mc->hot_add_cpu = pc_hot_add_cpu;
hw/i386/pc_piix.c +1 -0
@@ -440,6 +440,7 @@ static void pc_i440fx_5_0_machine_options(MachineClass *m) m->is_default = false;compat_props_add (m->compat_props, hw_compat_5_0, hw_compat_5_0_len);compat_props_add (m->compat_props, pc_compat_5_0, pc_compat_5_0_len); + m->auto_enable_numa_with_memdev = false; } DEFINE_I440FX_MACHINE(v5_0, "pc-i440fx-5.0", NULL,
hw/i386/pc_q35.c +1 -0
@@ -368,6 +368,7 @@ static void pc_q35_5_0_machine_options(MachineClass *m) m->alias =NULL ;compat_props_add (m->compat_props, hw_compat_5_0, hw_compat_5_0_len);compat_props_add (m->compat_props, pc_compat_5_0, pc_compat_5_0_len); + m->auto_enable_numa_with_memhp = false; } DEFINE_Q35_MACHINE(v5_0, "pc-q35-5.0", NULL,
include/hw/boards.h +1 -0
@@ -207,6 +207,7 @@ struct MachineClass {const char **valid_cpu_types;strList *allowed_dynamic_sysbus_devices;bool auto_enable_numa_with_memhp; +bool auto_enable_numa_with_memdev;void (*numa_auto_assign_ram)(MachineClass *mc,NodeInfo *nodes,int nb_nodes,ram_addr_t size);bool ignore_boot_device_suffixes;
Signature · 1 line
2.26.2
Quoted 4 lines · David Hildenbrand
This is the very basic, initial version of virtio-mem. More info on virtio-mem in general can be found in the Linux kernel driver v2 posting
[1] and in patch #10. The Linux driver is currently on its way upstream.
Quoted 8 lines
This series is based on [3]:
"[PATCH v1] pc: Support coldplugging of virtio-pmem-pci devices on all
buses"
And [4]:
"[PATCH v2] hmp: Make json format optional for qom-set"The patches can be found at:
https://github.com/davidhildenbrand/qemu.git virtio-mem-v4
OK so looks sane to me.
IIUC there are a couple of minor tweaks still so I'm expecting v5 of this.
You want me to merge this, right?
Quoted 144 lines
"The basic idea of virtio-mem is to provide a flexible, cross-architecture memory hot(un)plug solution that avoids many limitations imposed by existing technologies, architectures, and interfaces."
There are a lot of addons in the works (esp. protection of unplugged memory, better hugepage support (esp. when reading unplugged memory), resizeable memory backends, support for more architectures, ...), this is the very basic version to get the ball rolling.
The first 8 patches make sure we don't have any sudden surprises e.g., if somebody tries to pin all memory in RAM blocks, resulting in a higher memory consumption than desired. The remaining patches add basic virtio-mem along with support for x86-64. The last patch indicates to the guest OS the maximum possible PFN using ACPI SRAT, such that Linux can properly enable the swiotlb when booting only with DMA memory.
[1] https://lkml.kernel.org/r/[email protected]
[2] https://lkml.kernel.org/r/[email protected]
[3] https://lkml.kernel.org/r/[email protected]
[3] https://lkml.kernel.org/r/[email protected]Based-on: <[email protected]>
Based-on: <[email protected]>
Cc: teawater <[email protected]>
Cc: Pankaj Gupta <[email protected]>v3 -> v4
- Adapt to virtio-mem config layout change (block size now is 64bit)
- Added "numa: Auto-enable NUMA when any memory devices are possible"v2 -> v3:
- Rebased on upstream/[3]
- "virtio-mem: Exclude unplugged memory during migration" -- Added
- "virtio-mem: Paravirtualized memory hot(un)plug" -- Simplify bitmap operations, find consecutive areas -- Tweak error messages
-- Reshuffle some checks
-- Minor cleanups
- "accel/kvm: Convert toram_block_discard_disable()"
- "target/i386: sev: Useram_block_discard_disable()" -- Keep asserts clean of functional thingsv1 -> v2:
- Rebased to object_property_*() changes
- "exec: Introduce ram_block_discard_(disable|require)()"
-- Change the function names and rephrase/add comments
- "virtio-balloon: Rip outqemu_balloon_inhibit()"
-- Add and use "migration_in_incoming_postcopy()"
- "migration/rdma: Useram_block_discard_disable()"
-- Add a comment regardingpin_allvs. !pin_all
- "virtio-mem: Paravirtualized memory hot(un)plug"
-- Replacevirtio_mem_discard_inhibited()by
migration_in_incoming_postcopy()
-- Drop some asserts
-- Dropvirtio_mem_bad_request(), usevirtio_error()directly, printing
more information
-- Replace "Note: Discarding should never fail ..." comments by
error_report()
-- Replacevirtio_stw_p()bycpu_to_le16()
-- Dropmigration_addrandmigration_block_size
-- Minor cleanups
- "linux-headers: update to contain virtio-mem"
-- Updated to latest v4 in Linux
- General changes
-- Fixup the users of the renamedram_block_discard_(disable|require)-- Use "X: cannot disable RAM discard"-styled error messages
- Added
-- "virtio-mem: Migration sanity checks"
-- "virtio-mem: Add trace events"David Hildenbrand (21):
exec: Introduce ram_block_discard_(disable|require)()
vfio: Convert toram_block_discard_disable()accel/kvm: Convert toram_block_discard_disable()s390x/pv: Convert toram_block_discard_disable()
virtio-balloon: Rip outqemu_balloon_inhibit()target/i386: sev: Useram_block_discard_disable()migration/rdma: Useram_block_discard_disable()migration/colo: Useram_block_discard_disable()
linux-headers: update to contain virtio-mem
virtio-mem: Paravirtualized memory hot(un)plug
virtio-pci: Proxy for virtio-mem
MAINTAINERS: Add myself as virtio-mem maintainer
hmp: Handle virtio-mem when printing memory device info
numa: Handle virtio-mem in NUMA stats
pc: Support for virtio-mem-pci
virtio-mem: Allow notifiers for size changes
virtio-pci: Send qapi events when the virtio-mem size changes
virtio-mem: Migration sanity checks
virtio-mem: Add trace events
virtio-mem: Exclude unplugged memory during migration
numa: Auto-enable NUMA when any memory devices are possibleMAINTAINERS | 8 + accel/kvm/kvm-all.c | 4 +- balloon.c | 17 - exec.c | 52 ++ hw/arm/virt.c | 2 + hw/core/numa.c | 17 +- hw/i386/Kconfig | 1 + hw/i386/microvm.c | 1 + hw/i386/pc.c | 50 +- hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c | 1 + hw/s390x/s390-virtio-ccw.c | 22 +- hw/vfio/ap.c | 10 +- hw/vfio/ccw.c | 11 +- hw/vfio/common.c | 53 +- hw/vfio/pci.c | 6 +- hw/virtio/Kconfig | 11 + hw/virtio/Makefile.objs | 2 + hw/virtio/trace-events | 10 + hw/virtio/virtio-balloon.c | 8 +- hw/virtio/virtio-mem-pci.c | 157 ++++ hw/virtio/virtio-mem-pci.h | 34 + hw/virtio/virtio-mem.c | 872 ++++++++++++++++++++ include/exec/memory.h | 41 + include/hw/boards.h | 1 + include/hw/pci/pci.h | 1 + include/hw/vfio/vfio-common.h | 4 +- include/hw/virtio/virtio-mem.h | 86 ++ include/migration/colo.h | 2 +- include/migration/misc.h | 2 + include/standard-headers/linux/virtio_ids.h | 1 + include/standard-headers/linux/virtio_mem.h | 211 +++++ include/sysemu/balloon.h | 2 - migration/migration.c | 15 +- migration/postcopy-ram.c | 23 - migration/rdma.c | 18 +- migration/savevm.c | 11 +- monitor/hmp-cmds.c | 16 + monitor/monitor.c | 1 + qapi/misc.json | 64 +- target/i386/sev.c | 7 + 41 files changed, 1730 insertions(+), 126 deletions(-) create mode 100644 hw/virtio/virtio-mem-pci.c create mode 100644 hw/virtio/virtio-mem-pci.h create mode 100644 hw/virtio/virtio-mem.c create mode 100644 include/hw/virtio/virtio-mem.h create mode 100644 include/standard-headers/linux/virtio_mem.hSignature · 1 line
2.26.2
Quoted 11 lines · Michael S. Tsirkin
Quoted 10 lines · David Hildenbrand
This is the very basic, initial version of virtio-mem. More info on virtio-mem in general can be found in the Linux kernel driver v2 posting
[1] and in patch #10. The Linux driver is currently on its way upstream.This series is based on [3]:
"[PATCH v1] pc: Support coldplugging of virtio-pmem-pci devices on all
buses"
And [4]:
"[PATCH v2] hmp: Make json format optional for qom-set"
Quoted 8 lines
Quoted 2 lines
The patches can be found at:
https://github.com/davidhildenbrand/qemu.git virtio-mem-v4OK so looks sane to me.
IIUC there are a couple of minor tweaks still so I'm expecting v5 of this.
You want me to merge this, right?
Yes, I am planning to send (rebased) v5 most probably tomorrow. Thanks!
Signature · 3 lines
Thanks, David / dhildenb