From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 669D8568D for ; Fri, 15 Jan 2016 07:26:01 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP; 14 Jan 2016 22:26:00 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,298,1449561600"; d="scan'208";a="881828944" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49]) by fmsmga001.fm.intel.com with ESMTP; 14 Jan 2016 22:25:58 -0800 Date: Fri, 15 Jan 2016 14:27:26 +0800 From: Yuanhan Liu To: Santosh Shukla Message-ID: <20160115062726.GS19531@yliu-dev.sh.intel.com> References: <1452778117-30178-1-git-send-email-sshukla@mvista.com> <1452778117-30178-9-git-send-email-sshukla@mvista.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1452778117-30178-9-git-send-email-sshukla@mvista.com> User-Agent: Mutt/1.5.23 (2014-03-12) 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 06:26:01 -0000 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. > + */ > + 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. > +}; > + > 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 "?:". --yliu