Re: [PATCH v2 3/6] vhost: log used vring changes

[Peter Xu <peterx@redhat.com>]

On Tue, Dec 22, 2015 at 03:13:49PM +0800, Yuanhan Liu wrote:
> On Tue, Dec 22, 2015 at 02:55:52PM +0800, Peter Xu wrote:
> > On Thu, Dec 17, 2015 at 11:11:58AM +0800, Yuanhan Liu wrote:
> > > +static inline void __attribute__((always_inline))
> > > +vhost_log_used_vring(struct virtio_net *dev, struct vhost_virtqueue *vq,
> > > +		     uint64_t offset, uint64_t len)
> > > +{
> > 
> > One thing optional: I feel it a little bit confusing regarding to
> > the helper function name here, for the reasons:
> > 
> > 1. It more sounds like "logging all the vrings we used", however,
> >    what I understand is that, here we are logging dirty pages for
> >    guest memory. Or say, there is merely nothing to do directly with
> >    vring (although many vring ops might call this function, we are
> >    only passing [buf, len] pairs).
> > 
> > 2. It may also let people think of "vring_used", which is part of
> >    virtio protocol. However, it does not mean it too.
> 
> Yes, it does. Here log_guest_addr is set to physical address of
> vring_used. (check code at vhost_virtqueue_set_addr() of qemu).
> 
>      627 static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
>      628                                     struct vhost_virtqueue *vq,
>      629                                     unsigned idx, bool enable_log)
>      630 {
>      631     struct vhost_vring_addr addr = {
>      632         .index = idx,
>      633         .desc_user_addr = (uint64_t)(unsigned long)vq->desc,
>      634         .avail_user_addr = (uint64_t)(unsigned long)vq->avail,
>      635         .used_user_addr = (uint64_t)(unsigned long)vq->used,
> ==>  636         .log_guest_addr = vq->used_phys,
>      637         .flags = enable_log ? (1 << VHOST_VRING_F_LOG) : 0,
>      638     };
> 
> So, this function does log changes to used vring.

Yes. I have made a mistake.

Thanks!
Peter