* [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers
@ 2016-04-25 12:46 Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 1/8] libqos: use virtio_ids.h for device ID definitions Stefan Hajnoczi
` (8 more replies)
0 siblings, 9 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2016-04-25 12:46 UTC (permalink / raw)
To: qemu-devel; +Cc: marc.mari.barcelo, Paolo Bonzini, Stefan Hajnoczi
This patch series eliminates code duplication in libqos virtio.
include/standard-headers/ contains the Linux virtio header files so we don't
need to define our own version of the structs and constants.
Stefan Hajnoczi (8):
libqos: use virtio_ids.h for device ID definitions
libqos: drop duplicated PCI vendor ID definition
libqos: drop duplicated virtio_config.h definitions
libqos: drop duplicated virtio_ring.h bit definitions
libqos: drop duplicated virtio_vring.h structs
libqos: drop duplicated virtio_blk.h definitions
libqos: drop duplicated virtio_scsi.h definitions
libqos: drop duplicated virtio_pci.h definitions
tests/libqos/virtio-mmio.c | 5 +--
tests/libqos/virtio-pci.c | 50 ++++++++++++++-------------
tests/libqos/virtio-pci.h | 17 ---------
tests/libqos/virtio.c | 42 +++++++++++-----------
tests/libqos/virtio.h | 73 ++++-----------------------------------
tests/virtio-blk-test.c | 86 ++++++++++++++++++++--------------------------
tests/virtio-net-test.c | 10 +++---
tests/virtio-scsi-test.c | 53 +++++++++++-----------------
8 files changed, 123 insertions(+), 213 deletions(-)
--
2.5.5
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH for-2.7 1/8] libqos: use virtio_ids.h for device ID definitions
2016-04-25 12:46 [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers Stefan Hajnoczi
@ 2016-04-25 12:46 ` Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 2/8] libqos: drop duplicated PCI vendor ID definition Stefan Hajnoczi
` (7 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2016-04-25 12:46 UTC (permalink / raw)
To: qemu-devel; +Cc: marc.mari.barcelo, Paolo Bonzini, Stefan Hajnoczi
Avoid redefining device IDs. Use the standard Linux headers that are
already in the source tree.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/libqos/virtio.h | 9 ---------
tests/virtio-blk-test.c | 7 ++++---
tests/virtio-net-test.c | 5 +++--
tests/virtio-scsi-test.c | 5 +++--
4 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 0101278..af03793 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -19,15 +19,6 @@
#define QVIRTIO_DRIVER 0x2
#define QVIRTIO_DRIVER_OK 0x4
-#define QVIRTIO_NET_DEVICE_ID 0x1
-#define QVIRTIO_BLK_DEVICE_ID 0x2
-#define QVIRTIO_CONSOLE_DEVICE_ID 0x3
-#define QVIRTIO_RNG_DEVICE_ID 0x4
-#define QVIRTIO_BALLOON_DEVICE_ID 0x5
-#define QVIRTIO_RPMSG_DEVICE_ID 0x7
-#define QVIRTIO_SCSI_DEVICE_ID 0x8
-#define QVIRTIO_9P_DEVICE_ID 0x9
-
#define QVIRTIO_F_NOTIFY_ON_EMPTY 0x01000000
#define QVIRTIO_F_ANY_LAYOUT 0x08000000
#define QVIRTIO_F_RING_INDIRECT_DESC 0x10000000
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 3a66630..02107a6 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -19,6 +19,7 @@
#include "libqos/malloc-pc.h"
#include "libqos/malloc-generic.h"
#include "qemu/bswap.h"
+#include "standard-headers/linux/virtio_ids.h"
#define QVIRTIO_BLK_F_BARRIER 0x00000001
#define QVIRTIO_BLK_F_SIZE_MAX 0x00000002
@@ -119,9 +120,9 @@ static QVirtioPCIDevice *virtio_blk_pci_init(QPCIBus *bus, int slot)
{
QVirtioPCIDevice *dev;
- dev = qvirtio_pci_device_find(bus, QVIRTIO_BLK_DEVICE_ID);
+ dev = qvirtio_pci_device_find(bus, VIRTIO_ID_BLOCK);
g_assert(dev != NULL);
- g_assert_cmphex(dev->vdev.device_type, ==, QVIRTIO_BLK_DEVICE_ID);
+ g_assert_cmphex(dev->vdev.device_type, ==, VIRTIO_ID_BLOCK);
g_assert_cmphex(dev->pdev->devfn, ==, ((slot << 3) | PCI_FN));
qvirtio_pci_device_enable(dev);
@@ -733,7 +734,7 @@ static void mmio_basic(void)
dev = qvirtio_mmio_init_device(MMIO_DEV_BASE_ADDR, MMIO_PAGE_SIZE);
g_assert(dev != NULL);
- g_assert_cmphex(dev->vdev.device_type, ==, QVIRTIO_BLK_DEVICE_ID);
+ g_assert_cmphex(dev->vdev.device_type, ==, VIRTIO_ID_BLOCK);
qvirtio_reset(&qvirtio_mmio, &dev->vdev);
qvirtio_set_acknowledge(&qvirtio_mmio, &dev->vdev);
diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index 04cfcd5..7fd7a2f 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -21,6 +21,7 @@
#include "libqos/malloc-generic.h"
#include "qemu/bswap.h"
#include "hw/virtio/virtio-net.h"
+#include "standard-headers/linux/virtio_ids.h"
#define PCI_SLOT_HP 0x06
#define PCI_SLOT 0x04
@@ -40,9 +41,9 @@ static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot)
{
QVirtioPCIDevice *dev;
- dev = qvirtio_pci_device_find(bus, QVIRTIO_NET_DEVICE_ID);
+ dev = qvirtio_pci_device_find(bus, VIRTIO_ID_NET);
g_assert(dev != NULL);
- g_assert_cmphex(dev->vdev.device_type, ==, QVIRTIO_NET_DEVICE_ID);
+ g_assert_cmphex(dev->vdev.device_type, ==, VIRTIO_ID_NET);
qvirtio_pci_device_enable(dev);
qvirtio_reset(&qvirtio_pci, &dev->vdev);
diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index d78747a..625bd6d 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -18,6 +18,7 @@
#include "libqos/malloc.h"
#include "libqos/malloc-pc.h"
#include "libqos/malloc-generic.h"
+#include "standard-headers/linux/virtio_ids.h"
#define PCI_SLOT 0x02
#define PCI_FN 0x00
@@ -164,10 +165,10 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
vs->alloc = pc_alloc_init();
vs->bus = qpci_init_pc();
- dev = qvirtio_pci_device_find(vs->bus, QVIRTIO_SCSI_DEVICE_ID);
+ dev = qvirtio_pci_device_find(vs->bus, VIRTIO_ID_SCSI);
vs->dev = (QVirtioDevice *)dev;
g_assert(dev != NULL);
- g_assert_cmphex(vs->dev->device_type, ==, QVIRTIO_SCSI_DEVICE_ID);
+ g_assert_cmphex(vs->dev->device_type, ==, VIRTIO_ID_SCSI);
qvirtio_pci_device_enable(dev);
qvirtio_reset(&qvirtio_pci, vs->dev);
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH for-2.7 2/8] libqos: drop duplicated PCI vendor ID definition
2016-04-25 12:46 [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 1/8] libqos: use virtio_ids.h for device ID definitions Stefan Hajnoczi
@ 2016-04-25 12:46 ` Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 3/8] libqos: drop duplicated virtio_config.h definitions Stefan Hajnoczi
` (6 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2016-04-25 12:46 UTC (permalink / raw)
To: qemu-devel; +Cc: marc.mari.barcelo, Paolo Bonzini, Stefan Hajnoczi
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/libqos/virtio-pci.c | 3 ++-
tests/libqos/virtio.h | 2 --
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index fde2ff0..275c14d 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -17,6 +17,7 @@
#include "libqos/malloc.h"
#include "libqos/malloc-pc.h"
+#include "hw/pci/pci.h"
#include "hw/pci/pci_regs.h"
typedef struct QVirtioPCIForeachData {
@@ -264,7 +265,7 @@ void qvirtio_pci_foreach(QPCIBus *bus, uint16_t device_type,
.device_type = device_type,
.user_data = data };
- qpci_device_foreach(bus, QVIRTIO_VENDOR_ID, -1,
+ qpci_device_foreach(bus, PCI_VENDOR_ID_REDHAT_QUMRANET, -1,
qvirtio_pci_foreach_callback, &d);
}
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index af03793..e663bcf 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -12,8 +12,6 @@
#include "libqos/malloc.h"
-#define QVIRTIO_VENDOR_ID 0x1AF4
-
#define QVIRTIO_RESET 0x0
#define QVIRTIO_ACKNOWLEDGE 0x1
#define QVIRTIO_DRIVER 0x2
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH for-2.7 3/8] libqos: drop duplicated virtio_config.h definitions
2016-04-25 12:46 [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 1/8] libqos: use virtio_ids.h for device ID definitions Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 2/8] libqos: drop duplicated PCI vendor ID definition Stefan Hajnoczi
@ 2016-04-25 12:46 ` Stefan Hajnoczi
2016-05-08 18:22 ` Marc Marí
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 4/8] libqos: drop duplicated virtio_ring.h bit definitions Stefan Hajnoczi
` (5 subsequent siblings)
8 siblings, 1 reply; 14+ messages in thread
From: Stefan Hajnoczi @ 2016-04-25 12:46 UTC (permalink / raw)
To: qemu-devel; +Cc: marc.mari.barcelo, Paolo Bonzini, Stefan Hajnoczi
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/libqos/virtio.c | 19 ++++++++++---------
tests/libqos/virtio.h | 9 ---------
tests/virtio-blk-test.c | 5 +++--
3 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index 613dece..ee9e892 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -11,6 +11,7 @@
#include <glib.h>
#include "libqtest.h"
#include "libqos/virtio.h"
+#include "standard-headers/linux/virtio_config.h"
uint8_t qvirtio_config_readb(const QVirtioBus *bus, QVirtioDevice *d,
uint64_t addr)
@@ -55,28 +56,28 @@ QVirtQueue *qvirtqueue_setup(const QVirtioBus *bus, QVirtioDevice *d,
void qvirtio_reset(const QVirtioBus *bus, QVirtioDevice *d)
{
- bus->set_status(d, QVIRTIO_RESET);
- g_assert_cmphex(bus->get_status(d), ==, QVIRTIO_RESET);
+ bus->set_status(d, 0);
+ g_assert_cmphex(bus->get_status(d), ==, 0);
}
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);
+ bus->set_status(d, bus->get_status(d) | VIRTIO_CONFIG_S_ACKNOWLEDGE);
+ g_assert_cmphex(bus->get_status(d), ==, VIRTIO_CONFIG_S_ACKNOWLEDGE);
}
void qvirtio_set_driver(const QVirtioBus *bus, QVirtioDevice *d)
{
- bus->set_status(d, bus->get_status(d) | QVIRTIO_DRIVER);
+ bus->set_status(d, bus->get_status(d) | VIRTIO_CONFIG_S_DRIVER);
g_assert_cmphex(bus->get_status(d), ==,
- QVIRTIO_DRIVER | QVIRTIO_ACKNOWLEDGE);
+ VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_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);
+ bus->set_status(d, bus->get_status(d) | VIRTIO_CONFIG_S_DRIVER_OK);
+ g_assert_cmphex(bus->get_status(d), ==, VIRTIO_CONFIG_S_DRIVER_OK |
+ VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_ACKNOWLEDGE);
}
void qvirtio_wait_queue_isr(const QVirtioBus *bus, QVirtioDevice *d,
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index e663bcf..993ae0e 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -12,13 +12,6 @@
#include "libqos/malloc.h"
-#define QVIRTIO_RESET 0x0
-#define QVIRTIO_ACKNOWLEDGE 0x1
-#define QVIRTIO_DRIVER 0x2
-#define QVIRTIO_DRIVER_OK 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
@@ -27,8 +20,6 @@
#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
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 02107a6..bfeffc4 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -20,6 +20,7 @@
#include "libqos/malloc-generic.h"
#include "qemu/bswap.h"
#include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/virtio_config.h"
#define QVIRTIO_BLK_F_BARRIER 0x00000001
#define QVIRTIO_BLK_F_SIZE_MAX 0x00000002
@@ -240,7 +241,7 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
guest_free(alloc, req_addr);
- if (features & QVIRTIO_F_ANY_LAYOUT) {
+ if (features & VIRTIO_F_ANY_LAYOUT) {
/* Write and read with 2 descriptor layout */
/* Write request */
req.type = QVIRTIO_BLK_T_OUT;
@@ -607,7 +608,7 @@ static void pci_idx(void)
features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
features = features & ~(QVIRTIO_F_BAD_FEATURE |
QVIRTIO_F_RING_INDIRECT_DESC |
- QVIRTIO_F_NOTIFY_ON_EMPTY | QVIRTIO_BLK_F_SCSI);
+ VIRTIO_F_NOTIFY_ON_EMPTY | QVIRTIO_BLK_F_SCSI);
qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH for-2.7 4/8] libqos: drop duplicated virtio_ring.h bit definitions
2016-04-25 12:46 [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers Stefan Hajnoczi
` (2 preceding siblings ...)
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 3/8] libqos: drop duplicated virtio_config.h definitions Stefan Hajnoczi
@ 2016-04-25 12:46 ` Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 5/8] libqos: drop duplicated virtio_vring.h structs Stefan Hajnoczi
` (4 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2016-04-25 12:46 UTC (permalink / raw)
To: qemu-devel; +Cc: marc.mari.barcelo, Paolo Bonzini, Stefan Hajnoczi
Note that virtio_ring.h defines feature bits using their bit number:
#define VIRTIO_RING_F_INDIRECT_DESC 28
On the other hand libqos virtio.h uses the bit mask:
#define QVIRTIO_F_RING_INDIRECT_DESC 0x10000000
The patch makes the necessary adjustments.
I have used "1 << BITMASK" instead of "1ULL << BITMASK" because the
64-bit feature fields are not implemented in libqos virtio.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/libqos/virtio-mmio.c | 5 +++--
tests/libqos/virtio-pci.c | 5 +++--
tests/libqos/virtio.c | 13 +++++++------
tests/libqos/virtio.h | 14 --------------
tests/virtio-blk-test.c | 21 +++++++++++++--------
tests/virtio-net-test.c | 5 +++--
6 files changed, 29 insertions(+), 34 deletions(-)
diff --git a/tests/libqos/virtio-mmio.c b/tests/libqos/virtio-mmio.c
index a4382f3..7756b92 100644
--- a/tests/libqos/virtio-mmio.c
+++ b/tests/libqos/virtio-mmio.c
@@ -14,6 +14,7 @@
#include "libqos/virtio-mmio.h"
#include "libqos/malloc.h"
#include "libqos/malloc-generic.h"
+#include "standard-headers/linux/virtio_ring.h"
static uint8_t qvirtio_mmio_config_readb(QVirtioDevice *d, uint64_t addr)
{
@@ -136,8 +137,8 @@ static QVirtQueue *qvirtio_mmio_virtqueue_setup(QVirtioDevice *d,
vq->free_head = 0;
vq->num_free = vq->size;
vq->align = dev->page_size;
- vq->indirect = (dev->features & QVIRTIO_F_RING_INDIRECT_DESC) != 0;
- vq->event = (dev->features & QVIRTIO_F_RING_EVENT_IDX) != 0;
+ vq->indirect = (dev->features & (1 << VIRTIO_RING_F_INDIRECT_DESC)) != 0;
+ vq->event = (dev->features & (1 << VIRTIO_RING_F_EVENT_IDX)) != 0;
writel(dev->addr + QVIRTIO_MMIO_QUEUE_NUM, vq->size);
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 275c14d..4a0bef8 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -16,6 +16,7 @@
#include "libqos/pci-pc.h"
#include "libqos/malloc.h"
#include "libqos/malloc-pc.h"
+#include "standard-headers/linux/virtio_ring.h"
#include "hw/pci/pci.h"
#include "hw/pci/pci_regs.h"
@@ -213,8 +214,8 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *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->vq.event = (feat & QVIRTIO_F_RING_EVENT_IDX) != 0;
+ vqpci->vq.indirect = (feat & (1 << VIRTIO_RING_F_INDIRECT_DESC)) != 0;
+ vqpci->vq.event = (feat & (1 << VIRTIO_RING_F_EVENT_IDX)) != 0;
vqpci->msix_entry = -1;
vqpci->msix_addr = 0;
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index ee9e892..69b5426 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -12,6 +12,7 @@
#include "libqtest.h"
#include "libqos/virtio.h"
#include "standard-headers/linux/virtio_config.h"
+#include "standard-headers/linux/virtio_ring.h"
uint8_t qvirtio_config_readb(const QVirtioBus *bus, QVirtioDevice *d,
uint64_t addr)
@@ -173,7 +174,7 @@ QVRingIndirectDesc *qvring_indirect_desc_setup(QVirtioDevice *d,
/* indirect->desc[i].addr */
writeq(indirect->desc + (16 * i), 0);
/* indirect->desc[i].flags */
- writew(indirect->desc + (16 * i) + 12, QVRING_DESC_F_NEXT);
+ writew(indirect->desc + (16 * i) + 12, VRING_DESC_F_NEXT);
/* indirect->desc[i].next */
writew(indirect->desc + (16 * i) + 14, i + 1);
}
@@ -191,7 +192,7 @@ void qvring_indirect_desc_add(QVRingIndirectDesc *indirect, uint64_t data,
flags = readw(indirect->desc + (16 * indirect->index) + 12);
if (write) {
- flags |= QVRING_DESC_F_WRITE;
+ flags |= VRING_DESC_F_WRITE;
}
/* indirect->desc[indirect->index].addr */
@@ -211,11 +212,11 @@ uint32_t qvirtqueue_add(QVirtQueue *vq, uint64_t data, uint32_t len, bool write,
vq->num_free--;
if (write) {
- flags |= QVRING_DESC_F_WRITE;
+ flags |= VRING_DESC_F_WRITE;
}
if (next) {
- flags |= QVRING_DESC_F_NEXT;
+ flags |= VRING_DESC_F_NEXT;
}
/* vq->desc[vq->free_head].addr */
@@ -242,7 +243,7 @@ uint32_t qvirtqueue_add_indirect(QVirtQueue *vq, QVRingIndirectDesc *indirect)
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);
+ writew(vq->desc + (16 * vq->free_head) + 12, VRING_DESC_F_INDIRECT);
return vq->free_head++; /* Return and increase, in this order */
}
@@ -268,7 +269,7 @@ void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d, QVirtQueue *vq,
(sizeof(struct QVRingUsedElem) * vq->size));
/* < 1 because we add elements to avail queue one by one */
- if ((flags & QVRING_USED_F_NO_NOTIFY) == 0 &&
+ if ((flags & VRING_USED_F_NO_NOTIFY) == 0 &&
(!vq->event || (uint16_t)(idx-avail_event) < 1)) {
bus->virtqueue_kick(d, vq);
}
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 993ae0e..2af8036 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -12,22 +12,8 @@
#include "libqos/malloc.h"
-#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
-#define QVRING_DESC_F_INDIRECT 0x4
-
-#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;
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index bfeffc4..57a2635 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -21,6 +21,7 @@
#include "qemu/bswap.h"
#include "standard-headers/linux/virtio_ids.h"
#include "standard-headers/linux/virtio_config.h"
+#include "standard-headers/linux/virtio_ring.h"
#define QVIRTIO_BLK_F_BARRIER 0x00000001
#define QVIRTIO_BLK_F_SIZE_MAX 0x00000002
@@ -184,7 +185,8 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
features = qvirtio_get_features(bus, dev);
features = features & ~(QVIRTIO_F_BAD_FEATURE |
- QVIRTIO_F_RING_INDIRECT_DESC | QVIRTIO_F_RING_EVENT_IDX |
+ (1 << VIRTIO_RING_F_INDIRECT_DESC) |
+ (1 << VIRTIO_RING_F_EVENT_IDX) |
QVIRTIO_BLK_F_SCSI);
qvirtio_set_features(bus, dev, features);
@@ -350,9 +352,10 @@ static void pci_indirect(void)
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);
+ g_assert_cmphex(features & (1 << VIRTIO_RING_F_INDIRECT_DESC), !=, 0);
+ features = features & ~(QVIRTIO_F_BAD_FEATURE |
+ (1 << VIRTIO_RING_F_EVENT_IDX) |
+ QVIRTIO_BLK_F_SCSI);
qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
alloc = pc_alloc_init();
@@ -492,8 +495,9 @@ 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);
+ (1 << VIRTIO_RING_F_INDIRECT_DESC) |
+ (1 << VIRTIO_RING_F_EVENT_IDX) |
+ QVIRTIO_BLK_F_SCSI);
qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
@@ -607,8 +611,9 @@ static void pci_idx(void)
features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
features = features & ~(QVIRTIO_F_BAD_FEATURE |
- QVIRTIO_F_RING_INDIRECT_DESC |
- VIRTIO_F_NOTIFY_ON_EMPTY | QVIRTIO_BLK_F_SCSI);
+ (1 << VIRTIO_RING_F_INDIRECT_DESC) |
+ (1 << VIRTIO_F_NOTIFY_ON_EMPTY) |
+ QVIRTIO_BLK_F_SCSI);
qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index 7fd7a2f..9c8114d 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -22,6 +22,7 @@
#include "qemu/bswap.h"
#include "hw/virtio/virtio-net.h"
#include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/virtio_ring.h"
#define PCI_SLOT_HP 0x06
#define PCI_SLOT 0x04
@@ -71,8 +72,8 @@ static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev)
features = qvirtio_get_features(bus, dev);
features = features & ~(QVIRTIO_F_BAD_FEATURE |
- QVIRTIO_F_RING_INDIRECT_DESC |
- QVIRTIO_F_RING_EVENT_IDX);
+ (1 << VIRTIO_RING_F_INDIRECT_DESC) |
+ (1 << VIRTIO_RING_F_EVENT_IDX));
qvirtio_set_features(bus, dev, features);
qvirtio_set_driver_ok(bus, dev);
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH for-2.7 5/8] libqos: drop duplicated virtio_vring.h structs
2016-04-25 12:46 [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers Stefan Hajnoczi
` (3 preceding siblings ...)
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 4/8] libqos: drop duplicated virtio_ring.h bit definitions Stefan Hajnoczi
@ 2016-04-25 12:46 ` Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 6/8] libqos: drop duplicated virtio_blk.h definitions Stefan Hajnoczi
` (3 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2016-04-25 12:46 UTC (permalink / raw)
To: qemu-devel; +Cc: marc.mari.barcelo, Paolo Bonzini, Stefan Hajnoczi
The descriptor element, used, and avail vring structs are defined in
virtio_ring.h. There is no need to duplicate them in libqos virtio.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/libqos/virtio.c | 10 +++++-----
tests/libqos/virtio.h | 39 +++++++--------------------------------
2 files changed, 12 insertions(+), 37 deletions(-)
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index 69b5426..7910774 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -136,7 +136,7 @@ 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->avail = vq->desc + vq->size * sizeof(struct vring_desc);
vq->used = (uint64_t)((vq->avail + sizeof(uint16_t) * (3 + vq->size)
+ vq->align - 1) & ~(vq->align - 1));
@@ -157,7 +157,7 @@ void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq, uint64_t addr)
/* vq->used->flags */
writew(vq->used, 0);
/* vq->used->avail_event */
- writew(vq->used+2+(sizeof(struct QVRingUsedElem)*vq->size), 0);
+ writew(vq->used + 2 + sizeof(struct vring_used_elem) * vq->size, 0);
}
QVRingIndirectDesc *qvring_indirect_desc_setup(QVirtioDevice *d,
@@ -168,7 +168,7 @@ QVRingIndirectDesc *qvring_indirect_desc_setup(QVirtioDevice *d,
indirect->index = 0;
indirect->elem = elem;
- indirect->desc = guest_alloc(alloc, sizeof(QVRingDesc)*elem);
+ indirect->desc = guest_alloc(alloc, sizeof(struct vring_desc) * elem);
for (i = 0; i < elem - 1; ++i) {
/* indirect->desc[i].addr */
@@ -241,7 +241,7 @@ uint32_t qvirtqueue_add_indirect(QVirtQueue *vq, QVRingIndirectDesc *indirect)
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);
+ sizeof(struct vring_desc) * indirect->elem);
/* vq->desc[vq->free_head].flags */
writew(vq->desc + (16 * vq->free_head) + 12, VRING_DESC_F_INDIRECT);
@@ -266,7 +266,7 @@ void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d, QVirtQueue *vq,
/* Must read after idx is updated */
flags = readw(vq->avail);
avail_event = readw(vq->used + 4 +
- (sizeof(struct QVRingUsedElem) * vq->size));
+ sizeof(struct vring_used_elem) * vq->size);
/* < 1 because we add elements to avail queue one by one */
if ((flags & VRING_USED_F_NO_NOTIFY) == 0 &&
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 2af8036..c73fd8c 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -11,6 +11,7 @@
#define LIBQOS_VIRTIO_H
#include "libqos/malloc.h"
+#include "standard-headers/linux/virtio_ring.h"
#define QVIRTIO_F_BAD_FEATURE 0x40000000
@@ -19,36 +20,10 @@ typedef struct QVirtioDevice {
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 */
- uint16_t used_event;
-} 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 */
- uint16_t avail_event;
-} 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 */
+ uint64_t desc; /* This points to an array of struct vring_desc */
+ uint64_t avail; /* This points to a struct vring_avail */
+ uint64_t used; /* This points to a struct vring_desc */
uint16_t index;
uint32_t size;
uint32_t free_head;
@@ -59,7 +34,7 @@ typedef struct QVirtQueue {
} QVirtQueue;
typedef struct QVRingIndirectDesc {
- uint64_t desc; /* This points to an array fo QVRingDesc */
+ uint64_t desc; /* This points to an array fo struct vring_desc */
uint16_t index;
uint16_t elem;
} QVRingIndirectDesc;
@@ -110,9 +85,9 @@ typedef struct QVirtioBus {
static inline uint32_t qvring_size(uint32_t num, uint32_t align)
{
- return ((sizeof(struct QVRingDesc) * num + sizeof(uint16_t) * (3 + num)
+ return ((sizeof(struct vring_desc) * num + sizeof(uint16_t) * (3 + num)
+ align - 1) & ~(align - 1))
- + sizeof(uint16_t) * 3 + sizeof(struct QVRingUsedElem) * num;
+ + sizeof(uint16_t) * 3 + sizeof(struct vring_used_elem) * num;
}
uint8_t qvirtio_config_readb(const QVirtioBus *bus, QVirtioDevice *d,
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH for-2.7 6/8] libqos: drop duplicated virtio_blk.h definitions
2016-04-25 12:46 [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers Stefan Hajnoczi
` (4 preceding siblings ...)
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 5/8] libqos: drop duplicated virtio_vring.h structs Stefan Hajnoczi
@ 2016-04-25 12:46 ` Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 7/8] libqos: drop duplicated virtio_scsi.h definitions Stefan Hajnoczi
` (2 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2016-04-25 12:46 UTC (permalink / raw)
To: qemu-devel; +Cc: marc.mari.barcelo, Paolo Bonzini, Stefan Hajnoczi
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/virtio-blk-test.c | 50 ++++++++++++++++---------------------------------
1 file changed, 16 insertions(+), 34 deletions(-)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 57a2635..8e28a33 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -22,25 +22,7 @@
#include "standard-headers/linux/virtio_ids.h"
#include "standard-headers/linux/virtio_config.h"
#include "standard-headers/linux/virtio_ring.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
+#include "standard-headers/linux/virtio_blk.h"
#define TEST_IMAGE_SIZE (64 * 1024 * 1024)
#define QVIRTIO_BLK_TIMEOUT_US (30 * 1000 * 1000)
@@ -187,14 +169,14 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
features = features & ~(QVIRTIO_F_BAD_FEATURE |
(1 << VIRTIO_RING_F_INDIRECT_DESC) |
(1 << VIRTIO_RING_F_EVENT_IDX) |
- QVIRTIO_BLK_F_SCSI);
+ (1 << VIRTIO_BLK_F_SCSI));
qvirtio_set_features(bus, dev, features);
qvirtio_set_driver_ok(bus, dev);
/* Write and read with 3 descriptor layout */
/* Write request */
- req.type = QVIRTIO_BLK_T_OUT;
+ req.type = VIRTIO_BLK_T_OUT;
req.ioprio = 1;
req.sector = 0;
req.data = g_malloc0(512);
@@ -217,7 +199,7 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
guest_free(alloc, req_addr);
/* Read request */
- req.type = QVIRTIO_BLK_T_IN;
+ req.type = VIRTIO_BLK_T_IN;
req.ioprio = 1;
req.sector = 0;
req.data = g_malloc0(512);
@@ -246,7 +228,7 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
if (features & VIRTIO_F_ANY_LAYOUT) {
/* Write and read with 2 descriptor layout */
/* Write request */
- req.type = QVIRTIO_BLK_T_OUT;
+ req.type = VIRTIO_BLK_T_OUT;
req.ioprio = 1;
req.sector = 1;
req.data = g_malloc0(512);
@@ -267,7 +249,7 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
guest_free(alloc, req_addr);
/* Read request */
- req.type = QVIRTIO_BLK_T_IN;
+ req.type = VIRTIO_BLK_T_IN;
req.ioprio = 1;
req.sector = 1;
req.data = g_malloc0(512);
@@ -355,7 +337,7 @@ static void pci_indirect(void)
g_assert_cmphex(features & (1 << VIRTIO_RING_F_INDIRECT_DESC), !=, 0);
features = features & ~(QVIRTIO_F_BAD_FEATURE |
(1 << VIRTIO_RING_F_EVENT_IDX) |
- QVIRTIO_BLK_F_SCSI);
+ (1 << VIRTIO_BLK_F_SCSI));
qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
alloc = pc_alloc_init();
@@ -364,7 +346,7 @@ static void pci_indirect(void)
qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
/* Write request */
- req.type = QVIRTIO_BLK_T_OUT;
+ req.type = VIRTIO_BLK_T_OUT;
req.ioprio = 1;
req.sector = 0;
req.data = g_malloc0(512);
@@ -389,7 +371,7 @@ static void pci_indirect(void)
guest_free(alloc, req_addr);
/* Read request */
- req.type = QVIRTIO_BLK_T_IN;
+ req.type = VIRTIO_BLK_T_IN;
req.ioprio = 1;
req.sector = 0;
req.data = g_malloc0(512);
@@ -497,7 +479,7 @@ static void pci_msix(void)
features = features & ~(QVIRTIO_F_BAD_FEATURE |
(1 << VIRTIO_RING_F_INDIRECT_DESC) |
(1 << VIRTIO_RING_F_EVENT_IDX) |
- QVIRTIO_BLK_F_SCSI);
+ (1 << VIRTIO_BLK_F_SCSI));
qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
@@ -516,7 +498,7 @@ static void pci_msix(void)
g_assert_cmpint(capacity, ==, n_size / 512);
/* Write request */
- req.type = QVIRTIO_BLK_T_OUT;
+ req.type = VIRTIO_BLK_T_OUT;
req.ioprio = 1;
req.sector = 0;
req.data = g_malloc0(512);
@@ -540,7 +522,7 @@ static void pci_msix(void)
guest_free(alloc, req_addr);
/* Read request */
- req.type = QVIRTIO_BLK_T_IN;
+ req.type = VIRTIO_BLK_T_IN;
req.ioprio = 1;
req.sector = 0;
req.data = g_malloc0(512);
@@ -613,7 +595,7 @@ static void pci_idx(void)
features = features & ~(QVIRTIO_F_BAD_FEATURE |
(1 << VIRTIO_RING_F_INDIRECT_DESC) |
(1 << VIRTIO_F_NOTIFY_ON_EMPTY) |
- QVIRTIO_BLK_F_SCSI);
+ (1 << VIRTIO_BLK_F_SCSI));
qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
@@ -623,7 +605,7 @@ static void pci_idx(void)
qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
/* Write request */
- req.type = QVIRTIO_BLK_T_OUT;
+ req.type = VIRTIO_BLK_T_OUT;
req.ioprio = 1;
req.sector = 0;
req.data = g_malloc0(512);
@@ -642,7 +624,7 @@ static void pci_idx(void)
QVIRTIO_BLK_TIMEOUT_US);
/* Write request */
- req.type = QVIRTIO_BLK_T_OUT;
+ req.type = VIRTIO_BLK_T_OUT;
req.ioprio = 1;
req.sector = 1;
req.data = g_malloc0(512);
@@ -668,7 +650,7 @@ static void pci_idx(void)
guest_free(alloc, req_addr);
/* Read request */
- req.type = QVIRTIO_BLK_T_IN;
+ req.type = VIRTIO_BLK_T_IN;
req.ioprio = 1;
req.sector = 1;
req.data = g_malloc0(512);
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH for-2.7 7/8] libqos: drop duplicated virtio_scsi.h definitions
2016-04-25 12:46 [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers Stefan Hajnoczi
` (5 preceding siblings ...)
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 6/8] libqos: drop duplicated virtio_blk.h definitions Stefan Hajnoczi
@ 2016-04-25 12:46 ` Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 8/8] libqos: drop duplicated virtio_pci.h definitions Stefan Hajnoczi
2016-04-25 19:09 ` [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers John Snow
8 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2016-04-25 12:46 UTC (permalink / raw)
To: qemu-devel; +Cc: marc.mari.barcelo, Paolo Bonzini, Stefan Hajnoczi
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/virtio-scsi-test.c | 45 +++++++++++++++------------------------------
1 file changed, 15 insertions(+), 30 deletions(-)
diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index 625bd6d..8b8f5f3 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -19,11 +19,11 @@
#include "libqos/malloc-pc.h"
#include "libqos/malloc-generic.h"
#include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/virtio_scsi.h"
#define PCI_SLOT 0x02
#define PCI_FN 0x00
#define QVIRTIO_SCSI_TIMEOUT_US (1 * 1000 * 1000)
-#define CDB_SIZE 32
#define MAX_NUM_QUEUES 64
@@ -35,24 +35,6 @@ typedef struct {
QVirtQueue *vq[MAX_NUM_QUEUES + 2];
} QVirtIOSCSI;
-typedef struct {
- uint8_t lun[8];
- int64_t tag;
- uint8_t task_attr;
- uint8_t prio;
- uint8_t crn;
- uint8_t cdb[CDB_SIZE];
-} QEMU_PACKED QVirtIOSCSICmdReq;
-
-typedef struct {
- uint32_t sense_len;
- uint32_t resid;
- uint16_t status_qualifier;
- uint8_t status;
- uint8_t response;
- uint8_t sense[96];
-} QEMU_PACKED QVirtIOSCSICmdResp;
-
static void qvirtio_scsi_start(const char *extra_opts)
{
char *cmdline;
@@ -101,11 +83,11 @@ static uint8_t virtio_scsi_do_command(QVirtIOSCSI *vs, const uint8_t *cdb,
const uint8_t *data_in,
size_t data_in_len,
uint8_t *data_out, size_t data_out_len,
- QVirtIOSCSICmdResp *resp_out)
+ struct virtio_scsi_cmd_resp *resp_out)
{
QVirtQueue *vq;
- QVirtIOSCSICmdReq req = { { 0 } };
- QVirtIOSCSICmdResp resp = { .response = 0xff, .status = 0xff };
+ struct virtio_scsi_cmd_req req = { { 0 } };
+ struct virtio_scsi_cmd_resp resp = { .response = 0xff, .status = 0xff };
uint64_t req_addr, resp_addr, data_in_addr = 0, data_out_addr = 0;
uint8_t response;
uint32_t free_head;
@@ -114,7 +96,7 @@ static uint8_t virtio_scsi_do_command(QVirtIOSCSI *vs, const uint8_t *cdb,
req.lun[0] = 1; /* Select LUN */
req.lun[1] = 1; /* Select target 1 */
- memcpy(req.cdb, cdb, CDB_SIZE);
+ memcpy(req.cdb, cdb, VIRTIO_SCSI_CDB_SIZE);
/* XXX: Fix endian if any multi-byte field in req/resp is used */
@@ -139,7 +121,8 @@ static uint8_t virtio_scsi_do_command(QVirtIOSCSI *vs, const uint8_t *cdb,
qvirtqueue_kick(&qvirtio_pci, vs->dev, vq, free_head);
qvirtio_wait_queue_isr(&qvirtio_pci, vs->dev, vq, QVIRTIO_SCSI_TIMEOUT_US);
- response = readb(resp_addr + offsetof(QVirtIOSCSICmdResp, response));
+ response = readb(resp_addr +
+ offsetof(struct virtio_scsi_cmd_resp, response));
if (resp_out) {
memread(resp_addr, resp_out, sizeof(*resp_out));
@@ -154,10 +137,10 @@ static uint8_t virtio_scsi_do_command(QVirtIOSCSI *vs, const uint8_t *cdb,
static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
{
- const uint8_t test_unit_ready_cdb[CDB_SIZE] = {};
+ const uint8_t test_unit_ready_cdb[VIRTIO_SCSI_CDB_SIZE] = {};
QVirtIOSCSI *vs;
QVirtioPCIDevice *dev;
- QVirtIOSCSICmdResp resp;
+ struct virtio_scsi_cmd_resp resp;
void *addr;
int i;
@@ -240,10 +223,12 @@ static void test_unaligned_write_same(void)
QVirtIOSCSI *vs;
uint8_t buf1[512] = { 0 };
uint8_t buf2[512] = { 1 };
- const uint8_t write_same_cdb_1[CDB_SIZE] = { 0x41, 0x00, 0x00, 0x00, 0x00,
- 0x01, 0x00, 0x00, 0x02, 0x00 };
- const uint8_t write_same_cdb_2[CDB_SIZE] = { 0x41, 0x00, 0x00, 0x00, 0x00,
- 0x01, 0x00, 0x33, 0x00, 0x00 };
+ const uint8_t write_same_cdb_1[VIRTIO_SCSI_CDB_SIZE] = {
+ 0x41, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x02, 0x00
+ };
+ const uint8_t write_same_cdb_2[VIRTIO_SCSI_CDB_SIZE] = {
+ 0x41, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x33, 0x00, 0x00
+ };
qvirtio_scsi_start("-drive file=blkdebug::null-co://,if=none,id=dr1"
",format=raw,file.align=4k "
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH for-2.7 8/8] libqos: drop duplicated virtio_pci.h definitions
2016-04-25 12:46 [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers Stefan Hajnoczi
` (6 preceding siblings ...)
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 7/8] libqos: drop duplicated virtio_scsi.h definitions Stefan Hajnoczi
@ 2016-04-25 12:46 ` Stefan Hajnoczi
2016-04-25 19:09 ` [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers John Snow
8 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2016-04-25 12:46 UTC (permalink / raw)
To: qemu-devel; +Cc: marc.mari.barcelo, Paolo Bonzini, Stefan Hajnoczi
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/libqos/virtio-pci.c | 42 ++++++++++++++++++++++--------------------
tests/libqos/virtio-pci.h | 17 -----------------
tests/virtio-blk-test.c | 11 ++++++-----
tests/virtio-scsi-test.c | 3 ++-
4 files changed, 30 insertions(+), 43 deletions(-)
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 4a0bef8..d30d224 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -17,6 +17,7 @@
#include "libqos/malloc.h"
#include "libqos/malloc-pc.h"
#include "standard-headers/linux/virtio_ring.h"
+#include "standard-headers/linux/virtio_pci.h"
#include "hw/pci/pci.h"
#include "hw/pci/pci_regs.h"
@@ -104,31 +105,31 @@ static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr)
static uint32_t qvirtio_pci_get_features(QVirtioDevice *d)
{
QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
- return qpci_io_readl(dev->pdev, dev->addr + QVIRTIO_PCI_DEVICE_FEATURES);
+ return qpci_io_readl(dev->pdev, dev->addr + VIRTIO_PCI_HOST_FEATURES);
}
static void qvirtio_pci_set_features(QVirtioDevice *d, uint32_t features)
{
QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
- qpci_io_writel(dev->pdev, dev->addr + QVIRTIO_PCI_GUEST_FEATURES, features);
+ qpci_io_writel(dev->pdev, dev->addr + VIRTIO_PCI_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_PCI_GUEST_FEATURES);
+ return qpci_io_readl(dev->pdev, dev->addr + VIRTIO_PCI_GUEST_FEATURES);
}
static uint8_t qvirtio_pci_get_status(QVirtioDevice *d)
{
QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
- return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_PCI_DEVICE_STATUS);
+ return qpci_io_readb(dev->pdev, dev->addr + VIRTIO_PCI_STATUS);
}
static void qvirtio_pci_set_status(QVirtioDevice *d, uint8_t status)
{
QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
- qpci_io_writeb(dev->pdev, dev->addr + QVIRTIO_PCI_DEVICE_STATUS, status);
+ qpci_io_writeb(dev->pdev, dev->addr + VIRTIO_PCI_STATUS, status);
}
static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
@@ -152,7 +153,7 @@ static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
}
}
} else {
- return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_PCI_ISR_STATUS) & 1;
+ return qpci_io_readb(dev->pdev, dev->addr + VIRTIO_PCI_ISR) & 1;
}
}
@@ -176,26 +177,26 @@ static bool qvirtio_pci_get_config_isr_status(QVirtioDevice *d)
}
}
} else {
- return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_PCI_ISR_STATUS) & 2;
+ return qpci_io_readb(dev->pdev, dev->addr + VIRTIO_PCI_ISR) & 2;
}
}
static void qvirtio_pci_queue_select(QVirtioDevice *d, uint16_t index)
{
QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
- qpci_io_writeb(dev->pdev, dev->addr + QVIRTIO_PCI_QUEUE_SELECT, index);
+ qpci_io_writeb(dev->pdev, dev->addr + VIRTIO_PCI_QUEUE_SEL, index);
}
static uint16_t qvirtio_pci_get_queue_size(QVirtioDevice *d)
{
QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
- return qpci_io_readw(dev->pdev, dev->addr + QVIRTIO_PCI_QUEUE_SIZE);
+ return qpci_io_readw(dev->pdev, dev->addr + VIRTIO_PCI_QUEUE_NUM);
}
static void qvirtio_pci_set_queue_address(QVirtioDevice *d, uint32_t pfn)
{
QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
- qpci_io_writel(dev->pdev, dev->addr + QVIRTIO_PCI_QUEUE_ADDRESS, pfn);
+ qpci_io_writel(dev->pdev, dev->addr + VIRTIO_PCI_QUEUE_PFN, pfn);
}
static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
@@ -213,7 +214,7 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
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.align = VIRTIO_PCI_VRING_ALIGN;
vqpci->vq.indirect = (feat & (1 << VIRTIO_RING_F_INDIRECT_DESC)) != 0;
vqpci->vq.event = (feat & (1 << VIRTIO_RING_F_EVENT_IDX)) != 0;
@@ -227,9 +228,10 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
/* Check power of 2 */
g_assert_cmpint(vqpci->vq.size & (vqpci->vq.size - 1), ==, 0);
- addr = guest_alloc(alloc, qvring_size(vqpci->vq.size, QVIRTIO_PCI_ALIGN));
+ addr = guest_alloc(alloc, qvring_size(vqpci->vq.size,
+ VIRTIO_PCI_VRING_ALIGN));
qvring_init(alloc, &vqpci->vq, addr);
- qvirtio_pci_set_queue_address(d, vqpci->vq.desc / QVIRTIO_PCI_ALIGN);
+ qvirtio_pci_set_queue_address(d, vqpci->vq.desc / VIRTIO_PCI_VRING_ALIGN);
return &vqpci->vq;
}
@@ -237,7 +239,7 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
static void qvirtio_pci_virtqueue_kick(QVirtioDevice *d, QVirtQueue *vq)
{
QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
- qpci_io_writew(dev->pdev, dev->addr + QVIRTIO_PCI_QUEUE_NOTIFY, vq->index);
+ qpci_io_writew(dev->pdev, dev->addr + VIRTIO_PCI_QUEUE_NOTIFY, vq->index);
}
const QVirtioBus qvirtio_pci = {
@@ -317,9 +319,9 @@ void qvirtqueue_pci_msix_setup(QVirtioPCIDevice *d, QVirtQueuePCI *vqpci,
control & ~PCI_MSIX_ENTRY_CTRL_MASKBIT);
qvirtio_pci_queue_select(&d->vdev, vqpci->vq.index);
- qpci_io_writew(d->pdev, d->addr + QVIRTIO_PCI_MSIX_QUEUE_VECTOR, entry);
- vector = qpci_io_readw(d->pdev, d->addr + QVIRTIO_PCI_MSIX_QUEUE_VECTOR);
- g_assert_cmphex(vector, !=, QVIRTIO_MSI_NO_VECTOR);
+ qpci_io_writew(d->pdev, d->addr + VIRTIO_MSI_QUEUE_VECTOR, entry);
+ vector = qpci_io_readw(d->pdev, d->addr + VIRTIO_MSI_QUEUE_VECTOR);
+ g_assert_cmphex(vector, !=, VIRTIO_MSI_NO_VECTOR);
}
void qvirtio_pci_set_msix_configuration_vector(QVirtioPCIDevice *d,
@@ -349,7 +351,7 @@ void qvirtio_pci_set_msix_configuration_vector(QVirtioPCIDevice *d,
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_PCI_MSIX_CONF_VECTOR, entry);
- vector = qpci_io_readw(d->pdev, d->addr + QVIRTIO_PCI_MSIX_CONF_VECTOR);
- g_assert_cmphex(vector, !=, QVIRTIO_MSI_NO_VECTOR);
+ qpci_io_writew(d->pdev, d->addr + VIRTIO_MSI_CONFIG_VECTOR, entry);
+ vector = qpci_io_readw(d->pdev, d->addr + VIRTIO_MSI_CONFIG_VECTOR);
+ g_assert_cmphex(vector, !=, VIRTIO_MSI_NO_VECTOR);
}
diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
index 8f0e52a..efcac2d 100644
--- a/tests/libqos/virtio-pci.h
+++ b/tests/libqos/virtio-pci.h
@@ -13,23 +13,6 @@
#include "libqos/virtio.h"
#include "libqos/pci.h"
-#define QVIRTIO_PCI_DEVICE_FEATURES 0x00
-#define QVIRTIO_PCI_GUEST_FEATURES 0x04
-#define QVIRTIO_PCI_QUEUE_ADDRESS 0x08
-#define QVIRTIO_PCI_QUEUE_SIZE 0x0C
-#define QVIRTIO_PCI_QUEUE_SELECT 0x0E
-#define QVIRTIO_PCI_QUEUE_NOTIFY 0x10
-#define QVIRTIO_PCI_DEVICE_STATUS 0x12
-#define QVIRTIO_PCI_ISR_STATUS 0x13
-#define QVIRTIO_PCI_MSIX_CONF_VECTOR 0x14
-#define QVIRTIO_PCI_MSIX_QUEUE_VECTOR 0x16
-#define QVIRTIO_PCI_DEVICE_SPECIFIC_MSIX 0x18
-#define QVIRTIO_PCI_DEVICE_SPECIFIC_NO_MSIX 0x14
-
-#define QVIRTIO_PCI_ALIGN 4096
-
-#define QVIRTIO_MSI_NO_VECTOR 0xFFFF
-
typedef struct QVirtioPCIDevice {
QVirtioDevice vdev;
QPCIDevice *pdev;
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 8e28a33..fb44840 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -23,6 +23,7 @@
#include "standard-headers/linux/virtio_config.h"
#include "standard-headers/linux/virtio_ring.h"
#include "standard-headers/linux/virtio_blk.h"
+#include "standard-headers/linux/virtio_pci.h"
#define TEST_IMAGE_SIZE (64 * 1024 * 1024)
#define QVIRTIO_BLK_TIMEOUT_US (30 * 1000 * 1000)
@@ -292,7 +293,7 @@ static void pci_basic(void)
alloc, 0);
/* MSI-X is not enabled */
- addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_NO_MSIX;
+ addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
test_basic(&qvirtio_pci, &dev->vdev, alloc, &vqpci->vq,
(uint64_t)(uintptr_t)addr);
@@ -327,7 +328,7 @@ static void pci_indirect(void)
dev = virtio_blk_pci_init(bus, PCI_SLOT);
/* MSI-X is not enabled */
- addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_NO_MSIX;
+ addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
(uint64_t)(uintptr_t)addr);
@@ -422,7 +423,7 @@ static void pci_config(void)
dev = virtio_blk_pci_init(bus, PCI_SLOT);
/* MSI-X is not enabled */
- addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_NO_MSIX;
+ addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
(uint64_t)(uintptr_t)addr);
@@ -469,7 +470,7 @@ static void pci_msix(void)
qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
/* MSI-X is enabled */
- addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_MSIX;
+ addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(true);
capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
(uint64_t)(uintptr_t)addr);
@@ -585,7 +586,7 @@ static void pci_idx(void)
qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
/* MSI-X is enabled */
- addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_MSIX;
+ addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(true);
capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
(uint64_t)(uintptr_t)addr);
diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index 8b8f5f3..71f5aeb 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -19,6 +19,7 @@
#include "libqos/malloc-pc.h"
#include "libqos/malloc-generic.h"
#include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/virtio_pci.h"
#include "standard-headers/linux/virtio_scsi.h"
#define PCI_SLOT 0x02
@@ -158,7 +159,7 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
qvirtio_set_acknowledge(&qvirtio_pci, vs->dev);
qvirtio_set_driver(&qvirtio_pci, vs->dev);
- addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_NO_MSIX;
+ addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
vs->num_queues = qvirtio_config_readl(&qvirtio_pci, vs->dev,
(uint64_t)(uintptr_t)addr);
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers
2016-04-25 12:46 [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers Stefan Hajnoczi
` (7 preceding siblings ...)
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 8/8] libqos: drop duplicated virtio_pci.h definitions Stefan Hajnoczi
@ 2016-04-25 19:09 ` John Snow
2016-04-26 9:58 ` Stefan Hajnoczi
8 siblings, 1 reply; 14+ messages in thread
From: John Snow @ 2016-04-25 19:09 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel; +Cc: marc.mari.barcelo, Paolo Bonzini
On 04/25/2016 08:46 AM, Stefan Hajnoczi wrote:
> This patch series eliminates code duplication in libqos virtio.
> include/standard-headers/ contains the Linux virtio header files so we don't
> need to define our own version of the structs and constants.
>
Is that a good idea? I had thought some of the intentional purpose of
duplicating the headers was to test the other versions.
> Stefan Hajnoczi (8):
> libqos: use virtio_ids.h for device ID definitions
> libqos: drop duplicated PCI vendor ID definition
> libqos: drop duplicated virtio_config.h definitions
> libqos: drop duplicated virtio_ring.h bit definitions
> libqos: drop duplicated virtio_vring.h structs
> libqos: drop duplicated virtio_blk.h definitions
> libqos: drop duplicated virtio_scsi.h definitions
> libqos: drop duplicated virtio_pci.h definitions
>
> tests/libqos/virtio-mmio.c | 5 +--
> tests/libqos/virtio-pci.c | 50 ++++++++++++++-------------
> tests/libqos/virtio-pci.h | 17 ---------
> tests/libqos/virtio.c | 42 +++++++++++-----------
> tests/libqos/virtio.h | 73 ++++-----------------------------------
> tests/virtio-blk-test.c | 86 ++++++++++++++++++++--------------------------
> tests/virtio-net-test.c | 10 +++---
> tests/virtio-scsi-test.c | 53 +++++++++++-----------------
> 8 files changed, 123 insertions(+), 213 deletions(-)
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers
2016-04-25 19:09 ` [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers John Snow
@ 2016-04-26 9:58 ` Stefan Hajnoczi
0 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2016-04-26 9:58 UTC (permalink / raw)
To: John Snow; +Cc: Stefan Hajnoczi, qemu-devel, marc.mari.barcelo, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 876 bytes --]
On Mon, Apr 25, 2016 at 03:09:14PM -0400, John Snow wrote:
> On 04/25/2016 08:46 AM, Stefan Hajnoczi wrote:
> > This patch series eliminates code duplication in libqos virtio.
> > include/standard-headers/ contains the Linux virtio header files so we don't
> > need to define our own version of the structs and constants.
> >
>
> Is that a good idea? I had thought some of the intentional purpose of
> duplicating the headers was to test the other versions.
There is already a way to verify that the Linux virtio structs and
constants are correct: compare against the virtio specification.
The duplication adds extra work for anyone writing virtio device tests.
We need more tests, which won't happen if it's a pain to write tests.
Most likely the constants will just be copy-pasted from Linux anyway, so
I don't see any benefit in duplication.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.7 3/8] libqos: drop duplicated virtio_config.h definitions
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 3/8] libqos: drop duplicated virtio_config.h definitions Stefan Hajnoczi
@ 2016-05-08 18:22 ` Marc Marí
2016-05-08 18:40 ` Marc Marí
0 siblings, 1 reply; 14+ messages in thread
From: Marc Marí @ 2016-05-08 18:22 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Paolo Bonzini
On Mon, 25 Apr 2016 13:46:08 +0100
Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> tests/libqos/virtio.c | 19 ++++++++++---------
> tests/libqos/virtio.h | 9 ---------
> tests/virtio-blk-test.c | 5 +++--
> 3 files changed, 13 insertions(+), 20 deletions(-)
>
> diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
> index 613dece..ee9e892 100644
> --- a/tests/libqos/virtio.c
> +++ b/tests/libqos/virtio.c
> @@ -11,6 +11,7 @@
> #include <glib.h>
> #include "libqtest.h"
> #include "libqos/virtio.h"
> +#include "standard-headers/linux/virtio_config.h"
>
> uint8_t qvirtio_config_readb(const QVirtioBus *bus, QVirtioDevice *d,
> uint64_t
> addr) @@ -55,28 +56,28 @@ QVirtQueue *qvirtqueue_setup(const
> QVirtioBus *bus, QVirtioDevice *d,
> void qvirtio_reset(const QVirtioBus *bus, QVirtioDevice *d)
> {
> - bus->set_status(d, QVIRTIO_RESET);
> - g_assert_cmphex(bus->get_status(d), ==, QVIRTIO_RESET);
> + bus->set_status(d, 0);
> + g_assert_cmphex(bus->get_status(d), ==, 0);
> }
>
> 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);
> + bus->set_status(d, bus->get_status(d) |
> VIRTIO_CONFIG_S_ACKNOWLEDGE);
> + g_assert_cmphex(bus->get_status(d), ==,
> VIRTIO_CONFIG_S_ACKNOWLEDGE); }
>
> void qvirtio_set_driver(const QVirtioBus *bus, QVirtioDevice *d)
> {
> - bus->set_status(d, bus->get_status(d) | QVIRTIO_DRIVER);
> + bus->set_status(d, bus->get_status(d) | VIRTIO_CONFIG_S_DRIVER);
> g_assert_cmphex(bus->get_status(d), ==,
> - QVIRTIO_DRIVER |
> QVIRTIO_ACKNOWLEDGE);
> + VIRTIO_CONFIG_S_DRIVER |
> VIRTIO_CONFIG_S_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);
> + bus->set_status(d, bus->get_status(d) |
> VIRTIO_CONFIG_S_DRIVER_OK);
> + g_assert_cmphex(bus->get_status(d), ==,
> VIRTIO_CONFIG_S_DRIVER_OK |
> + VIRTIO_CONFIG_S_DRIVER |
> VIRTIO_CONFIG_S_ACKNOWLEDGE); }
>
> void qvirtio_wait_queue_isr(const QVirtioBus *bus, QVirtioDevice *d,
> diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
> index e663bcf..993ae0e 100644
> --- a/tests/libqos/virtio.h
> +++ b/tests/libqos/virtio.h
> @@ -12,13 +12,6 @@
>
> #include "libqos/malloc.h"
>
> -#define QVIRTIO_RESET 0x0
> -#define QVIRTIO_ACKNOWLEDGE 0x1
> -#define QVIRTIO_DRIVER 0x2
> -#define QVIRTIO_DRIVER_OK 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
> @@ -27,8 +20,6 @@
> #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
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index 02107a6..bfeffc4 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -20,6 +20,7 @@
> #include "libqos/malloc-generic.h"
> #include "qemu/bswap.h"
> #include "standard-headers/linux/virtio_ids.h"
> +#include "standard-headers/linux/virtio_config.h"
>
> #define QVIRTIO_BLK_F_BARRIER 0x00000001
> #define QVIRTIO_BLK_F_SIZE_MAX 0x00000002
> @@ -240,7 +241,7 @@ static void test_basic(const QVirtioBus *bus,
> QVirtioDevice *dev,
> guest_free(alloc, req_addr);
>
> - if (features & QVIRTIO_F_ANY_LAYOUT) {
> + if (features & VIRTIO_F_ANY_LAYOUT) {
> /* Write and read with 2 descriptor layout */
> /* Write request */
> req.type = QVIRTIO_BLK_T_OUT;
> @@ -607,7 +608,7 @@ static void pci_idx(void)
> features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
> features = features & ~(QVIRTIO_F_BAD_FEATURE |
> QVIRTIO_F_RING_INDIRECT_DESC |
> - QVIRTIO_F_NOTIFY_ON_EMPTY |
> QVIRTIO_BLK_F_SCSI);
> + VIRTIO_F_NOTIFY_ON_EMPTY |
> QVIRTIO_BLK_F_SCSI); qvirtio_set_features(&qvirtio_pci, &dev->vdev,
> features);
> vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci,
> &dev->vdev,
In standard-headers/linux/virtio_config.h, there is;
#define VIRTIO_F_NOTIFY_ON_EMPTY 24
#define VIRTIO_F_ANY_LAYOUT 27
whereas in tests/libqos.h, there is:
#define QVIRTIO_F_NOTIFY_ON_EMPTY 0x01000000
#define QVIRTIO_F_ANY_LAYOUT 0x08000000
It is necessary to use 2 << VIRTIO_F_NOTIFY_ON_EMPTY to make the change
properly.
Marc
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.7 3/8] libqos: drop duplicated virtio_config.h definitions
2016-05-08 18:22 ` Marc Marí
@ 2016-05-08 18:40 ` Marc Marí
2016-05-09 11:46 ` Stefan Hajnoczi
0 siblings, 1 reply; 14+ messages in thread
From: Marc Marí @ 2016-05-08 18:40 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Paolo Bonzini
On Sun, 8 May 2016 20:22:53 +0200
Marc Marí <marc.mari.barcelo@gmail.com> wrote:
> On Mon, 25 Apr 2016 13:46:08 +0100
> Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > tests/libqos/virtio.c | 19 ++++++++++---------
> > tests/libqos/virtio.h | 9 ---------
> > tests/virtio-blk-test.c | 5 +++--
> > 3 files changed, 13 insertions(+), 20 deletions(-)
> >
> > diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
> > index 613dece..ee9e892 100644
> > --- a/tests/libqos/virtio.c
> > +++ b/tests/libqos/virtio.c
> > @@ -11,6 +11,7 @@
> > #include <glib.h>
> > #include "libqtest.h"
> > #include "libqos/virtio.h"
> > +#include "standard-headers/linux/virtio_config.h"
> >
> > uint8_t qvirtio_config_readb(const QVirtioBus *bus, QVirtioDevice
> > *d, uint64_t
> > addr) @@ -55,28 +56,28 @@ QVirtQueue *qvirtqueue_setup(const
> > QVirtioBus *bus, QVirtioDevice *d,
> > void qvirtio_reset(const QVirtioBus *bus, QVirtioDevice *d)
> > {
> > - bus->set_status(d, QVIRTIO_RESET);
> > - g_assert_cmphex(bus->get_status(d), ==, QVIRTIO_RESET);
> > + bus->set_status(d, 0);
> > + g_assert_cmphex(bus->get_status(d), ==, 0);
> > }
> >
> > 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);
> > + bus->set_status(d, bus->get_status(d) |
> > VIRTIO_CONFIG_S_ACKNOWLEDGE);
> > + g_assert_cmphex(bus->get_status(d), ==,
> > VIRTIO_CONFIG_S_ACKNOWLEDGE); }
> >
> > void qvirtio_set_driver(const QVirtioBus *bus, QVirtioDevice *d)
> > {
> > - bus->set_status(d, bus->get_status(d) | QVIRTIO_DRIVER);
> > + bus->set_status(d, bus->get_status(d) |
> > VIRTIO_CONFIG_S_DRIVER); g_assert_cmphex(bus->get_status(d), ==,
> > - QVIRTIO_DRIVER |
> > QVIRTIO_ACKNOWLEDGE);
> > + VIRTIO_CONFIG_S_DRIVER |
> > VIRTIO_CONFIG_S_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);
> > + bus->set_status(d, bus->get_status(d) |
> > VIRTIO_CONFIG_S_DRIVER_OK);
> > + g_assert_cmphex(bus->get_status(d), ==,
> > VIRTIO_CONFIG_S_DRIVER_OK |
> > + VIRTIO_CONFIG_S_DRIVER |
> > VIRTIO_CONFIG_S_ACKNOWLEDGE); }
> >
> > void qvirtio_wait_queue_isr(const QVirtioBus *bus, QVirtioDevice
> > *d, diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
> > index e663bcf..993ae0e 100644
> > --- a/tests/libqos/virtio.h
> > +++ b/tests/libqos/virtio.h
> > @@ -12,13 +12,6 @@
> >
> > #include "libqos/malloc.h"
> >
> > -#define QVIRTIO_RESET 0x0
> > -#define QVIRTIO_ACKNOWLEDGE 0x1
> > -#define QVIRTIO_DRIVER 0x2
> > -#define QVIRTIO_DRIVER_OK 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
> > @@ -27,8 +20,6 @@
> > #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
> > diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> > index 02107a6..bfeffc4 100644
> > --- a/tests/virtio-blk-test.c
> > +++ b/tests/virtio-blk-test.c
> > @@ -20,6 +20,7 @@
> > #include "libqos/malloc-generic.h"
> > #include "qemu/bswap.h"
> > #include "standard-headers/linux/virtio_ids.h"
> > +#include "standard-headers/linux/virtio_config.h"
> >
> > #define QVIRTIO_BLK_F_BARRIER 0x00000001
> > #define QVIRTIO_BLK_F_SIZE_MAX 0x00000002
> > @@ -240,7 +241,7 @@ static void test_basic(const QVirtioBus *bus,
> > QVirtioDevice *dev,
> > guest_free(alloc, req_addr);
> >
> > - if (features & QVIRTIO_F_ANY_LAYOUT) {
> > + if (features & VIRTIO_F_ANY_LAYOUT) {
> > /* Write and read with 2 descriptor layout */
> > /* Write request */
> > req.type = QVIRTIO_BLK_T_OUT;
> > @@ -607,7 +608,7 @@ static void pci_idx(void)
> > features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
> > features = features & ~(QVIRTIO_F_BAD_FEATURE |
> > QVIRTIO_F_RING_INDIRECT_DESC |
> > - QVIRTIO_F_NOTIFY_ON_EMPTY |
> > QVIRTIO_BLK_F_SCSI);
> > + VIRTIO_F_NOTIFY_ON_EMPTY |
> > QVIRTIO_BLK_F_SCSI); qvirtio_set_features(&qvirtio_pci, &dev->vdev,
> > features);
> > vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci,
> > &dev->vdev,
>
> In standard-headers/linux/virtio_config.h, there is;
>
> #define VIRTIO_F_NOTIFY_ON_EMPTY 24
> #define VIRTIO_F_ANY_LAYOUT 27
>
> whereas in tests/libqos.h, there is:
>
> #define QVIRTIO_F_NOTIFY_ON_EMPTY 0x01000000
> #define QVIRTIO_F_ANY_LAYOUT 0x08000000
>
> It is necessary to use 2 << VIRTIO_F_NOTIFY_ON_EMPTY to make the
> change properly.
>
I meant 1 << VIRTIO_F_NOTIFY_ON_EMPTY, obviously.
And what I say is half corrected in the next patch. I think it would be
nicer to make the changes directly in this patch.
In any case, this line:
> if (features & VIRTIO_F_ANY_LAYOUT) {
is not corrected in any patch
Marc
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.7 3/8] libqos: drop duplicated virtio_config.h definitions
2016-05-08 18:40 ` Marc Marí
@ 2016-05-09 11:46 ` Stefan Hajnoczi
0 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2016-05-09 11:46 UTC (permalink / raw)
To: Marc Marí; +Cc: Stefan Hajnoczi, Paolo Bonzini, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1096 bytes --]
On Sun, May 08, 2016 at 08:40:36PM +0200, Marc Marí wrote:
> On Sun, 8 May 2016 20:22:53 +0200
> Marc Marí <marc.mari.barcelo@gmail.com> wrote:
>
> > On Mon, 25 Apr 2016 13:46:08 +0100
> > Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > In standard-headers/linux/virtio_config.h, there is;
> >
> > #define VIRTIO_F_NOTIFY_ON_EMPTY 24
> > #define VIRTIO_F_ANY_LAYOUT 27
> >
> > whereas in tests/libqos.h, there is:
> >
> > #define QVIRTIO_F_NOTIFY_ON_EMPTY 0x01000000
> > #define QVIRTIO_F_ANY_LAYOUT 0x08000000
> >
> > It is necessary to use 2 << VIRTIO_F_NOTIFY_ON_EMPTY to make the
> > change properly.
> >
>
> I meant 1 << VIRTIO_F_NOTIFY_ON_EMPTY, obviously.
>
> And what I say is half corrected in the next patch. I think it would be
> nicer to make the changes directly in this patch.
>
> In any case, this line:
> > if (features & VIRTIO_F_ANY_LAYOUT) {
> is not corrected in any patch
Thanks for pointing this out, I thought I had caught all instances where
(1 << X) and X differ.
Will fix in the next revision.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-05-09 11:46 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-25 12:46 [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 1/8] libqos: use virtio_ids.h for device ID definitions Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 2/8] libqos: drop duplicated PCI vendor ID definition Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 3/8] libqos: drop duplicated virtio_config.h definitions Stefan Hajnoczi
2016-05-08 18:22 ` Marc Marí
2016-05-08 18:40 ` Marc Marí
2016-05-09 11:46 ` Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 4/8] libqos: drop duplicated virtio_ring.h bit definitions Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 5/8] libqos: drop duplicated virtio_vring.h structs Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 6/8] libqos: drop duplicated virtio_blk.h definitions Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 7/8] libqos: drop duplicated virtio_scsi.h definitions Stefan Hajnoczi
2016-04-25 12:46 ` [Qemu-devel] [PATCH for-2.7 8/8] libqos: drop duplicated virtio_pci.h definitions Stefan Hajnoczi
2016-04-25 19:09 ` [Qemu-devel] [PATCH for-2.7 0/8] libqos: use standard virtio headers John Snow
2016-04-26 9:58 ` Stefan Hajnoczi
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.