[Qemu-devel] [PATCH 0/7] Virtio PCI libqos driver

[Qemu-devel] [PATCH 0/7] Virtio PCI libqos driver

[Marc Marí]

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

Marc Marí (7):
  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

 hw/block/virtio-blk.c     |   14 +--
 tests/Makefile            |    3 +-
 tests/libqos/malloc-pc.c  |    2 +-
 tests/libqos/malloc.h     |    2 +-
 tests/libqos/virtio-pci.c |  189 +++++++++++++++++++++++++++++++++++++
 tests/libqos/virtio-pci.h |   48 ++++++++++
 tests/libqos/virtio.c     |  120 ++++++++++++++++++++++++
 tests/libqos/virtio.h     |  127 +++++++++++++++++++++++++
 tests/libqtest.c          |   11 ++-
 tests/virtio-blk-test.c   |  228 +++++++++++++++++++++++++++++++++++++++++++--
 10 files changed, 725 insertions(+), 19 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

[Qemu-devel] [PATCH 1/7] tests: Functions bus_foreach and device_find from libqos virtio API

[Marc Marí]

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   |   67 +++++++++++++++++++++++++++++++++++-----
 5 files changed, 183 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..52894f4 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,65 @@
 
 #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 char *last_tmp_path;
+
+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);
+
+    last_tmp_path = g_malloc0(strlen(tmp_path));
+    strcpy(last_tmp_path, tmp_path);
+
+    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);
+
+    return qpci_init_pc();
+}
+
+static void test_end(void)
 {
+    qtest_end();
+    unlink(last_tmp_path);
+    g_free(last_tmp_path);
+}
+
+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 +76,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

[Qemu-devel] [PATCH 2/7] tests: Add virtio device initialization

[Marc Marí]

Add functions to read and write virtio header fields.
Add feature checking and 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 |   32 ++++++++++++++++++++++++++++++++
 tests/libqos/virtio-pci.h |   17 +++++++++++++++++
 tests/libqos/virtio.c     |   31 +++++++++++++++++++++++++++++++
 tests/libqos/virtio.h     |   19 +++++++++++++++++++
 tests/virtio-blk-test.c   |   14 ++++++++++++++
 6 files changed, 114 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..a04281e 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -55,6 +55,30 @@ static void qvirtio_pci_assign_device(QVirtioDevice *d, void *data)
     *vpcidev = (QVirtioPCIDevice *)d;
 }
 
+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 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 = {
+    .get_features = qvirtio_pci_get_features,
+    .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 +97,11 @@ QVirtioPCIDevice *qvirtio_pci_device_find(QPCIBus *bus, uint16_t device_type)
 
     return dev;
 }
+
+void qvirtio_pci_enable_device(QVirtioPCIDevice *d)
+{
+    qpci_device_enable(d->pdev);
+    d->addr = qpci_iomap(d->pdev, 0);
+    g_assert(d->addr != NULL);
+}
+
diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
index 5101abb..ef7744e 100644
--- a/tests/libqos/virtio-pci.h
+++ b/tests/libqos/virtio-pci.h
@@ -13,12 +13,29 @@
 #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_enable_device(QVirtioPCIDevice *d);
 #endif
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
new file mode 100644
index 0000000..118cb15
--- /dev/null
+++ b/tests/libqos/virtio.c
@@ -0,0 +1,31 @@
+/*
+ * 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"
+
+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..a2e3219 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,19 @@ typedef struct QVirtioDevice {
     uint16_t device_type;
 } QVirtioDevice;
 
+typedef struct QVirtioBus {
+    /* Get features of the device */
+    uint32_t (*get_features)(QVirtioDevice *d);
+
+    /* 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;
+
+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 52894f4..83be520 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -59,6 +59,8 @@ static void pci_basic(void)
 {
     QVirtioPCIDevice *dev;
     QPCIBus *bus;
+    void *addr;
+    uint64_t capacity;
 
     bus = test_start();
 
@@ -67,6 +69,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_enable_device(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 = qpci_io_readl(dev->pdev, addr) |
+                qpci_io_readl(dev->pdev, addr+4);
+    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE/512);
+
     g_free(dev);
     test_end();
 }
-- 
1.7.10.4

[Qemu-devel] [PATCH 3/7] libqtest: add QTEST_LOG for debugging qtest testcases

[Marc Marí]

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

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 98e8f4b..056275d 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,18 @@ QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
 
     /* No need to send anything for an empty QObject.  */
     if (qobj) {
+        size_t len;
+        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) {
+            len = write(2, str, size);
+            if (len != size) {
+                fprintf(stderr, "Could not log\n");
+            }
+        }
         /* Send QMP request */
         socket_send(s->qmp_fd, str, size);
 
