qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support
@ 2019-10-23 10:04 Stefan Hajnoczi
  2019-10-23 10:04 ` [PATCH v4 01/16] tests/virtio-blk-test: read config space after feature negotiation Stefan Hajnoczi
                   ` (16 more replies)
  0 siblings, 17 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini, Richard Henderson

v4:
 * Introduce bool d->features_negotiated so that tests can negotiate a
   0 feature bit set in Legacy mode [Thomas]
 * Make the FEATURES_OK code change in qvirtio_set_driver_ok() clearer and
   mention it in the commit description [Thomas]
 * Fix indentation in qvring_init() [Thomas]
v3:
 * Now implements VIRTIO 1.0 fully (vring, device initialization).
   This required several new patches to address the following issues:
   1. Tests that do not negotiate features (non-compliant!)
   2. Tests that access configuration space before feature negotiation
      (non-compliant!)
   3. Tests that set up virtqueues before feature negotiation (non-compliant!)
   4. vring accesses require byte-swapping when VIRTIO 1.0 is used with a
      big-endian guest because the qtest_readX/writeX() API automatically
      converts to guest-endian
   5. VIRTIO 1.0 has an additional FEATURES_OK step during Device
      Initialization
 * Change uint8_t bar_idx to int [Thomas]
 * Document qpci_find_capability() [Thomas]
 * Every commit tested with arm, ppc64, and x86_64 targets using:
   git rebase -i origin/master -x 'make tests/qos-test &&
   QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 tests/qos-test &&
   QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/qos-test'
   QTEST_QEMU_BINARY=arm-softmmu/qemu-system-arm tests/qos-test'
v2:
 * Fix checkpatch.pl issues, except MAINTAINERS file warning.  libqos already
   has maintainers and the new virtio-pci-modern.[ch] files don't need extra
   entries since they are already covered by the existing libqos/ entry.

New VIRTIO devices are Non-Transitional.  This means they only expose the
VIRTIO 1.0 interface, not the Legacy interface.

The libqos only supports Legacy and Transitional devices (in Legacy mode).
This patch series adds VIRTIO 1.0 support so that tests can run against
Non-Transitional devices too.

The virtio-fs device is Non-Transitional, so this is a prerequisite for
virtio-fs qos tests.

Stefan Hajnoczi (16):
  tests/virtio-blk-test: read config space after feature negotiation
  libqos: read QVIRTIO_MMIO_VERSION register
  libqos: extend feature bits to 64-bit
  virtio-scsi-test: add missing feature negotiation
  tests/virtio-blk-test: set up virtqueue after feature negotiation
  libqos: add missing virtio-9p feature negotiation
  libqos: enforce Device Initialization order
  libqos: implement VIRTIO 1.0 FEATURES_OK step
  libqos: access VIRTIO 1.0 vring in little-endian
  libqos: add iteration support to qpci_find_capability()
  libqos: pass full QVirtQueue to set_queue_address()
  libqos: add MSI-X callbacks to QVirtioPCIDevice
  libqos: expose common virtqueue setup/cleanup functions
  libqos: make the virtio-pci BAR index configurable
  libqos: extract Legacy virtio-pci.c code
  libqos: add VIRTIO PCI 1.0 support

 tests/Makefile.include           |   1 +
 tests/libqos/pci.h               |   2 +-
 tests/libqos/virtio-mmio.h       |   1 +
 tests/libqos/virtio-pci-modern.h |  17 ++
 tests/libqos/virtio-pci.h        |  34 ++-
 tests/libqos/virtio.h            |  19 +-
 tests/libqos/pci.c               |  30 ++-
 tests/libqos/virtio-9p.c         |   6 +
 tests/libqos/virtio-mmio.c       |  38 ++-
 tests/libqos/virtio-net.c        |   6 +-
 tests/libqos/virtio-pci-modern.c | 443 +++++++++++++++++++++++++++++++
 tests/libqos/virtio-pci.c        | 105 +++++---
 tests/libqos/virtio.c            | 160 ++++++++---
 tests/virtio-blk-test.c          |  66 +++--
 tests/virtio-scsi-test.c         |   8 +
 15 files changed, 802 insertions(+), 134 deletions(-)
 create mode 100644 tests/libqos/virtio-pci-modern.h
 create mode 100644 tests/libqos/virtio-pci-modern.c

-- 
2.21.0



^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v4 01/16] tests/virtio-blk-test: read config space after feature negotiation
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 10:04 ` [PATCH v4 02/16] libqos: read QVIRTIO_MMIO_VERSION register Stefan Hajnoczi
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini, Richard Henderson

The VIRTIO Configuration Space cannot be accessed before device feature
bits have been read because a driver doesn't know the endianness until
it has checked VIRTIO_F_VERSION_1.

Fix this problem in preparation for VIRTIO 1.0 support.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 tests/virtio-blk-test.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index ed13167392..f6674fb233 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -125,10 +125,6 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
     char *data;
     QTestState *qts = global_qtest;
 
-    capacity = qvirtio_config_readq(dev, 0);
-
-    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
-
     features = qvirtio_get_features(dev);
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
                     (1u << VIRTIO_RING_F_INDIRECT_DESC) |
@@ -136,6 +132,9 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
                     (1u << VIRTIO_BLK_F_SCSI));
     qvirtio_set_features(dev, features);
 
+    capacity = qvirtio_config_readq(dev, 0);
+    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
+
     qvirtio_set_driver_ok(dev);
 
     /* Write and read with 3 descriptor layout */
@@ -359,9 +358,6 @@ static void indirect(void *obj, void *u_data, QGuestAllocator *t_alloc)
     char *data;
     QTestState *qts = global_qtest;
 
-    capacity = qvirtio_config_readq(dev, 0);
-    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
-
     features = qvirtio_get_features(dev);
     g_assert_cmphex(features & (1u << VIRTIO_RING_F_INDIRECT_DESC), !=, 0);
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
@@ -369,6 +365,9 @@ static void indirect(void *obj, void *u_data, QGuestAllocator *t_alloc)
                             (1u << VIRTIO_BLK_F_SCSI));
     qvirtio_set_features(dev, features);
 
+    capacity = qvirtio_config_readq(dev, 0);
+    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
+
     vq = qvirtqueue_setup(dev, t_alloc, 0);
     qvirtio_set_driver_ok(dev);
 
@@ -434,8 +433,16 @@ static void config(void *obj, void *data, QGuestAllocator *t_alloc)
     QVirtioBlk *blk_if = obj;
     QVirtioDevice *dev = blk_if->vdev;
     int n_size = TEST_IMAGE_SIZE / 2;
+    uint64_t features;
     uint64_t capacity;
 
+    features = qvirtio_get_features(dev);
+    features = features & ~(QVIRTIO_F_BAD_FEATURE |
+                            (1u << VIRTIO_RING_F_INDIRECT_DESC) |
+                            (1u << VIRTIO_RING_F_EVENT_IDX) |
+                            (1u << VIRTIO_BLK_F_SCSI));
+    qvirtio_set_features(dev, features);
+
     capacity = qvirtio_config_readq(dev, 0);
     g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
 
@@ -475,9 +482,6 @@ static void msix(void *obj, void *u_data, QGuestAllocator *t_alloc)
     qpci_msix_enable(pdev->pdev);
     qvirtio_pci_set_msix_configuration_vector(pdev, t_alloc, 0);
 
-    capacity = qvirtio_config_readq(dev, 0);
-    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
-
     features = qvirtio_get_features(dev);
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
                             (1u << VIRTIO_RING_F_INDIRECT_DESC) |
@@ -485,6 +489,9 @@ static void msix(void *obj, void *u_data, QGuestAllocator *t_alloc)
                             (1u << VIRTIO_BLK_F_SCSI));
     qvirtio_set_features(dev, features);
 
+    capacity = qvirtio_config_readq(dev, 0);
+    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
+
     vq = qvirtqueue_setup(dev, t_alloc, 0);
     qvirtqueue_pci_msix_setup(pdev, (QVirtQueuePCI *)vq, t_alloc, 1);
 
@@ -584,9 +591,6 @@ static void idx(void *obj, void *u_data, QGuestAllocator *t_alloc)
     qpci_msix_enable(pdev->pdev);
     qvirtio_pci_set_msix_configuration_vector(pdev, t_alloc, 0);
 
-    capacity = qvirtio_config_readq(dev, 0);
-    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
-
     features = qvirtio_get_features(dev);
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
                             (1u << VIRTIO_RING_F_INDIRECT_DESC) |
@@ -594,6 +598,9 @@ static void idx(void *obj, void *u_data, QGuestAllocator *t_alloc)
                             (1u << VIRTIO_BLK_F_SCSI));
     qvirtio_set_features(dev, features);
 
+    capacity = qvirtio_config_readq(dev, 0);
+    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
+
     vq = qvirtqueue_setup(dev, t_alloc, 0);
     qvirtqueue_pci_msix_setup(pdev, (QVirtQueuePCI *)vq, t_alloc, 1);
 
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 02/16] libqos: read QVIRTIO_MMIO_VERSION register
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
  2019-10-23 10:04 ` [PATCH v4 01/16] tests/virtio-blk-test: read config space after feature negotiation Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 10:04 ` [PATCH v4 03/16] libqos: extend feature bits to 64-bit Stefan Hajnoczi
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini, Richard Henderson

There was no real virtio-mmio ABI change between Legacy and VIRTIO 1.0
except that the Version field was incremented from 1 to 2.

However, QEMU does not allow Legacy drivers to perform VIRTIO 1.0
operations like accessing 64-bit feature bits.  Since we will introduce
64-bit feature bit support we need a way to differentiate between
virtio-mmio Version 1 and 2 to avoid upsetting QEMU when we operate in
Legacy mode.

Stash away the Version field so later patches can change behavior
depending on the version.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 tests/libqos/virtio-mmio.h | 1 +
 tests/libqos/virtio-mmio.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/tests/libqos/virtio-mmio.h b/tests/libqos/virtio-mmio.h
index 17a17141c3..0e45778b07 100644
--- a/tests/libqos/virtio-mmio.h
+++ b/tests/libqos/virtio-mmio.h
@@ -40,6 +40,7 @@ typedef struct QVirtioMMIODevice {
     uint64_t addr;
     uint32_t page_size;
     uint32_t features; /* As it cannot be read later, save it */
+    uint32_t version;
 } QVirtioMMIODevice;
 
 extern const QVirtioBus qvirtio_mmio;
diff --git a/tests/libqos/virtio-mmio.c b/tests/libqos/virtio-mmio.c
index d0047876a8..7154b03c1d 100644
--- a/tests/libqos/virtio-mmio.c
+++ b/tests/libqos/virtio-mmio.c
@@ -223,6 +223,9 @@ void qvirtio_mmio_init_device(QVirtioMMIODevice *dev, QTestState *qts,
     magic = qtest_readl(qts, addr + QVIRTIO_MMIO_MAGIC_VALUE);
     g_assert(magic == ('v' | 'i' << 8 | 'r' << 16 | 't' << 24));
 
+    dev->version = qtest_readl(qts, addr + QVIRTIO_MMIO_VERSION);
+    g_assert(dev->version == 1 || dev->version == 2);
+
     dev->qts = qts;
     dev->addr = addr;
     dev->page_size = page_size;
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 03/16] libqos: extend feature bits to 64-bit
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
  2019-10-23 10:04 ` [PATCH v4 01/16] tests/virtio-blk-test: read config space after feature negotiation Stefan Hajnoczi
  2019-10-23 10:04 ` [PATCH v4 02/16] libqos: read QVIRTIO_MMIO_VERSION register Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 10:04 ` [PATCH v4 04/16] virtio-scsi-test: add missing feature negotiation Stefan Hajnoczi
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini,
	Philippe Mathieu-Daudé,
	Richard Henderson

In VIRTIO 1.0 feature bits changed from 32-bit to 64-bit.  (In fact, the
transports allow even more feature bits but nothing uses more than 64
bits today.)

Add 64-bit feature bit support to virtio-mmio and virtio-pci.  This will
be necessary for VIRTIO 1.0 support.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/libqos/virtio.h      | 12 ++++++------
 tests/libqos/virtio-mmio.c | 28 ++++++++++++++++++++++------
 tests/libqos/virtio-net.c  |  6 +++---
 tests/libqos/virtio-pci.c  | 12 ++++++------
 tests/libqos/virtio.c      |  4 ++--
 tests/virtio-blk-test.c    |  8 ++++----
 6 files changed, 43 insertions(+), 27 deletions(-)

diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 2cb2448f46..a5c99fb3c9 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -13,7 +13,7 @@
 #include "libqos/malloc.h"
 #include "standard-headers/linux/virtio_ring.h"
 
-#define QVIRTIO_F_BAD_FEATURE           0x40000000
+#define QVIRTIO_F_BAD_FEATURE           0x40000000ull
 
 typedef struct QVirtioBus QVirtioBus;
 
@@ -52,13 +52,13 @@ struct QVirtioBus {
     uint64_t (*config_readq)(QVirtioDevice *d, uint64_t addr);
 
     /* Get features of the device */
-    uint32_t (*get_features)(QVirtioDevice *d);
+    uint64_t (*get_features)(QVirtioDevice *d);
 
     /* Set features of the device */
-    void (*set_features)(QVirtioDevice *d, uint32_t features);
+    void (*set_features)(QVirtioDevice *d, uint64_t features);
 
     /* Get features of the guest */
-    uint32_t (*get_guest_features)(QVirtioDevice *d);
+    uint64_t (*get_guest_features)(QVirtioDevice *d);
 
     /* Get status of the device */
     uint8_t (*get_status)(QVirtioDevice *d);
@@ -103,8 +103,8 @@ uint8_t qvirtio_config_readb(QVirtioDevice *d, uint64_t addr);
 uint16_t qvirtio_config_readw(QVirtioDevice *d, uint64_t addr);
 uint32_t qvirtio_config_readl(QVirtioDevice *d, uint64_t addr);
 uint64_t qvirtio_config_readq(QVirtioDevice *d, uint64_t addr);
-uint32_t qvirtio_get_features(QVirtioDevice *d);
-void qvirtio_set_features(QVirtioDevice *d, uint32_t features);
+uint64_t qvirtio_get_features(QVirtioDevice *d);
+void qvirtio_set_features(QVirtioDevice *d, uint64_t features);
 bool qvirtio_is_big_endian(QVirtioDevice *d);
 
 void qvirtio_reset(QVirtioDevice *d);
diff --git a/tests/libqos/virtio-mmio.c b/tests/libqos/virtio-mmio.c
index 7154b03c1d..78066e6e05 100644
--- a/tests/libqos/virtio-mmio.c
+++ b/tests/libqos/virtio-mmio.c
@@ -40,22 +40,38 @@ static uint64_t qvirtio_mmio_config_readq(QVirtioDevice *d, uint64_t off)
     return qtest_readq(dev->qts, dev->addr + QVIRTIO_MMIO_DEVICE_SPECIFIC + off);
 }
 
