qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v4 00/11] Virtio PCI libqos driver
@ 2014-08-12 11:41 Marc Marí
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 01/11] tests: Functions bus_foreach and device_find from libqos virtio API Marc Marí
                   ` (10 more replies)
  0 siblings, 11 replies; 23+ messages in thread
From: Marc Marí @ 2014-08-12 11:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marc Marí, Paolo Bonzini, Stefan Hajnoczi

Add functions for virtio PCI libqos driver. Add more debugging tools. Solve
bugs found while generating tests.

v3: Solved problems, added indirect descriptor support and test for
    configuration changes
v4: Solved bugs, changed some interfaces, added MSI-X and event_idx support.

Marc Marí (11):
  tests: Functions bus_foreach and device_find from libqos virtio API
  tests: Add virtio device initialization
  libqtest: add QTEST_LOG for debugging qtest testcases
  libqos: Correct mask to align size to PAGE_SIZE in malloc-pc
  libqos: Change free function called in malloc
  virtio-blk: Correct bug in support for flexible descriptor layout
  libqos: Added basic virtqueue support to virtio implementation
  libqos: Added indirect descriptor support to virtio implementation
  libqos: Added test case for configuration changes in virtio-blk test
  libqos: Added MSI-X support
  libqos: Added EVENT_IDX support

 hw/block/virtio-blk.c     |   14 +-
 tests/Makefile            |    3 +-
 tests/libqos/malloc-pc.c  |    2 +-
 tests/libqos/malloc.h     |    2 +-
 tests/libqos/pci.c        |  110 +++++++++-
 tests/libqos/pci.h        |   10 +
 tests/libqos/virtio-pci.c |  337 ++++++++++++++++++++++++++++++
 tests/libqos/virtio-pci.h |   61 ++++++
 tests/libqos/virtio.c     |  255 +++++++++++++++++++++++
 tests/libqos/virtio.h     |  182 ++++++++++++++++
 tests/libqtest.c          |    7 +-
 tests/virtio-blk-test.c   |  501 ++++++++++++++++++++++++++++++++++++++++++++-
 12 files changed, 1463 insertions(+), 21 deletions(-)
 create mode 100644 tests/libqos/virtio-pci.c
 create mode 100644 tests/libqos/virtio-pci.h
 create mode 100644 tests/libqos/virtio.c
 create mode 100644 tests/libqos/virtio.h

-- 
1.7.10.4

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

* [Qemu-devel] [PATCH v4 01/11] tests: Functions bus_foreach and device_find from libqos virtio API
  2014-08-12 11:41 [Qemu-devel] [PATCH v4 00/11] Virtio PCI libqos driver Marc Marí
@ 2014-08-12 11:41 ` Marc Marí
  2014-08-13 16:52   ` Stefan Hajnoczi
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 02/11] tests: Add virtio device initialization Marc Marí
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Marc Marí @ 2014-08-12 11:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marc Marí, Paolo Bonzini, Stefan Hajnoczi

Virtio header has been changed to compile and work with a real device.
Functions bus_foreach and device_find have been implemented for PCI.
Virtio-blk test case now opens a fake device.

Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
 tests/Makefile            |    3 +-
 tests/libqos/virtio-pci.c |   75 +++++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/virtio-pci.h |   24 +++++++++++++++
 tests/libqos/virtio.h     |   23 ++++++++++++++
 tests/virtio-blk-test.c   |   61 +++++++++++++++++++++++++++++++-----
 5 files changed, 177 insertions(+), 9 deletions(-)
 create mode 100644 tests/libqos/virtio-pci.c
 create mode 100644 tests/libqos/virtio-pci.h
 create mode 100644 tests/libqos/virtio.h

diff --git a/tests/Makefile b/tests/Makefile
index 4b2e1bb..7c0f670 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -294,6 +294,7 @@ libqos-obj-y += tests/libqos/i2c.o
 libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
 libqos-pc-obj-y += tests/libqos/malloc-pc.o
 libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
+libqos-virtio-obj-y = $(libqos-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio-pci.o
 
 tests/rtc-test$(EXESUF): tests/rtc-test.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o
@@ -315,7 +316,7 @@ tests/vmxnet3-test$(EXESUF): tests/vmxnet3-test.o
 tests/ne2000-test$(EXESUF): tests/ne2000-test.o
 tests/wdt_ib700-test$(EXESUF): tests/wdt_ib700-test.o
 tests/virtio-balloon-test$(EXESUF): tests/virtio-balloon-test.o
-tests/virtio-blk-test$(EXESUF): tests/virtio-blk-test.o
+tests/virtio-blk-test$(EXESUF): tests/virtio-blk-test.o $(libqos-virtio-obj-y)
 tests/virtio-net-test$(EXESUF): tests/virtio-net-test.o
 tests/virtio-rng-test$(EXESUF): tests/virtio-rng-test.o
 tests/virtio-scsi-test$(EXESUF): tests/virtio-scsi-test.o
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
new file mode 100644
index 0000000..fde1b1f
--- /dev/null
+++ b/tests/libqos/virtio-pci.c
@@ -0,0 +1,75 @@
+/*
+ * libqos virtio PCI driver
+ *
+ * Copyright (c) 2014 Marc Marí
+ *
+ * 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 <glib.h>
+#include "libqtest.h"
+#include "libqos/virtio.h"
+#include "libqos/virtio-pci.h"
+#include "libqos/pci.h"
+#include "libqos/pci-pc.h"
+
+#include "hw/pci/pci_regs.h"
+
+typedef struct QVirtioPCIForeachData {
+    void (*func)(QVirtioDevice *d, void *data);
+    uint16_t device_type;
+    void *user_data;
+} QVirtioPCIForeachData;
+
+static QVirtioPCIDevice *qpcidevice_to_qvirtiodevice(QPCIDevice *pdev)
+{
+    QVirtioPCIDevice *vpcidev;
+    vpcidev = g_malloc0(sizeof(*vpcidev));
+
+    if (pdev) {
+        vpcidev->pdev = pdev;
+        vpcidev->vdev.device_type =
+                            qpci_config_readw(vpcidev->pdev, PCI_SUBSYSTEM_ID);
+    }
+
+    return vpcidev;
+}
+
+static void qvirtio_pci_foreach_callback(
+                        QPCIDevice *dev, int devfn, void *data)
+{
+    QVirtioPCIForeachData *d = data;
+    QVirtioPCIDevice *vpcidev = qpcidevice_to_qvirtiodevice(dev);
+
+    if (vpcidev->vdev.device_type == d->device_type) {
+        d->func(&vpcidev->vdev, d->user_data);
+    } else {
+        g_free(vpcidev);
+    }
+}
+
+static void qvirtio_pci_assign_device(QVirtioDevice *d, void *data)
+{
+    QVirtioPCIDevice **vpcidev = data;
+    *vpcidev = (QVirtioPCIDevice *)d;
+}
+
+void qvirtio_pci_foreach(QPCIBus *bus, uint16_t device_type,
+                void (*func)(QVirtioDevice *d, void *data), void *data)
+{
+    QVirtioPCIForeachData d = { .func = func,
+                                .device_type = device_type,
+                                .user_data = data };
+
+    qpci_device_foreach(bus, QVIRTIO_VENDOR_ID, -1,
+                                qvirtio_pci_foreach_callback, &d);
+}
+
+QVirtioPCIDevice *qvirtio_pci_device_find(QPCIBus *bus, uint16_t device_type)
+{
+    QVirtioPCIDevice *dev = NULL;
+    qvirtio_pci_foreach(bus, device_type, qvirtio_pci_assign_device, &dev);
+
+    return dev;
+}
diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
new file mode 100644
index 0000000..5101abb
--- /dev/null
+++ b/tests/libqos/virtio-pci.h
@@ -0,0 +1,24 @@
+/*
+ * libqos virtio PCI definitions
+ *
+ * Copyright (c) 2014 Marc Marí
+ *
+ * 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_H
+#define LIBQOS_VIRTIO_PCI_H
+
+#include "libqos/virtio.h"
+#include "libqos/pci.h"
+
+typedef struct QVirtioPCIDevice {
+    QVirtioDevice vdev;
+    QPCIDevice *pdev;
+} QVirtioPCIDevice;
+
+void qvirtio_pci_foreach(QPCIBus *bus, uint16_t device_type,
+                void (*func)(QVirtioDevice *d, void *data), void *data);
+QVirtioPCIDevice *qvirtio_pci_device_find(QPCIBus *bus, uint16_t device_type);
+#endif
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
new file mode 100644
index 0000000..2a05798
--- /dev/null
+++ b/tests/libqos/virtio.h
@@ -0,0 +1,23 @@
+/*
+ * libqos virtio definitions
+ *
+ * Copyright (c) 2014 Marc Marí
+ *
+ * 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_H
+#define LIBQOS_VIRTIO_H
+
+#define QVIRTIO_VENDOR_ID       0x1AF4
+
+#define QVIRTIO_NET_DEVICE_ID   0x1
+#define QVIRTIO_BLK_DEVICE_ID   0x2
+
+typedef struct QVirtioDevice {
+    /* Device type */
+    uint16_t device_type;
+} QVirtioDevice;
+
+#endif
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index d53f875..4d87a3e 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -2,6 +2,7 @@
  * QTest testcase for VirtIO Block Device
  *
  * Copyright (c) 2014 SUSE LINUX Products GmbH
+ * Copyright (c) 2014 Marc Marí
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
@@ -9,12 +10,59 @@
 
 #include <glib.h>
 #include <string.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdio.h>
 #include "libqtest.h"
-#include "qemu/osdep.h"
+#include "libqos/virtio.h"
+#include "libqos/virtio-pci.h"
+#include "libqos/pci-pc.h"
 
-/* Tests only initialization so far. TODO: Replace with functional tests */
-static void pci_nop(void)
+#define TEST_IMAGE_SIZE (64 * 1024 * 1024)
+#define PCI_SLOT    0x04
+#define PCI_FN      0x00
+
+static QPCIBus *test_start(void)
 {
+    char cmdline[100];
+    char tmp_path[] = "/tmp/qtest.XXXXXX";
+    int fd, ret;
+
+    /* Create a temporary raw image */
+    fd = mkstemp(tmp_path);
+    g_assert_cmpint(fd, >=, 0);
+    ret = ftruncate(fd, TEST_IMAGE_SIZE);
+    g_assert_cmpint(ret, ==, 0);
+    close(fd);
+
+    snprintf(cmdline, 100, "-drive if=none,id=drive0,file=%s "
+                            "-device virtio-blk-pci,drive=drive0,addr=%x.%x",
+                            tmp_path, PCI_SLOT, PCI_FN);
+    qtest_start(cmdline);
+    unlink(tmp_path);
+
+    return qpci_init_pc();
+}
+
+static void test_end(void)
+{
+    qtest_end();
+}
+
+static void pci_basic(void)
+{
+    QVirtioPCIDevice *dev;
+    QPCIBus *bus;
+
+    bus = test_start();
+
+    dev = qvirtio_pci_device_find(bus, QVIRTIO_BLK_DEVICE_ID);
+    g_assert(dev != NULL);
+    g_assert_cmphex(dev->vdev.device_type, ==, QVIRTIO_BLK_DEVICE_ID);
+    g_assert_cmphex(dev->pdev->devfn, ==, ((PCI_SLOT << 3) | PCI_FN));
+
+    g_free(dev);
+    test_end();
 }
 
 int main(int argc, char **argv)
@@ -22,13 +70,10 @@ int main(int argc, char **argv)
     int ret;
 
     g_test_init(&argc, &argv, NULL);
-    qtest_add_func("/virtio/blk/pci/nop", pci_nop);
 
-    qtest_start("-drive id=drv0,if=none,file=/dev/null "
-                "-device virtio-blk-pci,drive=drv0");
-    ret = g_test_run();
+    g_test_add_func("/virtio/blk/pci/basic", pci_basic);
 
-    qtest_end();
+    ret = g_test_run();
 
     return ret;
 }
-- 
1.7.10.4

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

* [Qemu-devel] [PATCH v4 02/11] tests: Add virtio device initialization
  2014-08-12 11:41 [Qemu-devel] [PATCH v4 00/11] Virtio PCI libqos driver Marc Marí
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 01/11] tests: Functions bus_foreach and device_find from libqos virtio API Marc Marí
@ 2014-08-12 11:41 ` Marc Marí
  2014-08-13 16:52   ` Stefan Hajnoczi
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 03/11] libqtest: add QTEST_LOG for debugging qtest testcases Marc Marí
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Marc Marí @ 2014-08-12 11:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marc Marí, Paolo Bonzini, Stefan Hajnoczi

Add functions to read and write virtio header fields.
Add status bit setting in virtio-blk-device.

Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
 tests/Makefile            |    2 +-
 tests/libqos/virtio-pci.c |   57 +++++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/virtio-pci.h |   18 ++++++++++++++
 tests/libqos/virtio.c     |   55 +++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/virtio.h     |   30 ++++++++++++++++++++++++
 tests/virtio-blk-test.c   |   14 +++++++++++
 6 files changed, 175 insertions(+), 1 deletion(-)
 create mode 100644 tests/libqos/virtio.c

diff --git a/tests/Makefile b/tests/Makefile
index 7c0f670..e0e203f 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -294,7 +294,7 @@ libqos-obj-y += tests/libqos/i2c.o
 libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
 libqos-pc-obj-y += tests/libqos/malloc-pc.o
 libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
-libqos-virtio-obj-y = $(libqos-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio-pci.o
+libqos-virtio-obj-y = $(libqos-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o
 
 tests/rtc-test$(EXESUF): tests/rtc-test.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index fde1b1f..f3ab578 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -55,6 +55,51 @@ static void qvirtio_pci_assign_device(QVirtioDevice *d, void *data)
     *vpcidev = (QVirtioPCIDevice *)d;
 }
 
+static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, void *addr)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    return qpci_io_readb(dev->pdev, addr);
+}
+
+static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, void *addr)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    return qpci_io_readw(dev->pdev, addr);
+}
+
+static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, void *addr)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    return qpci_io_readl(dev->pdev, addr);
+}
+
+static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, void *addr)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    return qpci_io_readl(dev->pdev, addr) | qpci_io_readl(dev->pdev, addr+4);
+}
+
+static uint8_t qvirtio_pci_get_status(QVirtioDevice *d)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_DEVICE_STATUS);
+}
+
+static void qvirtio_pci_set_status(QVirtioDevice *d, uint8_t val)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    qpci_io_writeb(dev->pdev, dev->addr + QVIRTIO_DEVICE_STATUS, val);
+}
+
+const QVirtioBus qvirtio_pci = {
+    .config_readb = qvirtio_pci_config_readb,
+    .config_readw = qvirtio_pci_config_readw,
+    .config_readl = qvirtio_pci_config_readl,
+    .config_readq = qvirtio_pci_config_readq,
+    .get_status = qvirtio_pci_get_status,
+    .set_status = qvirtio_pci_set_status,
+};
+
 void qvirtio_pci_foreach(QPCIBus *bus, uint16_t device_type,
                 void (*func)(QVirtioDevice *d, void *data), void *data)
 {
@@ -73,3 +118,15 @@ QVirtioPCIDevice *qvirtio_pci_device_find(QPCIBus *bus, uint16_t device_type)
 
     return dev;
 }
+
+void qvirtio_pci_device_enable(QVirtioPCIDevice *d)
+{
+    qpci_device_enable(d->pdev);
+    d->addr = qpci_iomap(d->pdev, 0);
+    g_assert(d->addr != NULL);
+}
+
+void qvirtio_pci_device_disable(QVirtioPCIDevice *d)
+{
+    qpci_iounmap(d->pdev, d->addr);
+}
diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
index 5101abb..26f902e 100644
--- a/tests/libqos/virtio-pci.h
+++ b/tests/libqos/virtio-pci.h
@@ -13,12 +13,30 @@
 #include "libqos/virtio.h"
 #include "libqos/pci.h"
 
+#define QVIRTIO_DEVICE_FEATURES         0x00
+#define QVIRTIO_GUEST_FEATURES          0x04
+#define QVIRTIO_QUEUE_ADDRESS           0x08
+#define QVIRTIO_QUEUE_SIZE              0x0C
+#define QVIRTIO_QUEUE_SELECT            0x0E
+#define QVIRTIO_QUEUE_NOTIFY            0x10
+#define QVIRTIO_DEVICE_STATUS           0x12
+#define QVIRTIO_ISR_STATUS              0x13
+#define QVIRTIO_MSIX_CONF_VECTOR        0x14
+#define QVIRTIO_MSIX_QUEUE_VECTOR       0x16
+#define QVIRTIO_DEVICE_SPECIFIC_MSIX    0x18
+#define QVIRTIO_DEVICE_SPECIFIC_NO_MSIX 0x14
+
 typedef struct QVirtioPCIDevice {
     QVirtioDevice vdev;
     QPCIDevice *pdev;
+    void *addr;
 } QVirtioPCIDevice;
 
+extern const QVirtioBus qvirtio_pci;
+
 void qvirtio_pci_foreach(QPCIBus *bus, uint16_t device_type,
                 void (*func)(QVirtioDevice *d, void *data), void *data);
 QVirtioPCIDevice *qvirtio_pci_device_find(QPCIBus *bus, uint16_t device_type);
+void qvirtio_pci_device_enable(QVirtioPCIDevice *d);
+void qvirtio_pci_device_disable(QVirtioPCIDevice *d);
 #endif
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
new file mode 100644
index 0000000..577d679
--- /dev/null
+++ b/tests/libqos/virtio.c
@@ -0,0 +1,55 @@
+/*
+ * libqos virtio driver
+ *
+ * Copyright (c) 2014 Marc Marí
+ *
+ * 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 <glib.h>
+#include "libqtest.h"
+#include "libqos/virtio.h"
+
+uint8_t qvirtio_config_readb(const QVirtioBus *bus, QVirtioDevice *d,
+                                                                void *addr)
+{
+    return bus->config_readb(d, addr);
+}
+
+uint16_t qvirtio_config_readw(const QVirtioBus *bus, QVirtioDevice *d,
+                                                                void *addr)
+{
+    return bus->config_readw(d, addr);
+}
+
+uint32_t qvirtio_config_readl(const QVirtioBus *bus, QVirtioDevice *d,
+                                                                void *addr)
+{
+    return bus->config_readl(d, addr);
+}
+
+uint64_t qvirtio_config_readq(const QVirtioBus *bus, QVirtioDevice *d,
+                                                                void *addr)
+{
+    return bus->config_readq(d, addr);
+}
+
+void qvirtio_reset(const QVirtioBus *bus, QVirtioDevice *d)
+{
+    bus->set_status(d, QVIRTIO_RESET);
+    g_assert_cmphex(bus->get_status(d), ==, QVIRTIO_RESET);
+}
+
+void qvirtio_set_acknowledge(const QVirtioBus *bus, QVirtioDevice *d)
+{
+    bus->set_status(d, bus->get_status(d) | QVIRTIO_ACKNOWLEDGE);
+    g_assert_cmphex(bus->get_status(d), ==, QVIRTIO_ACKNOWLEDGE);
+}
+
+void qvirtio_set_driver(const QVirtioBus *bus, QVirtioDevice *d)
+{
+    bus->set_status(d, bus->get_status(d) | QVIRTIO_DRIVER);
+    g_assert_cmphex(bus->get_status(d), ==,
+                                    QVIRTIO_DRIVER | QVIRTIO_ACKNOWLEDGE);
+}
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 2a05798..23e975b 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -12,6 +12,10 @@
 
 #define QVIRTIO_VENDOR_ID       0x1AF4
 
+#define QVIRTIO_RESET           0x0
+#define QVIRTIO_ACKNOWLEDGE     0x1
+#define QVIRTIO_DRIVER          0x2
+
 #define QVIRTIO_NET_DEVICE_ID   0x1
 #define QVIRTIO_BLK_DEVICE_ID   0x2
 
@@ -20,4 +24,30 @@ typedef struct QVirtioDevice {
     uint16_t device_type;
 } QVirtioDevice;
 
+typedef struct QVirtioBus {
+    uint8_t (*config_readb)(QVirtioDevice *d, void *addr);
+    uint16_t (*config_readw)(QVirtioDevice *d, void *addr);
+    uint32_t (*config_readl)(QVirtioDevice *d, void *addr);
+    uint64_t (*config_readq)(QVirtioDevice *d, void *addr);
+
+    /* Get status of the device */
+    uint8_t (*get_status)(QVirtioDevice *d);
+
+    /* Set status of the device  */
+    void (*set_status)(QVirtioDevice *d, uint8_t val);
+} QVirtioBus;
+
+uint8_t qvirtio_config_readb(const QVirtioBus *bus, QVirtioDevice *d,
+                                                                void *addr);
+uint16_t qvirtio_config_readw(const QVirtioBus *bus, QVirtioDevice *d,
+                                                                void *addr);
+uint32_t qvirtio_config_readl(const QVirtioBus *bus, QVirtioDevice *d,
+                                                                void *addr);
+uint64_t qvirtio_config_readq(const QVirtioBus *bus, QVirtioDevice *d,
+                                                                void *addr);
+
+void qvirtio_reset(const QVirtioBus *bus, QVirtioDevice *d);
+void qvirtio_set_acknowledge(const QVirtioBus *bus, QVirtioDevice *d);
+void qvirtio_set_driver(const QVirtioBus *bus, QVirtioDevice *d);
+
 #endif
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 4d87a3e..a534521 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -53,6 +53,8 @@ static void pci_basic(void)
 {
     QVirtioPCIDevice *dev;
     QPCIBus *bus;
+    void *addr;
+    uint64_t capacity;
 
     bus = test_start();
 
@@ -61,6 +63,18 @@ static void pci_basic(void)
     g_assert_cmphex(dev->vdev.device_type, ==, QVIRTIO_BLK_DEVICE_ID);
     g_assert_cmphex(dev->pdev->devfn, ==, ((PCI_SLOT << 3) | PCI_FN));
 
+    qvirtio_pci_device_enable(dev);
+    qvirtio_reset(&qvirtio_pci, &dev->vdev);
+    qvirtio_set_acknowledge(&qvirtio_pci, &dev->vdev);
+    qvirtio_set_driver(&qvirtio_pci, &dev->vdev);
+
+    /* MSI-X is not enabled */
+    addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX;
+
+    capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
+    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE/512);
+
+    qvirtio_pci_device_disable(dev);
     g_free(dev);
     test_end();
 }
-- 
1.7.10.4

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

* [Qemu-devel] [PATCH v4 03/11] libqtest: add QTEST_LOG for debugging qtest testcases
  2014-08-12 11:41 [Qemu-devel] [PATCH v4 00/11] Virtio PCI libqos driver Marc Marí
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 01/11] tests: Functions bus_foreach and device_find from libqos virtio API Marc Marí
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 02/11] tests: Add virtio device initialization Marc Marí
@ 2014-08-12 11:41 ` Marc Marí
  2014-08-13 16:53   ` Stefan Hajnoczi
  2014-08-13 16:56   ` Stefan Hajnoczi
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 04/11] libqos: Correct mask to align size to PAGE_SIZE in malloc-pc Marc Marí
                   ` (7 subsequent siblings)
  10 siblings, 2 replies; 23+ messages in thread
From: Marc Marí @ 2014-08-12 11:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marc Marí, Paolo Bonzini, Stefan Hajnoczi

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
 tests/libqtest.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 98e8f4b..fbd600d 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -167,11 +167,12 @@ QTestState *qtest_init(const char *extra_args)
     if (s->qemu_pid == 0) {
         command = g_strdup_printf("exec %s "
                                   "-qtest unix:%s,nowait "
-                                  "-qtest-log /dev/null "
+                                  "-qtest-log %s "
                                   "-qmp unix:%s,nowait "
                                   "-machine accel=qtest "
                                   "-display none "
                                   "%s", qemu_binary, socket_path,
+                                  getenv("QTEST_LOG") ? "/dev/fd/2" : "/dev/null",
                                   qmp_socket_path,
                                   extra_args ?: "");
         execlp("/bin/sh", "sh", "-c", command, NULL);
@@ -397,10 +398,14 @@ QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
 
     /* No need to send anything for an empty QObject.  */
     if (qobj) {
+        int log = getenv("QTEST_LOG") != NULL;
         QString *qstr = qobject_to_json(qobj);
         const char *str = qstring_get_str(qstr);
         size_t size = qstring_get_length(qstr);
 
+        if (log) {
+            fprintf(stderr, "%s", str);
+        }
         /* Send QMP request */
         socket_send(s->qmp_fd, str, size);
 
-- 
1.7.10.4

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

* [Qemu-devel] [PATCH v4 04/11] libqos: Correct mask to align size to PAGE_SIZE in malloc-pc
  2014-08-12 11:41 [Qemu-devel] [PATCH v4 00/11] Virtio PCI libqos driver Marc Marí
                   ` (2 preceding siblings ...)
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 03/11] libqtest: add QTEST_LOG for debugging qtest testcases Marc Marí
@ 2014-08-12 11:41 ` Marc Marí
  2014-08-13 16:56   ` Stefan Hajnoczi
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 05/11] libqos: Change free function called in malloc Marc Marí
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Marc Marí @ 2014-08-12 11:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marc Marí, Paolo Bonzini, Stefan Hajnoczi

Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
 tests/libqos/malloc-pc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index db1496c..2efd095 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -36,7 +36,7 @@ static uint64_t pc_alloc(QGuestAllocator *allocator, size_t size)
 
 
     size += (PAGE_SIZE - 1);
