Re: [Qemu-devel] vhost, iova, and dirty page tracking

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Sep 20, 2019 at 09:15:40AM +0800, Jason Wang wrote:
> 
> On 2019/9/19 下午10:06, Michael S. Tsirkin wrote:
> > On Thu, Sep 19, 2019 at 05:37:48PM +0800, Jason Wang wrote:
> > > On 2019/9/19 下午3:16, Tian, Kevin wrote:
> > > > +Paolo to help clarify here.
> > > > 
> > > > > From: Jason Wang [mailto:jasowang@redhat.com]
> > > > > Sent: Thursday, September 19, 2019 2:32 PM
> > > > > 
> > > > > 
> > > > > On 2019/9/19 下午2:17, Yan Zhao wrote:
> > > > > > On Thu, Sep 19, 2019 at 02:09:53PM +0800, Jason Wang wrote:
> > > > > > > On 2019/9/19 下午1:28, Yan Zhao wrote:
> > > > > > > > On Thu, Sep 19, 2019 at 09:05:12AM +0800, Jason Wang wrote:
> > > > > > > > > On 2019/9/18 下午4:37, Tian, Kevin wrote:
> > > > > > > > > > > From: Jason Wang [mailto:jasowang@redhat.com]
> > > > > > > > > > > Sent: Wednesday, September 18, 2019 2:10 PM
> > > > > > > > > > > 
> > > > > > > > > > > > > Note that the HVA to GPA mapping is not an 1:1 mapping. One
> > > > > HVA
> > > > > > > > > > > range
> > > > > > > > > > > > > could be mapped to several GPA ranges.
> > > > > > > > > > > > This is fine. Currently vfio_dma maintains IOVA->HVA mapping.
> > > > > > > > > > > > 
> > > > > > > > > > > > btw under what condition HVA->GPA is not 1:1 mapping? I didn't
> > > > > realize it.
> > > > > > > > > > > I don't remember the details e.g memory region alias? And neither
> > > > > kvm
> > > > > > > > > > > nor kvm API does forbid this if my memory is correct.
> > > > > > > > > > > 
> > > > > > > > > > I checked https://qemu.weilnetz.de/doc/devel/memory.html, which
> > > > > > > > > > provides an example of aliased layout. However, its aliasing is all
> > > > > > > > > > 1:1, instead of N:1. From guest p.o.v every writable GPA implies an
> > > > > > > > > > unique location. Why would we hit the situation where multiple
> > > > > > > > > > write-able GPAs are mapped to the same HVA (i.e. same physical
> > > > > > > > > > memory location)?
> > > > > > > > > I don't know, just want to say current API does not forbid this. So we
> > > > > > > > > probably need to take care it.
> > > > > > > > > 
> > > > > > > > yes, in KVM API level, it does not forbid two slots to have the same
> > > > > HVA(slot->userspace_addr).
> > > > > > > > But
> > > > > > > > (1) there's only one kvm instance for each vm for each qemu process.
> > > > > > > > (2) all ramblock->host (corresponds to HVA and slot->userspace_addr)
> > > > > in one qemu
> > > > > > > > process is non-overlapping as it's obtained from mmmap().
> > > > > > > > (3) qemu ensures two kvm slots will not point to the same section of
> > > > > one ramblock.
> > > > > > > > So, as long as kvm instance is not shared in two processes, and
> > > > > > > > there's no bug in qemu, we can assure that HVA to GPA is 1:1.
> > > > > > > Well, you leave this API for userspace, so you can't assume qemu is the
> > > > > > > only user or any its behavior. If you had you should limit it in the API
> > > > > > > level instead of open window for them.
> > > > > > > 
> > > > > > > 
> > > > > > > > But even if there are two processes operating on the same kvm
> > > > > instance
> > > > > > > > and manipulating on memory slots, adding an extra GPA along side
> > > > > current
> > > > > > > > IOVA & HVA to ioctl VFIO_IOMMU_MAP_DMA can still let driver knows
> > > > > the
> > > > > > > > right IOVA->GPA mapping, right?
> > > > > > > It looks fragile. Consider HVA was mapped to both GPA1 and GPA2.
> > > > > Guest
> > > > > > > maps IOVA to GPA2, so we have IOVA GPA2 HVA in the new ioctl and
> > > > > then
> > > > > > > log through GPA2. If userspace is trying to sync through GPA1, it will
> > > > > > > miss the dirty page. So for safety we need log both GPA1 and GPA2. (See
> > > > > > > what has been done in log_write_hva() in vhost.c). The only way to do
> > > > > > > that is to maintain an independent HVA to GPA mapping like what KVM
> > > > > or
> > > > > > > vhost did.
> > > > > > > 
> > > > > > why GPA1 and GPA2 should be both dirty?
> > > > > > even they have the same HVA due to overlaping virtual address space in
> > > > > > two processes, they still correspond to two physical pages.
> > > > > > don't get what's your meaning :)
> > > > > The point is not leave any corner case that is hard to debug or fix in
> > > > > the future.
> > > > > 
> > > > > Let's just start by a single process, the API allows userspace to maps
> > > > > HVA to both GPA1 and GPA2. Since it knows GPA1 and GPA2 are equivalent,
> > > > > it's ok to sync just through GPA1. That means if you only log GPA2, it
> > > > > won't work.
> > > > > 
> > > > I noted KVM itself doesn't consider such situation (one HVA is mapped
> > > > to multiple GPAs), when doing its dirty page tracking. If you look at
> > > > kvm_vcpu_mark_page_dirty, it simply finds the unique memslot which
> > > > contains the dirty gfn and then set the dirty bit within that slot. It
> > > > doesn't attempt to walk all memslots to find out any other GPA which
> > > > may be mapped to the same HVA.
> > > > 
> > > > So there must be some disconnect here. let's hear from Paolo first and
> > > > understand the rationale behind such situation.
> > > 
> > > Neither did vhost when IOTLB is disabled. And cc Michael who points out this
> > > issue at the beginning.
> > > 
> > > Thanks
> > > 
> > > 
> > > > Thanks
> > > > Kevin
> > Yes, we fixed with a kind of a work around, at the time I proposed
> > a new interace to fix it fully. I don't think we ever got around
> > to implementing it - right?
> 
> 
> Paolo said userspace just need to sync through all GPAs, so my understanding
> is that work around is ok by redundant, so did the API you proposed.
> Anything I miss?
> 
> Thanks

I just feel an extra lookup is awkward. We don't benchmark
the speed during migration right now but it's something
we might care about down the road.

HTH

-- 
MST