From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <peterx@redhat.com>
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
 by dpdk.org (Postfix) with ESMTP id 5C24A370
 for <dev@dpdk.org>; Tue, 22 Dec 2015 09:01:18 +0100 (CET)
Received: from int-mx11.intmail.prod.int.phx2.redhat.com
 (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24])
 by mx1.redhat.com (Postfix) with ESMTPS id AB2B8C00124A;
 Tue, 22 Dec 2015 08:01:17 +0000 (UTC)
Received: from pxdev.xzpeter.org (vpn1-6-183.pek2.redhat.com [10.72.6.183])
 by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id
 tBM816N8009904
 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO);
 Tue, 22 Dec 2015 03:01:11 -0500
Date: Tue, 22 Dec 2015 16:01:03 +0800
From: Peter Xu <peterx@redhat.com>
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Message-ID: <20151222080103.GE7532@pxdev.xzpeter.org>
References: <1449027793-30975-1-git-send-email-yuanhan.liu@linux.intel.com>
 <1450321921-27799-1-git-send-email-yuanhan.liu@linux.intel.com>
 <1450321921-27799-4-git-send-email-yuanhan.liu@linux.intel.com>
 <20151222065552.GC7532@pxdev.xzpeter.org>
 <20151222071349.GQ18863@yliu-dev.sh.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20151222071349.GQ18863@yliu-dev.sh.intel.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24
Cc: "Michael S. Tsirkin" <mst@redhat.com>, dev@dpdk.org,
 Victor Kaplansky <vkaplans@redhat.com>
Subject: Re: [dpdk-dev] [PATCH v2 3/6] vhost: log used vring changes
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 22 Dec 2015 08:01:18 -0000

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