-static uint32_t qvirtio_mmio_get_features(QVirtioDevice *d)
+static uint64_t qvirtio_mmio_get_features(QVirtioDevice *d)
 {
     QVirtioMMIODevice *dev = container_of(d, QVirtioMMIODevice, vdev);
+    uint64_t lo;
+    uint64_t hi = 0;
+
     qtest_writel(dev->qts, dev->addr + QVIRTIO_MMIO_HOST_FEATURES_SEL, 0);
-    return qtest_readl(dev->qts, dev->addr + QVIRTIO_MMIO_HOST_FEATURES);
+    lo = qtest_readl(dev->qts, dev->addr + QVIRTIO_MMIO_HOST_FEATURES);
+
+    if (dev->version >= 2) {
+        qtest_writel(dev->qts, dev->addr + QVIRTIO_MMIO_HOST_FEATURES_SEL, 1);
+        hi = qtest_readl(dev->qts, dev->addr + QVIRTIO_MMIO_HOST_FEATURES);
+    }
+
+    return (hi << 32) | lo;
 }
 
-static void qvirtio_mmio_set_features(QVirtioDevice *d, uint32_t features)
+static void qvirtio_mmio_set_features(QVirtioDevice *d, uint64_t features)
 {
     QVirtioMMIODevice *dev = container_of(d, QVirtioMMIODevice, vdev);
     dev->features = features;
     qtest_writel(dev->qts, dev->addr + QVIRTIO_MMIO_GUEST_FEATURES_SEL, 0);
     qtest_writel(dev->qts, dev->addr + QVIRTIO_MMIO_GUEST_FEATURES, features);
+
+    if (dev->version >= 2) {
+        qtest_writel(dev->qts, dev->addr + QVIRTIO_MMIO_GUEST_FEATURES_SEL, 1);
+        qtest_writel(dev->qts, dev->addr + QVIRTIO_MMIO_GUEST_FEATURES,
+                     features >> 32);
+    }
 }
 
-static uint32_t qvirtio_mmio_get_guest_features(QVirtioDevice *d)
+static uint64_t qvirtio_mmio_get_guest_features(QVirtioDevice *d)
 {
     QVirtioMMIODevice *dev = container_of(d, QVirtioMMIODevice, vdev);
     return dev->features;
@@ -149,8 +165,8 @@ static QVirtQueue *qvirtio_mmio_virtqueue_setup(QVirtioDevice *d,
     vq->free_head = 0;
     vq->num_free = vq->size;
     vq->align = dev->page_size;
-    vq->indirect = (dev->features & (1u << VIRTIO_RING_F_INDIRECT_DESC)) != 0;
-    vq->event = (dev->features & (1u << VIRTIO_RING_F_EVENT_IDX)) != 0;
+    vq->indirect = dev->features & (1ull << VIRTIO_RING_F_INDIRECT_DESC);
+    vq->event = dev->features & (1ull << VIRTIO_RING_F_EVENT_IDX);
 
     qtest_writel(dev->qts, dev->addr + QVIRTIO_MMIO_QUEUE_NUM, vq->size);
 
diff --git a/tests/libqos/virtio-net.c b/tests/libqos/virtio-net.c
index 6567beb553..710d440c3d 100644
--- a/tests/libqos/virtio-net.c
+++ b/tests/libqos/virtio-net.c
@@ -44,11 +44,11 @@ static void virtio_net_setup(QVirtioNet *interface)
 
     features = qvirtio_get_features(vdev);
     features &= ~(QVIRTIO_F_BAD_FEATURE |
-                  (1u << VIRTIO_RING_F_INDIRECT_DESC) |
-                  (1u << VIRTIO_RING_F_EVENT_IDX));
+                  (1ull << VIRTIO_RING_F_INDIRECT_DESC) |
+                  (1ull << VIRTIO_RING_F_EVENT_IDX));
     qvirtio_set_features(vdev, features);
 
-    if (features & (1u << VIRTIO_NET_F_MQ)) {
+    if (features & (1ull << VIRTIO_NET_F_MQ)) {
         interface->n_queues = qvirtio_config_readw(vdev, 8) * 2;
     } else {
         interface->n_queues = 2;
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 50499e75ef..1b6b760fc6 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -96,19 +96,19 @@ static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t off)
     return val;
 }
 
-static uint32_t qvirtio_pci_get_features(QVirtioDevice *d)
+static uint64_t qvirtio_pci_get_features(QVirtioDevice *d)
 {
     QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
     return qpci_io_readl(dev->pdev, dev->bar, VIRTIO_PCI_HOST_FEATURES);
 }
 
-static void qvirtio_pci_set_features(QVirtioDevice *d, uint32_t features)
+static void qvirtio_pci_set_features(QVirtioDevice *d, uint64_t features)
 {
     QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
     qpci_io_writel(dev->pdev, dev->bar, VIRTIO_PCI_GUEST_FEATURES, features);
 }
 
-static uint32_t qvirtio_pci_get_guest_features(QVirtioDevice *d)
+static uint64_t qvirtio_pci_get_guest_features(QVirtioDevice *d)
 {
     QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
     return qpci_io_readl(dev->pdev, dev->bar, VIRTIO_PCI_GUEST_FEATURES);
@@ -208,7 +208,7 @@ static void qvirtio_pci_set_queue_address(QVirtioDevice *d, uint32_t pfn)
 static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
                                         QGuestAllocator *alloc, uint16_t index)
 {
-    uint32_t feat;
+    uint64_t feat;
     uint64_t addr;
     QVirtQueuePCI *vqpci;
     QVirtioPCIDevice *qvpcidev = container_of(d, QVirtioPCIDevice, vdev);
@@ -222,8 +222,8 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
     vqpci->vq.free_head = 0;
     vqpci->vq.num_free = vqpci->vq.size;
     vqpci->vq.align = VIRTIO_PCI_VRING_ALIGN;
-    vqpci->vq.indirect = (feat & (1u << VIRTIO_RING_F_INDIRECT_DESC)) != 0;
-    vqpci->vq.event = (feat & (1u << VIRTIO_RING_F_EVENT_IDX)) != 0;
+    vqpci->vq.indirect = feat & (1ull << VIRTIO_RING_F_INDIRECT_DESC);
+    vqpci->vq.event = feat & (1ull << VIRTIO_RING_F_EVENT_IDX);
 
     vqpci->msix_entry = -1;
     vqpci->msix_addr = 0;
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index 0ae9956fc8..4f7e6bb8a1 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -33,12 +33,12 @@ uint64_t qvirtio_config_readq(QVirtioDevice *d, uint64_t addr)
     return d->bus->config_readq(d, addr);
 }
 
-uint32_t qvirtio_get_features(QVirtioDevice *d)
+uint64_t qvirtio_get_features(QVirtioDevice *d)
 {
     return d->bus->get_features(d);
 }
 
-void qvirtio_set_features(QVirtioDevice *d, uint32_t features)
+void qvirtio_set_features(QVirtioDevice *d, uint64_t features)
 {
     d->features = features;
     d->bus->set_features(d, features);
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index f6674fb233..31680cc159 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -119,7 +119,7 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
     QVirtioBlkReq req;
     uint64_t req_addr;
     uint64_t capacity;
-    uint32_t features;
+    uint64_t features;
     uint32_t free_head;
     uint8_t status;
     char *data;
@@ -352,7 +352,7 @@ static void indirect(void *obj, void *u_data, QGuestAllocator *t_alloc)
     QVRingIndirectDesc *indirect;
     uint64_t req_addr;
     uint64_t capacity;
-    uint32_t features;
+    uint64_t features;
     uint32_t free_head;
     uint8_t status;
     char *data;
@@ -467,7 +467,7 @@ static void msix(void *obj, void *u_data, QGuestAllocator *t_alloc)
     int n_size = TEST_IMAGE_SIZE / 2;
     uint64_t req_addr;
     uint64_t capacity;
-    uint32_t features;
+    uint64_t features;
     uint32_t free_head;
     uint8_t status;
     char *data;
@@ -574,7 +574,7 @@ static void idx(void *obj, void *u_data, QGuestAllocator *t_alloc)
     QVirtioBlkReq req;
     uint64_t req_addr;
     uint64_t capacity;
-    uint32_t features;
+    uint64_t features;
     uint32_t free_head;
     uint32_t write_head;
     uint32_t desc_idx;
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 04/16] virtio-scsi-test: add missing feature negotiation
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
                   ` (2 preceding siblings ...)
  2019-10-23 10:04 ` [PATCH v4 03/16] libqos: extend feature bits to 64-bit Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 10:19   ` Thomas Huth
  2019-10-23 10:04 ` [PATCH v4 05/16] tests/virtio-blk-test: set up virtqueue after " Stefan Hajnoczi
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini, Richard Henderson

VIRTIO Device Initialization requires feature negotiation.  Currently
virtio-scsi-test.c is non-compliant.

libqos tests acknowledge all feature bits advertised by the device,
except VIRTIO_F_BAD_FEATURE (which devices use to detect broken
drivers!) and VIRTIO_RING_F_EVENT_IDX (which is not implemented in
libqos and accepting it would break notifications).

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/virtio-scsi-test.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index 7c8f9b27f8..0415e75876 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -123,10 +123,16 @@ static QVirtioSCSIQueues *qvirtio_scsi_init(QVirtioDevice *dev)
     QVirtioSCSIQueues *vs;
     const uint8_t test_unit_ready_cdb[VIRTIO_SCSI_CDB_SIZE] = {};
     struct virtio_scsi_cmd_resp resp;
+    uint64_t features;
     int i;
 
     vs = g_new0(QVirtioSCSIQueues, 1);
     vs->dev = dev;
+
+    features = qvirtio_get_features(dev);
+    features &= ~(QVIRTIO_F_BAD_FEATURE | (1ull << VIRTIO_RING_F_EVENT_IDX));
+    qvirtio_set_features(dev, features);
+
     vs->num_queues = qvirtio_config_readl(dev, 0);
 
     g_assert_cmpint(vs->num_queues, <, MAX_NUM_QUEUES);
@@ -135,6 +141,8 @@ static QVirtioSCSIQueues *qvirtio_scsi_init(QVirtioDevice *dev)
         vs->vq[i] = qvirtqueue_setup(dev, alloc, i);
     }
 
+    qvirtio_set_driver_ok(dev);
+
     /* Clear the POWER ON OCCURRED unit attention */
     g_assert_cmpint(virtio_scsi_do_command(vs, test_unit_ready_cdb,
                                            NULL, 0, NULL, 0, &resp),
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 05/16] tests/virtio-blk-test: set up virtqueue after feature negotiation
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
                   ` (3 preceding siblings ...)
  2019-10-23 10:04 ` [PATCH v4 04/16] virtio-scsi-test: add missing feature negotiation Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 10:04 ` [PATCH v4 06/16] libqos: add missing virtio-9p " Stefan Hajnoczi
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini, Richard Henderson

VIRTIO Device Initialization requires that feature negotiation has
completed before virtqueues are set up.  This makes sense because the
driver must know whether it is operating in Legacy or VIRTIO 1.0 mode
before it can access vring fields with the correct endianness.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 tests/virtio-blk-test.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 31680cc159..fe0dc4a896 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -113,8 +113,8 @@ static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d,
     return addr;
 }
 
-static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
-                       QVirtQueue *vq)
+/* Returns the request virtqueue so the caller can perform further tests */
+static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
 {
     QVirtioBlkReq req;
     uint64_t req_addr;
@@ -124,6 +124,7 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
     uint8_t status;
     char *data;
     QTestState *qts = global_qtest;
+    QVirtQueue *vq;
 
     features = qvirtio_get_features(dev);
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
@@ -135,6 +136,8 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
     capacity = qvirtio_config_readq(dev, 0);
     g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
 
+    vq = qvirtqueue_setup(dev, alloc, 0);
+
     qvirtio_set_driver_ok(dev);
 
     /* Write and read with 3 descriptor layout */
@@ -331,14 +334,16 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
 
         guest_free(alloc, req_addr);
     }
+
+    return vq;
 }
 
 static void basic(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtioBlk *blk_if = obj;
     QVirtQueue *vq;
-    vq = qvirtqueue_setup(blk_if->vdev, t_alloc, 0);
-    test_basic(blk_if->vdev, t_alloc, vq);
+
+    vq = test_basic(blk_if->vdev, t_alloc);
     qvirtqueue_cleanup(blk_if->vdev->bus, vq, t_alloc);
 
 }
@@ -746,9 +751,7 @@ static void resize(void *obj, void *data, QGuestAllocator *t_alloc)
     QVirtQueue *vq;
     QTestState *qts = global_qtest;
 
-    vq = qvirtqueue_setup(dev, t_alloc, 0);
-
-    test_basic(dev, t_alloc, vq);
+    vq = test_basic(dev, t_alloc);
 
     qmp_discard_response("{ 'execute': 'block_resize', "
                          " 'arguments': { 'device': 'drive0', "
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 06/16] libqos: add missing virtio-9p feature negotiation
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
                   ` (4 preceding siblings ...)
  2019-10-23 10:04 ` [PATCH v4 05/16] tests/virtio-blk-test: set up virtqueue after " Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 10:20   ` Thomas Huth
  2019-10-23 10:04 ` [PATCH v4 07/16] libqos: enforce Device Initialization order Stefan Hajnoczi
                   ` (10 subsequent siblings)
  16 siblings, 1 reply; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini, Richard Henderson

