Re: [Qemu-devel] [PATCH v5 03/18] vfio: allow to notify unmap for very large region

[David Gibson]

On Tue, Jan 24, 2017 at 09:32:07AM -0700, Alex Williamson wrote:
> On Tue, 24 Jan 2017 18:25:56 +0800
> Peter Xu <address@hidden> wrote:
> 
> > Linux vfio driver supports to do VFIO_IOMMU_UNMAP_DMA for a very big
> > region. This can be leveraged by QEMU IOMMU implementation to cleanup
> > existing page mappings for an entire iova address space (by notifying
> > with an IOTLB with extremely huge addr_mask). However current
> > vfio_iommu_map_notify() does not allow that. It make sure that all the
> > translated address in IOTLB is falling into RAM range.
> > 
> > The check makes sense, but it should only be a sensible checker for
> > mapping operations, and mean little for unmap operations.
> > 
> > This patch moves this check into map logic only, so that we'll get
> > faster unmap handling (no need to translate again), and also we can then
> > better support unmapping a very big region when it covers non-ram ranges
> > or even not-existing ranges.
> > 
> > Signed-off-by: Peter Xu <address@hidden>
> > ---
> >  hw/vfio/common.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> > index ce55dff..4d90844 100644
> > --- a/hw/vfio/common.c
> > +++ b/hw/vfio/common.c
> > @@ -354,11 +354,10 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, 
> > IOMMUTLBEntry *iotlb)
> >          return;
> >      }
> >  
> > -    if (!vfio_get_vaddr(iotlb, &vaddr, &read_only)) {
> > -        return;
> > -    }
> > -
> >      if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
> > +        if (!vfio_get_vaddr(iotlb, &vaddr, &read_only)) {
> > +            return;
> > +        }
> 
> 
> David, is SPAPR going to freak out if it sees unmaps to ranges that
> extend beyond individual mappings, or perhaps include no mappings?
> This effectively allows unmapping the entire address space of the iommu
> in one pass, without validating or translating the backing.

Extending beyond an individual mapping will be fine.  However, if the
unmap extends beyond the extent of IOVAs mapped by a single TCE table,
then the unmap will fail (with ENXIO or EINVAL depending on whether
there's a problem with origin or only size).

With holidays I've lost the context of this thread, so I can't easily
find the whole patch series this relates to.  From your brief
description above it sounds likes it should be ok - a range covering
just the IOVA space (as long as that's actually correct for spapr tce)
should be ok.

> 
> >          ret = vfio_dma_map(container, iova,
> >                             iotlb->addr_mask + 1, vaddr,
> >                             read_only);
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature