From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yuanhan Liu Subject: Re: [PATCH v4 6/8] virtio-user: add new virtual pci driver for virtio Date: Mon, 16 May 2016 10:51:51 +0800 Message-ID: <20160516025151.GC5641@yliu-dev.sh.intel.com> References: <1461892716-19122-1-git-send-email-jianfeng.tan@intel.com> <1461892716-19122-7-git-send-email-jianfeng.tan@intel.com> <20160512021208.GA17474@yliu-dev.sh.intel.com> <20160512164015.GA5641@yliu-dev.sh.intel.com> <20160513044518.GB5641@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "dev@dpdk.org" , "Xie, Huawei" , "rich.lane@bigswitch.com" , "mst@redhat.com" , "nakajima.yoshihiro@lab.ntt.co.jp" , "p.fedin@samsung.com" , "ann.zhuangyanying@huawei.com" , "mukawa@igel.co.jp" , "nhorman@tuxdriver.com" To: "Tan, Jianfeng" Return-path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id A9B9A95DC for ; Mon, 16 May 2016 04:51:49 +0200 (CEST) Content-Disposition: inline In-Reply-To: List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Mon, May 16, 2016 at 01:48:01AM +0000, Tan, Jianfeng wrote: > > On Fri, May 13, 2016 at 09:54:33AM +0800, Tan, Jianfeng wrote: > > > > > > So, I'd suggest something like following: > > > > > > if (is_vdev(..)) { > > > > > > > > > The blocker issue of your suggestion is that we have no such condition. > > > > > > Previously, I use dev_type, but as David's comment said: > > > > That's not the only option. There should be others, for example, > > checking the existence of virtio_user_device. Or even, you could > > add a new flag inside virtio hw while initiating your vdev. > > > > > May I ask how many more such handling are needed, excluding the tx > > queue > > > header desc setup? And as stated, in generic, yes, we should try that. > > > > > > > > > Those which need special handling: > > > (1) vq->vq_ring_mem: it is set but never used, so it's out of question. > > > (2) vq->virtio_net_hdr_mem and vring_hdr_desc_init > > > > vring_hdr_desc_init is common. > > > > > (3) vq->offset > > > > > > Just (2) and (3) so far. And the question is quite clear: where to put these > > > two special handling. > > > > Apparently, you can't put it into the queue_setup(). And I still think > > my proposal works great here. > > OK, since it's indeed inappropriate to put these special handlings inside queue_setup() from semantic perspective, I'll add them according to if hw->vdev_private == NULL in the driver. I'm thinking maybe we could rename "vdev_private" to "virtio_user_dev" (or something like that), to make it explicit that it's for virtio user device. I mean, there should be no other vdevs after all. OTOH, using "hw->vdev_private != NULL" to say it's a virtio-user device is a bit weird; it doesn't even make too much sense. --yliu