-    size &= PAGE_SIZE;
+    size &= -PAGE_SIZE;
 
     g_assert_cmpint((s->start + size), <=, s->end);
 
-- 
1.7.10.4

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

* [Qemu-devel] [PATCH v4 05/11] libqos: Change free function called in malloc
  2014-08-12 11:41 [Qemu-devel] [PATCH v4 00/11] Virtio PCI libqos driver Marc Marí
                   ` (3 preceding siblings ...)
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 04/11] libqos: Correct mask to align size to PAGE_SIZE in malloc-pc Marc Marí
@ 2014-08-12 11:41 ` Marc Marí
  2014-08-13 16:56   ` Stefan Hajnoczi
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 06/11] virtio-blk: Correct bug in support for flexible descriptor layout Marc Marí
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Marc Marí @ 2014-08-12 11:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marc Marí, Paolo Bonzini, Stefan Hajnoczi

Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
 tests/libqos/malloc.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
index 46f6000..5565381 100644
--- a/tests/libqos/malloc.h
+++ b/tests/libqos/malloc.h
@@ -32,7 +32,7 @@ static inline uint64_t guest_alloc(QGuestAllocator *allocator, size_t size)
 
 static inline void guest_free(QGuestAllocator *allocator, uint64_t addr)
 {
-    allocator->alloc(allocator, addr);
+    allocator->free(allocator, addr);
 }
 
 #endif
-- 
1.7.10.4

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

* [Qemu-devel] [PATCH v4 06/11] virtio-blk: Correct bug in support for flexible descriptor layout
  2014-08-12 11:41 [Qemu-devel] [PATCH v4 00/11] Virtio PCI libqos driver Marc Marí
                   ` (4 preceding siblings ...)
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 05/11] libqos: Change free function called in malloc Marc Marí
@ 2014-08-12 11:41 ` Marc Marí
  2014-08-13 16:56   ` Stefan Hajnoczi
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 07/11] libqos: Added basic virtqueue support to virtio implementation Marc Marí
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Marc Marí @ 2014-08-12 11:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marc Marí, Paolo Bonzini, Stefan Hajnoczi

Without this correction, only a three descriptor layout is accepted, and
requests with just two descriptors are not completed and no error message is
displayed.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
 hw/block/virtio-blk.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index c241c50..302c39e 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -404,19 +404,19 @@ void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
          * NB: per existing s/n string convention the string is
          * terminated by '\0' only when shorter than buffer.
          */