VIRTIO Device Initialization requires feature negotiation.  The libqos
virtio-9p driver lacks feature negotiation and is therefore
non-compliant.

libqos tests acknowledge all feature bits advertised by the device,
except VIRTIO_F_BAD_FEATURE (which devices use to detect broken
drivers!) and VIRTIO_RING_F_EVENT_IDX (which is not implemented in
libqos and accepting it would break notifications).

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqos/virtio-9p.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/libqos/virtio-9p.c b/tests/libqos/virtio-9p.c
index 8c9efce3e1..77dbfb62ad 100644
--- a/tests/libqos/virtio-9p.c
+++ b/tests/libqos/virtio-9p.c
@@ -32,6 +32,12 @@ static void virtio_9p_cleanup(QVirtio9P *interface)
 
 static void virtio_9p_setup(QVirtio9P *interface)
 {
+    uint64_t features;
+
+    features = qvirtio_get_features(interface->vdev);
+    features &= ~(QVIRTIO_F_BAD_FEATURE | (1ull << VIRTIO_RING_F_EVENT_IDX));
+    qvirtio_set_features(interface->vdev, features);
+
     interface->vq = qvirtqueue_setup(interface->vdev, alloc, 0);
     qvirtio_set_driver_ok(interface->vdev);
 }
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 07/16] libqos: enforce Device Initialization order
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
                   ` (5 preceding siblings ...)
  2019-10-23 10:04 ` [PATCH v4 06/16] libqos: add missing virtio-9p " Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 11:23   ` Thomas Huth
  2019-10-23 10:04 ` [PATCH v4 08/16] libqos: implement VIRTIO 1.0 FEATURES_OK step Stefan Hajnoczi
                   ` (9 subsequent siblings)
  16 siblings, 1 reply; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini, Richard Henderson

According to VIRTIO 1.1 "3.1.1 Driver Requirements: Device
Initialization", configuration space and virtqueues cannot be accessed
before features have been negotiated.  Enforce this requirement.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
v4:
 * Introduce bool d->features_negotiated so that tests can negotiate a
   0 feature bit set in Legacy mode [Thomas]
---
 tests/libqos/virtio.h | 1 +
 tests/libqos/virtio.c | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index a5c99fb3c9..0e8f823c7b 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -23,6 +23,7 @@ typedef struct QVirtioDevice {
     uint16_t device_type;
     uint64_t features;
     bool big_endian;
+    bool features_negotiated;
 } QVirtioDevice;
 
 typedef struct QVirtQueue {
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index 4f7e6bb8a1..6049ff3b3e 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -15,21 +15,25 @@
 
 uint8_t qvirtio_config_readb(QVirtioDevice *d, uint64_t addr)
 {
+    g_assert_true(d->features_negotiated);
     return d->bus->config_readb(d, addr);
 }
 
 uint16_t qvirtio_config_readw(QVirtioDevice *d, uint64_t addr)
 {
+    g_assert_true(d->features_negotiated);
     return d->bus->config_readw(d, addr);
 }
 
 uint32_t qvirtio_config_readl(QVirtioDevice *d, uint64_t addr)
 {
+    g_assert_true(d->features_negotiated);
     return d->bus->config_readl(d, addr);
 }
 
 uint64_t qvirtio_config_readq(QVirtioDevice *d, uint64_t addr)
 {
+    g_assert_true(d->features_negotiated);
     return d->bus->config_readq(d, addr);
 }
 
@@ -42,11 +46,13 @@ void qvirtio_set_features(QVirtioDevice *d, uint64_t features)
 {
     d->features = features;
     d->bus->set_features(d, features);
+    d->features_negotiated = true;
 }
 
 QVirtQueue *qvirtqueue_setup(QVirtioDevice *d,
                              QGuestAllocator *alloc, uint16_t index)
 {
+    g_assert_true(d->features_negotiated);
     return d->bus->virtqueue_setup(d, alloc, index);
 }
 
@@ -60,6 +66,7 @@ void qvirtio_reset(QVirtioDevice *d)
 {
     d->bus->set_status(d, 0);
     g_assert_cmphex(d->bus->get_status(d), ==, 0);
+    d->features_negotiated = false;
 }
 
 void qvirtio_set_acknowledge(QVirtioDevice *d)
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 08/16] libqos: implement VIRTIO 1.0 FEATURES_OK step
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
                   ` (6 preceding siblings ...)
  2019-10-23 10:04 ` [PATCH v4 07/16] libqos: enforce Device Initialization order Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 11:04   ` Thomas Huth
  2019-10-23 10:04 ` [PATCH v4 09/16] libqos: access VIRTIO 1.0 vring in little-endian Stefan Hajnoczi
                   ` (8 subsequent siblings)
  16 siblings, 1 reply; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini, Richard Henderson

Device initialization has an extra step in VIRTIO 1.0.  The FEATURES_OK
status bit is set to indicate that feature negotiation has completed.
The driver then reads the status register again to check that the device
agrees with the final features.

Implement this step as part of qvirtio_set_features() instead of
introducing a separate function.  This way all existing code works
without modifications.

The check in qvirtio_set_driver_ok() needs to be updated because
FEATURES_OK will be set for VIRTIO 1.0 devices.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
v4:
 * Make FEATURES_OK change in qvirtio_set_driver_ok() clearer and
   mention it in the commit description [Thomas]
---
 tests/libqos/virtio.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index 6049ff3b3e..fa597c2481 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -46,6 +46,20 @@ void qvirtio_set_features(QVirtioDevice *d, uint64_t features)
 {
     d->features = features;
     d->bus->set_features(d, features);
+
+    /*
+     * This could be a separate function for drivers that want to access
+     * configuration space before setting FEATURES_OK, but no existing users
+     * need that and it's less code for callers if this is done implicitly.
+    */
+    if (features & (1ull << VIRTIO_F_VERSION_1)) {
+        uint8_t status = d->bus->get_status(d) |
+                         VIRTIO_CONFIG_S_FEATURES_OK;
+
+        d->bus->set_status(d, status);
+        g_assert_cmphex(d->bus->get_status(d), ==, status);
+    }
+
     d->features_negotiated = true;
 }
 
@@ -86,7 +100,9 @@ void qvirtio_set_driver_ok(QVirtioDevice *d)
 {
     d->bus->set_status(d, d->bus->get_status(d) | VIRTIO_CONFIG_S_DRIVER_OK);
     g_assert_cmphex(d->bus->get_status(d), ==, VIRTIO_CONFIG_S_DRIVER_OK |
-                    VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_ACKNOWLEDGE);
+                    VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_ACKNOWLEDGE |
+                    (d->features & (1ull << VIRTIO_F_VERSION_1) ?
+                     VIRTIO_CONFIG_S_FEATURES_OK : 0));
 }
 
 void qvirtio_wait_queue_isr(QTestState *qts, QVirtioDevice *d,
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 09/16] libqos: access VIRTIO 1.0 vring in little-endian
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
                   ` (7 preceding siblings ...)
  2019-10-23 10:04 ` [PATCH v4 08/16] libqos: implement VIRTIO 1.0 FEATURES_OK step Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 10:04 ` [PATCH v4 10/16] libqos: add iteration support to qpci_find_capability() Stefan Hajnoczi
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini, Richard Henderson

VIRTIO 1.0 uses little-endian for the vring.  Legacy VIRTIO uses guest
endianness.  Adjust the code to handle both.

Note that qvirtio_readq() is not defined because it has no users.  All
the other accessors are really needed.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
v4:
 * Fixed indentation in qvring_init() [Thomas]
---
 tests/libqos/virtio.h      |   4 +-
 tests/libqos/virtio-mmio.c |   1 +
 tests/libqos/virtio-pci.c  |   1 +
 tests/libqos/virtio.c      | 131 +++++++++++++++++++++++++++----------
 tests/virtio-blk-test.c    |   8 +--
 5 files changed, 106 insertions(+), 39 deletions(-)

diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 0e8f823c7b..ebbff5193b 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -27,6 +27,7 @@ typedef struct QVirtioDevice {
 } QVirtioDevice;
 
 typedef struct QVirtQueue {
+    QVirtioDevice *vdev;
     uint64_t desc; /* This points to an array of struct vring_desc */
     uint64_t avail; /* This points to a struct vring_avail */
     uint64_t used; /* This points to a struct vring_used */
@@ -135,7 +136,8 @@ void qvring_init(QTestState *qts, const QGuestAllocator *alloc, QVirtQueue *vq,
 QVRingIndirectDesc *qvring_indirect_desc_setup(QTestState *qs, QVirtioDevice *d,
                                                QGuestAllocator *alloc,
                                                uint16_t elem);
-void qvring_indirect_desc_add(QTestState *qts, QVRingIndirectDesc *indirect,
+void qvring_indirect_desc_add(QVirtioDevice *d, QTestState *qts,
+                              QVRingIndirectDesc *indirect,
                               uint64_t data, uint32_t len, bool write);
 uint32_t qvirtqueue_add(QTestState *qts, QVirtQueue *vq, uint64_t data,
                         uint32_t len, bool write, bool next);
diff --git a/tests/libqos/virtio-mmio.c b/tests/libqos/virtio-mmio.c
index 78066e6e05..4db1f1b8bc 100644
--- a/tests/libqos/virtio-mmio.c
+++ b/tests/libqos/virtio-mmio.c
@@ -157,6 +157,7 @@ static QVirtQueue *qvirtio_mmio_virtqueue_setup(QVirtioDevice *d,
     uint64_t addr;
 
     vq = g_malloc0(sizeof(*vq));
+    vq->vdev = d;
     qvirtio_mmio_queue_select(d, index);
     qtest_writel(dev->qts, dev->addr + QVIRTIO_MMIO_QUEUE_ALIGN, dev->page_size);
 
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 1b6b760fc6..7ecf5d0a52 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -217,6 +217,7 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
     feat = qvirtio_pci_get_guest_features(d);
 
     qvirtio_pci_queue_select(d, index);
+    vqpci->vq.vdev = d;
     vqpci->vq.index = index;
     vqpci->vq.size = qvirtio_pci_get_queue_size(d);
     vqpci->vq.free_head = 0;
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index fa597c2481..9aa360620c 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -8,11 +8,68 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/bswap.h"
 #include "libqtest.h"
 #include "libqos/virtio.h"
 #include "standard-headers/linux/virtio_config.h"
 #include "standard-headers/linux/virtio_ring.h"
 
+/*
+ * qtest_readX/writeX() functions transfer host endian from/to guest endian.
+ * This works great for Legacy VIRTIO devices where we need guest endian
+ * accesses.  For VIRTIO 1.0 the vring is little-endian so the automatic guest
+ * endianness conversion is not wanted.
+ *
+ * The following qvirtio_readX/writeX() functions handle Legacy and VIRTIO 1.0
+ * accesses seamlessly.
+ */
+static uint16_t qvirtio_readw(QVirtioDevice *d, QTestState *qts, uint64_t addr)
+{
+    uint16_t val = qtest_readw(qts, addr);
+
+    if (d->features & (1ull << VIRTIO_F_VERSION_1) && qtest_big_endian(qts)) {
+        val = bswap16(val);
+    }
+    return val;
+}
+
+static uint32_t qvirtio_readl(QVirtioDevice *d, QTestState *qts, uint64_t addr)
+{
+    uint32_t val = qtest_readl(qts, addr);
+
+    if (d->features & (1ull << VIRTIO_F_VERSION_1) && qtest_big_endian(qts)) {
+        val = bswap32(val);
+    }
+    return val;
+}
+
+static void qvirtio_writew(QVirtioDevice *d, QTestState *qts,
+                           uint64_t addr, uint16_t val)
+{
+    if (d->features & (1ull << VIRTIO_F_VERSION_1) && qtest_big_endian(qts)) {
+        val = bswap16(val);
+    }
+    qtest_writew(qts, addr, val);
+}
+
+static void qvirtio_writel(QVirtioDevice *d, QTestState *qts,
+                           uint64_t addr, uint32_t val)
+{
+    if (d->features & (1ull << VIRTIO_F_VERSION_1) && qtest_big_endian(qts)) {
+        val = bswap32(val);
+    }
+    qtest_writel(qts, addr, val);
+}
+
+static void qvirtio_writeq(QVirtioDevice *d, QTestState *qts,
+                           uint64_t addr, uint64_t val)
+{
+    if (d->features & (1ull << VIRTIO_F_VERSION_1) && qtest_big_endian(qts)) {
+        val = bswap64(val);
+    }
+    qtest_writeq(qts, addr, val);
+}
+
 uint8_t qvirtio_config_readb(QVirtioDevice *d, uint64_t addr)
 {
     g_assert_true(d->features_negotiated);
@@ -189,23 +246,23 @@ void qvring_init(QTestState *qts, const QGuestAllocator *alloc, QVirtQueue *vq,
 
     for (i = 0; i < vq->size - 1; i++) {
         /* vq->desc[i].addr */
-        qtest_writeq(qts, vq->desc + (16 * i), 0);
+        qvirtio_writeq(vq->vdev, qts, vq->desc + (16 * i), 0);
         /* vq->desc[i].next */
-        qtest_writew(qts, vq->desc + (16 * i) + 14, i + 1);
+        qvirtio_writew(vq->vdev, qts, vq->desc + (16 * i) + 14, i + 1);
     }
 
     /* vq->avail->flags */
-    qtest_writew(qts, vq->avail, 0);
+    qvirtio_writew(vq->vdev, qts, vq->avail, 0);
     /* vq->avail->idx */
-    qtest_writew(qts, vq->avail + 2, 0);
+    qvirtio_writew(vq->vdev, qts, vq->avail + 2, 0);
     /* vq->avail->used_event */
-    qtest_writew(qts, vq->avail + 4 + (2 * vq->size), 0);
+    qvirtio_writew(vq->vdev, qts, vq->avail + 4 + (2 * vq->size), 0);
 
     /* vq->used->flags */
-    qtest_writew(qts, vq->used, 0);
+    qvirtio_writew(vq->vdev, qts, vq->used, 0);
     /* vq->used->avail_event */
-    qtest_writew(qts, vq->used + 2 + sizeof(struct vring_used_elem) * vq->size,
-                 0);
+    qvirtio_writew(vq->vdev, qts, vq->used + 2 +
+                   sizeof(struct vring_used_elem) * vq->size, 0);
 }
 
 QVRingIndirectDesc *qvring_indirect_desc_setup(QTestState *qs, QVirtioDevice *d,
@@ -221,35 +278,39 @@ QVRingIndirectDesc *qvring_indirect_desc_setup(QTestState *qs, QVirtioDevice *d,
 
     for (i = 0; i < elem - 1; ++i) {
         /* indirect->desc[i].addr */
-        qtest_writeq(qs, indirect->desc + (16 * i), 0);
+        qvirtio_writeq(d, qs, indirect->desc + (16 * i), 0);
         /* indirect->desc[i].flags */
-        qtest_writew(qs, indirect->desc + (16 * i) + 12, VRING_DESC_F_NEXT);
+        qvirtio_writew(d, qs, indirect->desc + (16 * i) + 12,
+                       VRING_DESC_F_NEXT);
         /* indirect->desc[i].next */
-        qtest_writew(qs, indirect->desc + (16 * i) + 14, i + 1);
+        qvirtio_writew(d, qs, indirect->desc + (16 * i) + 14, i + 1);
     }
 
     return indirect;
 }
 
-void qvring_indirect_desc_add(QTestState *qts, QVRingIndirectDesc *indirect,
+void qvring_indirect_desc_add(QVirtioDevice *d, QTestState *qts,
+                              QVRingIndirectDesc *indirect,
                               uint64_t data, uint32_t len, bool write)
 {
     uint16_t flags;
 
     g_assert_cmpint(indirect->index, <, indirect->elem);
 
-    flags = qtest_readw(qts, indirect->desc + (16 * indirect->index) + 12);
+    flags = qvirtio_readw(d, qts, indirect->desc +
+                                  (16 * indirect->index) + 12);
 
     if (write) {
         flags |= VRING_DESC_F_WRITE;
     }
 
     /* indirect->desc[indirect->index].addr */
-    qtest_writeq(qts, indirect->desc + (16 * indirect->index), data);
+    qvirtio_writeq(d, qts, indirect->desc + (16 * indirect->index), data);
     /* indirect->desc[indirect->index].len */
-    qtest_writel(qts, indirect->desc + (16 * indirect->index) + 8, len);
+    qvirtio_writel(d, qts, indirect->desc + (16 * indirect->index) + 8, len);
     /* indirect->desc[indirect->index].flags */
-    qtest_writew(qts, indirect->desc + (16 * indirect->index) + 12, flags);
+    qvirtio_writew(d, qts, indirect->desc + (16 * indirect->index) + 12,
+                   flags);
 
     indirect->index++;
 }
@@ -269,11 +330,11 @@ uint32_t qvirtqueue_add(QTestState *qts, QVirtQueue *vq, uint64_t data,
     }
 
     /* vq->desc[vq->free_head].addr */
-    qtest_writeq(qts, vq->desc + (16 * vq->free_head), data);
+    qvirtio_writeq(vq->vdev, qts, vq->desc + (16 * vq->free_head), data);
     /* vq->desc[vq->free_head].len */
-    qtest_writel(qts, vq->desc + (16 * vq->free_head) + 8, len);
+    qvirtio_writel(vq->vdev, qts, vq->desc + (16 * vq->free_head) + 8, len);
     /* vq->desc[vq->free_head].flags */
-    qtest_writew(qts, vq->desc + (16 * vq->free_head) + 12, flags);
+    qvirtio_writew(vq->vdev, qts, vq->desc + (16 * vq->free_head) + 12, flags);
 
     return vq->free_head++; /* Return and increase, in this order */
 }
@@ -288,13 +349,14 @@ uint32_t qvirtqueue_add_indirect(QTestState *qts, QVirtQueue *vq,
     vq->num_free--;
 
     /* vq->desc[vq->free_head].addr */
-    qtest_writeq(qts, vq->desc + (16 * vq->free_head), indirect->desc);
+    qvirtio_writeq(vq->vdev, qts, vq->desc + (16 * vq->free_head),
+                   indirect->desc);
     /* vq->desc[vq->free_head].len */
-    qtest_writel(qts, vq->desc + (16 * vq->free_head) + 8,
-           sizeof(struct vring_desc) * indirect->elem);
+    qvirtio_writel(vq->vdev, qts, vq->desc + (16 * vq->free_head) + 8,
+                   sizeof(struct vring_desc) * indirect->elem);
     /* vq->desc[vq->free_head].flags */
-    qtest_writew(qts, vq->desc + (16 * vq->free_head) + 12,
-                 VRING_DESC_F_INDIRECT);
+    qvirtio_writew(vq->vdev, qts, vq->desc + (16 * vq->free_head) + 12,
+                   VRING_DESC_F_INDIRECT);
 
     return vq->free_head++; /* Return and increase, in this order */
 }
@@ -303,21 +365,21 @@ void qvirtqueue_kick(QTestState *qts, QVirtioDevice *d, QVirtQueue *vq,
                      uint32_t free_head)
 {
     /* vq->avail->idx */
-    uint16_t idx = qtest_readw(qts, vq->avail + 2);
+    uint16_t idx = qvirtio_readw(d, qts, vq->avail + 2);
     /* vq->used->flags */
     uint16_t flags;
     /* vq->used->avail_event */
     uint16_t avail_event;
 
     /* vq->avail->ring[idx % vq->size] */
-    qtest_writew(qts, vq->avail + 4 + (2 * (idx % vq->size)), free_head);
+    qvirtio_writew(d, qts, vq->avail + 4 + (2 * (idx % vq->size)), free_head);
     /* vq->avail->idx */
-    qtest_writew(qts, vq->avail + 2, idx + 1);
+    qvirtio_writew(d, qts, vq->avail + 2, idx + 1);
 
     /* Must read after idx is updated */
-    flags = qtest_readw(qts, vq->avail);
-    avail_event = qtest_readw(qts, vq->used + 4 +
-                                   sizeof(struct vring_used_elem) * vq->size);
+    flags = qvirtio_readw(d, qts, vq->avail);
+    avail_event = qvirtio_readw(d, qts, vq->used + 4 +
+                                sizeof(struct vring_used_elem) * vq->size);
 
     /* < 1 because we add elements to avail queue one by one */
     if ((flags & VRING_USED_F_NO_NOTIFY) == 0 &&
@@ -342,7 +404,8 @@ bool qvirtqueue_get_buf(QTestState *qts, QVirtQueue *vq, uint32_t *desc_idx,
     uint16_t idx;
     uint64_t elem_addr, addr;
 
-    idx = qtest_readw(qts, vq->used + offsetof(struct vring_used, idx));
+    idx = qvirtio_readw(vq->vdev, qts,
+                        vq->used + offsetof(struct vring_used, idx));
     if (idx == vq->last_used_idx) {
         return false;
     }
@@ -354,12 +417,12 @@ bool qvirtqueue_get_buf(QTestState *qts, QVirtQueue *vq, uint32_t *desc_idx,
 
     if (desc_idx) {
         addr = elem_addr + offsetof(struct vring_used_elem, id);
-        *desc_idx = qtest_readl(qts, addr);
+        *desc_idx = qvirtio_readl(vq->vdev, qts, addr);
     }
 
     if (len) {
         addr = elem_addr + offsetof(struct vring_used_elem, len);
-        *len = qtest_readw(qts, addr);
+        *len = qvirtio_readw(vq->vdev, qts, addr);
     }
 
     vq->last_used_idx++;
@@ -371,7 +434,7 @@ void qvirtqueue_set_used_event(QTestState *qts, QVirtQueue *vq, uint16_t idx)
     g_assert(vq->event);
 
     /* vq->avail->used_event */
-    qtest_writew(qts, vq->avail + 4 + (2 * vq->size), idx);
+    qvirtio_writew(vq->vdev, qts, vq->avail + 4 + (2 * vq->size), idx);
 }
 
 void qvirtio_start_device(QVirtioDevice *vdev)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index fe0dc4a896..2a23698211 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -388,8 +388,8 @@ static void indirect(void *obj, void *u_data, QGuestAllocator *t_alloc)
     g_free(req.data);
 
     indirect = qvring_indirect_desc_setup(qts, dev, t_alloc, 2);
-    qvring_indirect_desc_add(qts, indirect, req_addr, 528, false);
-    qvring_indirect_desc_add(qts, indirect, req_addr + 528, 1, true);
+    qvring_indirect_desc_add(dev, qts, indirect, req_addr, 528, false);
+    qvring_indirect_desc_add(dev, qts, indirect, req_addr + 528, 1, true);
     free_head = qvirtqueue_add_indirect(qts, vq, indirect);
     qvirtqueue_kick(qts, dev, vq, free_head);
 
@@ -413,8 +413,8 @@ static void indirect(void *obj, void *u_data, QGuestAllocator *t_alloc)
     g_free(req.data);
 
     indirect = qvring_indirect_desc_setup(qts, dev, t_alloc, 2);
-    qvring_indirect_desc_add(qts, indirect, req_addr, 16, false);
-    qvring_indirect_desc_add(qts, indirect, req_addr + 16, 513, true);
+    qvring_indirect_desc_add(dev, qts, indirect, req_addr, 16, false);
+    qvring_indirect_desc_add(dev, qts, indirect, req_addr + 16, 513, true);
     free_head = qvirtqueue_add_indirect(qts, vq, indirect);
     qvirtqueue_kick(qts, dev, vq, free_head);
 
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 10/16] libqos: add iteration support to qpci_find_capability()
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
                   ` (8 preceding siblings ...)
  2019-10-23 10:04 ` [PATCH v4 09/16] libqos: access VIRTIO 1.0 vring in little-endian Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 10:04 ` [PATCH v4 11/16] libqos: pass full QVirtQueue to set_queue_address() Stefan Hajnoczi
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini, Richard Henderson

VIRTIO 1.0 PCI devices have multiple PCI_CAP_ID_VNDR capabilities so we
need a way to iterate over them.  Extend qpci_find_capability() to take
the last address.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
--
v3:
 * Document qpci_find_capability()
---
 tests/libqos/pci.h |  2 +-
 tests/libqos/pci.c | 30 ++++++++++++++++++++++++------
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/tests/libqos/pci.h b/tests/libqos/pci.h
index a5389a5845..590c175190 100644
--- a/tests/libqos/pci.h
+++ b/tests/libqos/pci.h
@@ -86,7 +86,7 @@ bool qpci_has_buggy_msi(QPCIDevice *dev);
 bool qpci_check_buggy_msi(QPCIDevice *dev);
 
 void qpci_device_enable(QPCIDevice *dev);
-uint8_t qpci_find_capability(QPCIDevice *dev, uint8_t id);
+uint8_t qpci_find_capability(QPCIDevice *dev, uint8_t id, uint8_t start_addr);
 void qpci_msix_enable(QPCIDevice *dev);
 void qpci_msix_disable(QPCIDevice *dev);
 bool qpci_msix_pending(QPCIDevice *dev, uint16_t entry);
diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c
index 662ee7a517..2309a724e4 100644
--- a/tests/libqos/pci.c
+++ b/tests/libqos/pci.c
@@ -115,10 +115,28 @@ void qpci_device_enable(QPCIDevice *dev)
     g_assert_cmphex(cmd & PCI_COMMAND_MASTER, ==, PCI_COMMAND_MASTER);
 }
 
-uint8_t qpci_find_capability(QPCIDevice *dev, uint8_t id)
+/**
+ * qpci_find_capability:
+ * @dev: the PCI device
+ * @id: the PCI Capability ID (PCI_CAP_ID_*)
+ * @start_addr: 0 to begin iteration or the last return value to continue
+ *              iteration
+ *
+ * Iterate over the PCI Capabilities List.
+ *
+ * Returns: PCI Configuration Space offset of the capabililty structure or
+ *          0 if no further matching capability is found
+ */
+uint8_t qpci_find_capability(QPCIDevice *dev, uint8_t id, uint8_t start_addr)
 {
     uint8_t cap;
-    uint8_t addr = qpci_config_readb(dev, PCI_CAPABILITY_LIST);
+    uint8_t addr;
+
+    if (start_addr) {
+        addr = qpci_config_readb(dev, start_addr + PCI_CAP_LIST_NEXT);
+    } else {
+        addr = qpci_config_readb(dev, PCI_CAPABILITY_LIST);
+    }
 
     do {
         cap = qpci_config_readb(dev, addr);
@@ -138,7 +156,7 @@ void qpci_msix_enable(QPCIDevice *dev)
     uint8_t bir_table;
     uint8_t bir_pba;
 
-    addr = qpci_find_capability(dev, PCI_CAP_ID_MSIX);
+    addr = qpci_find_capability(dev, PCI_CAP_ID_MSIX, 0);
     g_assert_cmphex(addr, !=, 0);
 
     val = qpci_config_readw(dev, addr + PCI_MSIX_FLAGS);
@@ -167,7 +185,7 @@ void qpci_msix_disable(QPCIDevice *dev)
     uint16_t val;
 
     g_assert(dev->msix_enabled);
-    addr = qpci_find_capability(dev, PCI_CAP_ID_MSIX);
+    addr = qpci_find_capability(dev, PCI_CAP_ID_MSIX, 0);
     g_assert_cmphex(addr, !=, 0);
     val = qpci_config_readw(dev, addr + PCI_MSIX_FLAGS);
     qpci_config_writew(dev, addr + PCI_MSIX_FLAGS,
@@ -203,7 +221,7 @@ bool qpci_msix_masked(QPCIDevice *dev, uint16_t entry)
     uint64_t vector_off = dev->msix_table_off + entry * PCI_MSIX_ENTRY_SIZE;
 
     g_assert(dev->msix_enabled);
-    addr = qpci_find_capability(dev, PCI_CAP_ID_MSIX);
+    addr = qpci_find_capability(dev, PCI_CAP_ID_MSIX, 0);
     g_assert_cmphex(addr, !=, 0);
     val = qpci_config_readw(dev, addr + PCI_MSIX_FLAGS);
 
@@ -221,7 +239,7 @@ uint16_t qpci_msix_table_size(QPCIDevice *dev)
     uint8_t addr;
     uint16_t control;
 
-    addr = qpci_find_capability(dev, PCI_CAP_ID_MSIX);
+    addr = qpci_find_capability(dev, PCI_CAP_ID_MSIX, 0);
     g_assert_cmphex(addr, !=, 0);
 
     control = qpci_config_readw(dev, addr + PCI_MSIX_FLAGS);
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 11/16] libqos: pass full QVirtQueue to set_queue_address()
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
                   ` (9 preceding siblings ...)
  2019-10-23 10:04 ` [PATCH v4 10/16] libqos: add iteration support to qpci_find_capability() Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 10:04 ` [PATCH v4 12/16] libqos: add MSI-X callbacks to QVirtioPCIDevice Stefan Hajnoczi
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini,
	Philippe Mathieu-Daudé,
	Richard Henderson

Instead of just passing the vring page frame number, pass the full
QVirtQueue.  This will allow the VIRTIO 1.0 transport to program the
fine-grained vring address registers in the future.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/libqos/virtio.h      | 2 +-
 tests/libqos/virtio-mmio.c | 6 ++++--
 tests/libqos/virtio-pci.c  | 6 ++++--
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index ebbff5193b..529ef7555a 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -81,7 +81,7 @@ struct QVirtioBus {
     uint16_t (*get_queue_size)(QVirtioDevice *d);
 
     /* Set the address of the selected queue */
-    void (*set_queue_address)(QVirtioDevice *d, uint32_t pfn);
+    void (*set_queue_address)(QVirtioDevice *d, QVirtQueue *vq);
 
     /* Setup the virtqueue specified by index */
     QVirtQueue *(*virtqueue_setup)(QVirtioDevice *d, QGuestAllocator *alloc,
diff --git a/tests/libqos/virtio-mmio.c b/tests/libqos/virtio-mmio.c
index 4db1f1b8bc..e0a2bd7bc6 100644
--- a/tests/libqos/virtio-mmio.c
+++ b/tests/libqos/virtio-mmio.c
@@ -143,9 +143,11 @@ static uint16_t qvirtio_mmio_get_queue_size(QVirtioDevice *d)
     return (uint16_t)qtest_readl(dev->qts, dev->addr + QVIRTIO_MMIO_QUEUE_NUM_MAX);
 }
 
-static void qvirtio_mmio_set_queue_address(QVirtioDevice *d, uint32_t pfn)
+static void qvirtio_mmio_set_queue_address(QVirtioDevice *d, QVirtQueue *vq)
 {
     QVirtioMMIODevice *dev = container_of(d, QVirtioMMIODevice, vdev);
+    uint64_t pfn = vq->desc / dev->page_size;
+
     qtest_writel(dev->qts, dev->addr + QVIRTIO_MMIO_QUEUE_PFN, pfn);
 }
 
@@ -179,7 +181,7 @@ static QVirtQueue *qvirtio_mmio_virtqueue_setup(QVirtioDevice *d,
 
     addr = guest_alloc(alloc, qvring_size(vq->size, dev->page_size));
     qvring_init(dev->qts, alloc, vq, addr);
-    qvirtio_mmio_set_queue_address(d, vq->desc / dev->page_size);
+    qvirtio_mmio_set_queue_address(d, vq);
 
     return vq;
 }
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 7ecf5d0a52..e4fa318dcc 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -199,9 +199,11 @@ static uint16_t qvirtio_pci_get_queue_size(QVirtioDevice *d)
     return qpci_io_readw(dev->pdev, dev->bar, VIRTIO_PCI_QUEUE_NUM);
 }
 
-static void qvirtio_pci_set_queue_address(QVirtioDevice *d, uint32_t pfn)
+static void qvirtio_pci_set_queue_address(QVirtioDevice *d, QVirtQueue *vq)
 {
     QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+    uint64_t pfn = vq->desc / VIRTIO_PCI_VRING_ALIGN;
+
     qpci_io_writel(dev->pdev, dev->bar, VIRTIO_PCI_QUEUE_PFN, pfn);
 }
 
@@ -239,7 +241,7 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
     addr = guest_alloc(alloc, qvring_size(vqpci->vq.size,
                                           VIRTIO_PCI_VRING_ALIGN));
     qvring_init(qvpcidev->pdev->bus->qts, alloc, &vqpci->vq, addr);
-    qvirtio_pci_set_queue_address(d, vqpci->vq.desc / VIRTIO_PCI_VRING_ALIGN);
+    qvirtio_pci_set_queue_address(d, &vqpci->vq);
 
     return &vqpci->vq;
 }
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 12/16] libqos: add MSI-X callbacks to QVirtioPCIDevice
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
                   ` (10 preceding siblings ...)
  2019-10-23 10:04 ` [PATCH v4 11/16] libqos: pass full QVirtQueue to set_queue_address() Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 10:04 ` [PATCH v4 13/16] libqos: expose common virtqueue setup/cleanup functions Stefan Hajnoczi
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini, Richard Henderson

The MSI-X vectors are programmed differently in the VIRTIO 1.0 and
Legacy interfaces.  Introduce callbacks so different implementations can
be used depending on the interface version.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 tests/libqos/virtio-pci.h | 12 ++++++++++++
 tests/libqos/virtio-pci.c | 37 ++++++++++++++++++++++++++++---------
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
index 728b4715f1..4299efc023 100644
--- a/tests/libqos/virtio-pci.h
+++ b/tests/libqos/virtio-pci.h
@@ -14,16 +14,28 @@
 #include "libqos/pci.h"
 #include "libqos/qgraph.h"
 
+typedef struct QVirtioPCIMSIXOps QVirtioPCIMSIXOps;
+
 typedef struct QVirtioPCIDevice {
     QOSGraphObject obj;
     QVirtioDevice vdev;
     QPCIDevice *pdev;
     QPCIBar bar;
+    const QVirtioPCIMSIXOps *msix_ops;
     uint16_t config_msix_entry;
     uint64_t config_msix_addr;
     uint32_t config_msix_data;
 } QVirtioPCIDevice;
 
+struct QVirtioPCIMSIXOps {
+    /* Set the Configuration Vector for MSI-X */
+    void (*set_config_vector)(QVirtioPCIDevice *d, uint16_t entry);
+
+    /* Set the Queue Vector for MSI-X */
+    void (*set_queue_vector)(QVirtioPCIDevice *d, uint16_t vq_idx,
+                             uint16_t entry);
+};
+
 typedef struct QVirtQueuePCI {
     QVirtQueue vq;
     uint16_t msix_entry;
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index e4fa318dcc..0725777a8d 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -281,6 +281,31 @@ const QVirtioBus qvirtio_pci = {
     .virtqueue_kick = qvirtio_pci_virtqueue_kick,
 };
 
+static void qvirtio_pci_set_config_vector(QVirtioPCIDevice *d, uint16_t entry)
+{
+    uint16_t vector;
+
+    qpci_io_writew(d->pdev, d->bar, VIRTIO_MSI_CONFIG_VECTOR, entry);
+    vector = qpci_io_readw(d->pdev, d->bar, VIRTIO_MSI_CONFIG_VECTOR);
+    g_assert_cmphex(vector, !=, VIRTIO_MSI_NO_VECTOR);
+}
+
+static void qvirtio_pci_set_queue_vector(QVirtioPCIDevice *d, uint16_t vq_idx,
+                                         uint16_t entry)
+{
+    uint16_t vector;
+
+    qvirtio_pci_queue_select(&d->vdev, vq_idx);
+    qpci_io_writew(d->pdev, d->bar, VIRTIO_MSI_QUEUE_VECTOR, entry);
+    vector = qpci_io_readw(d->pdev, d->bar, VIRTIO_MSI_QUEUE_VECTOR);
+    g_assert_cmphex(vector, !=, VIRTIO_MSI_NO_VECTOR);
+}
+
+static const QVirtioPCIMSIXOps qvirtio_pci_msix_ops_legacy = {
+    .set_config_vector = qvirtio_pci_set_config_vector,
+    .set_queue_vector = qvirtio_pci_set_queue_vector,
+};
+
 void qvirtio_pci_device_enable(QVirtioPCIDevice *d)
 {
     qpci_device_enable(d->pdev);
@@ -295,7 +320,6 @@ void qvirtio_pci_device_disable(QVirtioPCIDevice *d)
 void qvirtqueue_pci_msix_setup(QVirtioPCIDevice *d, QVirtQueuePCI *vqpci,
                                         QGuestAllocator *alloc, uint16_t entry)
 {
-    uint16_t vector;
     uint32_t control;
     uint64_t off;
 
@@ -321,16 +345,12 @@ void qvirtqueue_pci_msix_setup(QVirtioPCIDevice *d, QVirtQueuePCI *vqpci,
                    off + PCI_MSIX_ENTRY_VECTOR_CTRL,
                    control & ~PCI_MSIX_ENTRY_CTRL_MASKBIT);
 
-    qvirtio_pci_queue_select(&d->vdev, vqpci->vq.index);
-    qpci_io_writew(d->pdev, d->bar, VIRTIO_MSI_QUEUE_VECTOR, entry);
-    vector = qpci_io_readw(d->pdev, d->bar, VIRTIO_MSI_QUEUE_VECTOR);
-    g_assert_cmphex(vector, !=, VIRTIO_MSI_NO_VECTOR);
+    d->msix_ops->set_queue_vector(d, vqpci->vq.index, entry);
 }
 
 void qvirtio_pci_set_msix_configuration_vector(QVirtioPCIDevice *d,
                                         QGuestAllocator *alloc, uint16_t entry)
 {
-    uint16_t vector;
     uint32_t control;
     uint64_t off;
 
@@ -358,9 +378,7 @@ void qvirtio_pci_set_msix_configuration_vector(QVirtioPCIDevice *d,
                    off + PCI_MSIX_ENTRY_VECTOR_CTRL,
                    control & ~PCI_MSIX_ENTRY_CTRL_MASKBIT);
 
-    qpci_io_writew(d->pdev, d->bar, VIRTIO_MSI_CONFIG_VECTOR, entry);
-    vector = qpci_io_readw(d->pdev, d->bar, VIRTIO_MSI_CONFIG_VECTOR);
-    g_assert_cmphex(vector, !=, VIRTIO_MSI_NO_VECTOR);
+    d->msix_ops->set_config_vector(d, entry);
 }
 
 void qvirtio_pci_destructor(QOSGraphObject *obj)
@@ -383,6 +401,7 @@ static void qvirtio_pci_init_from_pcidev(QVirtioPCIDevice *dev, QPCIDevice *pci_
     dev->vdev.device_type = qpci_config_readw(pci_dev, PCI_SUBSYSTEM_ID);
 
     dev->config_msix_entry = -1;
+    dev->msix_ops = &qvirtio_pci_msix_ops_legacy;
 
     dev->vdev.bus = &qvirtio_pci;
     dev->vdev.big_endian = qvirtio_pci_is_big_endian(dev);
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 13/16] libqos: expose common virtqueue setup/cleanup functions
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
                   ` (11 preceding siblings ...)
  2019-10-23 10:04 ` [PATCH v4 12/16] libqos: add MSI-X callbacks to QVirtioPCIDevice Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 10:04 ` [PATCH v4 14/16] libqos: make the virtio-pci BAR index configurable Stefan Hajnoczi
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini, Richard Henderson

The VIRTIO 1.0 code will need to perform additional steps but it will
reuse the common virtqueue setup/cleanup code.  Make these functions
public.

Make sure to invoke callbacks via QVirtioBus instead of directly calling
the virtio-pci Legacy versions of these functions.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 tests/libqos/virtio-pci.h |  8 ++++++++
 tests/libqos/virtio-pci.c | 19 ++++++++++---------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
index 4299efc023..0e4a8b7b00 100644
--- a/tests/libqos/virtio-pci.h
+++ b/tests/libqos/virtio-pci.h
@@ -65,4 +65,12 @@ void qvirtio_pci_set_msix_configuration_vector(QVirtioPCIDevice *d,
                                         QGuestAllocator *alloc, uint16_t entry);
 void qvirtqueue_pci_msix_setup(QVirtioPCIDevice *d, QVirtQueuePCI *vqpci,
                                         QGuestAllocator *alloc, uint16_t entry);
+
+/* Used by Legacy and Modern virtio-pci code */
+QVirtQueue *qvirtio_pci_virtqueue_setup_common(QVirtioDevice *d,
+                                               QGuestAllocator *alloc,
+                                               uint16_t index);
+void qvirtio_pci_virtqueue_cleanup_common(QVirtQueue *vq,
+                                          QGuestAllocator *alloc);
+
 #endif
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 0725777a8d..c900742f96 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -207,8 +207,9 @@ static void qvirtio_pci_set_queue_address(QVirtioDevice *d, QVirtQueue *vq)
     qpci_io_writel(dev->pdev, dev->bar, VIRTIO_PCI_QUEUE_PFN, pfn);
 }
 
-static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
-                                        QGuestAllocator *alloc, uint16_t index)
+QVirtQueue *qvirtio_pci_virtqueue_setup_common(QVirtioDevice *d,
+                                               QGuestAllocator *alloc,
+                                               uint16_t index)
 {
     uint64_t feat;
     uint64_t addr;
@@ -216,12 +217,12 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
     QVirtioPCIDevice *qvpcidev = container_of(d, QVirtioPCIDevice, vdev);
 
     vqpci = g_malloc0(sizeof(*vqpci));
-    feat = qvirtio_pci_get_guest_features(d);
+    feat = d->bus->get_guest_features(d);
 
-    qvirtio_pci_queue_select(d, index);
+    d->bus->queue_select(d, index);
     vqpci->vq.vdev = d;
     vqpci->vq.index = index;
-    vqpci->vq.size = qvirtio_pci_get_queue_size(d);
+    vqpci->vq.size = d->bus->get_queue_size(d);
     vqpci->vq.free_head = 0;
     vqpci->vq.num_free = vqpci->vq.size;
     vqpci->vq.align = VIRTIO_PCI_VRING_ALIGN;
@@ -241,12 +242,12 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
     addr = guest_alloc(alloc, qvring_size(vqpci->vq.size,
                                           VIRTIO_PCI_VRING_ALIGN));
     qvring_init(qvpcidev->pdev->bus->qts, alloc, &vqpci->vq, addr);
-    qvirtio_pci_set_queue_address(d, &vqpci->vq);
+    d->bus->set_queue_address(d, &vqpci->vq);
 
     return &vqpci->vq;
 }
 
-static void qvirtio_pci_virtqueue_cleanup(QVirtQueue *vq,
+void qvirtio_pci_virtqueue_cleanup_common(QVirtQueue *vq,
                                           QGuestAllocator *alloc)
 {
     QVirtQueuePCI *vqpci = container_of(vq, QVirtQueuePCI, vq);
@@ -276,8 +277,8 @@ const QVirtioBus qvirtio_pci = {
     .queue_select = qvirtio_pci_queue_select,
     .get_queue_size = qvirtio_pci_get_queue_size,
     .set_queue_address = qvirtio_pci_set_queue_address,
-    .virtqueue_setup = qvirtio_pci_virtqueue_setup,
-    .virtqueue_cleanup = qvirtio_pci_virtqueue_cleanup,
+    .virtqueue_setup = qvirtio_pci_virtqueue_setup_common,
+    .virtqueue_cleanup = qvirtio_pci_virtqueue_cleanup_common,
     .virtqueue_kick = qvirtio_pci_virtqueue_kick,
 };
 
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 14/16] libqos: make the virtio-pci BAR index configurable
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
                   ` (12 preceding siblings ...)
  2019-10-23 10:04 ` [PATCH v4 13/16] libqos: expose common virtqueue setup/cleanup functions Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 10:04 ` [PATCH v4 15/16] libqos: extract Legacy virtio-pci.c code Stefan Hajnoczi
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini,
	Philippe Mathieu-Daudé,
	Richard Henderson

The Legacy virtio-pci interface always uses BAR 0.  VIRTIO 1.0 may need
to use a different BAR index, so make it configurable.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v3:
 * Change uint8_t bar_idx to int [Thomas]
---
 tests/libqos/virtio-pci.h | 2 ++
 tests/libqos/virtio-pci.c | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
index 0e4a8b7b00..78a1c15c2a 100644
--- a/tests/libqos/virtio-pci.h
+++ b/tests/libqos/virtio-pci.h
@@ -25,6 +25,8 @@ typedef struct QVirtioPCIDevice {
     uint16_t config_msix_entry;
     uint64_t config_msix_addr;
     uint32_t config_msix_data;
+
+    int bar_idx;
 } QVirtioPCIDevice;
 
 struct QVirtioPCIMSIXOps {
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index c900742f96..e9595603f5 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -310,7 +310,7 @@ static const QVirtioPCIMSIXOps qvirtio_pci_msix_ops_legacy = {
 void qvirtio_pci_device_enable(QVirtioPCIDevice *d)
 {
     qpci_device_enable(d->pdev);
-    d->bar = qpci_iomap(d->pdev, 0, NULL);
+    d->bar = qpci_iomap(d->pdev, d->bar_idx, NULL);
 }
 
 void qvirtio_pci_device_disable(QVirtioPCIDevice *d)
@@ -400,6 +400,7 @@ static void qvirtio_pci_init_from_pcidev(QVirtioPCIDevice *dev, QPCIDevice *pci_
 {
     dev->pdev = pci_dev;
     dev->vdev.device_type = qpci_config_readw(pci_dev, PCI_SUBSYSTEM_ID);
+    dev->bar_idx = 0;
 
     dev->config_msix_entry = -1;
     dev->msix_ops = &qvirtio_pci_msix_ops_legacy;
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 15/16] libqos: extract Legacy virtio-pci.c code
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
                   ` (13 preceding siblings ...)
  2019-10-23 10:04 ` [PATCH v4 14/16] libqos: make the virtio-pci BAR index configurable Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 10:04 ` [PATCH v4 16/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
  2019-10-25 11:32 ` [PATCH v4 00/16] " Michael S. Tsirkin
  16 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini,
	Philippe Mathieu-Daudé,
	Richard Henderson

The current libqos virtio-pci.c code implements the VIRTIO Legacy
interface.  Extract existing code in preparation for VIRTIO 1.0 support.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/libqos/virtio-pci.h |  2 --
 tests/libqos/virtio-pci.c | 29 ++++++++++++-----------------
 2 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
index 78a1c15c2a..6b3a385b06 100644
--- a/tests/libqos/virtio-pci.h
+++ b/tests/libqos/virtio-pci.h
@@ -45,8 +45,6 @@ typedef struct QVirtQueuePCI {
     uint32_t msix_data;
 } QVirtQueuePCI;
 
-extern const QVirtioBus qvirtio_pci;
-
 void virtio_pci_init(QVirtioPCIDevice *dev, QPCIBus *bus, QPCIAddress * addr);
 QVirtioPCIDevice *virtio_pci_new(QPCIBus *bus, QPCIAddress * addr);
 
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index e9595603f5..11866f7772 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -35,14 +35,6 @@
  * original qvirtio_pci_destructor and qvirtio_pci_start_hw.
  */
 
-static inline bool qvirtio_pci_is_big_endian(QVirtioPCIDevice *dev)
-{
-    QPCIBus *bus = dev->pdev->bus;
-
-    /* FIXME: virtio 1.0 is always little-endian */
-    return qtest_big_endian(bus->qts);
-}
-
 #define CONFIG_BASE(dev) (VIRTIO_PCI_CONFIG_OFF((dev)->pdev->msix_enabled))
 
 static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t off)
@@ -55,8 +47,7 @@ static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t off)
  * but virtio ( < 1.0) is in guest order
  * so with a big-endian guest the order has been reversed,
  * reverse it again
- * virtio-1.0 is always little-endian, like PCI, but this
- * case will be managed inside qvirtio_pci_is_big_endian()
+ * virtio-1.0 is always little-endian, like PCI
  */
 
 static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t off)
@@ -262,7 +253,7 @@ static void qvirtio_pci_virtqueue_kick(QVirtioDevice *d, QVirtQueue *vq)
     qpci_io_writew(dev->pdev, dev->bar, VIRTIO_PCI_QUEUE_NOTIFY, vq->index);
 }
 
-const QVirtioBus qvirtio_pci = {
+static const QVirtioBus qvirtio_pci_legacy = {
     .config_readb = qvirtio_pci_config_readb,
     .config_readw = qvirtio_pci_config_readw,
     .config_readl = qvirtio_pci_config_readl,
@@ -396,17 +387,21 @@ void qvirtio_pci_start_hw(QOSGraphObject *obj)
     qvirtio_start_device(&dev->vdev);
 }
 
+static void qvirtio_pci_init_legacy(QVirtioPCIDevice *dev)
+{
+    dev->vdev.device_type = qpci_config_readw(dev->pdev, PCI_SUBSYSTEM_ID);
+    dev->bar_idx = 0;
+    dev->vdev.bus = &qvirtio_pci_legacy;
+    dev->msix_ops = &qvirtio_pci_msix_ops_legacy;
+    dev->vdev.big_endian = qtest_big_endian(dev->pdev->bus->qts);
+}
+
 static void qvirtio_pci_init_from_pcidev(QVirtioPCIDevice *dev, QPCIDevice *pci_dev)
 {
     dev->pdev = pci_dev;
-    dev->vdev.device_type = qpci_config_readw(pci_dev, PCI_SUBSYSTEM_ID);
-    dev->bar_idx = 0;
-
     dev->config_msix_entry = -1;
-    dev->msix_ops = &qvirtio_pci_msix_ops_legacy;
 
-    dev->vdev.bus = &qvirtio_pci;
-    dev->vdev.big_endian = qvirtio_pci_is_big_endian(dev);
+    qvirtio_pci_init_legacy(dev);
 
     /* each virtio-xxx-pci device should override at least this function */
     dev->obj.get_driver = NULL;
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 16/16] libqos: add VIRTIO PCI 1.0 support
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
                   ` (14 preceding siblings ...)
  2019-10-23 10:04 ` [PATCH v4 15/16] libqos: extract Legacy virtio-pci.c code Stefan Hajnoczi
@ 2019-10-23 10:04 ` Stefan Hajnoczi
  2019-10-23 11:22   ` Thomas Huth
  2019-10-25 11:32 ` [PATCH v4 00/16] " Michael S. Tsirkin
  16 siblings, 1 reply; 23+ messages in thread
From: Stefan Hajnoczi @ 2019-10-23 10:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Michael S. Tsirkin, Cornelia Huck, Stefan Hajnoczi,
	Christophe de Dinechin, Paolo Bonzini, Richard Henderson

Implement the VIRTIO 1.0 virtio-pci interface.  The main change here is
that the register layout is no longer a fixed layout in BAR 0.  Instead
we have to iterate of PCI Capabilities to find descriptions of where
various registers are located.  The vring registers are also more
fine-grained, allowing for more flexible vring layouts, but we don't
take advantage of that.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
---
 tests/Makefile.include           |   1 +
 tests/libqos/virtio-pci-modern.h |  17 ++
 tests/libqos/virtio-pci.h        |  10 +
 tests/libqos/virtio-pci-modern.c | 443 +++++++++++++++++++++++++++++++
 tests/libqos/virtio-pci.c        |   6 +-
 5 files changed, 476 insertions(+), 1 deletion(-)
 create mode 100644 tests/libqos/virtio-pci-modern.h
 create mode 100644 tests/libqos/virtio-pci-modern.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3543451ed3..3f633c8313 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -715,6 +715,7 @@ qos-test-obj-y += tests/libqos/virtio-blk.o
 qos-test-obj-y += tests/libqos/virtio-mmio.o
 qos-test-obj-y += tests/libqos/virtio-net.o
 qos-test-obj-y += tests/libqos/virtio-pci.o
+qos-test-obj-y += tests/libqos/virtio-pci-modern.o
 qos-test-obj-y += tests/libqos/virtio-rng.o
 qos-test-obj-y += tests/libqos/virtio-scsi.o
 qos-test-obj-y += tests/libqos/virtio-serial.o
diff --git a/tests/libqos/virtio-pci-modern.h b/tests/libqos/virtio-pci-modern.h
new file mode 100644
index 0000000000..6bf2b207c3
--- /dev/null
+++ b/tests/libqos/virtio-pci-modern.h
@@ -0,0 +1,17 @@
+/*
+ * libqos virtio PCI VIRTIO 1.0 definitions
+ *
+ * Copyright (c) 2019 Red Hat, Inc
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef LIBQOS_VIRTIO_PCI_MODERN_H
+#define LIBQOS_VIRTIO_PCI_MODERN_H
+
+#include "virtio-pci.h"
+
+bool qvirtio_pci_init_virtio_1(QVirtioPCIDevice *dev);
+
+#endif /* LIBQOS_VIRTIO_PCI_MODERN_H */
diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
index 6b3a385b06..294d5567ee 100644
--- a/tests/libqos/virtio-pci.h
+++ b/tests/libqos/virtio-pci.h
@@ -27,6 +27,13 @@ typedef struct QVirtioPCIDevice {
     uint32_t config_msix_data;
 
     int bar_idx;
+
+    /* VIRTIO 1.0 */
+    uint32_t common_cfg_offset;
+    uint32_t notify_cfg_offset;
+    uint32_t notify_off_multiplier;
+    uint32_t isr_cfg_offset;
+    uint32_t device_cfg_offset;
 } QVirtioPCIDevice;
 
 struct QVirtioPCIMSIXOps {
@@ -43,6 +50,9 @@ typedef struct QVirtQueuePCI {
     uint16_t msix_entry;
     uint64_t msix_addr;
     uint32_t msix_data;
+
+    /* VIRTIO 1.0 */
+    uint64_t notify_offset;
 } QVirtQueuePCI;
 
 void virtio_pci_init(QVirtioPCIDevice *dev, QPCIBus *bus, QPCIAddress * addr);
diff --git a/tests/libqos/virtio-pci-modern.c b/tests/libqos/virtio-pci-modern.c
new file mode 100644
index 0000000000..18d118866f
--- /dev/null
+++ b/tests/libqos/virtio-pci-modern.c
@@ -0,0 +1,443 @@
+/*
+ * libqos VIRTIO 1.0 PCI driver
+ *
+ * Copyright (c) 2019 Red Hat, Inc
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "standard-headers/linux/pci_regs.h"
+#include "standard-headers/linux/virtio_pci.h"
+#include "standard-headers/linux/virtio_config.h"
+#include "virtio-pci-modern.h"
+
+static uint8_t config_readb(QVirtioDevice *d, uint64_t addr)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+    return qpci_io_readb(dev->pdev, dev->bar, dev->device_cfg_offset + addr);
+}
+
+static uint16_t config_readw(QVirtioDevice *d, uint64_t addr)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+    return qpci_io_readw(dev->pdev, dev->bar, dev->device_cfg_offset + addr);
+}
+
+static uint32_t config_readl(QVirtioDevice *d, uint64_t addr)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+    return qpci_io_readl(dev->pdev, dev->bar, dev->device_cfg_offset + addr);
+}
+
+static uint64_t config_readq(QVirtioDevice *d, uint64_t addr)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+    return qpci_io_readq(dev->pdev, dev->bar, dev->device_cfg_offset + addr);
+}
+
+static uint64_t get_features(QVirtioDevice *d)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+    uint64_t lo, hi;
+
+    qpci_io_writel(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg,
+                            device_feature_select),
+                   0);
+    lo = qpci_io_readl(dev->pdev, dev->bar, dev->common_cfg_offset +
+                       offsetof(struct virtio_pci_common_cfg, device_feature));
+
+    qpci_io_writel(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg,
+                            device_feature_select),
+                   1);
+    hi = qpci_io_readl(dev->pdev, dev->bar, dev->common_cfg_offset +
+                       offsetof(struct virtio_pci_common_cfg, device_feature));
+
+    return (hi << 32) | lo;
+}
+
+static void set_features(QVirtioDevice *d, uint64_t features)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+
+    /* Drivers must enable VIRTIO 1.0 or else use the Legacy interface */
+    g_assert_cmphex(features & (1ull << VIRTIO_F_VERSION_1), !=, 0);
+
+    qpci_io_writel(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg,
+                            guest_feature_select),
+                   0);
+    qpci_io_writel(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg,
+                            guest_feature),
+                   features);
+    qpci_io_writel(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg,
+                            guest_feature_select),
+                   1);
+    qpci_io_writel(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg,
+                            guest_feature),
+                   features >> 32);
+}
+
+static uint64_t get_guest_features(QVirtioDevice *d)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+    uint64_t lo, hi;
+
+    qpci_io_writel(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg,
+                            guest_feature_select),
+                   0);
+    lo = qpci_io_readl(dev->pdev, dev->bar, dev->common_cfg_offset +
+                       offsetof(struct virtio_pci_common_cfg, guest_feature));
+
+    qpci_io_writel(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg,
+                            guest_feature_select),
+                   1);
+    hi = qpci_io_readl(dev->pdev, dev->bar, dev->common_cfg_offset +
+                       offsetof(struct virtio_pci_common_cfg, guest_feature));
+
+    return (hi << 32) | lo;
+}
+
+static uint8_t get_status(QVirtioDevice *d)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+
+    return qpci_io_readb(dev->pdev, dev->bar, dev->common_cfg_offset +
+                         offsetof(struct virtio_pci_common_cfg,
+                                  device_status));
+}
+
+static void set_status(QVirtioDevice *d, uint8_t status)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+
+    return qpci_io_writeb(dev->pdev, dev->bar, dev->common_cfg_offset +
+                          offsetof(struct virtio_pci_common_cfg,
+                                   device_status),
+                          status);
+}
+
+static bool get_msix_status(QVirtioPCIDevice *dev, uint32_t msix_entry,
+                            uint32_t msix_addr, uint32_t msix_data)
+{
+    uint32_t data;
+
+    g_assert_cmpint(msix_entry, !=, -1);
+    if (qpci_msix_masked(dev->pdev, msix_entry)) {
+        /* No ISR checking should be done if masked, but read anyway */
+        return qpci_msix_pending(dev->pdev, msix_entry);
+    }
+
+    data = qtest_readl(dev->pdev->bus->qts, msix_addr);
+    if (data == msix_data) {
+        qtest_writel(dev->pdev->bus->qts, msix_addr, 0);
+        return true;
+    } else {
+        return false;
+    }
+}
+
+static bool get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+
+    if (dev->pdev->msix_enabled) {
+        QVirtQueuePCI *vqpci = container_of(vq, QVirtQueuePCI, vq);
+
+        return get_msix_status(dev, vqpci->msix_entry, vqpci->msix_addr,
+                               vqpci->msix_data);
+    }
+
+    return qpci_io_readb(dev->pdev, dev->bar, dev->isr_cfg_offset) & 1;
+}
+
+static bool get_config_isr_status(QVirtioDevice *d)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+
+    if (dev->pdev->msix_enabled) {
+        return get_msix_status(dev, dev->config_msix_entry,
+                               dev->config_msix_addr, dev->config_msix_data);
+    }
+
+    return qpci_io_readb(dev->pdev, dev->bar, dev->isr_cfg_offset) & 2;
+}
+
+static void wait_config_isr_status(QVirtioDevice *d, gint64 timeout_us)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+    gint64 start_time = g_get_monotonic_time();
+
+    do {
+        g_assert(g_get_monotonic_time() - start_time <= timeout_us);
+        qtest_clock_step(dev->pdev->bus->qts, 100);
+    } while (!get_config_isr_status(d));
+}
+
+static void queue_select(QVirtioDevice *d, uint16_t index)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+
+    qpci_io_writew(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg, queue_select),
+                   index);
+}
+
+static uint16_t get_queue_size(QVirtioDevice *d)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+
+    return qpci_io_readw(dev->pdev, dev->bar, dev->common_cfg_offset +
+                         offsetof(struct virtio_pci_common_cfg, queue_size));
+}
+
+static void set_queue_address(QVirtioDevice *d, QVirtQueue *vq)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+
+    qpci_io_writel(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg, queue_desc_lo),
+                   vq->desc);
+    qpci_io_writel(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg, queue_desc_hi),
+                   vq->desc >> 32);
+
+    qpci_io_writel(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg, queue_avail_lo),
+                   vq->avail);
+    qpci_io_writel(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg, queue_avail_hi),
+                   vq->avail >> 32);
+
+    qpci_io_writel(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg, queue_used_lo),
+                   vq->used);
+    qpci_io_writel(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg, queue_used_hi),
+                   vq->used >> 32);
+}
+
+static QVirtQueue *virtqueue_setup(QVirtioDevice *d, QGuestAllocator *alloc,
+                                   uint16_t index)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+    QVirtQueue *vq;
+    QVirtQueuePCI *vqpci;
+    uint16_t notify_off;
+
+    vq = qvirtio_pci_virtqueue_setup_common(d, alloc, index);
+    vqpci = container_of(vq, QVirtQueuePCI, vq);
+
+    notify_off = qpci_io_readw(dev->pdev, dev->bar, dev->common_cfg_offset +
+                               offsetof(struct virtio_pci_common_cfg,
+                                        queue_notify_off));
+
+    vqpci->notify_offset = dev->notify_cfg_offset +
+                           notify_off * dev->notify_off_multiplier;
+
+    qpci_io_writew(dev->pdev, dev->bar, dev->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg, queue_enable), 1);
+
+    return vq;
+}
+
+static void virtqueue_kick(QVirtioDevice *d, QVirtQueue *vq)
+{
+    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
+    QVirtQueuePCI *vqpci = container_of(vq, QVirtQueuePCI, vq);
+
+    qpci_io_writew(dev->pdev, dev->bar, vqpci->notify_offset, vq->index);
+}
+
+static const QVirtioBus qvirtio_pci_virtio_1 = {
+    .config_readb = config_readb,
+    .config_readw = config_readw,
+    .config_readl = config_readl,
+    .config_readq = config_readq,
+    .get_features = get_features,
+    .set_features = set_features,
+    .get_guest_features = get_guest_features,
+    .get_status = get_status,
+    .set_status = set_status,
+    .get_queue_isr_status = get_queue_isr_status,
+    .wait_config_isr_status = wait_config_isr_status,
+    .queue_select = queue_select,
+    .get_queue_size = get_queue_size,
+    .set_queue_address = set_queue_address,
+    .virtqueue_setup = virtqueue_setup,
+    .virtqueue_cleanup = qvirtio_pci_virtqueue_cleanup_common,
+    .virtqueue_kick = virtqueue_kick,
+};
+
+static void set_config_vector(QVirtioPCIDevice *d, uint16_t entry)
+{
+    uint16_t vector;
+
+    qpci_io_writew(d->pdev, d->bar, d->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg, msix_config), entry);
+    vector = qpci_io_readw(d->pdev, d->bar, d->common_cfg_offset +
+                           offsetof(struct virtio_pci_common_cfg,
+                                    msix_config));
+    g_assert_cmphex(vector, !=, VIRTIO_MSI_NO_VECTOR);
+}
+
+static void set_queue_vector(QVirtioPCIDevice *d, uint16_t vq_idx,
+                             uint16_t entry)
+{
+    uint16_t vector;
+
+    queue_select(&d->vdev, vq_idx);
+    qpci_io_writew(d->pdev, d->bar, d->common_cfg_offset +
+                   offsetof(struct virtio_pci_common_cfg, queue_msix_vector),
+                   entry);
+    vector = qpci_io_readw(d->pdev, d->bar, d->common_cfg_offset +
+                           offsetof(struct virtio_pci_common_cfg,
+                                    queue_msix_vector));
+    g_assert_cmphex(vector, !=, VIRTIO_MSI_NO_VECTOR);
+}
+
+static const QVirtioPCIMSIXOps qvirtio_pci_msix_ops_virtio_1 = {
+    .set_config_vector = set_config_vector,
+    .set_queue_vector = set_queue_vector,
+};
+
+static bool probe_device_type(QVirtioPCIDevice *dev)
+{
+    uint16_t vendor_id;
+    uint16_t device_id;
+
+    /* "Drivers MUST match devices with the PCI Vendor ID 0x1AF4" */
+    vendor_id = qpci_config_readw(dev->pdev, PCI_VENDOR_ID);
+    if (vendor_id != 0x1af4) {
+        return false;
+    }
+
+    /*
+     * "Any PCI device with ... PCI Device ID 0x1000 through 0x107F inclusive
+     * is a virtio device"
+     */
+    device_id = qpci_config_readw(dev->pdev, PCI_DEVICE_ID);
+    if (device_id < 0x1000 || device_id > 0x107f) {
+        return false;
+    }
+
+    /*
+     * "Devices MAY utilize a Transitional PCI Device ID range, 0x1000 to
+     * 0x103F depending on the device type"
+     */
+    if (device_id < 0x1040) {
+        /*
+         * "Transitional devices MUST have the PCI Subsystem Device ID matching
+         * the Virtio Device ID"
+         */
+        dev->vdev.device_type = qpci_config_readw(dev->pdev, PCI_SUBSYSTEM_ID);
+    } else {
+        /*
+         * "The PCI Device ID is calculated by adding 0x1040 to the Virtio
+         * Device ID"
+         */
+        dev->vdev.device_type = device_id - 0x1040;
+    }
+
+    return true;
+}
+
+/* Find the first VIRTIO 1.0 PCI structure for a given type */
+static bool find_structure(QVirtioPCIDevice *dev, uint8_t cfg_type,
+                           uint8_t *bar, uint32_t *offset, uint32_t *length,
+                           uint8_t *cfg_addr)
+{
+    uint8_t addr = 0;
+
+    while ((addr = qpci_find_capability(dev->pdev, PCI_CAP_ID_VNDR,
+                                        addr)) != 0) {
+        uint8_t type;
+
+        type = qpci_config_readb(dev->pdev,
+                addr + offsetof(struct virtio_pci_cap, cfg_type));
+        if (type != cfg_type) {
+            continue;
+        }
+
+        *bar = qpci_config_readb(dev->pdev,
+                addr + offsetof(struct virtio_pci_cap, bar));
+        *offset = qpci_config_readl(dev->pdev,
+                addr + offsetof(struct virtio_pci_cap, offset));
+        *length = qpci_config_readl(dev->pdev,
+                addr + offsetof(struct virtio_pci_cap, length));
+        if (cfg_addr) {
+            *cfg_addr = addr;
+        }
+
+        return true;
+    }
+
+    return false;
+}
+
+static bool probe_device_layout(QVirtioPCIDevice *dev)
+{
+    uint8_t bar;
+    uint8_t cfg_addr;
+    uint32_t length;
+
+    /*
+     * Due to the qpci_iomap() API we only support devices that put all
+     * structures in the same PCI BAR.  Luckily this is true with QEMU.
+     */
+
+    if (!find_structure(dev, VIRTIO_PCI_CAP_COMMON_CFG, &bar,
+                        &dev->common_cfg_offset, &length, NULL)) {
+        return false;
+    }
+    dev->bar_idx = bar;
+
+    if (!find_structure(dev, VIRTIO_PCI_CAP_NOTIFY_CFG, &bar,
+                        &dev->notify_cfg_offset, &length, &cfg_addr)) {
+        return false;
+    }
+    g_assert_cmphex(bar, ==, dev->bar_idx);
+
+    dev->notify_off_multiplier = qpci_config_readl(dev->pdev,
+            cfg_addr + offsetof(struct virtio_pci_notify_cap,
+                                notify_off_multiplier));
+
+    if (!find_structure(dev, VIRTIO_PCI_CAP_ISR_CFG, &bar,
+                        &dev->isr_cfg_offset, &length, NULL)) {
+        return false;
+    }
+    g_assert_cmphex(bar, ==, dev->bar_idx);
+
+    if (!find_structure(dev, VIRTIO_PCI_CAP_DEVICE_CFG, &bar,
+                        &dev->device_cfg_offset, &length, NULL)) {
+        return false;
+    }
+    g_assert_cmphex(bar, ==, dev->bar_idx);
+
+    return true;
+}
+
+/* Probe a VIRTIO 1.0 device */
+bool qvirtio_pci_init_virtio_1(QVirtioPCIDevice *dev)
+{
+    if (!probe_device_type(dev)) {
+        return false;
+    }
+
+    if (!probe_device_layout(dev)) {
+        return false;
+    }
+
+    dev->vdev.bus = &qvirtio_pci_virtio_1;
+    dev->msix_ops = &qvirtio_pci_msix_ops_virtio_1;
+    dev->vdev.big_endian = false;
+    return true;
+}
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 11866f7772..62851c29bb 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -22,6 +22,8 @@
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_regs.h"
 
