libqos: Give qvirtio_config_read*() consistent semantics
authorDavid Gibson <david@gibson.dropbear.id.au>
Thu, 20 Oct 2016 03:08:07 +0000 (14:08 +1100)
committerDavid Gibson <david@gibson.dropbear.id.au>
Thu, 27 Oct 2016 22:38:27 +0000 (09:38 +1100)
The 'addr' parameter to qvirtio_config_read*() doesn't have a consistent
meaning: when using the virtio-pci versions, it's a full PCI space address,
but for virtio-mmio, it's an offset from the device's base mmio address.

This means that the callers need to do different things to calculate the
addresses in the two cases, which rather defeats the purpose of function
pointer backends.

All the current users of these functions are using them to retrieve
variables from the device specific portion of the virtio config space.
So, this patch alters the semantics to always be an offset into that
device specific config area.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
tests/libqos/virtio-mmio.c
tests/libqos/virtio-pci.c
tests/virtio-9p-test.c
tests/virtio-blk-test.c
tests/virtio-scsi-test.c

index bced6803e61f5b92dc72e88e4e80677840daf991..7aa83833383bc1c40b0861884d7f6669c9af9b71 100644 (file)
 #include "libqos/malloc-generic.h"
 #include "standard-headers/linux/virtio_ring.h"
 
-static uint8_t qvirtio_mmio_config_readb(QVirtioDevice *d, uint64_t addr)
+static uint8_t qvirtio_mmio_config_readb(QVirtioDevice *d, uint64_t off)
 {
     QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
-    return readb(dev->addr + addr);
+    return readb(dev->addr + QVIRTIO_MMIO_DEVICE_SPECIFIC + off);
 }
 
-static uint16_t qvirtio_mmio_config_readw(QVirtioDevice *d, uint64_t addr)
+static uint16_t qvirtio_mmio_config_readw(QVirtioDevice *d, uint64_t off)
 {
     QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
-    return readw(dev->addr + addr);
+    return readw(dev->addr + QVIRTIO_MMIO_DEVICE_SPECIFIC + off);
 }
 
-static uint32_t qvirtio_mmio_config_readl(QVirtioDevice *d, uint64_t addr)
+static uint32_t qvirtio_mmio_config_readl(QVirtioDevice *d, uint64_t off)
 {
     QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
-    return readl(dev->addr + addr);
+    return readl(dev->addr + QVIRTIO_MMIO_DEVICE_SPECIFIC + off);
 }
 
-static uint64_t qvirtio_mmio_config_readq(QVirtioDevice *d, uint64_t addr)
+static uint64_t qvirtio_mmio_config_readq(QVirtioDevice *d, uint64_t off)
 {
     QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
-    return readq(dev->addr + addr);
+    return readq(dev->addr + QVIRTIO_MMIO_DEVICE_SPECIFIC + off);
 }
 
 static uint32_t qvirtio_mmio_get_features(QVirtioDevice *d)
index 7e60b3a427e8786573aede57143eb5246333c521..fa82132ab7d2ea84f0ab779d44ff7321ce2aba24 100644 (file)
@@ -62,10 +62,13 @@ static void qvirtio_pci_assign_device(QVirtioDevice *d, void *data)
     *vpcidev = (QVirtioPCIDevice *)d;
 }
 
-static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr)
+#define CONFIG_BASE(dev) \
+    ((dev)->addr + VIRTIO_PCI_CONFIG_OFF((dev)->pdev->msix_enabled))
+
+static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t off)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-    return qpci_io_readb(dev->pdev, (void *)(uintptr_t)addr);
+    return qpci_io_readb(dev->pdev, CONFIG_BASE(dev) + off);
 }
 
 /* PCI is always read in little-endian order
@@ -76,31 +79,31 @@ static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr)
  * case will be managed inside qvirtio_is_big_endian()
  */
 
-static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t addr)
+static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t off)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
     uint16_t value;
 
-    value = qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr);
+    value = qpci_io_readw(dev->pdev, CONFIG_BASE(dev) + off);
     if (qvirtio_is_big_endian(d)) {
         value = bswap16(value);
     }
     return value;
 }
 
-static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, uint64_t addr)
+static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, uint64_t off)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
     uint32_t value;
 
-    value = qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr);
+    value = qpci_io_readl(dev->pdev, CONFIG_BASE(dev) + off);
     if (qvirtio_is_big_endian(d)) {
         value = bswap32(value);
     }
     return value;
 }
 
-static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr)
+static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t off)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
     int i;