-        strncpy(req->elem.in_sg[0].iov_base,
-                s->blk.serial ? s->blk.serial : "",
-                MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
+        const char *serial = s->blk.serial ? s->blk.serial : "";
+        size_t size = MIN(strlen(serial) + 1,
+                          MIN(iov_size(in_iov, in_num),
+                              VIRTIO_BLK_ID_BYTES));
+        iov_from_buf(in_iov, in_num, 0, serial, size);
         virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
         virtio_blk_free_request(req);
     } else if (type & VIRTIO_BLK_T_OUT) {
-        qemu_iovec_init_external(&req->qiov, &req->elem.out_sg[1],
-                                 req->elem.out_num - 1);
+        qemu_iovec_init_external(&req->qiov, iov, out_num);
         virtio_blk_handle_write(req, mrb);
     } else if (type == VIRTIO_BLK_T_IN || type == VIRTIO_BLK_T_BARRIER) {
         /* VIRTIO_BLK_T_IN is 0, so we can't just & it. */
-        qemu_iovec_init_external(&req->qiov, &req->elem.in_sg[0],
-                                 req->elem.in_num - 1);
+        qemu_iovec_init_external(&req->qiov, in_iov, in_num);
         virtio_blk_handle_read(req);
     } else {
         virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
-- 
1.7.10.4

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

* [Qemu-devel] [PATCH v4 07/11] libqos: Added basic virtqueue support to virtio implementation
  2014-08-12 11:41 [Qemu-devel] [PATCH v4 00/11] Virtio PCI libqos driver Marc Marí
                   ` (5 preceding siblings ...)
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 06/11] virtio-blk: Correct bug in support for flexible descriptor layout Marc Marí
@ 2014-08-12 11:41 ` Marc Marí
  2014-08-13 17:27   ` Stefan Hajnoczi
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 08/11] libqos: Added indirect descriptor " Marc Marí
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Marc Marí @ 2014-08-12 11:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marc Marí, Paolo Bonzini, Stefan Hajnoczi

Add status changing and feature negotiation.
Add basic virtqueue support for adding and sending virtqueue requests.
Add ISR checking.

Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
 tests/libqos/virtio-pci.c |   91 ++++++++++++++++++++++++++-
 tests/libqos/virtio-pci.h |    2 +
 tests/libqos/virtio.c     |  100 +++++++++++++++++++++++++++++
 tests/libqos/virtio.h     |  101 ++++++++++++++++++++++++++++-
 tests/virtio-blk-test.c   |  154 +++++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 441 insertions(+), 7 deletions(-)

diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index f3ab578..5f92df7 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -13,6 +13,8 @@
 #include "libqos/virtio-pci.h"
 #include "libqos/pci.h"
 #include "libqos/pci-pc.h"
+#include "libqos/malloc.h"
+#include "libqos/malloc-pc.h"
 
 #include "hw/pci/pci_regs.h"
 
@@ -79,16 +81,93 @@ static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, void *addr)
     return qpci_io_readl(dev->pdev, addr) | qpci_io_readl(dev->pdev, addr+4);
 }
 
+static uint32_t qvirtio_pci_get_features(QVirtioDevice *d)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    return qpci_io_readl(dev->pdev, dev->addr + QVIRTIO_DEVICE_FEATURES);
+}
+
+static void qvirtio_pci_set_features(QVirtioDevice *d, uint32_t features)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    qpci_io_writel(dev->pdev, dev->addr + QVIRTIO_GUEST_FEATURES, features);
+}
+
 static uint8_t qvirtio_pci_get_status(QVirtioDevice *d)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
     return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_DEVICE_STATUS);
 }
 
-static void qvirtio_pci_set_status(QVirtioDevice *d, uint8_t val)
+static void qvirtio_pci_set_status(QVirtioDevice *d, uint8_t status)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    qpci_io_writeb(dev->pdev, dev->addr + QVIRTIO_DEVICE_STATUS, status);
+}
+
+static uint8_t qvirtio_pci_get_isr_status(QVirtioDevice *d)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_ISR_STATUS);
+}
+
+static void qvirtio_pci_queue_select(QVirtioDevice *d, uint16_t index)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    qpci_io_writeb(dev->pdev, dev->addr + QVIRTIO_QUEUE_SELECT, index);
+}
+
+static uint16_t qvirtio_pci_get_queue_size(QVirtioDevice *d)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    return qpci_io_readw(dev->pdev, dev->addr + QVIRTIO_QUEUE_SIZE);
+}
+
+static void qvirtio_pci_set_queue_address(QVirtioDevice *d, uint32_t pfn)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    qpci_io_writel(dev->pdev, dev->addr + QVIRTIO_QUEUE_ADDRESS, pfn);
+}
+
+static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
+                                        QGuestAllocator *alloc, uint16_t index)
+{
+    uint16_t aux;
+    uint64_t addr;
+    QVirtQueue *vq;
+
+    vq = g_malloc0(sizeof(*vq));
+
+    qvirtio_pci_queue_select(d, index);
+    vq->index = index;
+    vq->size = qvirtio_pci_get_queue_size(d);
+    vq->free_head = 0;
+    vq->num_free = vq->size;
+    vq->align = QVIRTIO_PCI_ALIGN;
+
+    /* Check different than 0 */
+    g_assert_cmpint(vq->size, !=, 0);
+
+    /* Check power of 2 */
+    aux = vq->size;
+    while ((aux & 1) != 0) {
+        aux = aux >> 1;
+    }
+    g_assert_cmpint(aux, !=, 1);
+
+    addr = guest_alloc(alloc, qvring_size(vq->size, QVIRTIO_PCI_ALIGN));
+    qvring_init(alloc, vq, addr);
+    qvirtio_pci_set_queue_address(d, vq->desc/4096);
+
+    /* TODO: MSI-X configuration */
+
+    return vq;
+}
+
+static void qvirtio_pci_virtqueue_kick(QVirtioDevice *d, QVirtQueue *vq)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-    qpci_io_writeb(dev->pdev, dev->addr + QVIRTIO_DEVICE_STATUS, val);
+    qpci_io_writew(dev->pdev, dev->addr + QVIRTIO_QUEUE_NOTIFY, vq->index);
 }
 
 const QVirtioBus qvirtio_pci = {
@@ -96,8 +175,16 @@ const QVirtioBus qvirtio_pci = {
     .config_readw = qvirtio_pci_config_readw,
     .config_readl = qvirtio_pci_config_readl,
     .config_readq = qvirtio_pci_config_readq,
+    .get_features = qvirtio_pci_get_features,
+    .set_features = qvirtio_pci_set_features,
     .get_status = qvirtio_pci_get_status,
     .set_status = qvirtio_pci_set_status,
+    .get_isr_status = qvirtio_pci_get_isr_status,
+    .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_kick = qvirtio_pci_virtqueue_kick,
 };
 
 void qvirtio_pci_foreach(QPCIBus *bus, uint16_t device_type,
diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
index 26f902e..40bd12d 100644
--- a/tests/libqos/virtio-pci.h
+++ b/tests/libqos/virtio-pci.h
@@ -26,6 +26,8 @@
 #define QVIRTIO_DEVICE_SPECIFIC_MSIX    0x18
 #define QVIRTIO_DEVICE_SPECIFIC_NO_MSIX 0x14
 
+#define QVIRTIO_PCI_ALIGN   4096
+
 typedef struct QVirtioPCIDevice {
     QVirtioDevice vdev;
     QPCIDevice *pdev;
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index 577d679..762bc4c 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -35,6 +35,23 @@ uint64_t qvirtio_config_readq(const QVirtioBus *bus, QVirtioDevice *d,
     return bus->config_readq(d, addr);
 }
 
+uint32_t qvirtio_get_features(const QVirtioBus *bus, QVirtioDevice *d)
+{
+    return bus->get_features(d);
+}
+
+void qvirtio_set_features(const QVirtioBus *bus, QVirtioDevice *d,
+                                                            uint32_t features)
+{
+    bus->set_features(d, features);
+}
+
+QVirtQueue *qvirtio_virtqueue_setup(const QVirtioBus *bus, QVirtioDevice *d,
+                                        QGuestAllocator *alloc, uint16_t index)
+{
+    return bus->virtqueue_setup(d, alloc, index);
+}
+
 void qvirtio_reset(const QVirtioBus *bus, QVirtioDevice *d)
 {
     bus->set_status(d, QVIRTIO_RESET);
@@ -53,3 +70,86 @@ void qvirtio_set_driver(const QVirtioBus *bus, QVirtioDevice *d)
     g_assert_cmphex(bus->get_status(d), ==,
                                     QVIRTIO_DRIVER | QVIRTIO_ACKNOWLEDGE);
 }
+
+void qvirtio_set_driver_ok(const QVirtioBus *bus, QVirtioDevice *d)
+{
+    bus->set_status(d, bus->get_status(d) | QVIRTIO_DRIVER_OK);
+    g_assert_cmphex(bus->get_status(d), ==,
+                QVIRTIO_DRIVER_OK | QVIRTIO_DRIVER | QVIRTIO_ACKNOWLEDGE);
+}
+
+bool qvirtio_wait_isr(const QVirtioBus *bus, QVirtioDevice *d, uint8_t mask,
+                                                            uint64_t timeout)
+{
+    do {
+        clock_step(10);
+        if (bus->get_isr_status(d) & mask) {
+            break; /* It has ended */
+        }
+    } while (--timeout);
+
+    return timeout != 0;
+}
+
+void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq, uint64_t addr)
+{
+    int i;
+
+    vq->desc = addr;
+    vq->avail = vq->desc + vq->size*sizeof(QVRingDesc);
+    vq->used = (uint64_t)((vq->avail + sizeof(uint16_t) * (3 + vq->size)
+        +vq->align-1) & ~(vq->align - 1));
+
+    for (i = 0; i < vq->size-1; i++) {
+        /* vq->desc[i].addr */
+        writew(vq->desc+(16*i), 0);
+        /* vq->desc[i].next */
+        writew(vq->desc+(16*i)+14, i+1);
+    }
+
+    /* vq->avail->flags */
+    writew(vq->avail, 0);
+    /* vq->avail->idx */
+    writew(vq->avail+2, 0);
+
+    /* vq->used->flags */
+    writew(vq->used, 0);
+}
+
+uint32_t qvirtqueue_add(QVirtQueue *vq, uint64_t data, uint32_t len, bool write,
+                                                                    bool next)
+{
+    uint16_t flags = 0;
+    vq->num_free--;
+
+    if (write) {
+        flags |= QVRING_DESC_F_WRITE;
+    }
+
+    if (next) {
+        flags |= QVRING_DESC_F_NEXT;
+    }
+
+    /* vq->desc[vq->free_head].addr */
+    writeq(vq->desc+(16*vq->free_head), data);
+    /* vq->desc[vq->free_head].len */
+    writel(vq->desc+(16*vq->free_head)+8, len);
+    /* vq->desc[vq->free_head].flags */
+    writew(vq->desc+(16*vq->free_head)+12, flags);
+
+    return vq->free_head++; /* Return and increase, in this order */
+}
+
+void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d, QVirtQueue *vq,
+                                                            uint32_t free_head)
+{
+    /* vq->avail->idx */
+    uint16_t idx = readl(vq->avail+2);
+
+    /* vq->avail->ring[idx % vq->size] */
+    writel(vq->avail+4+(2*(idx % vq->size)), free_head);
+    /* vq->avail->idx */
+    writel(vq->avail+2, idx+1);
+
+    bus->virtqueue_kick(d, vq);
+}
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 23e975b..7a38910 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -10,33 +10,117 @@
 #ifndef LIBQOS_VIRTIO_H
 #define LIBQOS_VIRTIO_H
 
+#include "libqos/malloc.h"
+
 #define QVIRTIO_VENDOR_ID       0x1AF4
 
 #define QVIRTIO_RESET           0x0
 #define QVIRTIO_ACKNOWLEDGE     0x1
 #define QVIRTIO_DRIVER          0x2
+#define QVIRTIO_DRIVER_OK       0x4
 
 #define QVIRTIO_NET_DEVICE_ID   0x1
 #define QVIRTIO_BLK_DEVICE_ID   0x2
 
+#define QVRING_DESC_F_NEXT      0x1
+#define QVRING_DESC_F_WRITE     0x2
+#define QVRING_DESC_F_INDIRECT  0x4
+
+#define QVIRTIO_F_NOTIFY_ON_EMPTY       0x01000000
+#define QVIRTIO_F_ANY_LAYOUT            0x08000000
+#define QVIRTIO_F_RING_INDIRECT_DESC    0x10000000
+#define QVIRTIO_F_RING_EVENT_IDX        0x20000000
+#define QVIRTIO_F_BAD_FEATURE           0x40000000
+
+#define QVRING_AVAIL_F_NO_INTERRUPT     1
+
+#define QVRING_USED_F_NO_NOTIFY     1
+
 typedef struct QVirtioDevice {
     /* Device type */
     uint16_t device_type;
 } QVirtioDevice;
 
+typedef struct QVRingDesc {
+    uint64_t addr;
+    uint32_t len;
+    uint16_t flags;
+    uint16_t next;
+} QVRingDesc;
+
+typedef struct QVRingAvail {
+    uint16_t flags;
+    uint16_t idx;
+    uint16_t ring[0]; /* This is an array of uint16_t */
+} QVRingAvail;
+
+typedef struct QVRingUsedElem {
+    uint32_t id;
+    uint32_t len;
+} QVRingUsedElem;
+
+typedef struct QVRingUsed {
+    uint16_t flags;
+    uint16_t idx;
+    QVRingUsedElem ring[0]; /* This is an array of QVRingUsedElem structs */
+} QVRingUsed;
+
+typedef struct QVirtQueue {
+    uint64_t desc; /* This points to an array of QVRingDesc */
+    uint64_t avail; /* This points to a QVRingAvail */
+    uint64_t used; /* This points to a QVRingDesc */
+    uint16_t index;
+    uint32_t size;
+    uint32_t free_head;
+    uint32_t num_free;
+    uint32_t align;
+} QVirtQueue;
+
 typedef struct QVirtioBus {
     uint8_t (*config_readb)(QVirtioDevice *d, void *addr);
     uint16_t (*config_readw)(QVirtioDevice *d, void *addr);
     uint32_t (*config_readl)(QVirtioDevice *d, void *addr);
     uint64_t (*config_readq)(QVirtioDevice *d, void *addr);
 
+    /* Get features of the device */
+    uint32_t (*get_features)(QVirtioDevice *d);
+
+    /* Get features of the device */
+    void (*set_features)(QVirtioDevice *d, uint32_t features);
+
     /* Get status of the device */
     uint8_t (*get_status)(QVirtioDevice *d);
 
     /* Set status of the device  */
-    void (*set_status)(QVirtioDevice *d, uint8_t val);
+    void (*set_status)(QVirtioDevice *d, uint8_t status);
+
+    /* Get the ISR status of the device */
+    uint8_t (*get_isr_status)(QVirtioDevice *d);
+
+    /* Select a queue to work on */
+    void (*queue_select)(QVirtioDevice *d, uint16_t index);
+
+    /* Get the size of the selected queue */
+    uint16_t (*get_queue_size)(QVirtioDevice *d);
+
+    /* Set the address of the selected queue */
+    void (*set_queue_address)(QVirtioDevice *d, uint32_t pfn);
+
+    /* Setup the virtqueue specified by index */
+    QVirtQueue *(*virtqueue_setup)(QVirtioDevice *d, QGuestAllocator *alloc,
+                                                                uint16_t index);
+
+    /* Notify changes in virtqueue */
+    void (*virtqueue_kick)(QVirtioDevice *d, QVirtQueue *vq);
 } QVirtioBus;
 
+static inline unsigned qvring_size(uint32_t num, uint32_t align)
+{
+    return ((sizeof(struct QVRingDesc) * num + sizeof(uint16_t) * (3 + num)
+        + align - 1) & ~(align - 1))
+        + sizeof(uint16_t) * 3 + sizeof(struct QVRingUsedElem) * num;
+}
+
 uint8_t qvirtio_config_readb(const QVirtioBus *bus, QVirtioDevice *d,
                                                                 void *addr);
 uint16_t qvirtio_config_readw(const QVirtioBus *bus, QVirtioDevice *d,
@@ -45,9 +129,24 @@ uint32_t qvirtio_config_readl(const QVirtioBus *bus, QVirtioDevice *d,
                                                                 void *addr);
 uint64_t qvirtio_config_readq(const QVirtioBus *bus, QVirtioDevice *d,
                                                                 void *addr);
+uint32_t qvirtio_get_features(const QVirtioBus *bus, QVirtioDevice *d);
+void qvirtio_set_features(const QVirtioBus *bus, QVirtioDevice *d,
+                                                            uint32_t features);
 
 void qvirtio_reset(const QVirtioBus *bus, QVirtioDevice *d);
 void qvirtio_set_acknowledge(const QVirtioBus *bus, QVirtioDevice *d);
 void qvirtio_set_driver(const QVirtioBus *bus, QVirtioDevice *d);
+void qvirtio_set_driver_ok(const QVirtioBus *bus, QVirtioDevice *d);
+
+bool qvirtio_wait_isr(const QVirtioBus *bus, QVirtioDevice *d, uint8_t mask,
+                                                            uint64_t timeout);
+QVirtQueue *qvirtio_virtqueue_setup(const QVirtioBus *bus, QVirtioDevice *d,
+                                        QGuestAllocator *alloc, uint16_t index);
+
+void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq, uint64_t addr);
+uint32_t qvirtqueue_add(QVirtQueue *vq, uint64_t data, uint32_t len, bool write,
+                                                                    bool next);
+void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d, QVirtQueue *vq,
+                                                            uint32_t free_head);
 
 #endif
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index a534521..4f72144 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -17,10 +17,40 @@
 #include "libqos/virtio.h"
 #include "libqos/virtio-pci.h"
 #include "libqos/pci-pc.h"
-
-#define TEST_IMAGE_SIZE (64 * 1024 * 1024)
-#define PCI_SLOT    0x04
-#define PCI_FN      0x00
+#include "libqos/malloc.h"
+#include "libqos/malloc-pc.h"
+
+#define QVIRTIO_BLK_F_BARRIER       0x00000001
+#define QVIRTIO_BLK_F_SIZE_MAX      0x00000002
+#define QVIRTIO_BLK_F_SEG_MAX       0x00000004
+#define QVIRTIO_BLK_F_GEOMETRY      0x00000010
+#define QVIRTIO_BLK_F_RO            0x00000020
+#define QVIRTIO_BLK_F_BLK_SIZE      0x00000040
+#define QVIRTIO_BLK_F_SCSI          0x00000080
+#define QVIRTIO_BLK_F_WCE           0x00000200
+#define QVIRTIO_BLK_F_TOPOLOGY      0x00000400
+#define QVIRTIO_BLK_F_CONFIG_WCE    0x00000800
+
+#define QVIRTIO_BLK_T_IN            0
+#define QVIRTIO_BLK_T_OUT           1
+#define QVIRTIO_BLK_T_SCSI_CMD      2
+#define QVIRTIO_BLK_T_SCSI_CMD_OUT  3
+#define QVIRTIO_BLK_T_FLUSH         4
+#define QVIRTIO_BLK_T_FLUSH_OUT     5
+#define QVIRTIO_BLK_T_GET_ID        8
+
+#define TEST_IMAGE_SIZE         (64 * 1024 * 1024)
+#define QVIRTIO_BLK_TIMEOUT     100
+#define PCI_SLOT                0x04
+#define PCI_FN                  0x00
+
+typedef struct QVirtioBlkReq {
+    uint32_t type;
+    uint32_t ioprio;
+    uint64_t sector;
+    uint8_t data[0];
+    uint8_t status;
+} QVirtioBlkReq;
 
 static QPCIBus *test_start(void)
 {
@@ -53,8 +83,16 @@ static void pci_basic(void)
 {
     QVirtioPCIDevice *dev;
     QPCIBus *bus;
+    QVirtQueue *vq;
+    QGuestAllocator *alloc;
     void *addr;
+    uint64_t w_req;
+    uint64_t r_req;
     uint64_t capacity;
+    uint32_t features;
+    uint32_t free_head;
+    uint8_t status;
+    char *data;
 
     bus = test_start();
 
@@ -74,6 +112,114 @@ static void pci_basic(void)
     capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
     g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE/512);
 
+    features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
+    features = features & ~(QVIRTIO_F_BAD_FEATURE |
+                    QVIRTIO_F_RING_INDIRECT_DESC | QVIRTIO_F_RING_EVENT_IDX |
+                            QVIRTIO_BLK_F_SCSI);
+    qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
+
+    alloc = pc_alloc_init();
+    vq = qvirtio_virtqueue_setup(&qvirtio_pci, &dev->vdev, alloc, 0);
+
+    qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
+
+    /* Write and read with 2 descriptor layout */
+    data = g_malloc0(512);
+    strcpy(data, "TEST");
+
+    /* Write request */
+    w_req = guest_alloc(alloc, sizeof(QVirtioBlkReq)+512);
+    /* w_req->type */
+    writel(w_req, QVIRTIO_BLK_T_OUT);
+    /* w_req->ioprio */
+    writel(w_req+4, 1);
+    /* w_req->sector */
+    writeq(w_req+8, 0);
+    /* w_req->data */
+    memwrite(w_req+16, data, 512);
+    /* w_req->status */
+    writeb(w_req+528, 0xFF);
+
+    g_free(data);
+
+    free_head = qvirtqueue_add(vq, w_req, 528, false, true);
+    qvirtqueue_add(vq, w_req+528, 1, true, false);
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+
+    g_assert(qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+                                                        QVIRTIO_BLK_TIMEOUT));
+    status = readb(w_req+528);
+    g_assert_cmpint(status, ==, 0);
+
+    /* Read request */
+    r_req = guest_alloc(alloc, sizeof(QVirtioBlkReq)+512);
+    /* r_req->type */
+    writel(r_req, QVIRTIO_BLK_T_IN);
+    /* r_req->ioprio */
+    writel(r_req+4, 1);
+    /* r_req->sector */
+    writeq(r_req+8, 0);
+    /* r_req->status */
+    writeb(r_req+528, 0xFF);
+
+    free_head = qvirtqueue_add(vq, r_req, 16, false, true);
+    qvirtqueue_add(vq, r_req+16, 513, true, false);
+
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+
+    g_assert(qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+                                                        QVIRTIO_BLK_TIMEOUT));
+    status = readb(r_req+528);
+    g_assert_cmpint(status, ==, 0);
+
+    data = g_malloc0(512);
+    memread(r_req+16, data, 512);
+    g_assert_cmpstr(data, ==, "TEST");
+    g_free(data);
+
+    /* Write and read with 3 descriptor layout */
+    /* w_req->sector */
+    writeq(w_req+8, 1);
+    /* w_req->status */
+    writeb(w_req+528, 0xFF);
+
+    free_head = qvirtqueue_add(vq, w_req, 16, false, true);
+    qvirtqueue_add(vq, w_req+16, 512, false, true);
+    qvirtqueue_add(vq, w_req+528, 1, true, false);
+
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+
+    g_assert(qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+                                                        QVIRTIO_BLK_TIMEOUT));
+    status = readb(w_req+528);
+    g_assert_cmpint(status, ==, 0);
+
+    /* r_req->sector */
+    writeq(r_req+8, 1);
+    /* r_req->status */
+    writeb(r_req+528, 0xFF);
+
+    free_head = qvirtqueue_add(vq, r_req, 16, false, true);
+    qvirtqueue_add(vq, r_req+16, 512, true, true);
+    qvirtqueue_add(vq, r_req+528, 1, true, false);
+
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+
+    g_assert(qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+                                                        QVIRTIO_BLK_TIMEOUT));
+    status = readb(w_req+528);
+    g_assert_cmpint(status, ==, 0);
+
+    data = g_malloc0(512);
+    memread(r_req+16, data, 512);
+    g_assert_cmpstr(data, ==, "TEST");
+    g_free(data);
+
+    guest_free(alloc, w_req);
+    guest_free(alloc, r_req);
+
+    /* End test */
+    guest_free(alloc, vq->desc);
     qvirtio_pci_device_disable(dev);
     g_free(dev);
     test_end();
-- 
1.7.10.4

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

* [Qemu-devel] [PATCH v4 08/11] libqos: Added indirect descriptor support to virtio implementation
  2014-08-12 11:41 [Qemu-devel] [PATCH v4 00/11] Virtio PCI libqos driver Marc Marí
                   ` (6 preceding siblings ...)
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 07/11] libqos: Added basic virtqueue support to virtio implementation Marc Marí
@ 2014-08-12 11:41 ` Marc Marí
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 09/11] libqos: Added test case for configuration changes in virtio-blk test Marc Marí
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Marc Marí @ 2014-08-12 11:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marc Marí, Paolo Bonzini, Stefan Hajnoczi

Add functions necessary for working with indirect descriptors.
Add test using new functions.

Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
 tests/libqos/virtio-pci.c |   10 ++++
 tests/libqos/virtio.c     |   63 ++++++++++++++++++++++++
 tests/libqos/virtio.h     |   22 ++++++++-
 tests/virtio-blk-test.c   |  118 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 212 insertions(+), 1 deletion(-)

diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 5f92df7..184c430 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -93,6 +93,12 @@ static void qvirtio_pci_set_features(QVirtioDevice *d, uint32_t features)
     qpci_io_writel(dev->pdev, dev->addr + QVIRTIO_GUEST_FEATURES, features);
 }
 
+static uint32_t qvirtio_pci_get_guest_features(QVirtioDevice *d)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    return qpci_io_readl(dev->pdev, dev->addr + QVIRTIO_GUEST_FEATURES);
+}
+
 static uint8_t qvirtio_pci_get_status(QVirtioDevice *d)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
@@ -133,10 +139,12 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
                                         QGuestAllocator *alloc, uint16_t index)
 {
     uint16_t aux;
+    uint32_t feat;
     uint64_t addr;
     QVirtQueue *vq;
 
     vq = g_malloc0(sizeof(*vq));
+    feat = qvirtio_pci_get_guest_features(d);
 
     qvirtio_pci_queue_select(d, index);
     vq->index = index;
@@ -144,6 +152,7 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
     vq->free_head = 0;
     vq->num_free = vq->size;
     vq->align = QVIRTIO_PCI_ALIGN;
+    vq->indirect = (feat & QVIRTIO_F_RING_INDIRECT_DESC) != 0 ? true : false;
 
     /* Check different than 0 */
     g_assert_cmpint(vq->size, !=, 0);
@@ -177,6 +186,7 @@ const QVirtioBus qvirtio_pci = {
     .config_readq = qvirtio_pci_config_readq,
     .get_features = qvirtio_pci_get_features,
     .set_features = qvirtio_pci_set_features,
+    .get_guest_features = qvirtio_pci_get_guest_features,
     .get_status = qvirtio_pci_get_status,
     .set_status = qvirtio_pci_set_status,
     .get_isr_status = qvirtio_pci_get_isr_status,
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index 762bc4c..6310c48 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -116,6 +116,51 @@ void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq, uint64_t addr)
     writew(vq->used, 0);
 }
 
+QVRingIndirectDesc *qvring_indirect_desc_setup(QVirtioDevice *d,
+                                        QGuestAllocator *alloc, uint16_t elem)
+{
+    int i;
+    QVRingIndirectDesc *indirect = g_malloc(sizeof(*indirect));
+
+    indirect->index = 0;
+    indirect->elem = elem;
+    indirect->desc = guest_alloc(alloc, sizeof(QVRingDesc)*elem);
+
+    for (i = 0; i < elem-1; ++i) {
+        /* indirect->desc[i].addr */
+        writeq(indirect->desc+(16*i), 0);
+        /* indirect->desc[i].flags */
+        writew(indirect->desc+(16*i)+12, QVRING_DESC_F_NEXT);
+        /* indirect->desc[i].next */
+        writew(indirect->desc+(16*i)+14, i+1);
+    }
+
+    return indirect;
+}
+
+void qvring_indirect_desc_add(QVRingIndirectDesc *indirect, uint64_t data,
+                                                    uint32_t len, bool write)
+{
+    uint16_t flags;
+
+    g_assert_cmpint(indirect->index, <, indirect->elem);
+
+    flags = readw(indirect->desc+(16*indirect->index)+12);
+
+    if (write) {
+        flags |= QVRING_DESC_F_WRITE;
+    }
+
+    /* indirect->desc[indirect->index].addr */
+    writeq(indirect->desc+(16*indirect->index), data);
+    /* indirect->desc[indirect->index].len */
+    writel(indirect->desc+(16*indirect->index)+8, len);
+    /* indirect->desc[indirect->index].flags */
+    writew(indirect->desc+(16*indirect->index)+12, flags);
+
+    indirect->index++;
+}
+
 uint32_t qvirtqueue_add(QVirtQueue *vq, uint64_t data, uint32_t len, bool write,
                                                                     bool next)
 {
@@ -140,6 +185,24 @@ uint32_t qvirtqueue_add(QVirtQueue *vq, uint64_t data, uint32_t len, bool write,
     return vq->free_head++; /* Return and increase, in this order */
 }
 
+uint32_t qvirtqueue_add_indirect(QVirtQueue *vq, QVRingIndirectDesc *indirect)
+{
+    g_assert(vq->indirect);
+    g_assert_cmpint(vq->size, >=, indirect->elem);
+    g_assert_cmpint(indirect->index, ==, indirect->elem);
+
+    vq->num_free--;
+
+    /* vq->desc[vq->free_head].addr */
+    writeq(vq->desc+(16*vq->free_head), indirect->desc);
+    /* vq->desc[vq->free_head].len */
+    writel(vq->desc+(16*vq->free_head)+8, sizeof(QVRingDesc)*indirect->elem);
+    /* vq->desc[vq->free_head].flags */
+    writew(vq->desc+(16*vq->free_head)+12, QVRING_DESC_F_INDIRECT);
+
+    return vq->free_head++; /* Return and increase, in this order */
+}
+
 void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d, QVirtQueue *vq,
                                                             uint32_t free_head)
 {
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 7a38910..d0c84a6 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -22,6 +22,11 @@
 #define QVIRTIO_NET_DEVICE_ID   0x1
 #define QVIRTIO_BLK_DEVICE_ID   0x2
 
+#define QVIRTIO_F_NOTIFY_ON_EMPTY       0x01000000
+#define QVIRTIO_F_ANY_LAYOUT            0x08000000
+#define QVIRTIO_F_RING_INDIRECT_DESC    0x10000000
+#define QVIRTIO_F_RING_EVENT_IDX        0x20000000
+
 #define QVRING_DESC_F_NEXT      0x1
 #define QVRING_DESC_F_WRITE     0x2
 #define QVRING_DESC_F_INDIRECT  0x4
@@ -74,8 +79,15 @@ typedef struct QVirtQueue {
     uint32_t free_head;
     uint32_t num_free;
     uint32_t align;
+    bool indirect;
 } QVirtQueue;
 
+typedef struct QVRingIndirectDesc {
+    uint64_t desc; /* This points to an array fo QVRingDesc */
+    uint16_t index;
+    uint16_t elem;
+} QVRingIndirectDesc;
+
 typedef struct QVirtioBus {
     uint8_t (*config_readb)(QVirtioDevice *d, void *addr);
     uint16_t (*config_readw)(QVirtioDevice *d, void *addr);
@@ -85,9 +97,12 @@ typedef struct QVirtioBus {
     /* Get features of the device */
     uint32_t (*get_features)(QVirtioDevice *d);
 
-    /* Get features of the device */
+    /* Set features of the device */
     void (*set_features)(QVirtioDevice *d, uint32_t features);
 
+    /* Get features of the guest */
+    uint32_t (*get_guest_features)(QVirtioDevice *d);
+
     /* Get status of the device */
     uint8_t (*get_status)(QVirtioDevice *d);
 
@@ -144,8 +159,13 @@ QVirtQueue *qvirtio_virtqueue_setup(const QVirtioBus *bus, QVirtioDevice *d,
                                         QGuestAllocator *alloc, uint16_t index);
 
 void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq, uint64_t addr);
+QVRingIndirectDesc *qvring_indirect_desc_setup(QVirtioDevice *d,
+                                        QGuestAllocator *alloc, uint16_t elem);
+void qvring_indirect_desc_add(QVRingIndirectDesc *indirect, uint64_t data,
+                                                    uint32_t len, bool write);
 uint32_t qvirtqueue_add(QVirtQueue *vq, uint64_t data, uint32_t len, bool write,
                                                                     bool next);
+uint32_t qvirtqueue_add_indirect(QVirtQueue *vq, QVRingIndirectDesc *indirect);
 void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d, QVirtQueue *vq,
                                                             uint32_t free_head);
 
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 4f72144..06a9ac8 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -225,6 +225,123 @@ static void pci_basic(void)
     test_end();
 }
 
+static void pci_indirect(void)
+{
+    QVirtioPCIDevice *dev;
+    QPCIBus *bus;
+    QVirtQueue *vq;
+    QGuestAllocator *alloc;
+    QVRingIndirectDesc *indirect;
+    void *addr;
+    uint64_t w_req;
+    uint64_t r_req;
+    uint64_t capacity;
+    uint32_t features;
+    uint32_t free_head;
+    uint8_t status;
+    char *data;
+
+    bus = test_start();
+
+    dev = qvirtio_pci_device_find(bus, QVIRTIO_BLK_DEVICE_ID);
+    g_assert(dev != NULL);
+    g_assert_cmphex(dev->vdev.device_type, ==, QVIRTIO_BLK_DEVICE_ID);
+    g_assert_cmphex(dev->pdev->devfn, ==, ((PCI_SLOT << 3) | PCI_FN));
+
+    qvirtio_pci_device_enable(dev);
+    qvirtio_reset(&qvirtio_pci, &dev->vdev);
+    qvirtio_set_acknowledge(&qvirtio_pci, &dev->vdev);
+    qvirtio_set_driver(&qvirtio_pci, &dev->vdev);
+
+    /* MSI-X is not enabled */
+    addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX;
+
+    capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
+    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE/512);
+
+    features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
+    g_assert_cmphex(features & QVIRTIO_F_RING_INDIRECT_DESC, !=, 0);
+    features = features & ~(QVIRTIO_F_BAD_FEATURE | QVIRTIO_F_RING_EVENT_IDX |
+                                                            QVIRTIO_BLK_F_SCSI);
+    qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
+
+    alloc = pc_alloc_init();
+    vq = qvirtio_virtqueue_setup(&qvirtio_pci, &dev->vdev, alloc, 0);
+
+    qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
+
+    /* Write and read with 2 descriptor layout */
+    data = g_malloc0(512);
+    strcpy(data, "TEST");
+
+    /* Write request */
+    w_req = guest_alloc(alloc, sizeof(QVirtioBlkReq)+512);
+    /* w_req->type */
+    writel(w_req, QVIRTIO_BLK_T_OUT);
+    /* w_req->ioprio */
+    writel(w_req+4, 1);
+    /* w_req->sector */
+    writeq(w_req+8, 0);
+    /* w_req->data */
+    memwrite(w_req+16, data, 512);
+    /* w_req->status */
+    writeb(w_req+528, 0xFF);
+
+    g_free(data);
+
+    indirect = qvring_indirect_desc_setup(&dev->vdev, alloc, 2);
+    qvring_indirect_desc_add(indirect, w_req, 528, 0);
+    qvring_indirect_desc_add(indirect, w_req+528, 1, 1);
+    free_head = qvirtqueue_add_indirect(vq, indirect);
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+
+    g_assert(qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+                                                        QVIRTIO_BLK_TIMEOUT));
+    status = readb(w_req+528);
+    g_assert_cmpint(status, ==, 0);
+
+    g_free(indirect);
+
+    /* Read request */
+    r_req = guest_alloc(alloc, sizeof(QVirtioBlkReq)+512);
+    /* r_req->type */
+    writel(r_req, QVIRTIO_BLK_T_IN);
+    /* r_req->ioprio */
+    writel(r_req+4, 1);
+    /* r_req->sector */
+    writeq(r_req+8, 0);
+    /* r_req->status */
+    writeb(r_req+528, 0xFF);
+
+    indirect = qvring_indirect_desc_setup(&dev->vdev, alloc, 2);
+    qvring_indirect_desc_add(indirect, r_req, 16, false);
+    qvring_indirect_desc_add(indirect, r_req+16, 513, true);
+    free_head = qvirtqueue_add_indirect(vq, indirect);
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+
+    g_assert(qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+                                                        QVIRTIO_BLK_TIMEOUT));
+    status = readb(r_req+528);
+    g_assert_cmpint(status, ==, 0);
+
+    g_free(indirect);
+
+    data = g_malloc0(512);
+    memread(r_req+16, data, 512);
+    g_assert_cmpstr(data, ==, "TEST");
+    g_free(data);
+    g_free(indirect);
+
+    guest_free(alloc, w_req);
+    guest_free(alloc, r_req);
+
+    /* End test */
+    guest_free(alloc, vq->desc);
+    qvirtio_pci_device_disable(dev);
+    g_free(dev);
+    test_end();
+}
+
 int main(int argc, char **argv)
 {
     int ret;
@@ -232,6 +349,7 @@ int main(int argc, char **argv)
     g_test_init(&argc, &argv, NULL);
 
     g_test_add_func("/virtio/blk/pci/basic", pci_basic);
+    g_test_add_func("/virtio/blk/pci/indirect", pci_indirect);
 
     ret = g_test_run();
 
-- 
1.7.10.4

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

* [Qemu-devel] [PATCH v4 09/11] libqos: Added test case for configuration changes in virtio-blk test
  2014-08-12 11:41 [Qemu-devel] [PATCH v4 00/11] Virtio PCI libqos driver Marc Marí
                   ` (7 preceding siblings ...)
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 08/11] libqos: Added indirect descriptor " Marc Marí
@ 2014-08-12 11:41 ` Marc Marí
  2014-08-13 17:34   ` Stefan Hajnoczi
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 10/11] libqos: Added MSI-X support Marc Marí
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 11/11] libqos: Added EVENT_IDX support Marc Marí
  10 siblings, 1 reply; 23+ messages in thread