+#include "virtio-pci-modern.h"
+
 /* virtio-pci is a superclass of all virtio-xxx-pci devices;
  * the relation between virtio-pci and virtio-xxx-pci is implicit,
  * and therefore virtio-pci does not produce virtio and is not
@@ -401,7 +403,9 @@ static void qvirtio_pci_init_from_pcidev(QVirtioPCIDevice *dev, QPCIDevice *pci_
     dev->pdev = pci_dev;
     dev->config_msix_entry = -1;
 
-    qvirtio_pci_init_legacy(dev);
+    if (!qvirtio_pci_init_virtio_1(dev)) {
+        qvirtio_pci_init_legacy(dev);
+    }
 
     /* each virtio-xxx-pci device should override at least this function */
     dev->obj.get_driver = NULL;
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 04/16] virtio-scsi-test: add missing feature negotiation
  2019-10-23 10:04 ` [PATCH v4 04/16] virtio-scsi-test: add missing feature negotiation Stefan Hajnoczi
@ 2019-10-23 10:19   ` Thomas Huth
  0 siblings, 0 replies; 23+ messages in thread
From: Thomas Huth @ 2019-10-23 10:19 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Fam Zheng, Laurent Vivier, qemu-block, slp, Michael S. Tsirkin,
	Cornelia Huck, qemu-devel, Christophe de Dinechin, Paolo Bonzini,
	Richard Henderson