-- 
1.7.10.4

[Qemu-devel] [PATCH 4/7] libqos: Correct mask to align size to PAGE_SIZE in malloc-pc

[Marc Marí]

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

[Qemu-devel] [PATCH 5/7] libqos: Change free function called in malloc

[Marc Marí]

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

[Qemu-devel] [PATCH 6/7] virtio-blk: Correct bug in support for flexible descriptor layout

[Marc Marí]

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

[Qemu-devel] [PATCH 7/7] libqos: Added basic virtqueue support to virtio implementation

[Marc Marí]

Add status changing.
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 |   86 ++++++++++++++++++++++++-
 tests/libqos/virtio-pci.h |    7 ++
 tests/libqos/virtio.c     |   89 ++++++++++++++++++++++++++
 tests/libqos/virtio.h     |   87 ++++++++++++++++++++++++-
 tests/virtio-blk-test.c   |  155 +++++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 417 insertions(+), 7 deletions(-)

diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index a04281e..fdbb10e 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -8,11 +8,15 @@
  */
 
 #include <glib.h>
+#include <stdio.h>
+#include <inttypes.h>
 #include "libqtest.h"
 #include "libqos/virtio.h"
 #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"
 
@@ -61,22 +65,100 @@ static uint32_t qvirtio_pci_get_features(QVirtioDevice *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, uint16_t addr)
+{
+    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+    qpci_io_writel(dev->pdev, dev->addr + QVIRTIO_QUEUE_ADDRESS, addr);
+}
+
+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 = {
     .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 ef7744e..ec48d19 100644
--- a/tests/libqos/virtio-pci.h
+++ b/tests/libqos/virtio-pci.h
@@ -26,6 +26,13 @@
 #define QVIRTIO_DEVICE_SPECIFIC_MSIX    0x18
 #define QVIRTIO_DEVICE_SPECIFIC_NO_MSIX 0x14
 
+#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_PCI_ALIGN   4096
+
 typedef struct QVirtioPCIDevice {
     QVirtioDevice vdev;
     QPCIDevice *pdev;
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index 118cb15..d196495 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -8,6 +8,8 @@
  */
 
 #include <glib.h>
+#include <stdio.h>
+#include <inttypes.h>
 #include "libqtest.h"
 #include "libqos/virtio.h"
 
@@ -29,3 +31,90 @@ 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);
+}
+
+int 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);
+
+    if (timeout != 0) {
+        return 0;
+    } else {
+        return -1;
+    }
+}
+
+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[aux].addr */
+        writew(vq->desc+(16*i), 0);
+        /* vq->desc[aux].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, int write,
+                                                                    int 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 */
+    uint32_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 a2e3219..f684e38 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -10,33 +10,118 @@
 #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 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 {
     /* 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, uint16_t addr);
+
+    /* 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;
+}
+
 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);
+
+int qvirtio_wait_isr(const QVirtioBus *bus, QVirtioDevice *d, uint8_t mask,
+                                                            uint64_t timeout);
+
+void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq, uint64_t addr);
+uint32_t qvirtqueue_add(QVirtQueue *vq, uint64_t data, uint32_t len, int write,
+                                                                    int 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 83be520..909817d 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     10000
+#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 char *last_tmp_path;
 
@@ -59,8 +89,17 @@ static void pci_basic(void)
 {
     QVirtioPCIDevice *dev;
     QPCIBus *bus;
+    QVirtQueue *vq;
+    QGuestAllocator *alloc;
+    int isr_result;
     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();
 
@@ -81,6 +120,114 @@ static void pci_basic(void)
                 qpci_io_readl(dev->pdev, addr+4);
     g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE/512);
 
+    features = qvirtio_pci.get_features(&dev->vdev);
+    features = features & ~(QVIRTIO_F_RING_INDIRECT_DESC |
+                                QVIRTIO_F_RING_EVENT_IDX | QVIRTIO_BLK_F_SCSI);
+    qvirtio_pci.set_features(&dev->vdev, features);
+
+    alloc = pc_alloc_init();
+    vq = qvirtio_pci.virtqueue_setup(&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, 0, 1);
+    qvirtqueue_add(vq, w_req+528, 1, 1, 0);
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+
+    isr_result = qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+                                                        QVIRTIO_BLK_TIMEOUT);
+    g_assert_cmpint(isr_result, ==, 0);
+    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, 0, 1);
+    qvirtqueue_add(vq, r_req+16, 513, 1, 0);
+
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+
+    isr_result = qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+                                                        QVIRTIO_BLK_TIMEOUT);
+    g_assert_cmpint(isr_result, ==, 0);
+    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, 0, 1);
+    qvirtqueue_add(vq, w_req+16, 512, 0, 1);
+    qvirtqueue_add(vq, w_req+528, 1, 1, 0);
+
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+
+    isr_result = qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+                                                        QVIRTIO_BLK_TIMEOUT);
+    g_assert_cmpint(isr_result, ==, 0);
+    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, 0, 1);
+    qvirtqueue_add(vq, r_req+16, 512, 1, 1);
+    qvirtqueue_add(vq, r_req+528, 1, 1, 0);
+
+    qvirtqueue_kick(&qvirtio_pci, &dev->vdev, vq, free_head);
+
+    isr_result = qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x1,
+                                                        QVIRTIO_BLK_TIMEOUT);
+    g_assert_cmpint(isr_result, ==, 0);
+    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);
+
+    /* End test */
+    guest_free(alloc, vq->desc);
     g_free(dev);
     test_end();
 }
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 1/7] tests: Functions bus_foreach and device_find from libqos virtio API