From: Marc Marí @ 2014-08-12 11:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marc Marí, Paolo Bonzini, Stefan Hajnoczi

Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
 tests/virtio-blk-test.c |   14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 06a9ac8..7269ade 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -225,13 +225,14 @@ static void pci_basic(void)
     test_end();
 }
 
-static void pci_indirect(void)
+static void pci_indirect_config(void)
 {
     QVirtioPCIDevice *dev;
     QPCIBus *bus;
     QVirtQueue *vq;
     QGuestAllocator *alloc;
     QVRingIndirectDesc *indirect;
+    int n_size = TEST_IMAGE_SIZE/2;
     void *addr;
     uint64_t w_req;
     uint64_t r_req;
@@ -270,6 +271,15 @@ static void pci_indirect(void)
 
     qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
 
+    qmp("{ 'execute': 'block_resize', 'arguments': { 'device': 'drive0', "
+                                                    " 'size': %d } }", n_size);
+
+    g_assert(qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x2,
+                                                        QVIRTIO_BLK_TIMEOUT));
+
+    capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
+    g_assert_cmpint(capacity, ==, n_size/512);
+
     /* Write and read with 2 descriptor layout */
     data = g_malloc0(512);
     strcpy(data, "TEST");
@@ -349,7 +359,7 @@ int main(int argc, char **argv)
     g_test_init(&argc, &argv, NULL);
 
     g_test_add_func("/virtio/blk/pci/basic", pci_basic);