@@ -108,13 +111,13 @@ static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr)
 
     if (qvirtio_is_big_endian(d)) {
         for (i = 0; i < 8; ++i) {
-            u64 |= (uint64_t)qpci_io_readb(dev->pdev,
-                                (void *)(uintptr_t)addr + i) << (7 - i) * 8;
+            u64 |= (uint64_t)qpci_io_readb(dev->pdev, CONFIG_BASE(dev)
+                                           + off + i) << (7 - i) * 8;
         }
     } else {
         for (i = 0; i < 8; ++i) {
-            u64 |= (uint64_t)qpci_io_readb(dev->pdev,
-                                (void *)(uintptr_t)addr + i) << i * 8;
+            u64 |= (uint64_t)qpci_io_readb(dev->pdev, CONFIG_BASE(dev)
+                                           + off + i) << i * 8;
         }
     }
 
index 693920af5f3cfdf66e38f5aa6fac3d41ab68d919..9c4f6cb406475d9867229d7075659916e0a2d5f0 100644 (file)
@@ -95,7 +95,6 @@ static void qvirtio_9p_pci_free(QVirtIO9P *v9p)
 static void pci_basic_config(void)
 {
     QVirtIO9P *v9p;
-    void *addr;
     size_t tag_len;
     char *tag;
     int i;
@@ -104,15 +103,12 @@ static void pci_basic_config(void)
     qs = qvirtio_9p_start();
     v9p = qvirtio_9p_pci_init(qs);
 
-    addr = ((QVirtioPCIDevice *) v9p->dev)->addr + VIRTIO_PCI_CONFIG_OFF(false);
-    tag_len = qvirtio_config_readw(v9p->dev,
-                                   (uint64_t)(uintptr_t)addr);
+    tag_len = qvirtio_config_readw(v9p->dev, 0);
     g_assert_cmpint(tag_len, ==, strlen(mount_tag));
-    addr += sizeof(uint16_t);
 
     tag = g_malloc(tag_len);
     for (i = 0; i < tag_len; i++) {
-        tag[i] = qvirtio_config_readb(v9p->dev, (uint64_t)(uintptr_t)addr + i);
+        tag[i] = qvirtio_config_readb(v9p->dev, i + 2);
     }
     g_assert_cmpmem(tag, tag_len, mount_tag, tag_len);
     g_free(tag);
index f737c406e351b30235462a110d1389358a8ec1cc..0e32e416dd190e884b612a594005d89fc0e218f7 100644 (file)
@@ -155,7 +155,7 @@ static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d,
 }
 
 static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
-                       QVirtQueue *vq, uint64_t device_specific)
+                       QVirtQueue *vq)
 {
     QVirtioBlkReq req;
     uint64_t req_addr;
@@ -165,7 +165,7 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
     uint8_t status;
     char *data;
 
-    capacity = qvirtio_config_readq(dev, device_specific);
+    capacity = qvirtio_config_readq(dev, 0);
 
     g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
 
@@ -285,17 +285,13 @@ static void pci_basic(void)
     QVirtioPCIDevice *dev;
     QOSState *qs;
     QVirtQueuePCI *vqpci;
-    void *addr;
 
     qs = pci_test_start();
     dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
 
     vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&dev->vdev, qs->alloc, 0);
 
-    /* MSI-X is not enabled */
-    addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
-
-    test_basic(&dev->vdev, qs->alloc, &vqpci->vq, (uint64_t)(uintptr_t)addr);
+    test_basic(&dev->vdev, qs->alloc, &vqpci->vq);
 
     /* End test */
     qvirtqueue_cleanup(dev->vdev.bus, &vqpci->vq, qs->alloc);
@@ -311,7 +307,6 @@ static void pci_indirect(void)
     QOSState *qs;
     QVirtioBlkReq req;
     QVRingIndirectDesc *indirect;
-    void *addr;
     uint64_t req_addr;
     uint64_t capacity;
     uint32_t features;
@@ -323,10 +318,7 @@ static void pci_indirect(void)
 
     dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
 
-    /* MSI-X is not enabled */
-    addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
-
-    capacity = qvirtio_config_readq(&dev->vdev, (uint64_t)(uintptr_t)addr);
+    capacity = qvirtio_config_readq(&dev->vdev, 0);
     g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
 
     features = qvirtio_get_features(&dev->vdev);
@@ -406,17 +398,13 @@ static void pci_config(void)
     QVirtioPCIDevice *dev;
     QOSState *qs;
     int n_size = TEST_IMAGE_SIZE / 2;
-    void *addr;
     uint64_t capacity;
 
     qs = pci_test_start();
 
     dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
 
