Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback

From: Peter Xu <peterx@redhat.com>
To: Jason Wang <jasowang@redhat.com>
Cc: "Liu, Yi L" <yi.l.liu@intel.com>,
	"'alex.williamson@redhat.com'" <alex.williamson@redhat.com>,
	"Lan, Tianyu" <tianyu.lan@intel.com>,
	"Tian, Kevin" <kevin.tian@intel.com>,
	"'mst@redhat.com'" <mst@redhat.com>,
	"'jan.kiszka@siemens.com'" <jan.kiszka@siemens.com>,
	"'bd.aviv@gmail.com'" <bd.aviv@gmail.com>,
	'David Gibson' <david@gibson.dropbear.id.au>,
	"'qemu-devel@nongnu.org'" <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH v7 14/17] memory: add MemoryRegionIOMMUOps.replay() callback
Date: Fri, 31 Mar 2017 13:28:04 +0800	[thread overview]
Message-ID: <20170331052804.GG3981@pxdev.xzpeter.org> (raw)
In-Reply-To: <9a5bfd93-2d61-96c8-7a95-bccb5a0c819d@redhat.com>

On Fri, Mar 31, 2017 at 01:12:56PM +0800, Jason Wang wrote:
> 
> 
> On 2017年03月31日 13:01, Peter Xu wrote:
> >On Fri, Mar 31, 2017 at 12:21:23PM +0800, Jason Wang wrote:
> >>
> >>On 2017年03月31日 10:56, Peter Xu wrote:
> >>>>>Just come to mind that there may be a corner case here.
> >>>>>
> >>>>>Intel VT-d actually has a "pt" mode which allows device use physical address
> >>>>>even when VT-d is enabled. In kernel, there is a iommu_identity_mapping.
> >>>>>If a device is in this map, then it would use "pt" mode. So that IOMMU driver
> >>>>>would not build second-level page table for it.
> >>>>Yes, but qemu does not support ECAP_PT now, so guest will still have a page
> >>>>table in this case.
> >>>>
> >>>>>Back to the virtual IOVA implementation, if an assigned device is in the
> >>>>>iommu_identity_mapping(e.g. VGA controller), it uses GPA directly to do DMA.
> >>>>>So it demands a GPA->HPA mapping in host. However, the iommu->ops.replay
> >>>>>is not able to build it when guest SL page table is empty.
> >>>>>
> >>>>>So I think building an entire guest PA->HPA mapping before guest kernel boot
> >>>>>would be recommended. Any thoughts?
> >>>>We plan to add PT in 2.10, a possible rough idea is disabled iommu dmar
> >>>>region and use another region without iommu_ops. Then
> >>>>vfio_listener_region_add() will just do the correct mappings.
> >>>Even without any new region. With the patch 16/17 ("intel_iommu: allow
> >>>dynamic switch of IOMMU region"), we can just turn the IOMMU region
> >>>on/off, following the device's PT bit, maybe using the new
> >>>vtd_switch_address_space() interface. That should be enough.
> >>Right. For vhost it was probably need more works, e.g setting up static
> >>mappings during region_add().
> >Do we need to?
> 
> Not a must if we don't care about performance.
> 
> >
> >VFIO will need it for building up shadow page table, even without a
> >vIOMMU. But imho that should not be needed by vhost, right?
> 
> Device IOTLB will be enabled unconditionally if iommu_platform is specified.
> If we don't set static mappings, vhost will send IOTLB miss request. The
> performance will be horrible in this case.

I see, thanks. So looks like we will need one more patch for PT
support now. :)

-- peterx