-    g_test_add_func("/virtio/blk/pci/indirect", pci_indirect);
+    g_test_add_func("/virtio/blk/pci/indirect_config", pci_indirect_config);
 
     ret = g_test_run();
 
-- 
1.7.10.4

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

* [Qemu-devel] [PATCH v4 10/11] libqos: Added MSI-X support
  2014-08-12 11:41 [Qemu-devel] [PATCH v4 00/11] Virtio PCI libqos driver Marc Marí
                   ` (8 preceding siblings ...)
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 09/11] libqos: Added test case for configuration changes in virtio-blk test Marc Marí
@ 2014-08-12 11:41 ` Marc Marí
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 11/11] libqos: Added EVENT_IDX support Marc Marí
  10 siblings, 0 replies; 23+ messages in thread
From: Marc Marí @ 2014-08-12 11:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marc Marí, Paolo Bonzini, Stefan Hajnoczi

Added MSI-X support for qtest PCI.
Added MSI-X support for virtio-pci.
Added MSI-X test case in virtio-blk-test.

Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
 tests/libqos/pci.c        |  110 ++++++++++++++++++++++++-
 tests/libqos/pci.h        |   10 +++
 tests/libqos/virtio-pci.c |  145 ++++++++++++++++++++++++++++-----
 tests/libqos/virtio-pci.h |   17 ++++
 tests/libqos/virtio.c     |   17 +++-
 tests/libqos/virtio.h     |   11 ++-
 tests/virtio-blk-test.c   |  197 +++++++++++++++++++++++++++++++++++++--------
 7 files changed, 447 insertions(+), 60 deletions(-)

diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c
index c9a0b91..82b487d 100644
--- a/tests/libqos/pci.c
+++ b/tests/libqos/pci.c
@@ -15,8 +15,6 @@
 #include "hw/pci/pci_regs.h"
 #include <glib.h>
 
-#include <stdio.h>
-
 void qpci_device_foreach(QPCIBus *bus, int vendor_id, int device_id,
                          void (*func)(QPCIDevice *dev, int devfn, void *data),
                          void *data)
@@ -75,6 +73,114 @@ void qpci_device_enable(QPCIDevice *dev)
     qpci_config_writew(dev, PCI_COMMAND, cmd);
 }
 
+uint8_t qpci_find_capability(QPCIDevice *dev, uint8_t id)
+{
+    uint8_t cap;
+    uint8_t addr = qpci_config_readb(dev, PCI_CAPABILITY_LIST);
+
+    do {
+        cap = qpci_config_readb(dev, addr);
+        if (cap != id) {
+            addr = qpci_config_readb(dev, addr+PCI_CAP_LIST_NEXT);
+        }
+    } while (cap != id && addr != 0);
+
+    return addr;
+}
+
+void qpci_msix_enable(QPCIDevice *dev)
+{
+    uint8_t addr;
+    uint16_t val;
+    uint32_t table;
+    uint8_t bir_table;
+    uint8_t bir_pba;
+    void *offset;
+
+    addr = qpci_find_capability(dev, PCI_CAP_ID_MSIX);
+    g_assert_cmphex(addr, !=, 0);
+
+    val = qpci_config_readw(dev, addr+PCI_MSIX_FLAGS);
+    qpci_config_writew(dev, addr+PCI_MSIX_FLAGS, val | PCI_MSIX_FLAGS_ENABLE);
+
+    table = qpci_config_readl(dev, addr+PCI_MSIX_TABLE);
+    bir_table = table & PCI_MSIX_FLAGS_BIRMASK;
+    offset = qpci_iomap(dev, bir_table);
+    dev->msix_table = offset+(table & ~PCI_MSIX_FLAGS_BIRMASK);
+
+    table = qpci_config_readl(dev, addr+PCI_MSIX_PBA);
+    bir_pba = table & PCI_MSIX_FLAGS_BIRMASK;
+    if (bir_pba != bir_table) {
+        offset = qpci_iomap(dev, bir_pba);
+    }
+    dev->msix_pba = offset+(table & ~PCI_MSIX_FLAGS_BIRMASK);
+
+    g_assert(dev->msix_table != NULL);
+    g_assert(dev->msix_pba != NULL);
+    dev->msix_enabled = true;
+}
+
+void qpci_msix_disable(QPCIDevice *dev)
+{
+    uint8_t addr;
+    uint16_t val;
+
+    g_assert(dev->msix_enabled);
+    addr = qpci_find_capability(dev, PCI_CAP_ID_MSIX);
+    g_assert_cmphex(addr, !=, 0);
+    val = qpci_config_readw(dev, addr+PCI_MSIX_FLAGS);
+    qpci_config_writew(dev, addr+PCI_MSIX_FLAGS, val & ~PCI_MSIX_FLAGS_ENABLE);
+
+    qpci_iounmap(dev, dev->msix_table);
+    qpci_iounmap(dev, dev->msix_pba);
+    dev->msix_enabled = 0;
+    dev->msix_table = NULL;
+    dev->msix_pba = NULL;
+}
+
+bool qpci_msix_pending(QPCIDevice *dev, uint16_t entry)
+{
+    uint32_t pba_entry;
+    uint8_t bit_n = entry % 32;
+    void *addr = dev->msix_pba + (entry/32) * PCI_MSIX_ENTRY_SIZE/4;
+
+    g_assert(dev->msix_enabled);
+    pba_entry = qpci_io_readl(dev, addr);
+    qpci_io_writel(dev, addr, pba_entry & ~(1 << bit_n));
+    return (pba_entry & (1 << bit_n)) != 0;
+}
+
+bool qpci_msix_masked(QPCIDevice *dev, uint16_t entry)
+{
+    uint8_t addr;
+    uint16_t val;
+    void *vector_addr = dev->msix_table + (entry * PCI_MSIX_ENTRY_SIZE);
+
+    g_assert(dev->msix_enabled);
+    addr = qpci_find_capability(dev, PCI_CAP_ID_MSIX);
+    g_assert_cmphex(addr, !=, 0);
+    val = qpci_config_readw(dev, addr+PCI_MSIX_FLAGS);
+
+    if (val & PCI_MSIX_FLAGS_MASKALL) {
+        return true;
+    } else {
+        return (qpci_io_readl(dev, vector_addr + PCI_MSIX_ENTRY_VECTOR_CTRL)
+                                            & PCI_MSIX_ENTRY_CTRL_MASKBIT) != 0;
+    }
+}
+
+uint16_t qpci_msix_table_size(QPCIDevice *dev)
+{
+    uint8_t addr;
+    uint16_t control;
+
+    addr = qpci_find_capability(dev, PCI_CAP_ID_MSIX);
+    g_assert_cmphex(addr, !=, 0);
+
+    control = qpci_config_readw(dev, addr+PCI_MSIX_FLAGS);
+    return (control & PCI_MSIX_FLAGS_QSIZE) + 1;
+}
+
 uint8_t qpci_config_readb(QPCIDevice *dev, uint8_t offset)
 {
     return dev->bus->config_readb(dev->bus, dev->devfn, offset);
diff --git a/tests/libqos/pci.h b/tests/libqos/pci.h
index 3439431..decf5f6 100644
--- a/tests/libqos/pci.h
+++ b/tests/libqos/pci.h
@@ -14,6 +14,7 @@
 #define LIBQOS_PCI_H
 
 #include <stdint.h>
+#include "libqtest.h"
 
 #define QPCI_DEVFN(dev, fn) (((dev) << 3) | (fn))
 
@@ -49,6 +50,9 @@ struct QPCIDevice
 {
     QPCIBus *bus;
     int devfn;
+    bool msix_enabled;
+    void *msix_table;
+    void *msix_pba;
 };
 
 void qpci_device_foreach(QPCIBus *bus, int vendor_id, int device_id,
@@ -57,6 +61,12 @@ void qpci_device_foreach(QPCIBus *bus, int vendor_id, int device_id,
 QPCIDevice *qpci_device_find(QPCIBus *bus, int devfn);
 
 void qpci_device_enable(QPCIDevice *dev);
+uint8_t qpci_find_capability(QPCIDevice *dev, uint8_t id);
+void qpci_msix_enable(QPCIDevice *dev);
+void qpci_msix_disable(QPCIDevice *dev);
+bool qpci_msix_pending(QPCIDevice *dev, uint16_t entry);
+bool qpci_msix_masked(QPCIDevice *dev, uint16_t entry);
+uint16_t qpci_msix_table_size(QPCIDevice *dev);
 
 uint8_t qpci_config_readb(QPCIDevice *dev, uint8_t offset);
 uint16_t qpci_config_readw(QPCIDevice *dev, uint8_t offset);
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 184c430..acf785a 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -16,6 +16,9 @@
 #include "libqos/malloc.h"
 #include "libqos/malloc-pc.h"
 
+#include <stdio.h>
+#include <inttypes.h>
+
 #include "hw/pci/pci_regs.h"
 
 typedef struct QVirtioPCIForeachData {
@@ -35,6 +38,8 @@ static QVirtioPCIDevice *qpcidevice_to_qvirtiodevice(QPCIDevice *pdev)
                             qpci_config_readw(vpcidev->pdev, PCI_SUBSYSTEM_ID);
     }
 
+    vpcidev->config_msix_entry = -1;
+
     return vpcidev;
 }
 
@@ -111,10 +116,45 @@ static void qvirtio_pci_set_status(QVirtioDevice *d, uint8_t status)
     qpci_io_writeb(dev->pdev, dev->addr + QVIRTIO_DEVICE_STATUS, status);
 }
 
-static uint8_t qvirtio_pci_get_isr_status(QVirtioDevice *d)
+static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    QVirtQueuePCI *vqpci = (QVirtQueuePCI *)vq;
+    uint32_t data;
+
+    if (dev->pdev->msix_enabled) {
+        g_assert_cmpint(vqpci->msix_entry, !=, -1);
+        if (qpci_msix_masked(dev->pdev, vqpci->msix_entry)) {
+            /* No ISR checking should be done if masked, but read anyway */
+            return qpci_msix_pending(dev->pdev, vqpci->msix_entry);
+        } else {
+            data = readl(vqpci->msix_addr);
+            writel(vqpci->msix_addr, 0);
+            return data == vqpci->msix_data;
+        }
+    } else {
+        return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_ISR_STATUS) & 1;
+    }
+}
+
+static bool qvirtio_pci_get_config_isr_status(QVirtioDevice *d)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-    return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_ISR_STATUS);
+    uint32_t data;
+
+    if (dev->pdev->msix_enabled) {
+        g_assert_cmpint(dev->config_msix_entry, !=, -1);
+        if (qpci_msix_masked(dev->pdev, dev->config_msix_entry)) {
+            /* No ISR checking should be done if masked, but read anyway */
+            return qpci_msix_pending(dev->pdev, dev->config_msix_entry);
+        } else {
+            data = readl(dev->config_msix_addr);
+            writel(dev->config_msix_addr, 0);
+            return data == dev->config_msix_data;
+        }
+    } else {
+        return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_ISR_STATUS) & 2;
+    }
 }
 
 static void qvirtio_pci_queue_select(QVirtioDevice *d, uint16_t index)