-    /* MSI-X is not enabled */
-    addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
-
-    capacity = qvirtio_config_readq(&dev->vdev, (uint64_t)(uintptr_t)addr);
+    capacity = qvirtio_config_readq(&dev->vdev, 0);
     g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
 
     qvirtio_set_driver_ok(&dev->vdev);
@@ -425,7 +413,7 @@ static void pci_config(void)
                                                     " 'size': %d } }", n_size);
     qvirtio_wait_config_isr(&dev->vdev, QVIRTIO_BLK_TIMEOUT_US);
 
-    capacity = qvirtio_config_readq(&dev->vdev, (uint64_t)(uintptr_t)addr);
+    capacity = qvirtio_config_readq(&dev->vdev, 0);
     g_assert_cmpint(capacity, ==, n_size / 512);
 
     qvirtio_pci_device_disable(dev);
@@ -441,7 +429,6 @@ static void pci_msix(void)
     QVirtQueuePCI *vqpci;
     QVirtioBlkReq req;
     int n_size = TEST_IMAGE_SIZE / 2;
-    void *addr;
     uint64_t req_addr;
     uint64_t capacity;
     uint32_t features;
@@ -456,10 +443,7 @@ static void pci_msix(void)
 
     qvirtio_pci_set_msix_configuration_vector(dev, qs->alloc, 0);
 
-    /* MSI-X is enabled */
-    addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(true);
-
-    capacity = qvirtio_config_readq(&dev->vdev, (uint64_t)(uintptr_t)addr);
+    capacity = qvirtio_config_readq(&dev->vdev, 0);
     g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
 
     features = qvirtio_get_features(&dev->vdev);
@@ -479,7 +463,7 @@ static void pci_msix(void)
 
     qvirtio_wait_config_isr(&dev->vdev, QVIRTIO_BLK_TIMEOUT_US);
 
-    capacity = qvirtio_config_readq(&dev->vdev, (uintptr_t)addr);
+    capacity = qvirtio_config_readq(&dev->vdev, 0);
     g_assert_cmpint(capacity, ==, n_size / 512);
 
     /* Write request */
@@ -550,7 +534,6 @@ static void pci_idx(void)
     QOSState *qs;
     QVirtQueuePCI *vqpci;
     QVirtioBlkReq req;
-    void *addr;
     uint64_t req_addr;
     uint64_t capacity;
     uint32_t features;
@@ -565,10 +548,7 @@ static void pci_idx(void)
 
     qvirtio_pci_set_msix_configuration_vector(dev, qs->alloc, 0);
 
-    /* MSI-X is enabled */
-    addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(true);
-
-    capacity = qvirtio_config_readq(&dev->vdev, (uint64_t)(uintptr_t)addr);
+    capacity = qvirtio_config_readq(&dev->vdev, 0);
     g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
 
     features = qvirtio_get_features(&dev->vdev);
@@ -709,14 +689,14 @@ static void mmio_basic(void)
     alloc = generic_alloc_init(MMIO_RAM_ADDR, MMIO_RAM_SIZE, MMIO_PAGE_SIZE);
     vq = qvirtqueue_setup(&dev->vdev, alloc, 0);
 
-    test_basic(&dev->vdev, alloc, vq, QVIRTIO_MMIO_DEVICE_SPECIFIC);
+    test_basic(&dev->vdev, alloc, vq);
 
     qmp("{ 'execute': 'block_resize', 'arguments': { 'device': 'drive0', "
                                                     " 'size': %d } }", n_size);
 
     qvirtio_wait_queue_isr(&dev->vdev, vq, QVIRTIO_BLK_TIMEOUT_US);
 
-    capacity = qvirtio_config_readq(&dev->vdev, QVIRTIO_MMIO_DEVICE_SPECIFIC);
+    capacity = qvirtio_config_readq(&dev->vdev, 0);
     g_assert_cmpint(capacity, ==, n_size / 512);
 
     /* End test */
index 60dc9abd60493e8cb6b72d837246224c732ba450..69220ef07b9e9e023c0fefec0822bd046160e2b3 100644 (file)
@@ -143,7 +143,6 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
     QVirtIOSCSI *vs;
     QVirtioPCIDevice *dev;
     struct virtio_scsi_cmd_resp resp;
-    void *addr;
     int i;
 
     vs = g_new0(QVirtIOSCSI, 1);
@@ -161,8 +160,7 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
     qvirtio_set_acknowledge(vs->dev);
     qvirtio_set_driver(vs->dev);
 
-    addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
-    vs->num_queues = qvirtio_config_readl(vs->dev, (uint64_t)(uintptr_t)addr);
+    vs->num_queues = qvirtio_config_readl(vs->dev, 0);
 
     g_assert_cmpint(vs->num_queues, <, MAX_NUM_QUEUES);