Re: [Qemu-devel] [PULL 08/41] intel_iommu: support device iotlb descriptor

[Peter Xu]

On Thu, Feb 16, 2017 at 05:36:06AM +0000, Liu, Yi L wrote:
> > -----Original Message-----
> > From: Qemu-devel [mailto:address@hidden
> > On Behalf Of Michael S. Tsirkin
> > Sent: Tuesday, January 10, 2017 1:40 PM
> > To: address@hidden
> > Cc: Peter Maydell <address@hidden>; Eduardo Habkost
> > <address@hidden>; Jason Wang <address@hidden>; Peter Xu
> > <address@hidden>; Paolo Bonzini <address@hidden>; Richard
> > Henderson <address@hidden>
> > Subject: [Qemu-devel] [PULL 08/41] intel_iommu: support device iotlb
> > descriptor
> > 
> > From: Jason Wang <address@hidden>
> > 
> > This patch enables device IOTLB support for intel iommu. The major work is 
> > to
> > implement QI device IOTLB descriptor processing and notify the device 
> > through
> > iommu notifier.
> >
> Hi Jason/Michael,
>       
> Recently Peter Xu's patch also touched intel-iommu emulation. His patch 
> shadows
> second-level page table by capturing iotlb flush from guest. It would result 
> in page
> table updating in host. Does this patch also use the same map/umap API 
> provided
> by VFIO? If it is, then I think it would also update page table in host.

I haven't considered complex scenarios, but if simply we have a VM
with one vhost device and one vfio-pci device, imho that should not be
a problem - device iotlb is targeting SID rather than DOMAIN. So
device iotlb invalidations for vhost will be sent to vhost device
only.

However, vhost may receive two invalidations here, but it won't matter
much since vhost is just flushing caches twice.

> It looks to be
> a duplicate update. Pls refer to the following snapshot captured from section 
> 6.5.2.5
> of vtd spec. 
> 
> "Since translation requests from a device may be serviced by hardware from 
> the IOTLB, software must
> always request IOTLB invalidation (iotlb_inv_dsc) before requesting 
> corresponding Device-TLB
> (dev_tlb_inv_dsc) invalidation."
> 
> Maybe for device-iotlb, we need a separate API which just pass down the 
> invalidate
> info without updating page table. Any thoughts?

Now imho I slightly prefer just use the current UNMAP notifier type
even for device iotlb device. But, we may need to do one more check
that we skip sending general iotlb invalidations to ATS enabled
devices like vhost, to avoid duplicated cache flushing. From virt-svm
side, do we have specific requirement to introduce a new flag for it?

Thanks,

-- peterx