@@ -141,36 +181,38 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
     uint16_t aux;
     uint32_t feat;
     uint64_t addr;
-    QVirtQueue *vq;
+    QVirtQueuePCI *vqpci;
 
-    vq = g_malloc0(sizeof(*vq));
+    vqpci = g_malloc0(sizeof(*vqpci));
     feat = qvirtio_pci_get_guest_features(d);
 
     qvirtio_pci_queue_select(d, index);
-    vq->index = index;
-    vq->size = qvirtio_pci_get_queue_size(d);
-    vq->free_head = 0;
-    vq->num_free = vq->size;
-    vq->align = QVIRTIO_PCI_ALIGN;
-    vq->indirect = (feat & QVIRTIO_F_RING_INDIRECT_DESC) != 0 ? true : false;
+    vqpci->vq.index = index;
+    vqpci->vq.size = qvirtio_pci_get_queue_size(d);
+    vqpci->vq.free_head = 0;
+    vqpci->vq.num_free = vqpci->vq.size;
+    vqpci->vq.align = QVIRTIO_PCI_ALIGN;
+    vqpci->vq.indirect = (feat & QVIRTIO_F_RING_INDIRECT_DESC) != 0;
+
+    vqpci->msix_entry = -1;
+    vqpci->msix_addr = 0;
+    vqpci->msix_data = 0x12345678;
 
     /* Check different than 0 */
-    g_assert_cmpint(vq->size, !=, 0);
+    g_assert_cmpint(vqpci->vq.size, !=, 0);
 
     /* Check power of 2 */
-    aux = vq->size;
+    aux = vqpci->vq.size;
     while ((aux & 1) != 0) {
         aux = aux >> 1;
     }
     g_assert_cmpint(aux, !=, 1);
 
-    addr = guest_alloc(alloc, qvring_size(vq->size, QVIRTIO_PCI_ALIGN));
-    qvring_init(alloc, vq, addr);
-    qvirtio_pci_set_queue_address(d, vq->desc/4096);
-
-    /* TODO: MSI-X configuration */
+    addr = guest_alloc(alloc, qvring_size(vqpci->vq.size, QVIRTIO_PCI_ALIGN));
+    qvring_init(alloc, &vqpci->vq, addr);
+    qvirtio_pci_set_queue_address(d, vqpci->vq.desc/4096);
 
-    return vq;
+    return &vqpci->vq;
 }
 
 static void qvirtio_pci_virtqueue_kick(QVirtioDevice *d, QVirtQueue *vq)
@@ -189,7 +231,8 @@ const QVirtioBus qvirtio_pci = {
     .get_guest_features = qvirtio_pci_get_guest_features,
     .get_status = qvirtio_pci_get_status,
     .set_status = qvirtio_pci_set_status,
-    .get_isr_status = qvirtio_pci_get_isr_status,
+    .get_queue_isr_status = qvirtio_pci_get_queue_isr_status,
+    .get_config_isr_status = qvirtio_pci_get_config_isr_status,
     .queue_select = qvirtio_pci_queue_select,
     .get_queue_size = qvirtio_pci_get_queue_size,
     .set_queue_address = qvirtio_pci_set_queue_address,
@@ -226,4 +269,68 @@ void qvirtio_pci_device_enable(QVirtioPCIDevice *d)
 void qvirtio_pci_device_disable(QVirtioPCIDevice *d)
 {
     qpci_iounmap(d->pdev, d->addr);
+    d->addr = NULL;
+}
+
+void qvirtqueue_pci_msix_setup(QVirtioPCIDevice *d, QVirtQueuePCI *vqpci,
+                                        QGuestAllocator *alloc, uint16_t entry)
+{
+    uint16_t vector;
+    uint32_t control;
+    void *addr;
+
+    g_assert(d->pdev->msix_enabled);
+    addr = d->pdev->msix_table + (entry * 16);
+
+    g_assert_cmpint(entry, >=, 0);
+    g_assert_cmpint(entry, <, qpci_msix_table_size(d->pdev));
+    vqpci->msix_entry = entry;
+
+    vqpci->msix_addr = guest_alloc(alloc, 4);
+    qpci_io_writel(d->pdev, addr + PCI_MSIX_ENTRY_LOWER_ADDR,
+                                                    vqpci->msix_addr & ~0UL);
+    qpci_io_writel(d->pdev, addr + PCI_MSIX_ENTRY_UPPER_ADDR,
+                                            (vqpci->msix_addr >> 32) & ~0UL);
+    qpci_io_writel(d->pdev, addr + PCI_MSIX_ENTRY_DATA, vqpci->msix_data);
+
+    control = qpci_io_readl(d->pdev, addr + PCI_MSIX_ENTRY_VECTOR_CTRL);
+    qpci_io_writel(d->pdev, addr + 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->addr + QVIRTIO_MSIX_QUEUE_VECTOR, entry);
+    vector = qpci_io_readw(d->pdev, d->addr + QVIRTIO_MSIX_QUEUE_VECTOR);
+    g_assert_cmphex(vector, !=, VIRTIO_MSI_NO_VECTOR);
+}
+
+void qvirtio_pci_set_msix_configuration_vector(QVirtioPCIDevice *d,
+                                        QGuestAllocator *alloc, uint16_t entry)
+{
+    uint16_t vector;
+    uint32_t control;
+    void *addr;
+
+    g_assert(d->pdev->msix_enabled);
+    addr = d->pdev->msix_table + (entry * 16);
+
+    g_assert_cmpint(entry, >=, 0);
+    g_assert_cmpint(entry, <, qpci_msix_table_size(d->pdev));
+    d->config_msix_entry = entry;
+
+    d->config_msix_data = 0x12345678;
+    d->config_msix_addr = guest_alloc(alloc, 4);
+
+    qpci_io_writel(d->pdev, addr + PCI_MSIX_ENTRY_LOWER_ADDR,
+                                                    d->config_msix_addr & ~0UL);
+    qpci_io_writel(d->pdev, addr + PCI_MSIX_ENTRY_UPPER_ADDR,
+                                            (d->config_msix_addr >> 32) & ~0UL);
+    qpci_io_writel(d->pdev, addr + PCI_MSIX_ENTRY_DATA, d->config_msix_data);
+
+    control = qpci_io_readl(d->pdev, addr + PCI_MSIX_ENTRY_VECTOR_CTRL);
+    qpci_io_writel(d->pdev, addr + PCI_MSIX_ENTRY_VECTOR_CTRL,
+                                        control & ~PCI_MSIX_ENTRY_CTRL_MASKBIT);
+
+    qpci_io_writew(d->pdev, d->addr + QVIRTIO_MSIX_CONF_VECTOR, entry);
+    vector = qpci_io_readw(d->pdev, d->addr + QVIRTIO_MSIX_CONF_VECTOR);
+    g_assert_cmphex(vector, !=, VIRTIO_MSI_NO_VECTOR);
 }
diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
index 40bd12d..d6569aa 100644
--- a/tests/libqos/virtio-pci.h
+++ b/tests/libqos/virtio-pci.h
@@ -28,12 +28,24 @@
 
 #define QVIRTIO_PCI_ALIGN   4096
 
+#define VIRTIO_MSI_NO_VECTOR    0xFFFF
+
 typedef struct QVirtioPCIDevice {
     QVirtioDevice vdev;
     QPCIDevice *pdev;
     void *addr;
+    uint16_t config_msix_entry;
+    uint64_t config_msix_addr;
+    uint32_t config_msix_data;
 } QVirtioPCIDevice;
 
+typedef struct QVirtQueuePCI {
+    QVirtQueue vq;
+    uint16_t msix_entry;
+    uint64_t msix_addr;
+    uint32_t msix_data;
+} QVirtQueuePCI;
+
 extern const QVirtioBus qvirtio_pci;
 
 void qvirtio_pci_foreach(QPCIBus *bus, uint16_t device_type,
@@ -41,4 +53,9 @@ void qvirtio_pci_foreach(QPCIBus *bus, uint16_t device_type,
 QVirtioPCIDevice *qvirtio_pci_device_find(QPCIBus *bus, uint16_t device_type);
 void qvirtio_pci_device_enable(QVirtioPCIDevice *d);
 void qvirtio_pci_device_disable(QVirtioPCIDevice *d);
+
+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);
 #endif
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index 6310c48..7d1ee7c 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -78,12 +78,25 @@ void qvirtio_set_driver_ok(const QVirtioBus *bus, QVirtioDevice *d)
                 QVIRTIO_DRIVER_OK | QVIRTIO_DRIVER | QVIRTIO_ACKNOWLEDGE);
 }
 
-bool qvirtio_wait_isr(const QVirtioBus *bus, QVirtioDevice *d, uint8_t mask,
+bool qvirtio_wait_queue_isr(const QVirtioBus *bus, QVirtioDevice *d,
+                                            QVirtQueue *vq, uint64_t timeout)
+{
+    do {
+        clock_step(10);
+        if (bus->get_queue_isr_status(d, vq)) {
+            break; /* It has ended */
+        }
+    } while (--timeout);
+
+    return timeout != 0;
+}
+
+bool qvirtio_wait_config_isr(const QVirtioBus *bus, QVirtioDevice *d,
                                                             uint64_t timeout)
 {
     do {
         clock_step(10);
-        if (bus->get_isr_status(d) & mask) {
+        if (bus->get_config_isr_status(d)) {
             break; /* It has ended */
         }
     } while (--timeout);
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index d0c84a6..5010cfb 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -109,8 +109,11 @@ typedef struct QVirtioBus {
     /* Set status of the device  */
     void (*set_status)(QVirtioDevice *d, uint8_t status);
 
-    /* Get the ISR status of the device */
-    uint8_t (*get_isr_status)(QVirtioDevice *d);
+    /* Get the queue ISR status of the device */
+    bool (*get_queue_isr_status)(QVirtioDevice *d, QVirtQueue *vq);
+
+    /* Get the configuration ISR status of the device */
+    bool (*get_config_isr_status)(QVirtioDevice *d);
 
     /* Select a queue to work on */
     void (*queue_select)(QVirtioDevice *d, uint16_t index);
@@ -153,7 +156,9 @@ void qvirtio_set_acknowledge(const QVirtioBus *bus, QVirtioDevice *d);
 void qvirtio_set_driver(const QVirtioBus *bus, QVirtioDevice *d);
 void qvirtio_set_driver_ok(const QVirtioBus *bus, QVirtioDevice *d);
 
-bool qvirtio_wait_isr(const QVirtioBus *bus, QVirtioDevice *d, uint8_t mask,
+bool qvirtio_wait_queue_isr(const QVirtioBus *bus, QVirtioDevice *d,
+                                            QVirtQueue *vq, uint64_t timeout);
+bool qvirtio_wait_config_isr(const QVirtioBus *bus, QVirtioDevice *d,
                                                             uint64_t timeout);
 QVirtQueue *qvirtio_virtqueue_setup(const QVirtioBus *bus, QVirtioDevice *d,
                                         QGuestAllocator *alloc, uint16_t index);
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 7269ade..8a8b6ec 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -83,7 +83,7 @@ static void pci_basic(void)
 {
     QVirtioPCIDevice *dev;
     QPCIBus *bus;
-    QVirtQueue *vq;
+    QVirtQueuePCI *vqpci;
     QGuestAllocator *alloc;
     void *addr;
     uint64_t w_req;
@@ -119,7 +119,8 @@ static void pci_basic(void)
     qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
 
     alloc = pc_alloc_init();
-    vq = qvirtio_virtqueue_setup(&qvirtio_pci, &dev->vdev, alloc, 0);
+    vqpci = (QVirtQueuePCI *)qvirtio_virtqueue_setup(&qvirtio_pci, &dev->vdev,
+                                                                    alloc, 0);
 
     qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
 
@@ -142,11 +143,11 @@ static void pci_basic(void)
 
     g_free(data);
 
-    free_head = qvirtqueue_add(vq, w_req, 528, false, true);
-    qvirtqueue_add(vq, w_req+528, 1, true, false);
-    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+    free_head = qvirtqueue_add(&vqpci->vq, w_req, 528, false, true);
+    qvirtqueue_add(&vqpci->vq, w_req+528, 1, true, false);
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);
 
-    g_assert(qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+    g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
                                                         QVIRTIO_BLK_TIMEOUT));
     status = readb(w_req+528);
     g_assert_cmpint(status, ==, 0);
@@ -162,12 +163,12 @@ static void pci_basic(void)
     /* r_req->status */
     writeb(r_req+528, 0xFF);
 
-    free_head = qvirtqueue_add(vq, r_req, 16, false, true);
-    qvirtqueue_add(vq, r_req+16, 513, true, false);
+    free_head = qvirtqueue_add(&vqpci->vq, r_req, 16, false, true);
+    qvirtqueue_add(&vqpci->vq, r_req+16, 513, true, false);
 
-    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);
 
-    g_assert(qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+    g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
                                                         QVIRTIO_BLK_TIMEOUT));
     status = readb(r_req+528);
     g_assert_cmpint(status, ==, 0);
@@ -183,13 +184,13 @@ static void pci_basic(void)
     /* w_req->status */
     writeb(w_req+528, 0xFF);
 
-    free_head = qvirtqueue_add(vq, w_req, 16, false, true);
-    qvirtqueue_add(vq, w_req+16, 512, false, true);
-    qvirtqueue_add(vq, w_req+528, 1, true, false);
+    free_head = qvirtqueue_add(&vqpci->vq, w_req, 16, false, true);
+    qvirtqueue_add(&vqpci->vq, w_req+16, 512, false, true);
+    qvirtqueue_add(&vqpci->vq, w_req+528, 1, true, false);
 
-    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);
 
-    g_assert(qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+    g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
                                                         QVIRTIO_BLK_TIMEOUT));
     status = readb(w_req+528);
     g_assert_cmpint(status, ==, 0);
