From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f44.google.com (mail-pa0-f44.google.com [209.85.220.44]) by dpdk.org (Postfix) with ESMTP id 52F70952 for ; Fri, 15 Jan 2016 14:42:05 +0100 (CET) Received: by mail-pa0-f44.google.com with SMTP id cy9so395098822pac.0 for ; Fri, 15 Jan 2016 05:42:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mvista-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=xcxEIIB67St0kPMM2sOeHhKVbSn8GvfTaXkVpeJKutU=; b=Gb69088rAhjJGNYTz+nXGs8OvzB4tff7/8n6iWoR+0KoiGb0ZYo9dEWuWjlysvwQGF 2/wKFe7zLmMZ/+d9ki8Ci2Y68OSpZzphQ8+GgjEDYP4mrjWg3Bp7jlVX5UUF+rfVYQQj DjVH/ibS1G58RbJ0CNzeGD7GRyg1ZHtQbTxDlAWxMZrs3eVfe1jjfZpxXyPWhN1rIRnY bL7K5QYOLWQUHFZkG2Latz1f8xZSMPE6RLZrLKD1bw4JJzRwfXwli88ddyUgWBNXTnEO myv9sfqQKKP0CC0Q7gcbNtULmGbVTKxIOTqyZ4tpprCiVkYahK2FzbvG/dUdk3KlbSTU YoPQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=xcxEIIB67St0kPMM2sOeHhKVbSn8GvfTaXkVpeJKutU=; b=NQImuHfFgZcb8n91Ou0FVm4QAfhdIVln4ZG2Y99N4fNILtBOLKPf17TwZtvnkg1i3O 8DIvcfAlkkMjy4AhA2sWhuUt0dlMGWfVU5RvEHO3WO+ekiX98wOOYRf/qbWD2ETWcnbM GzVOzNHLIJ4Pj9stWrRc6BDar9X1EOkZv3W5FA0fZV8LRddMkEVRPurC/ODK8jzMhqlD rhzr4af6K3pxi5EnjSChQNotnaBhpR8DvL2KTVnfT/LIrm4Il3KMb3eyo49U9vIwE7oi +IC/X1GQHUJR7NASrzuaNk/BCcPGCEG8oTcNz2bl8wF3r82joALxLucGjmGnq2xmadwm 8YXg== X-Gm-Message-State: ALoCoQkjRYRpIU9mfNmPWO0hISdfArN+Xi8jRlI/2rFKR6H8Lhk7FxrRvy5VF8ztC0TMwhYsFpNKJDXf4XAosd4mfWq2hLlRSkZ4A432BYG6f7hg3K1avR0= MIME-Version: 1.0 X-Received: by 10.66.161.227 with SMTP id xv3mr15002205pab.117.1452865324712; Fri, 15 Jan 2016 05:42:04 -0800 (PST) Received: by 10.66.196.81 with HTTP; Fri, 15 Jan 2016 05:42:04 -0800 (PST) In-Reply-To: References: <1452778117-30178-1-git-send-email-sshukla@mvista.com> <1452778117-30178-9-git-send-email-sshukla@mvista.com> <20160115062726.GS19531@yliu-dev.sh.intel.com> Date: Fri, 15 Jan 2016 19:12:04 +0530 Message-ID: From: Santosh Shukla To: Yuanhan Liu Content-Type: text/plain; charset=UTF-8 Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v4 08/14] virtio: pci: extend virtio pci rw api for vfio interface 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: Fri, 15 Jan 2016 13:42:05 -0000 On Fri, Jan 15, 2016 at 6:13 PM, Santosh Shukla wrote: > On Fri, Jan 15, 2016 at 11:57 AM, Yuanhan Liu > wrote: >> On Thu, Jan 14, 2016 at 06:58:31PM +0530, Santosh Shukla wrote: >>> So far virtio handle rw access for uio / ioport interface, This patch to extend >>> the support for vfio interface. For that introducing private struct >>> virtio_vfio_dev{ >>> - is_vfio >>> - pci_dev >>> }; >>> Signed-off-by: Santosh Shukla >> ... >>> +/* For vfio only */ >>> +struct virtio_vfio_dev { >>> + bool is_vfio; /* True: vfio i/f, >>> + * False: not a vfio i/f >> >> Well, this is weird; you are adding a flag to tell whether it's a >> vfio device __inside__ a vfio struct. >> >> Back to the topic, this flag is not necessary to me: you can >> check the pci_dev->kdrv flag. >> > > yes, I'll replace is_vfio with pci_dev->kdrv. > >>> + */ >>> + struct rte_pci_device *pci_dev; /* vfio dev */ >> >> Note that I have already added this field into virtio_hw struct >> at my latest virtio 1.0 pmd patchset. >> >> While I told you before that you should not develop patches based >> on my patcheset, I guess you can do that now. Since it should be >> in good shape and close to be merged. > > Okay, Before rebasing my v5 patch on your 1.0 virtio patch, I like to > understand which qemu version support virtio 1.0 spec? Ignore, I figured out in other thread, qemu version >2.4, such as 2.4.1 or 2.5.0. >>> +}; >>> + >>> struct virtio_hw { >>> struct virtqueue *cvq; >>> uint32_t io_base; >>> @@ -176,6 +186,7 @@ struct virtio_hw { >>> uint8_t use_msix; >>> uint8_t started; >>> uint8_t mac_addr[ETHER_ADDR_LEN]; >>> + struct virtio_vfio_dev dev; >>> }; >>> >>> /* >>> @@ -231,20 +242,65 @@ outl_p(unsigned int data, unsigned int port) >>> #define VIRTIO_PCI_REG_ADDR(hw, reg) \ >>> (unsigned short)((hw)->io_base + (reg)) >>> >>> -#define VIRTIO_READ_REG_1(hw, reg) \ >>> - inb((VIRTIO_PCI_REG_ADDR((hw), (reg)))) >>> -#define VIRTIO_WRITE_REG_1(hw, reg, value) \ >>> - outb_p((unsigned char)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg)))) >>> - >>> -#define VIRTIO_READ_REG_2(hw, reg) \ >>> - inw((VIRTIO_PCI_REG_ADDR((hw), (reg)))) >>> -#define VIRTIO_WRITE_REG_2(hw, reg, value) \ >>> - outw_p((unsigned short)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg)))) >>> - >>> -#define VIRTIO_READ_REG_4(hw, reg) \ >>> - inl((VIRTIO_PCI_REG_ADDR((hw), (reg)))) >>> -#define VIRTIO_WRITE_REG_4(hw, reg, value) \ >>> - outl_p((unsigned int)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg)))) >>> +#define VIRTIO_READ_REG_1(hw, reg) \ >>> +({ \ >>> + uint8_t ret; \ >>> + struct virtio_vfio_dev *vdev; \ >>> + (vdev) = (&(hw)->dev); \ >>> + (((vdev)->is_vfio) ? \ >>> + (ioport_inb(((vdev)->pci_dev), reg, &ret)) : \ >>> + ((ret) = (inb((VIRTIO_PCI_REG_ADDR((hw), (reg))))))); \ >>> + ret; \ >>> +}) >> >> It becomes unreadable. I'd suggest to define them as iniline >> functions, and use "if .. else .." instead of "?:". >> Ok. >> --yliu