From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tetsuya Mukawa Subject: Re: [PATCH v2 0/7] virtio 1.0 enabling for virtio pmd driver Date: Mon, 18 Jan 2016 18:04:50 +0900 Message-ID: <569CAAB2.4070501@igel.co.jp> References: <1449719650-3482-1-git-send-email-yuanhan.liu@linux.intel.com> <1452581944-24838-1-git-send-email-yuanhan.liu@linux.intel.com> <569723B9.5080904@igel.co.jp> <56973B8E.6010808@intel.com> <56974315.2090106@igel.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, "Michael S. Tsirkin" To: "Tan, Jianfeng" Return-path: Received: from mail-pa0-f50.google.com (mail-pa0-f50.google.com [209.85.220.50]) by dpdk.org (Postfix) with ESMTP id 24E9ADE6 for ; Mon, 18 Jan 2016 10:04:53 +0100 (CET) Received: by mail-pa0-f50.google.com with SMTP id cy9so431182110pac.0 for ; Mon, 18 Jan 2016 01:04:53 -0800 (PST) In-Reply-To: <56974315.2090106@igel.co.jp> 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 2016/01/14 15:41, Tetsuya Mukawa wrote: > >>> 2. Abstraction of read/write accesses. >>> >>> It may be difficult to cleanly rebase my patches on this patches, >>> because virtio_read_caps() is not abstracted. >>> Let me describe it more. >>> So far, we need to handle below 3 virtio-net devices.. >>> - physical virtio-net device. >>> - virtual virtio-net device in virtio-net PMD. (Jianfeng's patch) >>> - virtual virtio-net device in QEMU. (my patch) >>> >>> Almost all code of the virtio-net PMD can be shared between above >>> different cases. >>> Probably big difference is how to access to configuration space. >>> >>> Yuanhan's patch introduces an abstraction layer to hide configuration >>> space layout and how to access it. >>> Is it possible to separate? >>> I guess "access method" will be nice to be abstracted separately from >>> "configuration space layout". >>> Probably access method will be defined by "eth_dev->dev_type" and the >>> PMD name like "eth_cvio". >>> And "configuration space layout" will be defined by capability list of >>> PCI configuration layout. >>> >>> For example, if access method like below are abstracted separately and >>> current "virtio_pci.c" is implemented on this abstraction, we can easily >>> re-use virtio_read_caps(). >>> - how to read/write virtio configuration space. >>> - how to mmap PCI configuration space. >>> - how to read/(write) PCI configuration space. >> >> I basically agree with you. We have two dimensions here: >> >> legacy modern >> physical virtio device: Use >> virtio_read_caps_phys() to distinguish >> virtual virtio device (Tetsuya): Use virtio_read_caps_virt() to >> distinguish >> virtual virtio device (Jianfeng): does not need a "configuration >> space layout", no need to distinguish >> >> So in vtpci_init(), we needs to test "eth_dev->dev_type" firstly >> >> vtpci_init() { >> if (eth_dev->dev_type == RTE_ETH_DEV_PCI) { >> if (virtio_read_caps_phys()) { >> // modern >> } else { >> // legacy >> } >> } else { >> if (Tetsuya's way) { >> if (virtio_read_caps_virt()) { >> // modern >> } else { >> // legacy >> } >> } else { >> // Jianfeng's way >> } >> } >> } >> >> And from Yuanhan's angle, I think he does not need to address this >> problem. How do you think? > Yes, I agree he doesn't need. > > Firstly, I have implemented like above, then I noticed that > virtio_read_caps_phy() and virtio_read_caps_virt() are same except for > access method. > Anyway, I guess abstracting access method is not so difficult. > If you are OK, I want to send RFC on Yuanhan's patch. Is it OK? Hi Jianfeng, I will submit patches to abstract pci access method. The patches will be on Yuanhan's latest virtio-1.0 patches. I guess your container patches also can be on the patches. Could you please check it? Thanks, Tetsuya