On 20.05.25 12:28, Chenyi Qiang wrote:
A new field, ram_shared, was introduced in RAMBlock to link to a
RamBlockAttribute object, which centralizes all guest_memfd state
information (such as fd and shared_bitmap) within a RAMBlock.
Create and initialize the RamBlockAttribute object upon ram_block_add().
Meanwhile, register the object in the target RAMBlock's MemoryRegion.
After that, guest_memfd-backed RAMBlock is associated with the
RamDiscardManager interface, and the users will execute
RamDiscardManager specific handling. For example, VFIO will register the
RamDiscardListener as expected. The live migration path needs to be
avoided since it is not supported yet in confidential VMs.
Additionally, use the ram_block_attribute_state_change() helper to
notify the registered RamDiscardListener of these changes.
Signed-off-by: Chenyi Qiang <chenyi.qi...@intel.com>
---
Changes in v5:
- Revert to use RamDiscardManager interface.
- Move the object_new() into the ram_block_attribute_create()
helper.
- Add some check in migration path.
Changes in v4:
- Remove the replay operations for attribute changes which will be
handled in a listener in following patches.
- Add some comment in the error path of realize() to remind the
future development of the unified error path.
Changes in v3:
- Use ram_discard_manager_reply_populated/discarded() to set the
memory attribute and add the undo support if state_change()
failed.
- Didn't add Reviewed-by from Alexey due to the new changes in this
commit.
Changes in v2:
- Introduce a new field memory_attribute_manager in RAMBlock.
- Move the state_change() handling during page conversion in this patch.
- Undo what we did if it fails to set.
- Change the order of close(guest_memfd) and memory_attribute_manager
cleanup.
---
accel/kvm/kvm-all.c | 9 +++++++++
migration/ram.c | 28 ++++++++++++++++++++++++++++
system/physmem.c | 14 ++++++++++++++
3 files changed, 51 insertions(+)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 51526d301b..2d7ecaeb6a 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3089,6 +3089,15 @@ int kvm_convert_memory(hwaddr start, hwaddr size, bool
to_private)
addr = memory_region_get_ram_ptr(mr) + section.offset_within_region;
rb = qemu_ram_block_from_host(addr, false, &offset);
+ ret = ram_block_attribute_state_change(RAM_BLOCK_ATTRIBUTE(mr->rdm),
+ offset, size, to_private);
+ if (ret) {
+ error_report("Failed to notify the listener the state change of "
+ "(0x%"HWADDR_PRIx" + 0x%"HWADDR_PRIx") to %s",
+ start, size, to_private ? "private" : "shared");
+ goto out_unref;
+ }
+
if (to_private) {
if (rb->page_size != qemu_real_host_page_size()) {
/*
diff --git a/migration/ram.c b/migration/ram.c
index c004f37060..69c9a42f16 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -890,6 +890,13 @@ static uint64_t
ramblock_dirty_bitmap_clear_discarded_pages(RAMBlock *rb)
if (rb->mr && rb->bmap && memory_region_has_ram_discard_manager(rb->mr)) {
RamDiscardManager *rdm =
memory_region_get_ram_discard_manager(rb->mr);
+
+ if (object_dynamic_cast(OBJECT(rdm), TYPE_RAM_BLOCK_ATTRIBUTE)) {
+ error_report("%s: Live migration for confidential VM is not "
+ "supported yet.", __func__);
+ exit(1);
+ }
These checks seem conceptually wrong.
I think if we were to special-case specific implementations, we should
do so using a different callback.
But why should we bother at all checking basic live migration handling
("unsupported for confidential VMs") on this level, and even just
exit()'ing the process?
All these object_dynamic_cast() checks in this patch should be dropped.
--
Cheers,
David / dhildenb
