All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.