----- Original Message -----
> From: "Stefan Hajnoczi" <stefanha@redhat.com>
> Sent: Wednesday, October 23, 2019 12:04:13 PM
> 
> VIRTIO Device Initialization requires feature negotiation.  Currently
> virtio-scsi-test.c is non-compliant.
> 
> libqos tests acknowledge all feature bits advertised by the device,
> except VIRTIO_F_BAD_FEATURE (which devices use to detect broken
> drivers!) and VIRTIO_RING_F_EVENT_IDX (which is not implemented in
> libqos and accepting it would break notifications).
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/virtio-scsi-test.c | 8 ++++++++
>  1 file changed, 8 insertions(+)

Reviewed-by: Thomas Huth" <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 06/16] libqos: add missing virtio-9p feature negotiation
  2019-10-23 10:04 ` [PATCH v4 06/16] libqos: add missing virtio-9p " Stefan Hajnoczi
@ 2019-10-23 10:20   ` Thomas Huth
  0 siblings, 0 replies; 23+ messages in thread
From: Thomas Huth @ 2019-10-23 10:20 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Fam Zheng, Laurent Vivier, qemu-block, slp, Michael S. Tsirkin,
	Cornelia Huck, qemu-devel, Christophe de Dinechin, Paolo Bonzini,
	Richard Henderson

