Re: [PATCH v4 08/14] virtio: pci: extend virtio pci rw api for vfio interface
From: Yuanhan Liu <hidden>
Date: 2016-01-15 06:26:01
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 <redacted>...
+/* For vfio only */
+struct virtio_vfio_dev {
+ bool is_vfio; /* True: vfio i/f,
+ * False: not a vfio i/fWell, 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.
quoted hunk ↗ jump to hunk
+}; + 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