> -----Original Message----- > From: Peter Xu [mailto:pet...@redhat.com] > Sent: Thursday, December 29, 2016 5:56 PM > To: Liu, Yi L <yi.l....@intel.com> > Cc: Tian, Kevin <kevin.t...@intel.com>; Lan, Tianyu <tianyu....@intel.com>; > 'qemu- > de...@nongnu.org' <qemu-devel@nongnu.org>; 'bd.a...@gmail.com' > <bd.a...@gmail.com> > Subject: Re: [Qemu-devel] [RFC PATCH 11/13] intel_iommu: provide its own > replay() > callback > > On Thu, Dec 29, 2016 at 07:38:20AM +0000, Liu, Yi L wrote: > > [...] > > > > +static void vtd_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n) > > > +{ > > > + VTDAddressSpace *vtd_as = container_of(mr, VTDAddressSpace, iommu); > > > + IntelIOMMUState *s = vtd_as->iommu_state; > > > + uint8_t bus_n = pci_bus_num(vtd_as->bus); > > > + VTDContextEntry ce; > > > + > > > + if (vtd_dev_to_context_entry(s, bus_n, vtd_as->devfn, &ce) == 0) { > > > > Hi Peter, > > > > Again I'd like to give my cheers on this patch set. > > Thanks. > > > > > Hereby, I have a minor concern on this part. > > The replay would be performed in vfio_listener_region_add() when device is > > assigned. Since guest is just started, so there will be no valid context > > entry for the > > assigned device. So there will be no vtd_page_walk. Thus no change to SL > > page > > table in host. The SL page table would still be a IOVA->HPA mapping. It's > > true > > that the gIOVA->HPA mapping can be built by page map/unmap when guest > > trying to issue dma. So it is just ok without relpay in the beginning. Hot > > plug should > > be all the same. I guess it is the design here? > > I am not sure whether I get your point here - I think it's by design. > We don't really need replay if we are sure that the IOMMU address > space has totally no mapping (i.e., when the guest init). Replay is > only useful when there are existing mappings in the IOMMU address > space. yes, if we are sure no existing mapping, it is ok. There is another case which may complain it. If a device is hotplug in, then there should be existing mapping. Just looked code again. If corresponding context entry is not present, the vtd_page_walk should be bypassed.
> > Another thing to mention: IMHO IOVA -> HPA mapping should be built by > page map/unmap when guest issues IOTLB invalidations, not DMA (guest > will first issue IOTLB invalidations, only after that the iova address > would be a valid DMA address). Exactly. thx for the correction. > > > > > However, it may be incompatible with a future work in which we expose an > > vIOMMU to guest but guest disable IOVA usage. In this scenario, the SL page > > table in host would be a GPA->HPA instead of a gIOVA->HPA mapping. It would > > rely on memory_replay to build a GPA->HPA mapping. If I'm correct, I think > > it > > would be expected to have a replay in the time device is assigned. > > If you need a GPA -> HPA mapping (AFAICT you mean virtualized SVM and > the so-called first level translation), IMHO the default mapping > behavior of vfio_listener_region_add() suites here (when > memory_region_is_iommu(section->mr) == false), which maps the whole > guest physical address space, just like the case when we use vfio > devices without vIOMMU. Yes, I mean SVM virtualizaion. To virtualize SVM, an vIOMMU would be exposed to guest. So memory_region_is_iommu(section->mr) would be true. Then, the original mapping the whole guest physical address space would not run. Thus no GPA->HPA mapping would be built. This is what I really concern here. Thanks, Yi L
