On Tue, Jan 31, 2017 at 02:35:00PM +1100, David Gibson wrote:
> On Tue, Jan 24, 2017 at 09:32:07AM -0700, Alex Williamson wrote:
> > On Tue, 24 Jan 2017 18:25:56 +0800
> > Peter Xu <pet...@redhat.com> 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 <pet...@redhat.com>
> > > ---
> > > 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.
Thanks for the confirmation! Then I'll move ahead for the next spin.
Thanks,
-- peterx
