From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 1DA051BE76 for ; Thu, 5 Jul 2018 07:12:55 +0200 (CEST) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Jul 2018 22:12:55 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,310,1526367600"; d="scan'208";a="64282757" Received: from debian.sh.intel.com (HELO debian) ([10.67.104.228]) by fmsmga002.fm.intel.com with ESMTP; 04 Jul 2018 22:12:53 -0700 Date: Thu, 5 Jul 2018 13:12:41 +0800 From: Tiwei Bie To: Maxime Coquelin Cc: zhihong.wang@intel.com, jfreimann@redhat.com, dev@dpdk.org, mst@redhat.com, jasowang@redhat.com, wexu@redhat.com Message-ID: <20180705051241.GA20322@debian> References: <20180704215438.5579-1-maxime.coquelin@redhat.com> <20180704215438.5579-15-maxime.coquelin@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20180704215438.5579-15-maxime.coquelin@redhat.com> User-Agent: Mutt/1.9.5 (2018-04-13) Subject: Re: [dpdk-dev] [PATCH v7 14/15] vhost: add notification for packed ring X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Jul 2018 05:12:56 -0000 On Wed, Jul 04, 2018 at 11:54:37PM +0200, Maxime Coquelin wrote: [...] > @@ -225,6 +231,15 @@ struct vring_desc_packed { > uint16_t index; > uint16_t flags; > }; > + > +#define VRING_EVENT_F_ENABLE 0x0 > +#define VRING_EVENT_F_DISABLE 0x1 > +#define VRING_EVENT_F_DESC 0x2 > + > +struct vring_packed_desc_event { > + uint16_t desc_event_off_wrap; > + uint16_t desc_event_flags; > +}; As all above types (including struct vring_desc_packed) and macros are being protected by VIRTIO_F_RING_PACKED, and they won't be defined if VIRTIO_F_RING_PACKED is defined in kernel header. We may want to unify the names. For the types, we may have below types defined in linux uapi: struct vring_packed; struct vring_packed_desc; struct vring_packed_desc_event; They can also be named as: struct vring_packed; struct vring_desc_packed; struct vring_packed_desc_event; We need to choose one of them or something else. For the `struct vring_packed_desc_event`, it can be defined as: struct vring_packed_desc_event { uint16_t off_wrap; uint16_t flags; }; or struct vring_packed_desc_event { uint16_t desc_event_off_wrap; uint16_t desc_event_flags; }; We need to choose one of them or something else. For the `struct vring_packed_desc`, it can be defined as: struct vring_packed_desc { uint64_t addr; uint32_t len; uint16_t index; uint16_t flags; }; or struct vring_packed_desc { uint64_t addr; uint32_t len; uint16_t id; // index -> id uint16_t flags; }; We need to choose one of them or something else. > #endif > [...] > +static __rte_always_inline void > +vhost_vring_call_packed(struct virtio_net *dev, struct vhost_virtqueue *vq) > +{ > + uint16_t old, new, off, off_wrap; > + bool kick = false; > + > + /* Flush used desc update. */ > + rte_smp_mb(); > + > + if (!(dev->features & (1ULL << VIRTIO_RING_F_EVENT_IDX))) { > + if (vq->driver_event->desc_event_flags != > + VRING_EVENT_F_DISABLE) > + kick = true; > + goto kick; > + } > + > + old = vq->signalled_used; We also need to check whether vq->signalled_used is valid? > + new = vq->last_used_idx; > + vq->signalled_used = new; > + > + if (vq->driver_event->desc_event_flags != VRING_EVENT_F_DESC) { > + if (vq->driver_event->desc_event_flags != > + VRING_EVENT_F_DISABLE) > + kick = true; > + goto kick; > + } > + > + rte_smp_rmb(); > + > + off_wrap = vq->driver_event->desc_event_off_wrap; > + off = off_wrap & ~(1 << 15); > + > + if (vq->used_wrap_counter != off_wrap >> 15) > + off -= vq->size; > + > + if (vhost_need_event(off, new, old)) > + kick = true; If new <= old, old needs to -= vq->size? > +kick: > + if (kick) > + eventfd_write(vq->callfd, (eventfd_t)1); > +} > + [...]