Re: [Qemu-devel] [RFC PATCH v3 3/3] VFIO Type1 IOMMU change: to support with iommu and without iommu

[Tian, Kevin]

> From: Song, Jike
> Sent: Thursday, May 05, 2016 2:56 PM
> >
> > The only reason I can come up with for why we'd want to integrate an
> > api-only domain into the existing type1 code would be to avoid page
> > accounting issues where we count locked pages once for a normal
> > assigned device and again for a vgpu, but that's not what we're doing
> > here.  We're not only locking the pages again regardless of them
> > already being locked, we're counting every time we lock them through
> > this new interface.  So there's really no point at all to making type1
> > become this unsupportable.  In that case we should be pulling out the
> > common code that we want to share from type1 and making a new type1
> > compatible vfio iommu backend rather than conditionalizing everything
> > here.
> >
> >> +
> >> +          // add to pfn_list
> >> +          lpfn = kzalloc(sizeof(*lpfn), GFP_KERNEL);
> >> +          if (!lpfn) {
> >> +                  ret = -ENOMEM;
> >> +                  mutex_unlock(&domain_vgpu->lock);
> >> +                  goto pin_done;
> >> +          }
> >> +          lpfn->vmm_va = remote_vaddr;
> >> +          lpfn->iova = iova;
> >> +          lpfn->pfn = pfn[i];
> >> +          lpfn->npage = 1;
> >> +          lpfn->prot = prot;
> >> +          atomic_inc(&lpfn->ref_count);
> >> +          vfio_link_vgpu_pfn(domain_vgpu, lpfn);
> >> +          mutex_unlock(&domain_vgpu->lock);
> >> +  }
> >> +
> >> +  ret = i;
> >> +
> >> +pin_done:
> >> +  return ret;
> >> +}
> >> +EXPORT_SYMBOL(vfio_pin_pages);
> >> +
> 
> IIUC, an api-only domain is a VFIO domain *without* underlying IOMMU
> hardware. It just, as you said in another mail, "rather than
> programming them into an IOMMU for a device, it simply stores the
> translations for use by later requests".
> 
> That imposes a constraint on gfx driver: hardware IOMMU must be disabled.
> Otherwise, if IOMMU is present, the gfx driver eventually programs
> the hardware IOMMU with IOVA returned by pci_map_page or dma_map_page;
> Meanwhile, the IOMMU backend for vgpu only maintains GPA <-> HPA
> translations without any knowledge about hardware IOMMU, how is the
> device model supposed to do to get an IOVA for a given GPA (thereby HPA
> by the IOMMU backend here)?
> 
> If things go as guessed above, as vfio_pin_pages() indicates, it
> pin & translate vaddr to PFN, then it will be very difficult for the
> device model to figure out:
> 
>       1, for a given GPA, how to avoid calling dma_map_page multiple times?
>       2, for which page to call dma_unmap_page?
> 
> --

We have to support both w/ iommu and w/o iommu case, since
that fact is out of GPU driver control. A simple way is to use
dma_map_page which internally will cope with w/ and w/o iommu
case gracefully, i.e. return HPA w/o iommu and IOVA w/ iommu.
Then in this file we only need to cache GPA to whatever dmadr_t
returned by dma_map_page.

Thanks
Kevin