[Stefan Hajnoczi]

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

On Thu, Jul 24, 2014 at 08:30:59PM +0200, Marc Marí wrote:
> +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);
> +
> +    last_tmp_path = g_malloc0(strlen(tmp_path));
> +    strcpy(last_tmp_path, tmp_path);
> +
> +    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);

Please unlink the temporary disk image file here.

The QEMU process has a file descriptor open when we reach this point, so
it's safe to delete it on disk (the file stays allocated until the last
file descriptor is closed).

This is important so that the temporary file is always deleted in
failure cases.  We do not reach test_end() when an assertion fails.

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

Re: [Qemu-devel] [PATCH 2/7] tests: Add virtio device initialization

[Stefan Hajnoczi]

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

On Thu, Jul 24, 2014 at 08:31:00PM +0200, Marc Marí wrote:
> +static uint32_t qvirtio_pci_get_features(QVirtioDevice *d)
> +{
> +    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> +    return qpci_io_readl(dev->pdev, dev->addr + QVIRTIO_DEVICE_FEATURES);
> +}

Unused?  If it's unused, then it's untested.

> +
> +static uint8_t qvirtio_pci_get_status(QVirtioDevice *d)
> +{
> +    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> +    return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_DEVICE_STATUS);
> +}

Unused?

> +
> +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);

Unused?

> @@ -73,3 +97,11 @@ QVirtioPCIDevice *qvirtio_pci_device_find(QPCIBus *bus, uint16_t device_type)
>  
>      return dev;
>  }
> +
> +void qvirtio_pci_enable_device(QVirtioPCIDevice *d)
> +{
> +    qpci_device_enable(d->pdev);
> +    d->addr = qpci_iomap(d->pdev, 0);
> +    g_assert(d->addr != NULL);
> +}

Where is qpci_iounmap() called to clean up?

> @@ -67,6 +69,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_enable_device(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 = qpci_io_readl(dev->pdev, addr) |
> +                qpci_io_readl(dev->pdev, addr+4);
> +    g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE/512);

Please add a qvirtio_config_read() function instead of directly
accessing the virtio configuration space via PCI.

Stefan

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

Re: [Qemu-devel] [PATCH 3/7] libqtest: add QTEST_LOG for debugging qtest testcases

[Stefan Hajnoczi]

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

On Thu, Jul 24, 2014 at 08:31:01PM +0200, Marc Marí wrote:
> @@ -397,10 +398,18 @@ QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
>  
>      /* No need to send anything for an empty QObject.  */
>      if (qobj) {
> +        size_t len;
> +        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) {
> +            len = write(2, str, size);
> +            if (len != size) {
> +                fprintf(stderr, "Could not log\n");

It's a bit funny that we print an error message to stderr after failing
to write to stderr.

Why not just fprintf(stderr, "%s", str) instead of using write()?

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

Re: [Qemu-devel] [PATCH 4/7] libqos: Correct mask to align size to PAGE_SIZE in malloc-pc

[Stefan Hajnoczi]

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

On Thu, Jul 24, 2014 at 08:31:02PM +0200, Marc Marí wrote:
> 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(-)

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

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

Re: [Qemu-devel] [PATCH 5/7] libqos: Change free function called in malloc

[Stefan Hajnoczi]

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

On Thu, Jul 24, 2014 at 08:31:03PM +0200, Marc Marí wrote:
> 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);
>  }

