Re: [PATCH 08/18] vfio/common: Record DMA mapped IOVA ranges

[Alex Williamson]

On Sun, 12 Feb 2023 17:40:06 +0200
Avihai Horon <avihaih@nvidia.com> wrote:

> On 27/01/2023 23:42, Alex Williamson wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On Thu, 26 Jan 2023 20:49:38 +0200
> > Avihai Horon <avihaih@nvidia.com> wrote:
> >  
> >> From: Joao Martins <joao.m.martins@oracle.com>
> >>
> >> According to the device DMA logging uAPI, IOVA ranges to be logged by
> >> the device must be provided all at once upon DMA logging start.
> >>
> >> As preparation for the following patches which will add device dirty
> >> page tracking, keep a record of all DMA mapped IOVA ranges so later they
> >> can be used for DMA logging start.
> >>
> >> Note that when vIOMMU is enabled DMA mapped IOVA ranges are not tracked.
> >> This is due to the dynamic nature of vIOMMU DMA mapping/unmapping.
> >> Following patches will address the vIOMMU case specifically.
> >>
> >> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> >> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> >> ---
> >>   include/hw/vfio/vfio-common.h |  3 ++
> >>   hw/vfio/common.c              | 86 +++++++++++++++++++++++++++++++++--
> >>   2 files changed, 86 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> >> index 88c2194fb9..d54000d7ae 100644
> >> --- a/include/hw/vfio/vfio-common.h
> >> +++ b/include/hw/vfio/vfio-common.h
> >> @@ -23,6 +23,7 @@
> >>
> >>   #include "exec/memory.h"
> >>   #include "qemu/queue.h"
> >> +#include "qemu/iova-tree.h"
> >>   #include "qemu/notify.h"
> >>   #include "ui/console.h"
> >>   #include "hw/display/ramfb.h"
> >> @@ -94,6 +95,8 @@ typedef struct VFIOContainer {
> >>       uint64_t max_dirty_bitmap_size;
> >>       unsigned long pgsizes;
> >>       unsigned int dma_max_mappings;
> >> +    IOVATree *mappings;
> >> +    QemuMutex mappings_mutex;
> >>       QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
> >>       QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
> >>       QLIST_HEAD(, VFIOGroup) group_list;
> >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> >> index e554573eb5..fafc361cea 100644
> >> --- a/hw/vfio/common.c
> >> +++ b/hw/vfio/common.c
> >> @@ -43,6 +43,7 @@
> >>   #include "migration/misc.h"
> >>   #include "migration/qemu-file.h"
> >>   #include "sysemu/tpm.h"
> >> +#include "qemu/iova-tree.h"
> >>
> >>   VFIOGroupList vfio_group_list =
> >>       QLIST_HEAD_INITIALIZER(vfio_group_list);
> >> @@ -373,6 +374,11 @@ bool vfio_mig_active(void)
> >>       return true;
> >>   }
> >>
> >> +static bool vfio_have_giommu(VFIOContainer *container)
> >> +{
> >> +    return !QLIST_EMPTY(&container->giommu_list);
> >> +}
> >> +
> >>   static void vfio_set_migration_error(int err)
> >>   {
> >>       MigrationState *ms = migrate_get_current();
> >> @@ -450,6 +456,51 @@ static bool 
> >> vfio_devices_all_running_and_mig_active(VFIOContainer *container)
> >>       return true;
> >>   }
> >>
> >> +static int vfio_record_mapping(VFIOContainer *container, hwaddr iova,
> >> +                               hwaddr size, bool readonly)
> >> +{
> >> +    DMAMap map = {
> >> +        .iova = iova,
> >> +        .size = size - 1, /* IOVATree is inclusive, so subtract 1 from 
> >> size */  
> > <facepalm>  
> 
> I am not sure what's the error you are referring here.
> Is it because DMAMap.size is not actually the size?
> Something else?

Sorry, I'm just lamenting what a terrible interface IOVATree provides
with this inclusive range nonsense.  Thanks,

Alex