From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id AF01B68F5 for ; Thu, 12 May 2016 04:07:12 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP; 11 May 2016 19:07:11 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,609,1455004800"; d="scan'208";a="964112442" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by fmsmga001.fm.intel.com with ESMTP; 11 May 2016 19:07:09 -0700 Date: Wed, 11 May 2016 19:12:08 -0700 From: Yuanhan Liu To: Jianfeng Tan Cc: dev@dpdk.org, Huawei Xie , 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 Message-ID: <20160512021208.GA17474@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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1461892716-19122-7-git-send-email-jianfeng.tan@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v4 6/8] virtio-user: add new virtual pci driver for virtio X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 May 2016 02:07:13 -0000 On Fri, Apr 29, 2016 at 01:18:34AM +0000, Jianfeng Tan wrote: > +static void > +vdev_read_dev_config(struct virtio_hw *hw, uint64_t offset, > + void *dst, int length) > +{ > + int i; > + struct virtio_user_hw *uhw = (struct virtio_user_hw *)hw->vdev_private; Unnecessary cast. > +static int > +vdev_setup_queue(struct virtio_hw *hw __rte_unused, struct virtqueue *vq) > +{ > + /* Changed to use virtual addr */ > + vq->vq_ring_mem = (phys_addr_t)vq->mz->addr; > + if (vq->virtio_net_hdr_mz) { > + vq->virtio_net_hdr_mem = > + (phys_addr_t)vq->virtio_net_hdr_mz->addr; > + /* Do it one more time after we reset virtio_net_hdr_mem */ > + vring_hdr_desc_init(vq); > + } > + vq->offset = offsetof(struct rte_mbuf, buf_addr); > + return 0; Here as last email said, you should not mix vq stuff. What's more, why do you invoke vring_hdr_desc_init() here? If it needs a special handling, do it in driver. The "setup_queue" method is actually for telling the device where desc, avail and used vring are located. Hence, the implementation could be simple: just log them. > + > +const struct virtio_pci_ops vdev_ops = { Note that this is the interface for the driver to talk to the device, we should put this file into upper layer then, in the driver. And let me make a summary, trying to make it clear: - We should not use any structures/functions from the virtio driver here, unless it's really a must. - It's allowed for driver to make *few* special handling for the virtio user device. And that's what the driver supposed to do: to handle different device variants. So, I think it's okay to export the virtio_user_device struct to driver and do all those kind of "fake pci" configration there. --yliu > + .read_dev_cfg = vdev_read_dev_config, > + .write_dev_cfg = vdev_write_dev_config, > + .reset = vdev_reset, > + .get_status = vdev_get_status, > + .set_status = vdev_set_status, > + .get_features = vdev_get_features, > + .set_features = vdev_set_features, > + .get_isr = vdev_get_isr, > + .set_config_irq = vdev_set_config_irq, > + .get_queue_num = vdev_get_queue_num, > + .setup_queue = vdev_setup_queue, > + .del_queue = vdev_del_queue, > + .notify_queue = vdev_notify_queue, > +}; > -- > 2.1.4