----- Original Message -----
> From: "Stefan Hajnoczi" <stefanha@redhat.com>
> Sent: Wednesday, October 23, 2019 12:04:15 PM
> 
> VIRTIO Device Initialization requires feature negotiation.  The libqos
> virtio-9p driver lacks feature negotiation and is therefore
> non-compliant.
> 
> libqos tests acknowledge all feature bits advertised by the device,
> except VIRTIO_F_BAD_FEATURE (which devices use to detect broken
> drivers!) and VIRTIO_RING_F_EVENT_IDX (which is not implemented in
> libqos and accepting it would break notifications).
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/libqos/virtio-9p.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Thomas Huth" <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 08/16] libqos: implement VIRTIO 1.0 FEATURES_OK step
  2019-10-23 10:04 ` [PATCH v4 08/16] libqos: implement VIRTIO 1.0 FEATURES_OK step Stefan Hajnoczi
@ 2019-10-23 11:04   ` Thomas Huth
  0 siblings, 0 replies; 23+ messages in thread
From: Thomas Huth @ 2019-10-23 11:04 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Fam Zheng, Laurent Vivier, qemu-block, slp, Michael S. Tsirkin,
	Cornelia Huck, qemu-devel, Christophe de Dinechin, Paolo Bonzini,
	Richard Henderson

