From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f175.google.com (mail-pf0-f175.google.com [209.85.192.175]) by dpdk.org (Postfix) with ESMTP id 0EB9E568D for ; Fri, 15 Jan 2016 13:43:52 +0100 (CET) Received: by mail-pf0-f175.google.com with SMTP id q63so120871389pfb.1 for ; Fri, 15 Jan 2016 04:43:51 -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=GJL+q7vC5Ov3fN8kSW2Dy/0rckEBjC1vdOj7ema0/z8=; b=aijjv19LR129CKwKlyPDeF54vHUC59EOkk9MMOIK9viLjCcvF169aWEoP8aaY/EzPC LepZABL0Wpj6r4C4yKzmL4B9jjrkvxWYpqIr/rENTOb7CGbI+7e0Dz9b2d0uznZKfy7X gvptJUzy8xZhVqGp1hAUEhRiubTfY9LISYoCAJ16GjKB7zxkOf5mX62EMxQIpvH7wJZt Ujy9wR1C1uzFixS4hQkRd+zdjlsAowS5RuBjfrrj2anM8rahpCo7CaWbfZw3VVcKdjuJ 0Qh7vFpaQqyepzQaS+xvh96O6+NXC/fS7gwMkIJR1v8hYK8r1U5X/9eeLFeh/Z1DCDfk 37Xg== 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=GJL+q7vC5Ov3fN8kSW2Dy/0rckEBjC1vdOj7ema0/z8=; b=mXf1ceGY3VwDEJRJDmMvCJ7DChtryU/15Y1AHf8lp9boXZ8xDI3DtsGb4ZqEdU85uK FpU5Cnmyc68Ij3Lt/UXvJqFQQBLmki30Mu5JFkR5s0KQgrL+1Q2gi8pEoMhEclUoY3RF iUwjw8tw6x1JTmtjk0cQeKyjn2ulEt7480rn5AWwPKAil10w4f6VR+5GoJIpY9hMgMTc hvKdC9efGEihSp09UQMv/KzYKAbCqHMVfpZ6Dx/IsXG959xPPrO0lZQfwaUUKbBXIgVI GSGwxaxLv0DL6CfgqTqBTyxb2VpmOq4Z6GEzJXjYmXV5DN8Oa0YbrEYkQOmO2rnVbLQd g0Nw== X-Gm-Message-State: ALoCoQlJGLZtM359wuYpBJZhR0/fwcXyhlud0NGKD6h0B1hP5wB3xe5sk5dMzlTU0UmdPPIQ1Ku+5HORD4R2mn2sGwx9p9PU6c7cUw3N14uV4qbAAM4vGNs= MIME-Version: 1.0 X-Received: by 10.98.12.131 with SMTP id 3mr14760637pfm.155.1452861831305; Fri, 15 Jan 2016 04:43:51 -0800 (PST) Received: by 10.66.196.81 with HTTP; Fri, 15 Jan 2016 04:43:51 -0800 (PST) In-Reply-To: <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> <20160115062726.GS19531@yliu-dev.sh.intel.com> Date: Fri, 15 Jan 2016 18:13:51 +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 12:43:52 -0000 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? > >> +}; >> + >> 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