@@ -199,13 +200,13 @@ static void pci_basic(void)
     /* r_req->status */
     writeb(r_req+528, 0xFF);
 
-    free_head = qvirtqueue_add(vq, r_req, 16, false, true);
-    qvirtqueue_add(vq, r_req+16, 512, true, true);
-    qvirtqueue_add(vq, r_req+528, 1, true, false);
+    free_head = qvirtqueue_add(&vqpci->vq, r_req, 16, false, true);
+    qvirtqueue_add(&vqpci->vq, r_req+16, 512, true, true);
+    qvirtqueue_add(&vqpci->vq, r_req+528, 1, true, false);
 
-    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);
 
-    g_assert(qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+    g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
                                                         QVIRTIO_BLK_TIMEOUT));
     status = readb(w_req+528);
     g_assert_cmpint(status, ==, 0);
@@ -219,7 +220,7 @@ static void pci_basic(void)
     guest_free(alloc, r_req);
 
     /* End test */
-    guest_free(alloc, vq->desc);
+    guest_free(alloc, vqpci->vq.desc);
     qvirtio_pci_device_disable(dev);
     g_free(dev);
     test_end();
@@ -229,7 +230,7 @@ static void pci_indirect_config(void)
 {
     QVirtioPCIDevice *dev;
     QPCIBus *bus;
-    QVirtQueue *vq;
+    QVirtQueuePCI *vqpci;
     QGuestAllocator *alloc;
     QVRingIndirectDesc *indirect;
     int n_size = TEST_IMAGE_SIZE/2;
@@ -267,14 +268,15 @@ static void pci_indirect_config(void)
     qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
 
     alloc = pc_alloc_init();
-    vq = qvirtio_virtqueue_setup(&qvirtio_pci, &dev->vdev, alloc, 0);
+    vqpci = (QVirtQueuePCI *)qvirtio_virtqueue_setup(&qvirtio_pci, &dev->vdev,
+                                                                    alloc, 0);
 
     qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
 
     qmp("{ 'execute': 'block_resize', 'arguments': { 'device': 'drive0', "
                                                     " 'size': %d } }", n_size);
 
-    g_assert(qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x2,
+    g_assert(qvirtio_wait_config_isr(&qvirtio_pci, &dev->vdev,
                                                         QVIRTIO_BLK_TIMEOUT));
 
     capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
@@ -300,12 +302,12 @@ static void pci_indirect_config(void)
     g_free(data);
 
     indirect = qvring_indirect_desc_setup(&dev->vdev, alloc, 2);
-    qvring_indirect_desc_add(indirect, w_req, 528, 0);
-    qvring_indirect_desc_add(indirect, w_req+528, 1, 1);
-    free_head = qvirtqueue_add_indirect(vq, indirect);
-    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+    qvring_indirect_desc_add(indirect, w_req, 528, false);
+    qvring_indirect_desc_add(indirect, w_req+528, 1, true);
+    free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);
 
-    g_assert(qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+    g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
                                                         QVIRTIO_BLK_TIMEOUT));
     status = readb(w_req+528);
     g_assert_cmpint(status, ==, 0);
@@ -326,10 +328,10 @@ static void pci_indirect_config(void)
     indirect = qvring_indirect_desc_setup(&dev->vdev, alloc, 2);
     qvring_indirect_desc_add(indirect, r_req, 16, false);
     qvring_indirect_desc_add(indirect, r_req+16, 513, true);
-    free_head = qvirtqueue_add_indirect(vq, indirect);
-    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+    free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);
 
-    g_assert(qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+    g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
                                                         QVIRTIO_BLK_TIMEOUT));
     status = readb(r_req+528);
     g_assert_cmpint(status, ==, 0);
@@ -340,13 +342,139 @@ static void pci_indirect_config(void)
     memread(r_req+16, data, 512);
     g_assert_cmpstr(data, ==, "TEST");
     g_free(data);
-    g_free(indirect);
 
     guest_free(alloc, w_req);
     guest_free(alloc, r_req);
 
     /* End test */
-    guest_free(alloc, vq->desc);
+    guest_free(alloc, vqpci->vq.desc);
+    qvirtio_pci_device_disable(dev);
+    g_free(dev);
+    test_end();
+}
+
+static void pci_msix(void)
+{
+    QVirtioPCIDevice *dev;
+    QPCIBus *bus;
+    QVirtQueuePCI *vqpci;
+    QGuestAllocator *alloc;
+    int n_size = TEST_IMAGE_SIZE/2;
+    void *addr;
+    uint64_t w_req;
+    uint64_t r_req;
+    uint64_t capacity;
+    uint32_t features;
+    uint32_t free_head;
+    uint8_t status;
+    char *data;
+
+    bus = test_start();
+    alloc = pc_alloc_init();
+
+    dev = qvirtio_pci_device_find(bus, QVIRTIO_BLK_DEVICE_ID);
+    g_assert(dev != NULL);
+    g_assert_cmphex(dev->vdev.device_type, ==, QVIRTIO_BLK_DEVICE_ID);
+    g_assert_cmphex(dev->pdev->devfn, ==, ((PCI_SLOT << 3) | PCI_FN));
+
+    qvirtio_pci_device_enable(dev);
+    qpci_msix_enable(dev->pdev);
+
+    qvirtio_reset(&qvirtio_pci, &dev->vdev);
+    qvirtio_set_acknowledge(&qvirtio_pci, &dev->vdev);
+    qvirtio_set_driver(&qvirtio_pci, &dev->vdev);
+
+    qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
+
+    /* MSI-X is enabled */
+    addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_MSIX;
+
+    capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
+    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE/512);
+
+    features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
+    features = features & ~(QVIRTIO_F_BAD_FEATURE |
+                            QVIRTIO_F_RING_INDIRECT_DESC |
+                            QVIRTIO_F_RING_EVENT_IDX | QVIRTIO_BLK_F_SCSI);
+    qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
+
+    vqpci = (QVirtQueuePCI *)qvirtio_virtqueue_setup(&qvirtio_pci, &dev->vdev,
+                                                                    alloc, 0);
+    qvirtqueue_pci_msix_setup(dev, vqpci, alloc, 1);
+
+    qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
+
+    qmp("{ 'execute': 'block_resize', 'arguments': { 'device': 'drive0', "
+                                                    " 'size': %d } }", n_size);
+
+    g_assert(qvirtio_wait_config_isr(&qvirtio_pci, &dev->vdev,
+                                                        QVIRTIO_BLK_TIMEOUT));
+
+    capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
+    g_assert_cmpint(capacity, ==, n_size/512);
+
+    /* Write and read with 2 descriptor layout */
+    data = g_malloc0(512);
+    strcpy(data, "TEST");
+
+    /* Write request */
+    w_req = guest_alloc(alloc, sizeof(QVirtioBlkReq)+512);
+    /* w_req->type */
+    writel(w_req, QVIRTIO_BLK_T_OUT);
+    /* w_req->ioprio */
+    writel(w_req+4, 1);
+    /* w_req->sector */
+    writeq(w_req+8, 0);
+    /* w_req->data */
+    memwrite(w_req+16, data, 512);
+    /* w_req->status */
+    writeb(w_req+528, 0xFF);
+
+    g_free(data);
+
+    free_head = qvirtqueue_add(&vqpci->vq, w_req, 528, false, true);
+    qvirtqueue_add(&vqpci->vq, w_req+528, 1, true, false);
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);
+
+    g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
+                                                        QVIRTIO_BLK_TIMEOUT));
+
+    status = readb(w_req+528);
+    g_assert_cmpint(status, ==, 0);
+
+    /* Read request */
+    r_req = guest_alloc(alloc, sizeof(QVirtioBlkReq)+512);
+    /* r_req->type */
+    writel(r_req, QVIRTIO_BLK_T_IN);
+    /* r_req->ioprio */
+    writel(r_req+4, 1);
+    /* r_req->sector */
+    writeq(r_req+8, 0);
+    /* r_req->status */
+    writeb(r_req+528, 0xFF);
+
+    free_head = qvirtqueue_add(&vqpci->vq, r_req, 16, false, true);
+    qvirtqueue_add(&vqpci->vq, r_req+16, 513, true, false);
+
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);
+
+    g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
+                                                        QVIRTIO_BLK_TIMEOUT));
+
+    status = readb(r_req+528);
+    g_assert_cmpint(status, ==, 0);
+
+    data = g_malloc0(512);
+    memread(r_req+16, data, 512);
+    g_assert_cmpstr(data, ==, "TEST");
+    g_free(data);
+
+    guest_free(alloc, w_req);
+    guest_free(alloc, r_req);
+
+    /* End test */
+    guest_free(alloc, vqpci->vq.desc);
+    qpci_msix_disable(dev->pdev);
     qvirtio_pci_device_disable(dev);
     g_free(dev);
     test_end();
@@ -360,6 +488,7 @@ int main(int argc, char **argv)
 
     g_test_add_func("/virtio/blk/pci/basic", pci_basic);
     g_test_add_func("/virtio/blk/pci/indirect_config", pci_indirect_config);
+    g_test_add_func("/virtio/blk/pci/msix", pci_msix);
 
     ret = g_test_run();
 
-- 
1.7.10.4

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

* [Qemu-devel] [PATCH v4 11/11] libqos: Added EVENT_IDX support
  2014-08-12 11:41 [Qemu-devel] [PATCH v4 00/11] Virtio PCI libqos driver Marc Marí
                   ` (9 preceding siblings ...)
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 10/11] libqos: Added MSI-X support Marc Marí
@ 2014-08-12 11:41 ` Marc Marí
  10 siblings, 0 replies; 23+ messages in thread
From: Marc Marí @ 2014-08-12 11:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marc Marí, Paolo Bonzini, Stefan Hajnoczi

Added avail_event and NO_NOTIFY check before notifying.
Added used_event setting.

Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
 tests/libqos/virtio-pci.c |    1 +
 tests/libqos/virtio.c     |   26 +++++++++++++++++++++++++-
 tests/libqos/virtio.h     |    5 +++++
 tests/virtio-blk-test.c   |   33 ++++++++++++++++++++++++++++-----
 4 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index acf785a..a35978d 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -193,6 +193,7 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
     vqpci->vq.num_free = vqpci->vq.size;
     vqpci->vq.align = QVIRTIO_PCI_ALIGN;
     vqpci->vq.indirect = (feat & QVIRTIO_F_RING_INDIRECT_DESC) != 0;
+    vqpci->vq.event = (feat & QVIRTIO_F_RING_EVENT_IDX) != 0;
 
     vqpci->msix_entry = -1;
     vqpci->msix_addr = 0;
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index 7d1ee7c..ae640f2 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -124,9 +124,13 @@ void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq, uint64_t addr)
     writew(vq->avail, 0);
     /* vq->avail->idx */
     writew(vq->avail+2, 0);
+    /* vq->avail->used_event */
+    writew(vq->avail+4+(2*vq->size), 0);
 
     /* vq->used->flags */
     writew(vq->used, 0);
+    /* vq->used->avail_event */
+    writew(vq->used+2+(sizeof(struct QVRingUsedElem)*vq->size), 0);
 }
 
 QVRingIndirectDesc *qvring_indirect_desc_setup(QVirtioDevice *d,
@@ -221,11 +225,31 @@ void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d, QVirtQueue *vq,
 {
     /* vq->avail->idx */
     uint16_t idx = readl(vq->avail+2);
+    /* vq->used->flags */
+    uint16_t flags;
+    /* vq->used->avail_event */
+    uint16_t avail_event;
 
     /* vq->avail->ring[idx % vq->size] */
     writel(vq->avail+4+(2*(idx % vq->size)), free_head);
     /* vq->avail->idx */
     writel(vq->avail+2, idx+1);
 
-    bus->virtqueue_kick(d, vq);
+    /* Must read after idx is updated */
+    flags = readw(vq->avail);
+    avail_event = readw(vq->used+4+(sizeof(struct QVRingUsedElem)*vq->size));
+
+    /* < 1 because we add elements to avail queue one by one */
+    if ((flags & QVRING_USED_F_NO_NOTIFY) == 0 &&
+                            (!vq->event || (uint16_t)(idx-avail_event) < 1)) {
+        bus->virtqueue_kick(d, vq);
+    }
+}
+
+void qvirtqueue_set_used_event(QVirtQueue *vq, uint16_t idx)
+{
+    g_assert(vq->event);
+
+    /* vq->avail->used_event */
+    writew(vq->avail+4+(2*vq->size), idx);
 }
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 5010cfb..95eb519 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -26,6 +26,7 @@
 #define QVIRTIO_F_ANY_LAYOUT            0x08000000
 #define QVIRTIO_F_RING_INDIRECT_DESC    0x10000000
 #define QVIRTIO_F_RING_EVENT_IDX        0x20000000
+#define QVIRTIO_F_BAD_FEATURE           0x40000000
 
 #define QVRING_DESC_F_NEXT      0x1
 #define QVRING_DESC_F_WRITE     0x2
@@ -57,6 +58,7 @@ typedef struct QVRingAvail {
     uint16_t flags;
     uint16_t idx;
     uint16_t ring[0]; /* This is an array of uint16_t */
+    uint16_t used_event;
 } QVRingAvail;
 
 typedef struct QVRingUsedElem {
@@ -68,6 +70,7 @@ typedef struct QVRingUsed {
     uint16_t flags;
     uint16_t idx;
     QVRingUsedElem ring[0]; /* This is an array of QVRingUsedElem structs */
+    uint16_t avail_event;
 } QVRingUsed;
 
 typedef struct QVirtQueue {
@@ -80,6 +83,7 @@ typedef struct QVirtQueue {
     uint32_t num_free;
     uint32_t align;
     bool indirect;
+    bool event;
 } QVirtQueue;
 
 typedef struct QVRingIndirectDesc {
@@ -174,4 +178,5 @@ uint32_t qvirtqueue_add_indirect(QVirtQueue *vq, QVRingIndirectDesc *indirect);
 void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d, QVirtQueue *vq,
                                                             uint32_t free_head);
 
+void qvirtqueue_set_used_event(QVirtQueue *vq, uint16_t idx);
 #endif
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 8a8b6ec..1203772 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -353,7 +353,7 @@ static void pci_indirect_config(void)
     test_end();
 }
 
-static void pci_msix(void)
+static void pci_msix_idx(void)
 {
     QVirtioPCIDevice *dev;
     QPCIBus *bus;
@@ -395,7 +395,7 @@ static void pci_msix(void)
     features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
                             QVIRTIO_F_RING_INDIRECT_DESC |
-                            QVIRTIO_F_RING_EVENT_IDX | QVIRTIO_BLK_F_SCSI);
+                            QVIRTIO_F_NOTIFY_ON_EMPTY | QVIRTIO_BLK_F_SCSI);
     qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
 
     vqpci = (QVirtQueuePCI *)qvirtio_virtqueue_setup(&qvirtio_pci, &dev->vdev,
@@ -413,7 +413,6 @@ static void pci_msix(void)
     capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
     g_assert_cmpint(capacity, ==, n_size/512);
 
-    /* Write and read with 2 descriptor layout */
     data = g_malloc0(512);
     strcpy(data, "TEST");
 
@@ -430,13 +429,36 @@ static void pci_msix(void)
     /* w_req->status */
     writeb(w_req+528, 0xFF);
 
+    free_head = qvirtqueue_add(&vqpci->vq, w_req, 528, false, true);
+    qvirtqueue_add(&vqpci->vq, w_req+528, 1, true, false);
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);
+
+    g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
+                                                        QVIRTIO_BLK_TIMEOUT));
+
+    /* Write request */
+    w_req = guest_alloc(alloc, sizeof(QVirtioBlkReq)+512);
+    /* w_req->type */
+    writel(w_req, QVIRTIO_BLK_T_OUT);
+    /* w_req->ioprio */
+    writel(w_req+4, 1);
+    /* w_req->sector */
+    writeq(w_req+8, 1);
+    /* w_req->data */
+    memwrite(w_req+16, data, 512);
+    /* w_req->status */
+    writeb(w_req+528, 0xFF);
+
     g_free(data);
 