----- Original Message -----
> From: "Stefan Hajnoczi" <stefanha@redhat.com>
> Sent: Wednesday, October 23, 2019 12:04:17 PM
> 
> Device initialization has an extra step in VIRTIO 1.0.  The FEATURES_OK
> status bit is set to indicate that feature negotiation has completed.
> The driver then reads the status register again to check that the device
> agrees with the final features.
> 
> Implement this step as part of qvirtio_set_features() instead of
> introducing a separate function.  This way all existing code works
> without modifications.
> 
> The check in qvirtio_set_driver_ok() needs to be updated because
> FEATURES_OK will be set for VIRTIO 1.0 devices.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> v4:
>  * Make FEATURES_OK change in qvirtio_set_driver_ok() clearer and
>    mention it in the commit description [Thomas]
> ---
>  tests/libqos/virtio.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)

Reviewed-by: Thomas Huth" <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 16/16] libqos: add VIRTIO PCI 1.0 support
  2019-10-23 10:04 ` [PATCH v4 16/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
@ 2019-10-23 11:22   ` Thomas Huth
  0 siblings, 0 replies; 23+ messages in thread
From: Thomas Huth @ 2019-10-23 11:22 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Fam Zheng, Laurent Vivier, qemu-block, slp, Michael S. Tsirkin,
	Cornelia Huck, qemu-devel, Christophe de Dinechin, Paolo Bonzini,
	Richard Henderson

----- Original Message -----
> From: "Stefan Hajnoczi" <stefanha@redhat.com>
> Sent: Wednesday, October 23, 2019 12:04:25 PM
> 
> Implement the VIRTIO 1.0 virtio-pci interface.  The main change here is
> that the register layout is no longer a fixed layout in BAR 0.  Instead
> we have to iterate of PCI Capabilities to find descriptions of where
> various registers are located.  The vring registers are also more
> fine-grained, allowing for more flexible vring layouts, but we don't
> take advantage of that.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Sergio Lopez <slp@redhat.com>
> ---
>  tests/Makefile.include           |   1 +
>  tests/libqos/virtio-pci-modern.h |  17 ++
>  tests/libqos/virtio-pci.h        |  10 +
>  tests/libqos/virtio-pci-modern.c | 443 +++++++++++++++++++++++++++++++
>  tests/libqos/virtio-pci.c        |   6 +-
>  5 files changed, 476 insertions(+), 1 deletion(-)
>  create mode 100644 tests/libqos/virtio-pci-modern.h
>  create mode 100644 tests/libqos/virtio-pci-modern.c

Acked-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 07/16] libqos: enforce Device Initialization order
  2019-10-23 10:04 ` [PATCH v4 07/16] libqos: enforce Device Initialization order Stefan Hajnoczi
@ 2019-10-23 11:23   ` Thomas Huth
  0 siblings, 0 replies; 23+ messages in thread
From: Thomas Huth @ 2019-10-23 11:23 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Fam Zheng, Laurent Vivier, qemu-block, slp, Michael S. Tsirkin,
	Cornelia Huck, qemu-devel, Christophe de Dinechin, Paolo Bonzini,
	Richard Henderson

----- Original Message -----
> From: "Stefan Hajnoczi" <stefanha@redhat.com>
> Sent: Wednesday, October 23, 2019 12:04:16 PM
> 
> According to VIRTIO 1.1 "3.1.1 Driver Requirements: Device
> Initialization", configuration space and virtqueues cannot be accessed
> before features have been negotiated.  Enforce this requirement.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> v4:
>  * Introduce bool d->features_negotiated so that tests can negotiate a
>    0 feature bit set in Legacy mode [Thomas]
> ---
>  tests/libqos/virtio.h | 1 +
>  tests/libqos/virtio.c | 7 +++++++
>  2 files changed, 8 insertions(+)

Reviewed-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support
  2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
                   ` (15 preceding siblings ...)
  2019-10-23 10:04 ` [PATCH v4 16/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
@ 2019-10-25 11:32 ` Michael S. Tsirkin
  16 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2019-10-25 11:32 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, slp,
	Cornelia Huck, qemu-devel, Christophe de Dinechin, Paolo Bonzini,
	Richard Henderson

On Wed, Oct 23, 2019 at 11:04:09AM +0100, Stefan Hajnoczi wrote:
> v4:
>  * Introduce bool d->features_negotiated so that tests can negotiate a
>    0 feature bit set in Legacy mode [Thomas]
>  * Make the FEATURES_OK code change in qvirtio_set_driver_ok() clearer and
>    mention it in the commit description [Thomas]
>  * Fix indentation in qvring_init() [Thomas]
> v3:
>  * Now implements VIRTIO 1.0 fully (vring, device initialization).
>    This required several new patches to address the following issues:
>    1. Tests that do not negotiate features (non-compliant!)
>    2. Tests that access configuration space before feature negotiation
>       (non-compliant!)
>    3. Tests that set up virtqueues before feature negotiation (non-compliant!)
>    4. vring accesses require byte-swapping when VIRTIO 1.0 is used with a
>       big-endian guest because the qtest_readX/writeX() API automatically
>       converts to guest-endian
>    5. VIRTIO 1.0 has an additional FEATURES_OK step during Device
>       Initialization
>  * Change uint8_t bar_idx to int [Thomas]
>  * Document qpci_find_capability() [Thomas]
>  * Every commit tested with arm, ppc64, and x86_64 targets using:
>    git rebase -i origin/master -x 'make tests/qos-test &&
>    QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 tests/qos-test &&
>    QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/qos-test'
>    QTEST_QEMU_BINARY=arm-softmmu/qemu-system-arm tests/qos-test'
> v2:
>  * Fix checkpatch.pl issues, except MAINTAINERS file warning.  libqos already
>    has maintainers and the new virtio-pci-modern.[ch] files don't need extra
>    entries since they are already covered by the existing libqos/ entry.
> 
> New VIRTIO devices are Non-Transitional.  This means they only expose the
> VIRTIO 1.0 interface, not the Legacy interface.
> 
> The libqos only supports Legacy and Transitional devices (in Legacy mode).
> This patch series adds VIRTIO 1.0 support so that tests can run against
> Non-Transitional devices too.

Very nice, thanks!
I'll queue this up in my tree.

> The virtio-fs device is Non-Transitional, so this is a prerequisite for
> virtio-fs qos tests.
> 
> Stefan Hajnoczi (16):
>   tests/virtio-blk-test: read config space after feature negotiation
>   libqos: read QVIRTIO_MMIO_VERSION register
>   libqos: extend feature bits to 64-bit
>   virtio-scsi-test: add missing feature negotiation
>   tests/virtio-blk-test: set up virtqueue after feature negotiation
>   libqos: add missing virtio-9p feature negotiation
>   libqos: enforce Device Initialization order
>   libqos: implement VIRTIO 1.0 FEATURES_OK step
>   libqos: access VIRTIO 1.0 vring in little-endian
>   libqos: add iteration support to qpci_find_capability()
>   libqos: pass full QVirtQueue to set_queue_address()
>   libqos: add MSI-X callbacks to QVirtioPCIDevice
>   libqos: expose common virtqueue setup/cleanup functions
>   libqos: make the virtio-pci BAR index configurable
>   libqos: extract Legacy virtio-pci.c code
>   libqos: add VIRTIO PCI 1.0 support
> 
>  tests/Makefile.include           |   1 +
>  tests/libqos/pci.h               |   2 +-
>  tests/libqos/virtio-mmio.h       |   1 +
>  tests/libqos/virtio-pci-modern.h |  17 ++
>  tests/libqos/virtio-pci.h        |  34 ++-
>  tests/libqos/virtio.h            |  19 +-
>  tests/libqos/pci.c               |  30 ++-
>  tests/libqos/virtio-9p.c         |   6 +
>  tests/libqos/virtio-mmio.c       |  38 ++-
>  tests/libqos/virtio-net.c        |   6 +-
>  tests/libqos/virtio-pci-modern.c | 443 +++++++++++++++++++++++++++++++
>  tests/libqos/virtio-pci.c        | 105 +++++---
>  tests/libqos/virtio.c            | 160 ++++++++---
>  tests/virtio-blk-test.c          |  66 +++--
>  tests/virtio-scsi-test.c         |   8 +
>  15 files changed, 802 insertions(+), 134 deletions(-)
>  create mode 100644 tests/libqos/virtio-pci-modern.h
>  create mode 100644 tests/libqos/virtio-pci-modern.c
> 
> -- 
> 2.21.0


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2019-10-25 11:35 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-23 10:04 [PATCH v4 00/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
2019-10-23 10:04 ` [PATCH v4 01/16] tests/virtio-blk-test: read config space after feature negotiation Stefan Hajnoczi
2019-10-23 10:04 ` [PATCH v4 02/16] libqos: read QVIRTIO_MMIO_VERSION register Stefan Hajnoczi
2019-10-23 10:04 ` [PATCH v4 03/16] libqos: extend feature bits to 64-bit Stefan Hajnoczi
2019-10-23 10:04 ` [PATCH v4 04/16] virtio-scsi-test: add missing feature negotiation Stefan Hajnoczi
2019-10-23 10:19   ` Thomas Huth
2019-10-23 10:04 ` [PATCH v4 05/16] tests/virtio-blk-test: set up virtqueue after " Stefan Hajnoczi
2019-10-23 10:04 ` [PATCH v4 06/16] libqos: add missing virtio-9p " Stefan Hajnoczi
2019-10-23 10:20   ` Thomas Huth
2019-10-23 10:04 ` [PATCH v4 07/16] libqos: enforce Device Initialization order Stefan Hajnoczi
2019-10-23 11:23   ` Thomas Huth
2019-10-23 10:04 ` [PATCH v4 08/16] libqos: implement VIRTIO 1.0 FEATURES_OK step Stefan Hajnoczi
2019-10-23 11:04   ` Thomas Huth
2019-10-23 10:04 ` [PATCH v4 09/16] libqos: access VIRTIO 1.0 vring in little-endian Stefan Hajnoczi
2019-10-23 10:04 ` [PATCH v4 10/16] libqos: add iteration support to qpci_find_capability() Stefan Hajnoczi
2019-10-23 10:04 ` [PATCH v4 11/16] libqos: pass full QVirtQueue to set_queue_address() Stefan Hajnoczi
2019-10-23 10:04 ` [PATCH v4 12/16] libqos: add MSI-X callbacks to QVirtioPCIDevice Stefan Hajnoczi
2019-10-23 10:04 ` [PATCH v4 13/16] libqos: expose common virtqueue setup/cleanup functions Stefan Hajnoczi
2019-10-23 10:04 ` [PATCH v4 14/16] libqos: make the virtio-pci BAR index configurable Stefan Hajnoczi
2019-10-23 10:04 ` [PATCH v4 15/16] libqos: extract Legacy virtio-pci.c code Stefan Hajnoczi
2019-10-23 10:04 ` [PATCH v4 16/16] libqos: add VIRTIO PCI 1.0 support Stefan Hajnoczi
2019-10-23 11:22   ` Thomas Huth
2019-10-25 11:32 ` [PATCH v4 00/16] " Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).