Hahahahahahaha!

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

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

Re: [Qemu-devel] [PATCH 7/7] libqos: Added basic virtqueue support to virtio implementation

[Stefan Hajnoczi]

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

On Thu, Jul 24, 2014 at 08:31:05PM +0200, Marc Marí wrote:
> +static void qvirtio_pci_set_queue_address(QVirtioDevice *d, uint16_t addr)
> +{
> +    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> +    qpci_io_writel(dev->pdev, dev->addr + QVIRTIO_QUEUE_ADDRESS, addr);
> +}

Why is addr uint16_t?  It should be a 32-bit Page Frame Number).  It's
probably clearer to name it "pfn" instead of "addr" since it's not an
address.

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

Re: [Qemu-devel] [PATCH 2/7] tests: Add virtio device initialization

[Marc Marí]

El Fri, 25 Jul 2014 16:19:41 +0100
Stefan Hajnoczi <stefanha@redhat.com> escribió:
> On Thu, Jul 24, 2014 at 08:31:00PM +0200, Marc Marí wrote:
> > +static uint32_t qvirtio_pci_get_features(QVirtioDevice *d)
> > +{
> > +    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> > +    return qpci_io_readl(dev->pdev, dev->addr +
> > QVIRTIO_DEVICE_FEATURES); +}
> 
> Unused?  If it's unused, then it's untested.

Yes, moved to the other patch for v2

> 
> > +
> > +static uint8_t qvirtio_pci_get_status(QVirtioDevice *d)
> > +{
> > +    QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> > +    return qpci_io_readb(dev->pdev, dev->addr +
> > QVIRTIO_DEVICE_STATUS); +}
> 
> Unused?

Used in virtio.c (qvirtio_reset / qvirtio_set_acknowledge /
qvirtio_set_driver).

 
> > +
> > +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);
> 
> Unused?

Also in virtio.c

> 
> > @@ -73,3 +97,11 @@ QVirtioPCIDevice
> > *qvirtio_pci_device_find(QPCIBus *bus, uint16_t device_type) 
> >      return dev;
> >  }
> > +
> > +void qvirtio_pci_enable_device(QVirtioPCIDevice *d)
> > +{
> > +    qpci_device_enable(d->pdev);
> > +    d->addr = qpci_iomap(d->pdev, 0);
> > +    g_assert(d->addr != NULL);
> > +}
> 
> Where is qpci_iounmap() called to clean up?

Missed. Also, it is unimplemented.
Marc

Re: [Qemu-devel] [PATCH 2/7] tests: Add virtio device initialization

[Stefan Hajnoczi]

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

On Fri, Jul 25, 2014 at 07:01:47PM +0200, Marc Marí wrote:
> > > @@ -73,3 +97,11 @@ QVirtioPCIDevice
> > > *qvirtio_pci_device_find(QPCIBus *bus, uint16_t device_type) 
> > >      return dev;
> > >  }
> > > +
> > > +void qvirtio_pci_enable_device(QVirtioPCIDevice *d)
> > > +{
> > > +    qpci_device_enable(d->pdev);
> > > +    d->addr = qpci_iomap(d->pdev, 0);
> > > +    g_assert(d->addr != NULL);
> > > +}
> > 
> > Where is qpci_iounmap() called to clean up?
> 
> Missed. Also, it is unimplemented.

It would be much harder to add in the appropriate guest_free() calls
later so users should still call it.  Just like we should call
guest_free() even if it is currently unimplemented.

Stefan

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

Re: [Qemu-devel] [PATCH 4/7] libqos: Correct mask to align size to PAGE_SIZE in malloc-pc

[John Snow]

On 07/24/2014 02:31 PM, Marc Marí wrote:
> 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);
>   
Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [PATCH 5/7] libqos: Change free function called in malloc

[John Snow]

On 07/24/2014 02:31 PM, Marc Marí wrote:
> 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
Imagine my surprise when I noticed the allocator was summoning up 
millions of bytes. "Oh good!"

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [PATCH 4/7] libqos: Correct mask to align size to PAGE_SIZE in malloc-pc

[Peter Maydell]

On 24 July 2014 19:31, Marc Marí <marc.mari.barcelo@gmail.com> wrote:
> 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;

Do we have access to the osdep.h ROUND_UP macro from libqos?
If so, we could replace these two lines with
     size = ROUND_UP(size, PAGE_SIZE);

thanks
-- PMM