ram-block-attribute update notifications are currently sent after conversions from/to private pages to trigger DMA maps/unmaps of shared GPA ranges (respectively). However, with in-place conversion additional requirements on the kernel side come into play which require this behavior to be adjusted.
For shared->private conversions: the attributes need to be set to private *after* the notification, since when using VFIO it may not be possible to update the attribute while it remains pinned due to the IOMMU mapping, so issue the notification first to ensure unmappings are done in advance. For private->shared conversions: the attributes need to be set to shared *before* the notification, since it will possibly result in the page being mapped into an IOMMU and trigger guest_memfd's fault handler, which will expect the page to have its attributes set to shared or otherwise SIGBUS. Implement this to enable passthrough support for CoCo guests with in-place conversion support enabled. For non-inplace conversion, pages mapped into the IOMMU are not the same physical pages as the one used for private accesses by the guest, so neither order risks DMA accesses to private memory and that path can be consolidated to use the same handling as well. Signed-off-by: Michael Roth <[email protected]> --- accel/kvm/kvm-all.c | 70 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 63 insertions(+), 7 deletions(-) diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index 0e6ff2de4b..62f2e8aa15 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -3456,6 +3456,47 @@ static int kvm_convert_section(MemoryRegionSection *section, bool to_private) return ret; } +static int kvm_pre_convert_section(MemoryRegionSection *section, bool to_private) +{ + hwaddr start = section->offset_within_address_space; + hwaddr size = int128_get64(section->size); + MemoryRegion *mr = section->mr; + ram_addr_t offset; + RAMBlock *rb; + void *addr; + int ret; + + addr = memory_region_get_ram_ptr(mr) + section->offset_within_region; + rb = qemu_ram_block_from_host(addr, false, &offset); + + /* + * The attributes need to be set to private *after* the notification + * of a shared->private conversion, since when using VFIO it may not + * be possible to update the attribute while it remains pinned due + * to the IOMMU mapping, so issue the notification first to ensure + * unmappings are done in advance. + * + * There is an asymmetry here in that if the subsequent memory + * attribute update fails, this notification is out of sync with the + * state as tracked by guest_memfd, which isn't ideal, but memory + * attribute failures are not expected to be recoverable any way so + * there it would be a waste of time to roll back the notification and + * re-trigger things like mapping the page via iommufd. + */ + if (to_private) { + ret = ram_block_attributes_state_change(rb->attributes, + 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, ret %d", + start, size, to_private ? "private" : "shared", ret); + return ret; + } + } + + return 0; +} + static int kvm_post_convert_section(MemoryRegionSection *section, bool to_private) { hwaddr start = section->offset_within_address_space; @@ -3469,13 +3510,22 @@ static int kvm_post_convert_section(MemoryRegionSection *section, bool to_privat addr = memory_region_get_ram_ptr(mr) + section->offset_within_region; rb = qemu_ram_block_from_host(addr, false, &offset); - ret = ram_block_attributes_state_change(rb->attributes, - 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, ret %d", - start, size, to_private ? "private" : "shared", ret); - return ret; + /* + * The attributes need to have been set to shared *before* the notification + * of a private->shared conversion, since it will possibly result in the + * page being mapped into an IOMMU when using VFIO and trigger + * guest_memfd's fault handler, which will expect the page to have its + * attributes set to shared. + */ + if (!to_private) { + ret = ram_block_attributes_state_change(rb->attributes, + 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, ret %d", + start, size, to_private ? "private" : "shared", ret); + return ret; + } } if (to_private) { @@ -3538,6 +3588,12 @@ int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private) continue; } + ret = kvm_pre_convert_section(§ion, to_private); + if (ret) { + memory_region_unref(section.mr); + break; + } + ret = kvm_convert_section(§ion, to_private); if (ret) { memory_region_unref(section.mr); -- 2.43.0
