On 14.01.21 16:57, David Hildenbrand wrote: > On 14.01.21 16:54, David Hildenbrand wrote: >> On 14.01.21 00:27, Alex Williamson wrote: >>> On Thu, 7 Jan 2021 14:34:16 +0100 >>> David Hildenbrand <da...@redhat.com> wrote: >>> >>>> Implement support for RamDiscardMgr, to prepare for virtio-mem >>>> support. Instead of mapping the whole memory section, we only map >>>> "populated" parts and update the mapping when notified about >>>> discarding/population of memory via the RamDiscardListener. Similarly, when >>>> syncing the dirty bitmaps, sync only the actually mapped (populated) parts >>>> by replaying via the notifier. >>>> >>>> Using virtio-mem with vfio is still blocked via >>>> ram_block_discard_disable()/ram_block_discard_require() after this patch. >>>> >>>> Cc: Paolo Bonzini <pbonz...@redhat.com> >>>> Cc: "Michael S. Tsirkin" <m...@redhat.com> >>>> Cc: Alex Williamson <alex.william...@redhat.com> >>>> Cc: Dr. David Alan Gilbert <dgilb...@redhat.com> >>>> Cc: Igor Mammedov <imamm...@redhat.com> >>>> Cc: Pankaj Gupta <pankaj.gupta.li...@gmail.com> >>>> Cc: Peter Xu <pet...@redhat.com> >>>> Cc: Auger Eric <eric.au...@redhat.com> >>>> Cc: Wei Yang <richard.weiy...@linux.alibaba.com> >>>> Cc: teawater <teawat...@linux.alibaba.com> >>>> Cc: Marek Kedzierski <mkedz...@redhat.com> >>>> Signed-off-by: David Hildenbrand <da...@redhat.com> >>>> --- >>>> hw/vfio/common.c | 200 ++++++++++++++++++++++++++++++++++ >>>> include/hw/vfio/vfio-common.h | 12 ++ >>>> 2 files changed, 212 insertions(+) >>>> >>>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c >>>> index 6ff1daa763..2bd219cf1d 100644 >>>> --- a/hw/vfio/common.c >>>> +++ b/hw/vfio/common.c >>>> @@ -654,6 +654,136 @@ out: >>>> rcu_read_unlock(); >>>> } >>>> >>>> +static void vfio_ram_discard_notify_discard(RamDiscardListener *rdl, >>>> + const MemoryRegion *mr, >>>> + ram_addr_t offset, ram_addr_t >>>> size) >>>> +{ >>>> + VFIORamDiscardListener *vrdl = container_of(rdl, >>>> VFIORamDiscardListener, >>>> + listener); >>>> + const hwaddr mr_start = MAX(offset, vrdl->offset_within_region); >>>> + const hwaddr mr_end = MIN(offset + size, >>>> + vrdl->offset_within_region + vrdl->size); >>>> + const hwaddr iova = mr_start - vrdl->offset_within_region + >>>> + vrdl->offset_within_address_space; >>>> + int ret; >>>> + >>>> + if (mr_start >= mr_end) { >>>> + return; >>>> + } >>>> + >>>> + /* Unmap with a single call. */ >>>> + ret = vfio_dma_unmap(vrdl->container, iova, mr_end - mr_start, NULL); >>>> + if (ret) { >>>> + error_report("%s: vfio_dma_unmap() failed: %s", __func__, >>>> + strerror(-ret)); >>>> + } >>>> +} >>>> + >>>> +static int vfio_ram_discard_notify_populate(RamDiscardListener *rdl, >>>> + const MemoryRegion *mr, >>>> + ram_addr_t offset, ram_addr_t >>>> size) >>>> +{ >>>> + VFIORamDiscardListener *vrdl = container_of(rdl, >>>> VFIORamDiscardListener, >>>> + listener); >>>> + const hwaddr mr_end = MIN(offset + size, >>>> + vrdl->offset_within_region + vrdl->size); >>>> + hwaddr mr_start = MAX(offset, vrdl->offset_within_region); >>>> + hwaddr mr_next, iova; >>>> + void *vaddr; >>>> + int ret; >>>> + >>>> + /* >>>> + * Map in (aligned within memory region) minimum granularity, so we >>>> can >>>> + * unmap in minimum granularity later. >>>> + */ >>>> + for (; mr_start < mr_end; mr_start = mr_next) { >>>> + mr_next = ROUND_UP(mr_start + 1, vrdl->granularity); >>>> + mr_next = MIN(mr_next, mr_end); >>>> + >>>> + iova = mr_start - vrdl->offset_within_region + >>>> + vrdl->offset_within_address_space; >>>> + vaddr = memory_region_get_ram_ptr(vrdl->mr) + mr_start; >>>> + >>>> + ret = vfio_dma_map(vrdl->container, iova, mr_next - mr_start, >>>> + vaddr, mr->readonly); >>>> + if (ret) { >>>> + /* Rollback */ >>>> + vfio_ram_discard_notify_discard(rdl, mr, offset, size); >>>> + return ret; >>>> + } >>>> + } >>>> + return 0; >>>> +} >>>> + >>>> +static void vfio_ram_discard_notify_discard_all(RamDiscardListener *rdl, >>>> + const MemoryRegion *mr) >>>> +{ >>>> + VFIORamDiscardListener *vrdl = container_of(rdl, >>>> VFIORamDiscardListener, >>>> + listener); >>>> + int ret; >>>> + >>>> + /* Unmap with a single call. */ >>>> + ret = vfio_dma_unmap(vrdl->container, >>>> vrdl->offset_within_address_space, >>>> + vrdl->size, NULL); >>>> + if (ret) { >>>> + error_report("%s: vfio_dma_unmap() failed: %s", __func__, >>>> + strerror(-ret)); >>>> + } >>>> +} >>>> + >>>> +static void vfio_register_ram_discard_notifier(VFIOContainer *container, >>>> + MemoryRegionSection >>>> *section) >>>> +{ >>>> + RamDiscardMgr *rdm = memory_region_get_ram_discard_mgr(section->mr); >>>> + RamDiscardMgrClass *rdmc = RAM_DISCARD_MGR_GET_CLASS(rdm); >>>> + VFIORamDiscardListener *vrdl; >>>> + >>>> + vrdl = g_new0(VFIORamDiscardListener, 1); >>>> + vrdl->container = container; >>>> + vrdl->mr = section->mr; >>>> + vrdl->offset_within_region = section->offset_within_region; >>>> + vrdl->offset_within_address_space = >>>> section->offset_within_address_space; >>>> + vrdl->size = int128_get64(section->size); >>>> + vrdl->granularity = rdmc->get_min_granularity(rdm, section->mr); >>>> + >>>> + /* Ignore some corner cases not relevant in practice. */ >>>> + g_assert(QEMU_IS_ALIGNED(vrdl->offset_within_region, >>>> TARGET_PAGE_SIZE)); >>>> + g_assert(QEMU_IS_ALIGNED(vrdl->offset_within_address_space, >>>> + TARGET_PAGE_SIZE)); >>>> + g_assert(QEMU_IS_ALIGNED(vrdl->size, TARGET_PAGE_SIZE)); >>> >>> Should probably toss in a test of vrdl->granularity vs >>> container->pgsizes too, right? Looks good otherwise: >> >> Makes sense as a sanity check. What about >> >> g_assert(vrdl->granularity && !is_power_of_2(vrdl->granularity)); > > ^ g_assert(vrdl->granularity >= 1 << ctz64(container->pgsizes)); >
Ehm, I guess my brain needs some time to cool down over the weekend :D + g_assert(vrdl->granularity && is_power_of_2(vrdl->granularity)); + g_assert(vrdl->granularity >= 1 << ctz64(container->pgsizes)); -- Thanks, David / dhildenb