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
