Re: [Qemu-devel] Logging dirty pages from vhost-net in-kernel with vIOMMU

[Jason Wang]

On 2018/12/5 下午9:32, Michael S. Tsirkin wrote:
> On Wed, Dec 05, 2018 at 11:02:11AM +0800, Jason Wang wrote:
> > On 2018/12/5 上午9:59, Michael S. Tsirkin wrote:
> > > On Wed, Dec 05, 2018 at 09:30:19AM +0800, Jason Wang wrote:
> > > > On 2018/12/5 上午2:37, Jintack Lim wrote:
> > > > > Hi,
> > > > >
> > > > > I'm wondering how the current implementation works when logging dirty
> > > > > pages during migration from vhost-net (in kernel) when used vIOMMU.
> > > > >
> > > > > I understand how vhost-net logs GPAs when not using vIOMMU. But when
> > > > > we use vhost with vIOMMU, then shouldn't vhost-net need to log the
> > > > > translated address (GPA) instead of the address written in the
> > > > > descriptor (IOVA) ? The current implementation looks like vhost-net
> > > > > just logs IOVA without translation in vhost_get_vq_desc() in
> > > > > drivers/vhost/net.c. It seems like QEMU doesn't do any further
> > > > > translation of the dirty log when syncing.
> > > > >
> > > > > I might be missing something. Could somebody shed some light on this?
> > > > Good catch. It looks like a bug to me. Want to post a patch for this?
> > > This isn't going to be a quick fix: IOTLB UAPI is translating
> > > IOVA values directly to uaddr.
> > >
> > > So to fix it, we need to change IOVA messages to translate to GPA
> > > so GPA can be logged.
> > >
> > > for existing userspace We can try reverse translation uaddr->gpa as a
> > > hack for logging but that translation was never guaranteed to be unique.
> >
> > We have memory table in vhost as well, so looks like we can do this in
> > kernel as well without disturbing UAPI?
> >
> > Thanks
> Let me try to rephrase.
>
> Yes, as a temporary bugfix we can do the uaddr to gpa translations.
> It is probably good enough for what QEMU does now.
>
> However it can break some legal userspace, since it is possible to
> have multiple UADDR mappings for a single GPA.
> In that setup the vhost table would only have one of these
> and it's possible that IOTLB would use another one.


Consider we are logging GPA, so it doesn't matter which UADDR in this 
case since we finally get a same GPA. Maybe you mean multiple GPA 
mappings for a single UADDR? Then we may want to log all possible GPA in 
this case.


> And generally it's a better idea security-wise to make
> iotlb talk in GPA terms. This way whoever sets the static
> GPA-to-UADDR mappings controls security, and the dynamic
> and more fragile iova mappings can not break QEMU security.


AFAIK, this may only work if memory table and IOTLB entries were set by 
different process I believe. Consider it's all set by qemu, and qemu 
will go through GPA-UADDR mapping before setting device IOTLB. It's 
probably not a gain for us now.


> So we need a UAPI extension with a feature flag.

Yes.

Thanks


> > > Jason I think you'll have to work on it given the complexity.
> > >
> > > > Thanks
> > > >
> > > >
> > > > > Thanks,
> > > > > Jintack