+    /* Notify after processing the third request */
+    qvirtqueue_set_used_event(&vqpci->vq, 2);
     free_head = qvirtqueue_add(&vqpci->vq, w_req, 528, false, true);
     qvirtqueue_add(&vqpci->vq, w_req+528, 1, true, false);
     qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);
 
-    g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
+    /* No notification expected */
+    g_assert(!qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
                                                         QVIRTIO_BLK_TIMEOUT));
 
     status = readb(w_req+528);
@@ -458,6 +480,7 @@ static void pci_msix(void)
 
     qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);
 
+
     g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
                                                         QVIRTIO_BLK_TIMEOUT));
 
@@ -488,7 +511,7 @@ int main(int argc, char **argv)
 
     g_test_add_func("/virtio/blk/pci/basic", pci_basic);
     g_test_add_func("/virtio/blk/pci/indirect_config", pci_indirect_config);
-    g_test_add_func("/virtio/blk/pci/msix", pci_msix);
+    g_test_add_func("/virtio/blk/pci/msix_idx", pci_msix_idx);
 
     ret = g_test_run();
 
-- 
1.7.10.4

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

* Re: [Qemu-devel] [PATCH v4 02/11] tests: Add virtio device initialization
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 02/11] tests: Add virtio device initialization Marc Marí
@ 2014-08-13 16:52   ` Stefan Hajnoczi
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-08-13 16:52 UTC (permalink / raw)
  To: Marc Marí; +Cc: Paolo Bonzini, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 562 bytes --]

On Tue, Aug 12, 2014 at 01:41:47PM +0200, Marc Marí wrote:
> +static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, void *addr)
> +{
> +    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> +    return qpci_io_readl(dev->pdev, addr) | qpci_io_readl(dev->pdev, addr+4);

This is broken because it never shifts bits into the upper 32 bits.
Which value to shift up depends on endianness.  Another solution is to
do byte reads into a uint64_t variable like memcpy.

I suggest you simply drop 64-bit accesses for now since they are
probably not used.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [Qemu-devel] [PATCH v4 01/11] tests: Functions bus_foreach and device_find from libqos virtio API
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 01/11] tests: Functions bus_foreach and device_find from libqos virtio API Marc Marí
@ 2014-08-13 16:52   ` Stefan Hajnoczi
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-08-13 16:52 UTC (permalink / raw)
  To: Marc Marí; +Cc: Paolo Bonzini, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 878 bytes --]

On Tue, Aug 12, 2014 at 01:41:46PM +0200, Marc Marí wrote:
> Virtio header has been changed to compile and work with a real device.
> Functions bus_foreach and device_find have been implemented for PCI.
> Virtio-blk test case now opens a fake device.
> 
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
>  tests/Makefile            |    3 +-
>  tests/libqos/virtio-pci.c |   75 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/libqos/virtio-pci.h |   24 +++++++++++++++
>  tests/libqos/virtio.h     |   23 ++++++++++++++
>  tests/virtio-blk-test.c   |   61 +++++++++++++++++++++++++++++++-----
>  5 files changed, 177 insertions(+), 9 deletions(-)
>  create mode 100644 tests/libqos/virtio-pci.c
>  create mode 100644 tests/libqos/virtio-pci.h
>  create mode 100644 tests/libqos/virtio.h

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [Qemu-devel] [PATCH v4 03/11] libqtest: add QTEST_LOG for debugging qtest testcases
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 03/11] libqtest: add QTEST_LOG for debugging qtest testcases Marc Marí
@ 2014-08-13 16:53   ` Stefan Hajnoczi
  2014-08-13 16:56   ` Stefan Hajnoczi
  1 sibling, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-08-13 16:53 UTC (permalink / raw)
  To: Marc Marí; +Cc: Paolo Bonzini, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 320 bytes --]

On Tue, Aug 12, 2014 at 01:41:48PM +0200, Marc Marí wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
>  tests/libqtest.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [Qemu-devel] [PATCH v4 03/11] libqtest: add QTEST_LOG for debugging qtest testcases
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 03/11] libqtest: add QTEST_LOG for debugging qtest testcases Marc Marí
  2014-08-13 16:53   ` Stefan Hajnoczi
@ 2014-08-13 16:56   ` Stefan Hajnoczi
  1 sibling, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-08-13 16:56 UTC (permalink / raw)
  To: Marc Marí; +Cc: Paolo Bonzini, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]

On Tue, Aug 12, 2014 at 01:41:48PM +0200, Marc Marí wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
>  tests/libqtest.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [Qemu-devel] [PATCH v4 04/11] libqos: Correct mask to align size to PAGE_SIZE in malloc-pc
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 04/11] libqos: Correct mask to align size to PAGE_SIZE in malloc-pc Marc Marí
@ 2014-08-13 16:56   ` Stefan Hajnoczi
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-08-13 16:56 UTC (permalink / raw)
  To: Marc Marí; +Cc: Paolo Bonzini, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]

On Tue, Aug 12, 2014 at 01:41:49PM +0200, Marc Marí wrote:
> Reviewed-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
>  tests/libqos/malloc-pc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [Qemu-devel] [PATCH v4 05/11] libqos: Change free function called in malloc
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 05/11] libqos: Change free function called in malloc Marc Marí
@ 2014-08-13 16:56   ` Stefan Hajnoczi
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-08-13 16:56 UTC (permalink / raw)
  To: Marc Marí; +Cc: Paolo Bonzini, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 405 bytes --]

On Tue, Aug 12, 2014 at 01:41:50PM +0200, Marc Marí wrote:
> Reviewed-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
>  tests/libqos/malloc.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [Qemu-devel] [PATCH v4 06/11] virtio-blk: Correct bug in support for flexible descriptor layout
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 06/11] virtio-blk: Correct bug in support for flexible descriptor layout Marc Marí
@ 2014-08-13 16:56   ` Stefan Hajnoczi
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-08-13 16:56 UTC (permalink / raw)
  To: Marc Marí; +Cc: Paolo Bonzini, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 550 bytes --]

On Tue, Aug 12, 2014 at 01:41:51PM +0200, Marc Marí wrote:
> Without this correction, only a three descriptor layout is accepted, and
> requests with just two descriptors are not completed and no error message is
> displayed.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
>  hw/block/virtio-blk.c |   14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [Qemu-devel] [PATCH v4 07/11] libqos: Added basic virtqueue support to virtio implementation
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 07/11] libqos: Added basic virtqueue support to virtio implementation Marc Marí
@ 2014-08-13 17:27   ` Stefan Hajnoczi
  2014-08-13 21:59     ` Marc Marí
  0 siblings, 1 reply; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-08-13 17:27 UTC (permalink / raw)
  To: Marc Marí; +Cc: Paolo Bonzini, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1802 bytes --]

On Tue, Aug 12, 2014 at 01:41:52PM +0200, Marc Marí wrote:
> +    /* Check power of 2 */
> +    aux = vq->size;
> +    while ((aux & 1) != 0) {
> +        aux = aux >> 1;
> +    }
> +    g_assert_cmpint(aux, !=, 1);

Power of 2 can be tested like this without a while loop:
g_assert_cmpint(vq->size & (vq->size - 1), ==, 0)

> +void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq, uint64_t addr)
> +{
> +    int i;
> +
> +    vq->desc = addr;
> +    vq->avail = vq->desc + vq->size*sizeof(QVRingDesc);
> +    vq->used = (uint64_t)((vq->avail + sizeof(uint16_t) * (3 + vq->size)
> +        +vq->align-1) & ~(vq->align - 1));
> +
> +    for (i = 0; i < vq->size-1; i++) {
> +        /* vq->desc[i].addr */
> +        writew(vq->desc+(16*i), 0);
> +        /* vq->desc[i].next */
> +        writew(vq->desc+(16*i)+14, i+1);
> +    }

Please use space between operators:
writew(vq->desc + (16 * i) + 14, i + 1)

This applies to all patches.

> +void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d, QVirtQueue *vq,
> +                                                            uint32_t free_head)
> +{
> +    /* vq->avail->idx */
> +    uint16_t idx = readl(vq->avail+2);
> +
> +    /* vq->avail->ring[idx % vq->size] */
> +    writel(vq->avail+4+(2*(idx % vq->size)), free_head);

If you want to use typed pointers you can.  It will make the code nicer
to read, just be careful never to dereference them and only to access
through writel()/readl():

typedef struct {
uint16_t foo; /* Forgot what these fields are called and didn't check */
uint16_t bar;
uint16_t ring[];
} VirtioAvail;

writel(&vq->avail.ring[idx % vq->size], free_head);

Now it looks more like normal C and the compiler is doing the address
calculations for us.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [Qemu-devel] [PATCH v4 09/11] libqos: Added test case for configuration changes in virtio-blk test
  2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 09/11] libqos: Added test case for configuration changes in virtio-blk test Marc Marí
@ 2014-08-13 17:34   ` Stefan Hajnoczi
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-08-13 17:34 UTC (permalink / raw)
  To: Marc Marí; +Cc: Paolo Bonzini, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]

On Tue, Aug 12, 2014 at 01:41:54PM +0200, Marc Marí wrote:
> @@ -349,7 +359,7 @@ int main(int argc, char **argv)
>      g_test_init(&argc, &argv, NULL);
>  
>      g_test_add_func("/virtio/blk/pci/basic", pci_basic);
> -    g_test_add_func("/virtio/blk/pci/indirect", pci_indirect);
> +    g_test_add_func("/virtio/blk/pci/indirect_config", pci_indirect_config);

Can you make this an independent test?  I think it is unrelated to
indirect descriptors.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [Qemu-devel] [PATCH v4 07/11] libqos: Added basic virtqueue support to virtio implementation
  2014-08-13 17:27   ` Stefan Hajnoczi
@ 2014-08-13 21:59     ` Marc Marí
  2014-08-17  9:37       ` Paolo Bonzini
  0 siblings, 1 reply; 23+ messages in thread
From: Marc Marí @ 2014-08-13 21:59 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Paolo Bonzini, qemu-devel

El Wed, 13 Aug 2014 18:27:40 +0100
Stefan Hajnoczi <stefanha@redhat.com> escribió:
> > +void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq,
> > uint64_t addr) +{
> > +    int i;
> > +
> > +    vq->desc = addr;
> > +    vq->avail = vq->desc + vq->size*sizeof(QVRingDesc);
> > +    vq->used = (uint64_t)((vq->avail + sizeof(uint16_t) * (3 +
> > vq->size)
> > +        +vq->align-1) & ~(vq->align - 1));
> > +
> > +    for (i = 0; i < vq->size-1; i++) {
> > +        /* vq->desc[i].addr */
> > +        writew(vq->desc+(16*i), 0);
> > +        /* vq->desc[i].next */
> > +        writew(vq->desc+(16*i)+14, i+1);
> > +    }
> 
> Please use space between operators:
> writew(vq->desc + (16 * i) + 14, i + 1)
> 
> This applies to all patches.
> 
> > +void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d,
> > QVirtQueue *vq,
> > +
> > uint32_t free_head) +{
> > +    /* vq->avail->idx */
> > +    uint16_t idx = readl(vq->avail+2);
> > +
> > +    /* vq->avail->ring[idx % vq->size] */
> > +    writel(vq->avail+4+(2*(idx % vq->size)), free_head);
> 
> If you want to use typed pointers you can.  It will make the code
> nicer to read, just be careful never to dereference them and only to
> access through writel()/readl():
> 
> typedef struct {
> uint16_t foo; /* Forgot what these fields are called and didn't check
> */ uint16_t bar;
> uint16_t ring[];
> } VirtioAvail;
> 
> writel(&vq->avail.ring[idx % vq->size], free_head);
> 
> Now it looks more like normal C and the compiler is doing the address
> calculations for us.

If it is not dereferenced, it can work. But all references will have to
be casted from (void *) to (uint64_t) always which is what readl and
writel expect. I think this is better than calculating the addresses,
but is still a bit ugly.

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

* Re: [Qemu-devel] [PATCH v4 07/11] libqos: Added basic virtqueue support to virtio implementation
  2014-08-13 21:59     ` Marc Marí
@ 2014-08-17  9:37       ` Paolo Bonzini
  0 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2014-08-17  9:37 UTC (permalink / raw)
  To: Marc Marí, Stefan Hajnoczi; +Cc: qemu-devel

Il 13/08/2014 23:59, Marc Marí ha scritto:
> If it is not dereferenced, it can work. But all references will have to
> be casted from (void *) to (uint64_t) always which is what readl and
> writel expect. I think this is better than calculating the addresses,
> but is still a bit ugly.

It would also break subtly on 32-bit builds if we ever try using
addresses above the first 4G (which could be an interesting test, and
doesn't require lots of host memory if one uses hotplug).

Paolo

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

end of thread, other threads:[~2014-08-17  9:37 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-12 11:41 [Qemu-devel] [PATCH v4 00/11] Virtio PCI libqos driver Marc Marí
2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 01/11] tests: Functions bus_foreach and device_find from libqos virtio API Marc Marí
2014-08-13 16:52   ` Stefan Hajnoczi
2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 02/11] tests: Add virtio device initialization Marc Marí
2014-08-13 16:52   ` Stefan Hajnoczi
2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 03/11] libqtest: add QTEST_LOG for debugging qtest testcases Marc Marí
2014-08-13 16:53   ` Stefan Hajnoczi
2014-08-13 16:56   ` Stefan Hajnoczi
2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 04/11] libqos: Correct mask to align size to PAGE_SIZE in malloc-pc Marc Marí
2014-08-13 16:56   ` Stefan Hajnoczi
2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 05/11] libqos: Change free function called in malloc Marc Marí
2014-08-13 16:56   ` Stefan Hajnoczi
2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 06/11] virtio-blk: Correct bug in support for flexible descriptor layout Marc Marí
2014-08-13 16:56   ` Stefan Hajnoczi
2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 07/11] libqos: Added basic virtqueue support to virtio implementation Marc Marí
2014-08-13 17:27   ` Stefan Hajnoczi
2014-08-13 21:59     ` Marc Marí
2014-08-17  9:37       ` Paolo Bonzini
2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 08/11] libqos: Added indirect descriptor " Marc Marí
2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 09/11] libqos: Added test case for configuration changes in virtio-blk test Marc Marí
2014-08-13 17:34   ` Stefan Hajnoczi
2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 10/11] libqos: Added MSI-X support Marc Marí
2014-08-12 11:41 ` [Qemu-devel] [PATCH v4 11/11] libqos: Added EVENT_IDX support Marc Marí

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).