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