* [Qemu-devel] [PATCH v2 1/5] libqos: Change use of pointers to uint64_t in virtio
2014-11-01 17:02 [Qemu-devel] [PATCH v2 0/5] libqos: Virtio MMIO driver Marc Marí
@ 2014-11-01 17:02 ` Marc Marí
2014-11-17 15:16 ` Stefan Hajnoczi
2014-11-01 17:02 ` [Qemu-devel] [PATCH v2 2/5] tests: Prepare virtio-blk-test for multi-arch implementation Marc Marí
` (4 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Marc Marí @ 2014-11-01 17:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc Marí, Andreas Färber, Stefan Hajnoczi
Convert use of pointers in functions of virtio to uint64_t in order to make it
platform-independent.
Add casting from pointers (in PCI functions) to uint64_t and vice versa through
uintptr_t.
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
tests/libqos/virtio-pci.c | 20 +++++++++++---------
tests/libqos/virtio.c | 8 ++++----
tests/libqos/virtio.h | 16 ++++++++--------
tests/virtio-blk-test.c | 21 ++++++++++++++-------
4 files changed, 37 insertions(+), 28 deletions(-)
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 788ebaf..92bcac1 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -60,25 +60,25 @@ static void qvirtio_pci_assign_device(QVirtioDevice *d, void *data)
*vpcidev = (QVirtioPCIDevice *)d;
}
-static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, void *addr)
+static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr)
{
QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
- return qpci_io_readb(dev->pdev, addr);
+ return qpci_io_readb(dev->pdev, (void *)(uintptr_t)addr);
}
-static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, void *addr)
+static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t addr)
{
QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
- return qpci_io_readw(dev->pdev, addr);
+ return qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr);
}
-static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, void *addr)
+static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, uint64_t addr)
{
QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
- return qpci_io_readl(dev->pdev, addr);
+ return qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr);
}
-static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, void *addr)
+static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr)
{
QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
int i;
@@ -86,11 +86,13 @@ static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, void *addr)
if (qtest_big_endian()) {
for (i = 0; i < 8; ++i) {
- u64 |= (uint64_t)qpci_io_readb(dev->pdev, addr + i) << (7 - i) * 8;
+ u64 |= (uint64_t)qpci_io_readb(dev->pdev,
+ (void *)(uintptr_t)addr + i) << (7 - i) * 8;
}
} else {
for (i = 0; i < 8; ++i) {
- u64 |= (uint64_t)qpci_io_readb(dev->pdev, addr + i) << i * 8;
+ u64 |= (uint64_t)qpci_io_readb(dev->pdev,
+ (void *)(uintptr_t)addr + i) << i * 8;
}
}
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index a061289..3205b88 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -12,25 +12,25 @@
#include "libqos/virtio.h"
uint8_t qvirtio_config_readb(const QVirtioBus *bus, QVirtioDevice *d,
- void *addr)
+ uint64_t addr)
{
return bus->config_readb(d, addr);
}
uint16_t qvirtio_config_readw(const QVirtioBus *bus, QVirtioDevice *d,
- void *addr)
+ uint64_t addr)
{
return bus->config_readw(d, addr);
}
uint32_t qvirtio_config_readl(const QVirtioBus *bus, QVirtioDevice *d,
- void *addr)
+ uint64_t addr)
{
return bus->config_readl(d, addr);
}
uint64_t qvirtio_config_readq(const QVirtioBus *bus, QVirtioDevice *d,
- void *addr)
+ uint64_t addr)
{
return bus->config_readq(d, addr);
}
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 29fbacb..2449fee 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -93,10 +93,10 @@ typedef struct QVRingIndirectDesc {
} QVRingIndirectDesc;
typedef struct QVirtioBus {
- uint8_t (*config_readb)(QVirtioDevice *d, void *addr);
- uint16_t (*config_readw)(QVirtioDevice *d, void *addr);
- uint32_t (*config_readl)(QVirtioDevice *d, void *addr);
- uint64_t (*config_readq)(QVirtioDevice *d, void *addr);
+ uint8_t (*config_readb)(QVirtioDevice *d, uint64_t addr);
+ uint16_t (*config_readw)(QVirtioDevice *d, uint64_t addr);
+ uint32_t (*config_readl)(QVirtioDevice *d, uint64_t addr);
+ uint64_t (*config_readq)(QVirtioDevice *d, uint64_t addr);
/* Get features of the device */
uint32_t (*get_features)(QVirtioDevice *d);
@@ -144,13 +144,13 @@ static inline uint32_t qvring_size(uint32_t num, uint32_t align)
}
uint8_t qvirtio_config_readb(const QVirtioBus *bus, QVirtioDevice *d,
- void *addr);
+ uint64_t addr);
uint16_t qvirtio_config_readw(const QVirtioBus *bus, QVirtioDevice *d,
- void *addr);
+ uint64_t addr);
uint32_t qvirtio_config_readl(const QVirtioBus *bus, QVirtioDevice *d,
- void *addr);
+ uint64_t addr);
uint64_t qvirtio_config_readq(const QVirtioBus *bus, QVirtioDevice *d,
- void *addr);
+ uint64_t addr);
uint32_t qvirtio_get_features(const QVirtioBus *bus, QVirtioDevice *d);
void qvirtio_set_features(const QVirtioBus *bus, QVirtioDevice *d,
uint32_t features);
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index ead3911..33e8094 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -157,7 +157,8 @@ static void pci_basic(void)
/* MSI-X is not enabled */
addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX;
- capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
+ capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
+ (uint64_t)(uintptr_t)addr);
g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
@@ -303,7 +304,8 @@ static void pci_indirect(void)
/* MSI-X is not enabled */
addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX;
- capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
+ capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
+ (uint64_t)(uintptr_t)addr);
g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
@@ -394,7 +396,8 @@ static void pci_config(void)
/* MSI-X is not enabled */
addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX;
- capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
+ capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
+ (uint64_t)(uintptr_t)addr);
g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
@@ -403,7 +406,8 @@ static void pci_config(void)
" 'size': %d } }", n_size);
qvirtio_wait_config_isr(&qvirtio_pci, &dev->vdev, QVIRTIO_BLK_TIMEOUT_US);
- capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
+ capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
+ (uint64_t)(uintptr_t)addr);
g_assert_cmpint(capacity, ==, n_size / 512);
qvirtio_pci_device_disable(dev);
@@ -438,7 +442,8 @@ static void pci_msix(void)
/* MSI-X is enabled */
addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_MSIX;
- capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
+ capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
+ (uint64_t)(uintptr_t)addr);
g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
@@ -458,7 +463,8 @@ static void pci_msix(void)
qvirtio_wait_config_isr(&qvirtio_pci, &dev->vdev, QVIRTIO_BLK_TIMEOUT_US);
- capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
+ capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
+ (uint64_t)(uintptr_t)addr);
g_assert_cmpint(capacity, ==, n_size / 512);
/* Write request */
@@ -547,7 +553,8 @@ static void pci_idx(void)
/* MSI-X is enabled */
addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_MSIX;
- capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
+ capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
+ (uint64_t)(uintptr_t)addr);
g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/5] libqos: Change use of pointers to uint64_t in virtio
2014-11-01 17:02 ` [Qemu-devel] [PATCH v2 1/5] libqos: Change use of pointers to uint64_t in virtio Marc Marí
@ 2014-11-17 15:16 ` Stefan Hajnoczi
2014-11-17 15:19 ` Marc Marí
0 siblings, 1 reply; 17+ messages in thread
From: Stefan Hajnoczi @ 2014-11-17 15:16 UTC (permalink / raw)
To: Marc Marí; +Cc: qemu-devel, Stefan Hajnoczi, Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 735 bytes --]
On Sat, Nov 01, 2014 at 06:02:26PM +0100, Marc Marí wrote:
> Convert use of pointers in functions of virtio to uint64_t in order to make it
> platform-independent.
>
> Add casting from pointers (in PCI functions) to uint64_t and vice versa through
> uintptr_t.
>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
> tests/libqos/virtio-pci.c | 20 +++++++++++---------
> tests/libqos/virtio.c | 8 ++++----
> tests/libqos/virtio.h | 16 ++++++++--------
> tests/virtio-blk-test.c | 21 ++++++++++++++-------
> 4 files changed, 37 insertions(+), 28 deletions(-)
This makes sense. To fully abolish pointers the PCI code also needs to
be converted. Do plan plan to do that?
Stefan
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/5] libqos: Change use of pointers to uint64_t in virtio
2014-11-17 15:16 ` Stefan Hajnoczi
@ 2014-11-17 15:19 ` Marc Marí
2014-11-17 15:25 ` Andreas Färber
0 siblings, 1 reply; 17+ messages in thread
From: Marc Marí @ 2014-11-17 15:19 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Stefan Hajnoczi, Andreas Färber
El Mon, 17 Nov 2014 15:16:21 +0000
Stefan Hajnoczi <stefanha@gmail.com> escribió:
> On Sat, Nov 01, 2014 at 06:02:26PM +0100, Marc Marí wrote:
> > Convert use of pointers in functions of virtio to uint64_t in order
> > to make it platform-independent.
> >
> > Add casting from pointers (in PCI functions) to uint64_t and vice
> > versa through uintptr_t.
> >
> > Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> > ---
> > tests/libqos/virtio-pci.c | 20 +++++++++++---------
> > tests/libqos/virtio.c | 8 ++++----
> > tests/libqos/virtio.h | 16 ++++++++--------
> > tests/virtio-blk-test.c | 21 ++++++++++++++-------
> > 4 files changed, 37 insertions(+), 28 deletions(-)
>
> This makes sense. To fully abolish pointers the PCI code also needs
> to be converted. Do plan plan to do that?
>
> Stefan
Not planned, but if asked, I may do it.
Marc
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/5] libqos: Change use of pointers to uint64_t in virtio
2014-11-17 15:19 ` Marc Marí
@ 2014-11-17 15:25 ` Andreas Färber
2014-11-18 14:05 ` Stefan Hajnoczi
0 siblings, 1 reply; 17+ messages in thread
From: Andreas Färber @ 2014-11-17 15:25 UTC (permalink / raw)
To: Marc Marí; +Cc: Stefan Hajnoczi, qemu-devel, Stefan Hajnoczi
Am 17.11.2014 um 16:19 schrieb Marc Marí:
> El Mon, 17 Nov 2014 15:16:21 +0000
> Stefan Hajnoczi <stefanha@gmail.com> escribió:
>> On Sat, Nov 01, 2014 at 06:02:26PM +0100, Marc Marí wrote:
>>> Convert use of pointers in functions of virtio to uint64_t in order
>>> to make it platform-independent.
>>>
>>> Add casting from pointers (in PCI functions) to uint64_t and vice
>>> versa through uintptr_t.
>>>
>>> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
>>> ---
>>> tests/libqos/virtio-pci.c | 20 +++++++++++---------
>>> tests/libqos/virtio.c | 8 ++++----
>>> tests/libqos/virtio.h | 16 ++++++++--------
>>> tests/virtio-blk-test.c | 21 ++++++++++++++-------
>>> 4 files changed, 37 insertions(+), 28 deletions(-)
>>
>> This makes sense. To fully abolish pointers the PCI code also needs
>> to be converted. Do plan plan to do that?
>>
>> Stefan
>
> Not planned, but if asked, I may do it.
Pretty please. :) I was wondering the same thing when reading the patch.
Regards,
Andreas
--
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/5] libqos: Change use of pointers to uint64_t in virtio
2014-11-17 15:25 ` Andreas Färber
@ 2014-11-18 14:05 ` Stefan Hajnoczi
0 siblings, 0 replies; 17+ messages in thread
From: Stefan Hajnoczi @ 2014-11-18 14:05 UTC (permalink / raw)
To: Andreas Färber; +Cc: Marc Marí, Stefan Hajnoczi, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1426 bytes --]
On Mon, Nov 17, 2014 at 04:25:50PM +0100, Andreas Färber wrote:
> Am 17.11.2014 um 16:19 schrieb Marc Marí:
> > El Mon, 17 Nov 2014 15:16:21 +0000
> > Stefan Hajnoczi <stefanha@gmail.com> escribió:
> >> On Sat, Nov 01, 2014 at 06:02:26PM +0100, Marc Marí wrote:
> >>> Convert use of pointers in functions of virtio to uint64_t in order
> >>> to make it platform-independent.
> >>>
> >>> Add casting from pointers (in PCI functions) to uint64_t and vice
> >>> versa through uintptr_t.
> >>>
> >>> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> >>> ---
> >>> tests/libqos/virtio-pci.c | 20 +++++++++++---------
> >>> tests/libqos/virtio.c | 8 ++++----
> >>> tests/libqos/virtio.h | 16 ++++++++--------
> >>> tests/virtio-blk-test.c | 21 ++++++++++++++-------
> >>> 4 files changed, 37 insertions(+), 28 deletions(-)
> >>
> >> This makes sense. To fully abolish pointers the PCI code also needs
> >> to be converted. Do plan plan to do that?
> >>
> >> Stefan
> >
> > Not planned, but if asked, I may do it.
>
> Pretty please. :) I was wondering the same thing when reading the patch.
That said, you do not need to convert PCI in this patch series.
If you do the PCI conversion in a separate patch series we can keep
merging code incrementally. It's more satisfying for you that way and
less intimidating for code reviews to review short series.
Stefan
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v2 2/5] tests: Prepare virtio-blk-test for multi-arch implementation
2014-11-01 17:02 [Qemu-devel] [PATCH v2 0/5] libqos: Virtio MMIO driver Marc Marí
2014-11-01 17:02 ` [Qemu-devel] [PATCH v2 1/5] libqos: Change use of pointers to uint64_t in virtio Marc Marí
@ 2014-11-01 17:02 ` Marc Marí
2014-11-17 15:21 ` Stefan Hajnoczi
2014-11-01 17:02 ` [Qemu-devel] [PATCH v2 3/5] libqos: Remove PCI assumptions in constants of virtio driver Marc Marí
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Marc Marí @ 2014-11-01 17:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc Marí, Andreas Färber, Stefan Hajnoczi
Modularize functions in virtio-blk-test and add PCI suffix for PCI specific
components.
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
tests/virtio-blk-test.c | 57 +++++++++++++++++++++++++++++++----------------
1 file changed, 38 insertions(+), 19 deletions(-)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 33e8094..6f07d5a 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -55,11 +55,11 @@ typedef struct QVirtioBlkReq {
uint8_t status;
} QVirtioBlkReq;
-static QPCIBus *test_start(void)
+static char *drive_create(void)
{
- char *cmdline;
- char tmp_path[] = "/tmp/qtest.XXXXXX";
int fd, ret;
+ char *tmp_path = malloc(18);
+ strncpy(tmp_path, "/tmp/qtest.XXXXXX", 18);
/* Create a temporary raw image */
fd = mkstemp(tmp_path);
@@ -68,6 +68,16 @@ static QPCIBus *test_start(void)
g_assert_cmpint(ret, ==, 0);
close(fd);
+ return tmp_path;
+}
+
+static QPCIBus *pci_test_start(void)
+{
+ char *cmdline;
+ char *tmp_path;
+
+ tmp_path = drive_create();
+
cmdline = g_strdup_printf("-drive if=none,id=drive0,file=%s "
"-drive if=none,id=drive1,file=/dev/null "
"-device virtio-blk-pci,id=drv0,drive=drive0,"
@@ -85,7 +95,7 @@ static void test_end(void)
qtest_end();
}
-static QVirtioPCIDevice *virtio_blk_init(QPCIBus *bus, int slot)
+static QVirtioPCIDevice *virtio_blk_pci_init(QPCIBus *bus, int slot)
{
QVirtioPCIDevice *dev;
@@ -150,9 +160,9 @@ static void pci_basic(void)
uint8_t status;
char *data;
- bus = test_start();
+ bus = pci_test_start();
- dev = virtio_blk_init(bus, PCI_SLOT);
+ dev = virtio_blk_pci_init(bus, PCI_SLOT);
/* MSI-X is not enabled */
addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX;
@@ -276,8 +286,10 @@ static void pci_basic(void)
/* End test */
guest_free(alloc, vqpci->vq.desc);
+ pc_alloc_uninit(alloc);
qvirtio_pci_device_disable(dev);
g_free(dev);
+ qpci_free_pc(bus);
test_end();
}
@@ -297,9 +309,9 @@ static void pci_indirect(void)
uint8_t status;
char *data;
- bus = test_start();
+ bus = pci_test_start();
- dev = virtio_blk_init(bus, PCI_SLOT);
+ dev = virtio_blk_pci_init(bus, PCI_SLOT);
/* MSI-X is not enabled */
addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX;
@@ -376,8 +388,10 @@ static void pci_indirect(void)
/* End test */
guest_free(alloc, vqpci->vq.desc);
+ pc_alloc_uninit(alloc);
qvirtio_pci_device_disable(dev);
g_free(dev);
+ qpci_free_pc(bus);
test_end();
}
@@ -389,9 +403,9 @@ static void pci_config(void)
void *addr;
uint64_t capacity;
- bus = test_start();
+ bus = pci_test_start();
- dev = virtio_blk_init(bus, PCI_SLOT);
+ dev = virtio_blk_pci_init(bus, PCI_SLOT);
/* MSI-X is not enabled */
addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX;
@@ -412,6 +426,7 @@ static void pci_config(void)
qvirtio_pci_device_disable(dev);
g_free(dev);
+ qpci_free_pc(bus);
test_end();
}
@@ -431,10 +446,10 @@ static void pci_msix(void)
uint8_t status;
char *data;
- bus = test_start();
+ bus = pci_test_start();
alloc = pc_alloc_init();
- dev = virtio_blk_init(bus, PCI_SLOT);
+ dev = virtio_blk_pci_init(bus, PCI_SLOT);
qpci_msix_enable(dev->pdev);
qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
@@ -520,10 +535,12 @@ static void pci_msix(void)
guest_free(alloc, req_addr);
/* End test */
- guest_free(alloc, (uint64_t)vqpci->vq.desc);
+ guest_free(alloc, vqpci->vq.desc);
+ pc_alloc_uninit(alloc);
qpci_msix_disable(dev->pdev);
qvirtio_pci_device_disable(dev);
g_free(dev);
+ qpci_free_pc(bus);
test_end();
}
@@ -542,10 +559,10 @@ static void pci_idx(void)
uint8_t status;
char *data;
- bus = test_start();
+ bus = pci_test_start();
alloc = pc_alloc_init();
- dev = virtio_blk_init(bus, PCI_SLOT);
+ dev = virtio_blk_pci_init(bus, PCI_SLOT);
qpci_msix_enable(dev->pdev);
qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
@@ -643,24 +660,26 @@ static void pci_idx(void)
/* End test */
guest_free(alloc, vqpci->vq.desc);
+ pc_alloc_uninit(alloc);
qpci_msix_disable(dev->pdev);
qvirtio_pci_device_disable(dev);
g_free(dev);
+ qpci_free_pc(bus);
test_end();
}
-static void hotplug(void)
+static void pci_hotplug(void)
{
QPCIBus *bus;
QVirtioPCIDevice *dev;
- bus = test_start();
+ bus = pci_test_start();
/* plug secondary disk */
qpci_plug_device_test("virtio-blk-pci", "drv1", PCI_SLOT_HP,
"'drive': 'drive1'");
- dev = virtio_blk_init(bus, PCI_SLOT_HP);
+ dev = virtio_blk_pci_init(bus, PCI_SLOT_HP);
g_assert(dev);
qvirtio_pci_device_disable(dev);
g_free(dev);
@@ -681,7 +700,7 @@ int main(int argc, char **argv)
g_test_add_func("/virtio/blk/pci/config", pci_config);
g_test_add_func("/virtio/blk/pci/msix", pci_msix);
g_test_add_func("/virtio/blk/pci/idx", pci_idx);
- g_test_add_func("/virtio/blk/pci/hotplug", hotplug);
+ g_test_add_func("/virtio/blk/pci/hotplug", pci_hotplug);
ret = g_test_run();
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/5] tests: Prepare virtio-blk-test for multi-arch implementation
2014-11-01 17:02 ` [Qemu-devel] [PATCH v2 2/5] tests: Prepare virtio-blk-test for multi-arch implementation Marc Marí
@ 2014-11-17 15:21 ` Stefan Hajnoczi
0 siblings, 0 replies; 17+ messages in thread
From: Stefan Hajnoczi @ 2014-11-17 15:21 UTC (permalink / raw)
To: Marc Marí; +Cc: qemu-devel, Stefan Hajnoczi, Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 1586 bytes --]
On Sat, Nov 01, 2014 at 06:02:27PM +0100, Marc Marí wrote:
> Modularize functions in virtio-blk-test and add PCI suffix for PCI specific
> components.
>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
> tests/virtio-blk-test.c | 57 +++++++++++++++++++++++++++++++----------------
> 1 file changed, 38 insertions(+), 19 deletions(-)
>
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index 33e8094..6f07d5a 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -55,11 +55,11 @@ typedef struct QVirtioBlkReq {
> uint8_t status;
> } QVirtioBlkReq;
>
> -static QPCIBus *test_start(void)
> +static char *drive_create(void)
> {
> - char *cmdline;
> - char tmp_path[] = "/tmp/qtest.XXXXXX";
> int fd, ret;
> + char *tmp_path = malloc(18);
> + strncpy(tmp_path, "/tmp/qtest.XXXXXX", 18);
Please use g_strdup():
https://developer.gnome.org/glib/stable/glib-String-Utility-Functions.html#g-strdup
QEMU does not use malloc(3)/free(3) directly. Always use glib memory
allocation (in this case g_strdup() allocates memory for you).
>
> /* Create a temporary raw image */
> fd = mkstemp(tmp_path);
> @@ -68,6 +68,16 @@ static QPCIBus *test_start(void)
> g_assert_cmpint(ret, ==, 0);
> close(fd);
>
> + return tmp_path;
> +}
> +
> +static QPCIBus *pci_test_start(void)
> +{
> + char *cmdline;
> + char *tmp_path;
> +
> + tmp_path = drive_create();
It seems tmp_path is leaked. You must free dynamically allocated
memory!
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v2 3/5] libqos: Remove PCI assumptions in constants of virtio driver
2014-11-01 17:02 [Qemu-devel] [PATCH v2 0/5] libqos: Virtio MMIO driver Marc Marí
2014-11-01 17:02 ` [Qemu-devel] [PATCH v2 1/5] libqos: Change use of pointers to uint64_t in virtio Marc Marí
2014-11-01 17:02 ` [Qemu-devel] [PATCH v2 2/5] tests: Prepare virtio-blk-test for multi-arch implementation Marc Marí
@ 2014-11-01 17:02 ` Marc Marí
2014-11-01 17:02 ` [Qemu-devel] [PATCH v2 4/5] libqos: Add malloc generic Marc Marí
` (2 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Marc Marí @ 2014-11-01 17:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc Marí, Andreas Färber, Stefan Hajnoczi
Convert PCI-specific constants names of libqos virtio driver.
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
tests/libqos/virtio-pci.c | 30 +++++++++++++++---------------
tests/libqos/virtio-pci.h | 24 ++++++++++++------------
tests/virtio-blk-test.c | 10 +++++-----
3 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 92bcac1..046a316 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -102,31 +102,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_DEVICE_FEATURES);
+ return qpci_io_readl(dev->pdev, dev->addr + QVIRTIO_PCI_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);
+ qpci_io_writel(dev->pdev, dev->addr + QVIRTIO_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_GUEST_FEATURES);
+ return qpci_io_readl(dev->pdev, dev->addr + QVIRTIO_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_DEVICE_STATUS);
+ return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_PCI_DEVICE_STATUS);
}
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);
+ qpci_io_writeb(dev->pdev, dev->addr + QVIRTIO_PCI_DEVICE_STATUS, status);
}
static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
@@ -146,7 +146,7 @@ static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
return data == vqpci->msix_data;
}
} else {
- return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_ISR_STATUS) & 1;
+ return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_PCI_ISR_STATUS) & 1;
}
}
@@ -166,26 +166,26 @@ static bool qvirtio_pci_get_config_isr_status(QVirtioDevice *d)
return data == dev->config_msix_data;
}
} else {
- return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_ISR_STATUS) & 2;
+ return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_PCI_ISR_STATUS) & 2;
}
}
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);
+ qpci_io_writeb(dev->pdev, dev->addr + QVIRTIO_PCI_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);
+ return qpci_io_readw(dev->pdev, dev->addr + QVIRTIO_PCI_QUEUE_SIZE);
}
static void qvirtio_pci_set_queue_address(QVirtioDevice *d, uint32_t pfn)
{
QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
- qpci_io_writel(dev->pdev, dev->addr + QVIRTIO_QUEUE_ADDRESS, pfn);
+ qpci_io_writel(dev->pdev, dev->addr + QVIRTIO_PCI_QUEUE_ADDRESS, pfn);
}
static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
@@ -227,7 +227,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_QUEUE_NOTIFY, vq->index);
+ qpci_io_writew(dev->pdev, dev->addr + QVIRTIO_PCI_QUEUE_NOTIFY, vq->index);
}
const QVirtioBus qvirtio_pci = {
@@ -307,8 +307,8 @@ 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_MSIX_QUEUE_VECTOR, entry);
- vector = qpci_io_readw(d->pdev, d->addr + QVIRTIO_MSIX_QUEUE_VECTOR);
+ 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);
}
@@ -339,7 +339,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_MSIX_CONF_VECTOR, entry);
- vector = qpci_io_readw(d->pdev, d->addr + QVIRTIO_MSIX_CONF_VECTOR);
+ 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);
}
diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
index 883f7ff..8f0e52a 100644
--- a/tests/libqos/virtio-pci.h
+++ b/tests/libqos/virtio-pci.h
@@ -13,18 +13,18 @@
#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
+#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
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 6f07d5a..449e8a4 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -165,7 +165,7 @@ static void pci_basic(void)
dev = virtio_blk_pci_init(bus, PCI_SLOT);
/* MSI-X is not enabled */
- addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX;
+ addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_NO_MSIX;
capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
(uint64_t)(uintptr_t)addr);
@@ -314,7 +314,7 @@ static void pci_indirect(void)
dev = virtio_blk_pci_init(bus, PCI_SLOT);
/* MSI-X is not enabled */
- addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX;
+ addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_NO_MSIX;
capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
(uint64_t)(uintptr_t)addr);
@@ -408,7 +408,7 @@ static void pci_config(void)
dev = virtio_blk_pci_init(bus, PCI_SLOT);
/* MSI-X is not enabled */
- addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX;
+ addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_NO_MSIX;
capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
(uint64_t)(uintptr_t)addr);
@@ -455,7 +455,7 @@ static void pci_msix(void)
qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
/* MSI-X is enabled */
- addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_MSIX;
+ addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_MSIX;
capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
(uint64_t)(uintptr_t)addr);
@@ -568,7 +568,7 @@ static void pci_idx(void)
qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
/* MSI-X is enabled */
- addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_MSIX;
+ addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_MSIX;
capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
(uint64_t)(uintptr_t)addr);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v2 4/5] libqos: Add malloc generic
2014-11-01 17:02 [Qemu-devel] [PATCH v2 0/5] libqos: Virtio MMIO driver Marc Marí
` (2 preceding siblings ...)
2014-11-01 17:02 ` [Qemu-devel] [PATCH v2 3/5] libqos: Remove PCI assumptions in constants of virtio driver Marc Marí
@ 2014-11-01 17:02 ` Marc Marí
2014-11-17 15:30 ` Stefan Hajnoczi
2014-11-01 17:02 ` [Qemu-devel] [PATCH v2 5/5] libqos: Add virtio MMIO support Marc Marí
2014-11-17 9:26 ` [Qemu-devel] [PATCH v2 0/5] libqos: Virtio MMIO driver Marc Marí
5 siblings, 1 reply; 17+ messages in thread
From: Marc Marí @ 2014-11-01 17:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc Marí, Andreas Färber, Stefan Hajnoczi
This malloc is a basic interface implementation that works for any platform.
It should be replaced in the future for a real malloc implementation for each
of the platforms.
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
tests/libqos/malloc-generic.c | 50 +++++++++++++++++++++++++++++++++++++++++
tests/libqos/malloc-generic.h | 21 +++++++++++++++++
2 files changed, 71 insertions(+)
create mode 100644 tests/libqos/malloc-generic.c
create mode 100644 tests/libqos/malloc-generic.h
diff --git a/tests/libqos/malloc-generic.c b/tests/libqos/malloc-generic.c
new file mode 100644
index 0000000..0049424
--- /dev/null
+++ b/tests/libqos/malloc-generic.c
@@ -0,0 +1,50 @@
+/*
+ * Basic libqos generic malloc support
+ *
+ * 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 "libqos/malloc-generic.h"
+#include "libqos/malloc.h"
+#include <glib.h>
+
+/*
+ * Mostly for valgrind happiness, but it does offer
+ * a chokepoint for debugging guest memory leaks, too.
+ */
+void generic_alloc_uninit(QGuestAllocator *allocator)
+{
+ alloc_uninit(allocator);
+}
+
+QGuestAllocator *generic_alloc_init_flags(uint64_t base_addr, uint64_t size,
+ uint32_t page_size, QAllocOpts flags)
+{
+ QGuestAllocator *s = g_malloc0(sizeof(*s));
+ MemBlock *node;
+
+ s->opts = flags;
+ s->page_size = page_size;
+
+ /* Start at 1MB */
+ s->start = base_addr + (1 << 20);
+
+ s->end = s->start + size;
+
+ QTAILQ_INIT(&s->used);
+ QTAILQ_INIT(&s->free);
+
+ node = mlist_new(s->start, s->end - s->start);
+ QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME);
+
+ return s;
+}
+
+inline QGuestAllocator *generic_alloc_init(uint64_t base_addr, uint64_t size,
+ uint32_t page_size)
+{
+ return generic_alloc_init_flags(base_addr, size, page_size, ALLOC_NO_FLAGS);
+}
diff --git a/tests/libqos/malloc-generic.h b/tests/libqos/malloc-generic.h
new file mode 100644
index 0000000..90104ec
--- /dev/null
+++ b/tests/libqos/malloc-generic.h
@@ -0,0 +1,21 @@
+/*
+ * Basic libqos generic malloc support
+ *
+ * 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_MALLOC_GENERIC_H
+#define LIBQOS_MALLOC_GENERIC_H
+
+#include "libqos/malloc.h"
+
+QGuestAllocator *generic_alloc_init(uint64_t base_addr, uint64_t size,
+ uint32_t page_size);
+QGuestAllocator *generic_alloc_init_flags(uint64_t base_addr, uint64_t size,
+ uint32_t page_size, QAllocOpts flags);
+void generic_alloc_uninit(QGuestAllocator *allocator);
+
+#endif
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v2 4/5] libqos: Add malloc generic
2014-11-01 17:02 ` [Qemu-devel] [PATCH v2 4/5] libqos: Add malloc generic Marc Marí
@ 2014-11-17 15:30 ` Stefan Hajnoczi
0 siblings, 0 replies; 17+ messages in thread
From: Stefan Hajnoczi @ 2014-11-17 15:30 UTC (permalink / raw)
To: Marc Marí; +Cc: qemu-devel, Stefan Hajnoczi, Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]
On Sat, Nov 01, 2014 at 06:02:29PM +0100, Marc Marí wrote:
> This malloc is a basic interface implementation that works for any platform.
> It should be replaced in the future for a real malloc implementation for each
> of the platforms.
>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
> tests/libqos/malloc-generic.c | 50 +++++++++++++++++++++++++++++++++++++++++
> tests/libqos/malloc-generic.h | 21 +++++++++++++++++
> 2 files changed, 71 insertions(+)
> create mode 100644 tests/libqos/malloc-generic.c
> create mode 100644 tests/libqos/malloc-generic.h
>
> diff --git a/tests/libqos/malloc-generic.c b/tests/libqos/malloc-generic.c
> new file mode 100644
> index 0000000..0049424
> --- /dev/null
> +++ b/tests/libqos/malloc-generic.c
> @@ -0,0 +1,50 @@
> +/*
> + * Basic libqos generic malloc support
> + *
> + * 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 "libqos/malloc-generic.h"
> +#include "libqos/malloc.h"
> +#include <glib.h>
Please always include system headers (<>) before user headers ("").
This ensures that QEMU's headers do not pollute the namespace or affect
system headers in any way.
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v2 5/5] libqos: Add virtio MMIO support
2014-11-01 17:02 [Qemu-devel] [PATCH v2 0/5] libqos: Virtio MMIO driver Marc Marí
` (3 preceding siblings ...)
2014-11-01 17:02 ` [Qemu-devel] [PATCH v2 4/5] libqos: Add malloc generic Marc Marí
@ 2014-11-01 17:02 ` Marc Marí
2014-11-17 15:48 ` Stefan Hajnoczi
2014-11-17 9:26 ` [Qemu-devel] [PATCH v2 0/5] libqos: Virtio MMIO driver Marc Marí
5 siblings, 1 reply; 17+ messages in thread
From: Marc Marí @ 2014-11-01 17:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc Marí, Andreas Färber, Stefan Hajnoczi
Add virtio MMIO support.
Add virtio-blk-test MMIO test case.
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
tests/Makefile | 4 +-
tests/libqos/virtio-mmio.c | 190 ++++++++++++++++++++++++++++++++++++++++++++
tests/libqos/virtio-mmio.h | 46 +++++++++++
tests/virtio-blk-test.c | 143 +++++++++++++++++++++++++++++++--
4 files changed, 375 insertions(+), 8 deletions(-)
create mode 100644 tests/libqos/virtio-mmio.c
create mode 100644 tests/libqos/virtio-mmio.h
diff --git a/tests/Makefile b/tests/Makefile
index 4ae0ca4..f193b03 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -183,6 +183,8 @@ gcov-files-sparc-y += hw/timer/m48t59.c
gcov-files-sparc64-y += hw/timer/m48t59.c
check-qtest-arm-y = tests/tmp105-test$(EXESUF)
gcov-files-arm-y += hw/misc/tmp105.c
+check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
+gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
check-qtest-ppc-y += tests/boot-order-test$(EXESUF)
check-qtest-ppc64-y += tests/boot-order-test$(EXESUF)
check-qtest-ppc64-y += tests/spapr-phb-test$(EXESUF)
@@ -300,8 +302,8 @@ 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.o tests/libqos/virtio-pci.o
libqos-usb-obj-y = $(libqos-pc-obj-y) tests/libqos/usb.o
+libqos-virtio-obj-y = $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o
tests/rtc-test$(EXESUF): tests/rtc-test.o
tests/m48t59-test$(EXESUF): tests/m48t59-test.o
diff --git a/tests/libqos/virtio-mmio.c b/tests/libqos/virtio-mmio.c
new file mode 100644
index 0000000..6896f7b
--- /dev/null
+++ b/tests/libqos/virtio-mmio.c
@@ -0,0 +1,190 @@
+/*
+ * libqos virtio MMIO 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-mmio.h"
+#include "libqos/malloc.h"
+#include "libqos/malloc-generic.h"
+#include <stdio.h>
+
+static uint8_t qvirtio_mmio_config_readb(QVirtioDevice *d, uint64_t addr)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ return readb(dev->addr + addr);
+}
+
+static uint16_t qvirtio_mmio_config_readw(QVirtioDevice *d, uint64_t addr)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ return readw(dev->addr + addr);
+}
+
+static uint32_t qvirtio_mmio_config_readl(QVirtioDevice *d, uint64_t addr)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ return readl(dev->addr + addr);
+}
+
+static uint64_t qvirtio_mmio_config_readq(QVirtioDevice *d, uint64_t addr)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ return readq(dev->addr + addr);
+}
+
+static uint32_t qvirtio_mmio_get_features(QVirtioDevice *d)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ writel(dev->addr + QVIRTIO_MMIO_HOST_FEATURES_SEL, 0);
+ return readl(dev->addr + QVIRTIO_MMIO_HOST_FEATURES);
+}
+
+static void qvirtio_mmio_set_features(QVirtioDevice *d, uint32_t features)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ dev->features = features;
+ writel(dev->addr + QVIRTIO_MMIO_GUEST_FEATURES_SEL, 0);
+ writel(dev->addr + QVIRTIO_MMIO_GUEST_FEATURES, features);
+}
+
+static uint32_t qvirtio_mmio_get_guest_features(QVirtioDevice *d)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ return dev->features;
+}
+
+static uint8_t qvirtio_mmio_get_status(QVirtioDevice *d)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ return (uint8_t)readl(dev->addr + QVIRTIO_MMIO_DEVICE_STATUS);
+}
+
+static void qvirtio_mmio_set_status(QVirtioDevice *d, uint8_t status)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ writel(dev->addr + QVIRTIO_MMIO_DEVICE_STATUS, (uint32_t)status);
+}
+
+static bool qvirtio_mmio_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ uint32_t isr;
+
+ isr = readl(dev->addr + QVIRTIO_MMIO_INTERRUPT_STATUS) & 1;
+ writel(dev->addr + QVIRTIO_MMIO_INTERRUPT_ACK, 1);
+ return isr != 0;
+}
+
+static bool qvirtio_mmio_get_config_isr_status(QVirtioDevice *d)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ uint32_t isr;
+
+ isr = readl(dev->addr + QVIRTIO_MMIO_INTERRUPT_STATUS) & 2;
+ writel(dev->addr + QVIRTIO_MMIO_INTERRUPT_ACK, 2);
+ return isr != 0;
+}
+
+static void qvirtio_mmio_queue_select(QVirtioDevice *d, uint16_t index)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ writel(dev->addr + QVIRTIO_MMIO_QUEUE_SEL, (uint32_t)index);
+
+ g_assert_cmphex(readl(dev->addr + QVIRTIO_MMIO_QUEUE_PFN), ==, 0);
+}
+
+static uint16_t qvirtio_mmio_get_queue_size(QVirtioDevice *d)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ return (uint16_t)readl(dev->addr + QVIRTIO_MMIO_QUEUE_NUM_MAX);
+}
+
+static void qvirtio_mmio_set_queue_address(QVirtioDevice *d, uint32_t pfn)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ writel(dev->addr + QVIRTIO_MMIO_QUEUE_PFN, pfn);
+}
+
+static QVirtQueue *qvirtio_mmio_virtqueue_setup(QVirtioDevice *d,
+ QGuestAllocator *alloc, uint16_t index)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ QVirtQueue *vq;
+ uint64_t addr;
+
+ vq = g_malloc0(sizeof(*vq));
+ qvirtio_mmio_queue_select(d, index);
+ writel(dev->addr + QVIRTIO_MMIO_QUEUE_ALIGN, dev->page_size);
+
+ vq->index = index;
+ vq->size = qvirtio_mmio_get_queue_size(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;
+
+ writel(dev->addr + QVIRTIO_MMIO_QUEUE_NUM, vq->size);
+
+ /* Check different than 0 */
+ g_assert_cmpint(vq->size, !=, 0);
+
+ /* Check power of 2 */
+ g_assert_cmpint(vq->size & (vq->size - 1), ==, 0);
+
+ addr = guest_alloc(alloc, qvring_size(vq->size, dev->page_size));
+ qvring_init(alloc, vq, addr);
+ qvirtio_mmio_set_queue_address(d, vq->desc / dev->page_size);
+
+ return vq;
+}
+
+static void qvirtio_mmio_virtqueue_kick(QVirtioDevice *d, QVirtQueue *vq)
+{
+ QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+ writel(dev->addr + QVIRTIO_MMIO_QUEUE_NOTIFY, vq->index);
+}
+
+const QVirtioBus qvirtio_mmio = {
+ .config_readb = qvirtio_mmio_config_readb,
+ .config_readw = qvirtio_mmio_config_readw,
+ .config_readl = qvirtio_mmio_config_readl,
+ .config_readq = qvirtio_mmio_config_readq,
+ .get_features = qvirtio_mmio_get_features,
+ .set_features = qvirtio_mmio_set_features,
+ .get_guest_features = qvirtio_mmio_get_guest_features,
+ .get_status = qvirtio_mmio_get_status,
+ .set_status = qvirtio_mmio_set_status,
+ .get_queue_isr_status = qvirtio_mmio_get_queue_isr_status,
+ .get_config_isr_status = qvirtio_mmio_get_config_isr_status,
+ .queue_select = qvirtio_mmio_queue_select,
+ .get_queue_size = qvirtio_mmio_get_queue_size,
+ .set_queue_address = qvirtio_mmio_set_queue_address,
+ .virtqueue_setup = qvirtio_mmio_virtqueue_setup,
+ .virtqueue_kick = qvirtio_mmio_virtqueue_kick,
+};
+
+QVirtioMMIODevice *qvirtio_mmio_init_device(uint64_t addr, uint32_t page_size)
+{
+ QVirtioMMIODevice *dev;
+ uint32_t magic;
+ dev = g_malloc0(sizeof(*dev));
+
+ magic = readl(addr + QVIRTIO_MMIO_MAGIC_VALUE);
+ g_assert(magic == ('v' | 'i' << 8 | 'r' << 16 | 't' << 24));
+
+ dev->addr = addr;
+ dev->page_size = page_size;
+ dev->vdev.device_type = readl(addr + QVIRTIO_MMIO_DEVICE_ID);
+
+ writel(addr + QVIRTIO_MMIO_GUEST_PAGE_SIZE, page_size);
+
+ return dev;
+}
diff --git a/tests/libqos/virtio-mmio.h b/tests/libqos/virtio-mmio.h
new file mode 100644
index 0000000..e3e52b9
--- /dev/null
+++ b/tests/libqos/virtio-mmio.h
@@ -0,0 +1,46 @@
+/*
+ * libqos virtio MMIO 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_MMIO_H
+#define LIBQOS_VIRTIO_MMIO_H
+
+#include "libqos/virtio.h"
+
+#define QVIRTIO_MMIO_MAGIC_VALUE 0x000
+#define QVIRTIO_MMIO_VERSION 0x004
+#define QVIRTIO_MMIO_DEVICE_ID 0x008
+#define QVIRTIO_MMIO_VENDOR_ID 0x00C
+#define QVIRTIO_MMIO_HOST_FEATURES 0x010
+#define QVIRTIO_MMIO_HOST_FEATURES_SEL 0x014
+#define QVIRTIO_MMIO_GUEST_FEATURES 0x020
+#define QVIRTIO_MMIO_GUEST_FEATURES_SEL 0x024
+#define QVIRTIO_MMIO_GUEST_PAGE_SIZE 0x028
+#define QVIRTIO_MMIO_QUEUE_SEL 0x030
+#define QVIRTIO_MMIO_QUEUE_NUM_MAX 0x034
+#define QVIRTIO_MMIO_QUEUE_NUM 0x038
+#define QVIRTIO_MMIO_QUEUE_ALIGN 0x03C
+#define QVIRTIO_MMIO_QUEUE_PFN 0x040
+#define QVIRTIO_MMIO_QUEUE_NOTIFY 0x050
+#define QVIRTIO_MMIO_INTERRUPT_STATUS 0x060
+#define QVIRTIO_MMIO_INTERRUPT_ACK 0x064
+#define QVIRTIO_MMIO_DEVICE_STATUS 0x070
+#define QVIRTIO_MMIO_DEVICE_SPECIFIC 0x100
+
+typedef struct QVirtioMMIODevice {
+ QVirtioDevice vdev;
+ uint64_t addr;
+ uint32_t page_size;
+ uint32_t features; /* As it cannot be read later, save it */
+} QVirtioMMIODevice;
+
+extern const QVirtioBus qvirtio_mmio;
+
+QVirtioMMIODevice *qvirtio_mmio_init_device(uint64_t addr, uint32_t page_size);
+
+#endif
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 449e8a4..bbfd5c8 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -16,9 +16,11 @@
#include "libqtest.h"
#include "libqos/virtio.h"
#include "libqos/virtio-pci.h"
+#include "libqos/virtio-mmio.h"
#include "libqos/pci-pc.h"
#include "libqos/malloc.h"
#include "libqos/malloc-pc.h"
+#include "libqos/malloc-generic.h"
#include "qemu/bswap.h"
#define QVIRTIO_BLK_F_BARRIER 0x00000001
@@ -42,10 +44,14 @@
#define TEST_IMAGE_SIZE (64 * 1024 * 1024)
#define QVIRTIO_BLK_TIMEOUT_US (30 * 1000 * 1000)
+#define PCI_SLOT_HP 0x06
#define PCI_SLOT 0x04
#define PCI_FN 0x00
-#define PCI_SLOT_HP 0x06
+#define MMIO_PAGE_SIZE 4096
+#define MMIO_DEV_BASE_ADDR 0x0A003E00
+#define MMIO_RAM_ADDR 0x40000000
+#define MMIO_RAM_SIZE 0x20000000
typedef struct QVirtioBlkReq {
uint32_t type;
@@ -90,6 +96,19 @@ static QPCIBus *pci_test_start(void)
return qpci_init_pc();
}
+static void arm_test_start(void)
+{
+ char cmdline[200];
+ char *tmp_path;
+
+ tmp_path = drive_create();
+
+ snprintf(cmdline, 200, "-machine virt -drive if=none,id=drive0,file=%s "
+ "-device virtio-blk-device,drive=drive0", tmp_path);
+ qtest_start(cmdline);
+ unlink(tmp_path);
+}
+
static void test_end(void)
{
qtest_end();
@@ -686,21 +705,131 @@ static void pci_hotplug(void)
/* unplug secondary disk */
qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP);
+ qpci_free_pc(bus);
+ test_end();
+}
+
+static void mmio_basic(void)
+{
+ QVirtioMMIODevice *dev;
+ QVirtQueue *vq;
+ QGuestAllocator *alloc;
+ QVirtioBlkReq req;
+ int n_size = TEST_IMAGE_SIZE / 2;
+ uint64_t req_addr;
+ uint64_t capacity;
+ uint32_t features;
+ uint32_t free_head;
+ uint8_t status;
+ char *data;
+
+ arm_test_start();
+
+ 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);
+
+ qvirtio_reset(&qvirtio_mmio, &dev->vdev);
+ qvirtio_set_acknowledge(&qvirtio_mmio, &dev->vdev);
+ qvirtio_set_driver(&qvirtio_mmio, &dev->vdev);
+
+ capacity = qvirtio_config_readq(&qvirtio_mmio, &dev->vdev,
+ QVIRTIO_MMIO_DEVICE_SPECIFIC);
+ g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
+
+ features = qvirtio_get_features(&qvirtio_mmio, &dev->vdev);
+ features = features & ~(QVIRTIO_F_RING_INDIRECT_DESC |
+ QVIRTIO_F_RING_EVENT_IDX | QVIRTIO_BLK_F_SCSI);
+ qvirtio_set_features(&qvirtio_mmio, &dev->vdev, features);
+
+ alloc = generic_alloc_init(MMIO_RAM_ADDR, MMIO_RAM_SIZE, MMIO_PAGE_SIZE);
+ vq = qvirtqueue_setup(&qvirtio_mmio, &dev->vdev, alloc, 0);
+
+ qvirtio_set_driver_ok(&qvirtio_mmio, &dev->vdev);
+
+ qmp("{ 'execute': 'block_resize', 'arguments': { 'device': 'drive0', "
+ " 'size': %d } }", n_size);
+
+ qvirtio_wait_queue_isr(&qvirtio_mmio, &dev->vdev, vq,
+ QVIRTIO_BLK_TIMEOUT_US);
+
+ capacity = qvirtio_config_readq(&qvirtio_mmio, &dev->vdev,
+ QVIRTIO_MMIO_DEVICE_SPECIFIC);
+ g_assert_cmpint(capacity, ==, n_size / 512);
+
+ /* Write request */
+ req.type = QVIRTIO_BLK_T_OUT;
+ req.ioprio = 1;
+ req.sector = 0;
+ req.data = g_malloc0(512);
+ strcpy(req.data, "TEST");
+
+ req_addr = virtio_blk_request(alloc, &req, 512);
+
+ g_free(req.data);
+
+ free_head = qvirtqueue_add(vq, req_addr, 528, false, true);
+ qvirtqueue_add(vq, req_addr + 528, 1, true, false);
+ qvirtqueue_kick(&qvirtio_mmio, &dev->vdev, vq, free_head);
+
+ qvirtio_wait_queue_isr(&qvirtio_mmio, &dev->vdev, vq,
+ QVIRTIO_BLK_TIMEOUT_US);
+ status = readb(req_addr + 528);
+ g_assert_cmpint(status, ==, 0);
+
+ guest_free(alloc, req_addr);
+
+ /* Read request */
+ req.type = QVIRTIO_BLK_T_IN;
+ req.ioprio = 1;
+ req.sector = 0;
+ req.data = g_malloc0(512);
+
+ req_addr = virtio_blk_request(alloc, &req, 512);
+
+ g_free(req.data);
+
+ free_head = qvirtqueue_add(vq, req_addr, 16, false, true);
+ qvirtqueue_add(vq, req_addr + 16, 513, true, false);
+
+ qvirtqueue_kick(&qvirtio_mmio, &dev->vdev, vq, free_head);
+
+ qvirtio_wait_queue_isr(&qvirtio_mmio, &dev->vdev, vq,
+ QVIRTIO_BLK_TIMEOUT_US);
+ status = readb(req_addr + 528);
+ g_assert_cmpint(status, ==, 0);
+
+ data = g_malloc0(512);
+ memread(req_addr + 16, data, 512);
+ g_assert_cmpstr(data, ==, "TEST");
+ g_free(data);
+
+ guest_free(alloc, req_addr);
+
+ /* End test */
+ guest_free(alloc, vq->desc);
+ generic_alloc_uninit(alloc);
+ g_free(dev);
test_end();
}
int main(int argc, char **argv)
{
int ret;
+ const char *arch = qtest_get_arch();
g_test_init(&argc, &argv, NULL);
- g_test_add_func("/virtio/blk/pci/basic", pci_basic);
- g_test_add_func("/virtio/blk/pci/indirect", pci_indirect);
- g_test_add_func("/virtio/blk/pci/config", pci_config);
- g_test_add_func("/virtio/blk/pci/msix", pci_msix);
- g_test_add_func("/virtio/blk/pci/idx", pci_idx);
- g_test_add_func("/virtio/blk/pci/hotplug", pci_hotplug);
+ if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+ qtest_add_func("/virtio/blk/pci/basic", pci_basic);
+ qtest_add_func("/virtio/blk/pci/indirect", pci_indirect);
+ qtest_add_func("/virtio/blk/pci/config", pci_config);
+ qtest_add_func("/virtio/blk/pci/msix", pci_msix);
+ qtest_add_func("/virtio/blk/pci/idx", pci_idx);
+ qtest_add_func("/virtio/blk/pci/hotplug", pci_hotplug);
+ } else if (strcmp(arch, "arm") == 0) {
+ qtest_add_func("/virtio/blk/mmio/basic", mmio_basic);
+ }
ret = g_test_run();
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v2 5/5] libqos: Add virtio MMIO support
2014-11-01 17:02 ` [Qemu-devel] [PATCH v2 5/5] libqos: Add virtio MMIO support Marc Marí
@ 2014-11-17 15:48 ` Stefan Hajnoczi
2014-11-23 11:41 ` Marc Marí
0 siblings, 1 reply; 17+ messages in thread
From: Stefan Hajnoczi @ 2014-11-17 15:48 UTC (permalink / raw)
To: Marc Marí; +Cc: qemu-devel, Stefan Hajnoczi, Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 6828 bytes --]
On Sat, Nov 01, 2014 at 06:02:30PM +0100, Marc Marí wrote:
> Add virtio MMIO support.
> Add virtio-blk-test MMIO test case.
>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
> tests/Makefile | 4 +-
> tests/libqos/virtio-mmio.c | 190 ++++++++++++++++++++++++++++++++++++++++++++
> tests/libqos/virtio-mmio.h | 46 +++++++++++
> tests/virtio-blk-test.c | 143 +++++++++++++++++++++++++++++++--
> 4 files changed, 375 insertions(+), 8 deletions(-)
> create mode 100644 tests/libqos/virtio-mmio.c
> create mode 100644 tests/libqos/virtio-mmio.h
>
> diff --git a/tests/Makefile b/tests/Makefile
> index 4ae0ca4..f193b03 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -183,6 +183,8 @@ gcov-files-sparc-y += hw/timer/m48t59.c
> gcov-files-sparc64-y += hw/timer/m48t59.c
> check-qtest-arm-y = tests/tmp105-test$(EXESUF)
> gcov-files-arm-y += hw/misc/tmp105.c
> +check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
> +gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
> check-qtest-ppc-y += tests/boot-order-test$(EXESUF)
> check-qtest-ppc64-y += tests/boot-order-test$(EXESUF)
> check-qtest-ppc64-y += tests/spapr-phb-test$(EXESUF)
> @@ -300,8 +302,8 @@ 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.o tests/libqos/virtio-pci.o
> libqos-usb-obj-y = $(libqos-pc-obj-y) tests/libqos/usb.o
> +libqos-virtio-obj-y = $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o
>
> tests/rtc-test$(EXESUF): tests/rtc-test.o
> tests/m48t59-test$(EXESUF): tests/m48t59-test.o
> diff --git a/tests/libqos/virtio-mmio.c b/tests/libqos/virtio-mmio.c
> new file mode 100644
> index 0000000..6896f7b
> --- /dev/null
> +++ b/tests/libqos/virtio-mmio.c
> @@ -0,0 +1,190 @@
> +/*
> + * libqos virtio MMIO 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-mmio.h"
> +#include "libqos/malloc.h"
> +#include "libqos/malloc-generic.h"
> +#include <stdio.h>
Please include system headers before user headers.
> +QVirtioMMIODevice *qvirtio_mmio_init_device(uint64_t addr, uint32_t page_size)
> +{
> + QVirtioMMIODevice *dev;
> + uint32_t magic;
> + dev = g_malloc0(sizeof(*dev));
> +
> + magic = readl(addr + QVIRTIO_MMIO_MAGIC_VALUE);
> + g_assert(magic == ('v' | 'i' << 8 | 'r' << 16 | 't' << 24));
The magic value is little-endian according to the virtio 1.0
specification and the Linux kernel's virtio_mmio implementation.
QEMU's virtio-mmio is native endian (i.e. host endian) and I think this
is a bug.
qtest's readl() is host-endian so it matches what QEMU does. I suppose
you can leave this for now but it may need to be changed in the future.
> +static void mmio_basic(void)
> +{
> + QVirtioMMIODevice *dev;
> + QVirtQueue *vq;
> + QGuestAllocator *alloc;
> + QVirtioBlkReq req;
> + int n_size = TEST_IMAGE_SIZE / 2;
> + uint64_t req_addr;
> + uint64_t capacity;
> + uint32_t features;
> + uint32_t free_head;
> + uint8_t status;
> + char *data;
> +
> + arm_test_start();
> +
> + 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);
> +
> + qvirtio_reset(&qvirtio_mmio, &dev->vdev);
> + qvirtio_set_acknowledge(&qvirtio_mmio, &dev->vdev);
> + qvirtio_set_driver(&qvirtio_mmio, &dev->vdev);
> +
> + capacity = qvirtio_config_readq(&qvirtio_mmio, &dev->vdev,
> + QVIRTIO_MMIO_DEVICE_SPECIFIC);
> + g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
> +
> + features = qvirtio_get_features(&qvirtio_mmio, &dev->vdev);
> + features = features & ~(QVIRTIO_F_RING_INDIRECT_DESC |
> + QVIRTIO_F_RING_EVENT_IDX | QVIRTIO_BLK_F_SCSI);
> + qvirtio_set_features(&qvirtio_mmio, &dev->vdev, features);
> +
> + alloc = generic_alloc_init(MMIO_RAM_ADDR, MMIO_RAM_SIZE, MMIO_PAGE_SIZE);
> + vq = qvirtqueue_setup(&qvirtio_mmio, &dev->vdev, alloc, 0);
> +
> + qvirtio_set_driver_ok(&qvirtio_mmio, &dev->vdev);
> +
> + qmp("{ 'execute': 'block_resize', 'arguments': { 'device': 'drive0', "
> + " 'size': %d } }", n_size);
> +
> + qvirtio_wait_queue_isr(&qvirtio_mmio, &dev->vdev, vq,
> + QVIRTIO_BLK_TIMEOUT_US);
> +
> + capacity = qvirtio_config_readq(&qvirtio_mmio, &dev->vdev,
> + QVIRTIO_MMIO_DEVICE_SPECIFIC);
> + g_assert_cmpint(capacity, ==, n_size / 512);
> +
> + /* Write request */
> + req.type = QVIRTIO_BLK_T_OUT;
> + req.ioprio = 1;
> + req.sector = 0;
> + req.data = g_malloc0(512);
> + strcpy(req.data, "TEST");
> +
> + req_addr = virtio_blk_request(alloc, &req, 512);
> +
> + g_free(req.data);
> +
> + free_head = qvirtqueue_add(vq, req_addr, 528, false, true);
> + qvirtqueue_add(vq, req_addr + 528, 1, true, false);
> + qvirtqueue_kick(&qvirtio_mmio, &dev->vdev, vq, free_head);
> +
> + qvirtio_wait_queue_isr(&qvirtio_mmio, &dev->vdev, vq,
> + QVIRTIO_BLK_TIMEOUT_US);
> + status = readb(req_addr + 528);
> + g_assert_cmpint(status, ==, 0);
> +
> + guest_free(alloc, req_addr);
> +
> + /* Read request */
> + req.type = QVIRTIO_BLK_T_IN;
> + req.ioprio = 1;
> + req.sector = 0;
> + req.data = g_malloc0(512);
> +
> + req_addr = virtio_blk_request(alloc, &req, 512);
> +
> + g_free(req.data);
> +
> + free_head = qvirtqueue_add(vq, req_addr, 16, false, true);
> + qvirtqueue_add(vq, req_addr + 16, 513, true, false);
> +
> + qvirtqueue_kick(&qvirtio_mmio, &dev->vdev, vq, free_head);
> +
> + qvirtio_wait_queue_isr(&qvirtio_mmio, &dev->vdev, vq,
> + QVIRTIO_BLK_TIMEOUT_US);
> + status = readb(req_addr + 528);
> + g_assert_cmpint(status, ==, 0);
> +
> + data = g_malloc0(512);
> + memread(req_addr + 16, data, 512);
> + g_assert_cmpstr(data, ==, "TEST");
> + g_free(data);
There is a lot of code duplication here. Can the test logic but shared
between PCI and MMIO?
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v2 5/5] libqos: Add virtio MMIO support
2014-11-17 15:48 ` Stefan Hajnoczi
@ 2014-11-23 11:41 ` Marc Marí
2014-11-24 11:59 ` Stefan Hajnoczi
0 siblings, 1 reply; 17+ messages in thread
From: Marc Marí @ 2014-11-23 11:41 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Stefan Hajnoczi, Andreas Färber
El Mon, 17 Nov 2014 15:48:09 +0000
Stefan Hajnoczi <stefanha@gmail.com> escribió:
> On Sat, Nov 01, 2014 at 06:02:30PM +0100, Marc Marí wrote:
>
> > +static void mmio_basic(void)
> > +{
> > + QVirtioMMIODevice *dev;
> > + QVirtQueue *vq;
> > + QGuestAllocator *alloc;
> > + QVirtioBlkReq req;
> > + int n_size = TEST_IMAGE_SIZE / 2;
> > + uint64_t req_addr;
> > + uint64_t capacity;
> > + uint32_t features;
> > + uint32_t free_head;
> > + uint8_t status;
> > + char *data;
> > +
> > + arm_test_start();
> > +
> > + 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); +
> > + qvirtio_reset(&qvirtio_mmio, &dev->vdev);
> > + qvirtio_set_acknowledge(&qvirtio_mmio, &dev->vdev);
> > + qvirtio_set_driver(&qvirtio_mmio, &dev->vdev);
> > +
> > + capacity = qvirtio_config_readq(&qvirtio_mmio, &dev->vdev,
> > +
> > QVIRTIO_MMIO_DEVICE_SPECIFIC);
> > + g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
> > +
> > + features = qvirtio_get_features(&qvirtio_mmio, &dev->vdev);
> > + features = features & ~(QVIRTIO_F_RING_INDIRECT_DESC |
> > + QVIRTIO_F_RING_EVENT_IDX |
> > QVIRTIO_BLK_F_SCSI);
> > + qvirtio_set_features(&qvirtio_mmio, &dev->vdev, features);
> > +
> > + alloc = generic_alloc_init(MMIO_RAM_ADDR, MMIO_RAM_SIZE,
> > MMIO_PAGE_SIZE);
> > + vq = qvirtqueue_setup(&qvirtio_mmio, &dev->vdev, alloc, 0);
> > +
> > + qvirtio_set_driver_ok(&qvirtio_mmio, &dev->vdev);
> > +
> > + qmp("{ 'execute': 'block_resize', 'arguments': { 'device':
> > 'drive0', "
> > + " 'size':
> > %d } }", n_size); +
> > + qvirtio_wait_queue_isr(&qvirtio_mmio, &dev->vdev, vq,
> > + QVIRTIO_BLK_TIMEOUT_US);
> > +
> > + capacity = qvirtio_config_readq(&qvirtio_mmio, &dev->vdev,
> > +
> > QVIRTIO_MMIO_DEVICE_SPECIFIC);
> > + g_assert_cmpint(capacity, ==, n_size / 512);
> > +
> > + /* Write request */
> > + req.type = QVIRTIO_BLK_T_OUT;
> > + req.ioprio = 1;
> > + req.sector = 0;
> > + req.data = g_malloc0(512);
> > + strcpy(req.data, "TEST");
> > +
> > + req_addr = virtio_blk_request(alloc, &req, 512);
> > +
> > + g_free(req.data);
> > +
> > + free_head = qvirtqueue_add(vq, req_addr, 528, false, true);
> > + qvirtqueue_add(vq, req_addr + 528, 1, true, false);
> > + qvirtqueue_kick(&qvirtio_mmio, &dev->vdev, vq, free_head);
> > +
> > + qvirtio_wait_queue_isr(&qvirtio_mmio, &dev->vdev, vq,
> > + QVIRTIO_BLK_TIMEOUT_US);
> > + status = readb(req_addr + 528);
> > + g_assert_cmpint(status, ==, 0);
> > +
> > + guest_free(alloc, req_addr);
> > +
> > + /* Read request */
> > + req.type = QVIRTIO_BLK_T_IN;
> > + req.ioprio = 1;
> > + req.sector = 0;
> > + req.data = g_malloc0(512);
> > +
> > + req_addr = virtio_blk_request(alloc, &req, 512);
> > +
> > + g_free(req.data);
> > +
> > + free_head = qvirtqueue_add(vq, req_addr, 16, false, true);
> > + qvirtqueue_add(vq, req_addr + 16, 513, true, false);
> > +
> > + qvirtqueue_kick(&qvirtio_mmio, &dev->vdev, vq, free_head);
> > +
> > + qvirtio_wait_queue_isr(&qvirtio_mmio, &dev->vdev, vq,
> > + QVIRTIO_BLK_TIMEOUT_US);
> > + status = readb(req_addr + 528);
> > + g_assert_cmpint(status, ==, 0);
> > +
> > + data = g_malloc0(512);
> > + memread(req_addr + 16, data, 512);
> > + g_assert_cmpstr(data, ==, "TEST");
> > + g_free(data);
>
> There is a lot of code duplication here. Can the test logic but
> shared between PCI and MMIO?
The code duplication that can be easily extracted and shared is
performing a simple write - read operation on the block device (which
is used in various test cases). Other places (for example, checking the
image size) use arch specific offsets. This could be abstracted, but I
think is a bit too complicated for a test case.
By now, I will extract just write - read operation. If you think that
adding abstraction for arch specific offsets is worth the effort, then
I'll add them.
Marc
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v2 5/5] libqos: Add virtio MMIO support
2014-11-23 11:41 ` Marc Marí
@ 2014-11-24 11:59 ` Stefan Hajnoczi
2014-11-24 12:30 ` Marc Marí
0 siblings, 1 reply; 17+ messages in thread
From: Stefan Hajnoczi @ 2014-11-24 11:59 UTC (permalink / raw)
To: Marc Marí; +Cc: Stefan Hajnoczi, qemu-devel, Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 4944 bytes --]
On Sun, Nov 23, 2014 at 12:41:25PM +0100, Marc Marí wrote:
> El Mon, 17 Nov 2014 15:48:09 +0000
> Stefan Hajnoczi <stefanha@gmail.com> escribió:
> > On Sat, Nov 01, 2014 at 06:02:30PM +0100, Marc Marí wrote:
> >
> > > +static void mmio_basic(void)
> > > +{
> > > + QVirtioMMIODevice *dev;
> > > + QVirtQueue *vq;
> > > + QGuestAllocator *alloc;
> > > + QVirtioBlkReq req;
> > > + int n_size = TEST_IMAGE_SIZE / 2;
> > > + uint64_t req_addr;
> > > + uint64_t capacity;
> > > + uint32_t features;
> > > + uint32_t free_head;
> > > + uint8_t status;
> > > + char *data;
> > > +
> > > + arm_test_start();
> > > +
> > > + 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); +
> > > + qvirtio_reset(&qvirtio_mmio, &dev->vdev);
> > > + qvirtio_set_acknowledge(&qvirtio_mmio, &dev->vdev);
> > > + qvirtio_set_driver(&qvirtio_mmio, &dev->vdev);
> > > +
> > > + capacity = qvirtio_config_readq(&qvirtio_mmio, &dev->vdev,
> > > +
> > > QVIRTIO_MMIO_DEVICE_SPECIFIC);
> > > + g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
> > > +
> > > + features = qvirtio_get_features(&qvirtio_mmio, &dev->vdev);
> > > + features = features & ~(QVIRTIO_F_RING_INDIRECT_DESC |
> > > + QVIRTIO_F_RING_EVENT_IDX |
> > > QVIRTIO_BLK_F_SCSI);
> > > + qvirtio_set_features(&qvirtio_mmio, &dev->vdev, features);
> > > +
> > > + alloc = generic_alloc_init(MMIO_RAM_ADDR, MMIO_RAM_SIZE,
> > > MMIO_PAGE_SIZE);
> > > + vq = qvirtqueue_setup(&qvirtio_mmio, &dev->vdev, alloc, 0);
> > > +
> > > + qvirtio_set_driver_ok(&qvirtio_mmio, &dev->vdev);
> > > +
> > > + qmp("{ 'execute': 'block_resize', 'arguments': { 'device':
> > > 'drive0', "
> > > + " 'size':
> > > %d } }", n_size); +
> > > + qvirtio_wait_queue_isr(&qvirtio_mmio, &dev->vdev, vq,
> > > + QVIRTIO_BLK_TIMEOUT_US);
> > > +
> > > + capacity = qvirtio_config_readq(&qvirtio_mmio, &dev->vdev,
> > > +
> > > QVIRTIO_MMIO_DEVICE_SPECIFIC);
> > > + g_assert_cmpint(capacity, ==, n_size / 512);
> > > +
> > > + /* Write request */
> > > + req.type = QVIRTIO_BLK_T_OUT;
> > > + req.ioprio = 1;
> > > + req.sector = 0;
> > > + req.data = g_malloc0(512);
> > > + strcpy(req.data, "TEST");
> > > +
> > > + req_addr = virtio_blk_request(alloc, &req, 512);
> > > +
> > > + g_free(req.data);
> > > +
> > > + free_head = qvirtqueue_add(vq, req_addr, 528, false, true);
> > > + qvirtqueue_add(vq, req_addr + 528, 1, true, false);
> > > + qvirtqueue_kick(&qvirtio_mmio, &dev->vdev, vq, free_head);
> > > +
> > > + qvirtio_wait_queue_isr(&qvirtio_mmio, &dev->vdev, vq,
> > > + QVIRTIO_BLK_TIMEOUT_US);
> > > + status = readb(req_addr + 528);
> > > + g_assert_cmpint(status, ==, 0);
> > > +
> > > + guest_free(alloc, req_addr);
> > > +
> > > + /* Read request */
> > > + req.type = QVIRTIO_BLK_T_IN;
> > > + req.ioprio = 1;
> > > + req.sector = 0;
> > > + req.data = g_malloc0(512);
> > > +
> > > + req_addr = virtio_blk_request(alloc, &req, 512);
> > > +
> > > + g_free(req.data);
> > > +
> > > + free_head = qvirtqueue_add(vq, req_addr, 16, false, true);
> > > + qvirtqueue_add(vq, req_addr + 16, 513, true, false);
> > > +
> > > + qvirtqueue_kick(&qvirtio_mmio, &dev->vdev, vq, free_head);
> > > +
> > > + qvirtio_wait_queue_isr(&qvirtio_mmio, &dev->vdev, vq,
> > > + QVIRTIO_BLK_TIMEOUT_US);
> > > + status = readb(req_addr + 528);
> > > + g_assert_cmpint(status, ==, 0);
> > > +
> > > + data = g_malloc0(512);
> > > + memread(req_addr + 16, data, 512);
> > > + g_assert_cmpstr(data, ==, "TEST");
> > > + g_free(data);
> >
> > There is a lot of code duplication here. Can the test logic but
> > shared between PCI and MMIO?
>
> The code duplication that can be easily extracted and shared is
> performing a simple write - read operation on the block device (which
> is used in various test cases). Other places (for example, checking the
> image size) use arch specific offsets. This could be abstracted, but I
> think is a bit too complicated for a test case.
Virtio config space is not transport-specific.
The Linux virtio_blk driver does:
virtio_cread(vdev, struct virtio_blk_config, capacity, &capacity);
Why does this patch use:
capacity = qvirtio_config_readq(&qvirtio_mmio, &dev->vdev, QVIRTIO_MMIO_DEVICE_SPECIFIC);
?
I don't have the virtio spec open right now, but I guess
qvirtio_config_readq() should always start at virtio configuration byte
0 and not rely on transport-specific offsets.
Stefan
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v2 5/5] libqos: Add virtio MMIO support
2014-11-24 11:59 ` Stefan Hajnoczi
@ 2014-11-24 12:30 ` Marc Marí
0 siblings, 0 replies; 17+ messages in thread
From: Marc Marí @ 2014-11-24 12:30 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Stefan Hajnoczi, qemu-devel, Andreas Färber
El Mon, 24 Nov 2014 11:59:19 +0000
Stefan Hajnoczi <stefanha@redhat.com> escribió:
> On Sun, Nov 23, 2014 at 12:41:25PM +0100, Marc Marí wrote:
> > El Mon, 17 Nov 2014 15:48:09 +0000
> > Stefan Hajnoczi <stefanha@gmail.com> escribió:
> > > On Sat, Nov 01, 2014 at 06:02:30PM +0100, Marc Marí wrote:
> > >
> > > > +static void mmio_basic(void)
> > > > +{
> > > > + QVirtioMMIODevice *dev;
> > > > + QVirtQueue *vq;
> > > > + QGuestAllocator *alloc;
> > > > + QVirtioBlkReq req;
> > > > + int n_size = TEST_IMAGE_SIZE / 2;
> > > > + uint64_t req_addr;
> > > > + uint64_t capacity;
> > > > + uint32_t features;
> > > > + uint32_t free_head;
> > > > + uint8_t status;
> > > > + char *data;
> > > > +
> > > > + arm_test_start();
> > > > +
> > > > + 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); +
> > > > + qvirtio_reset(&qvirtio_mmio, &dev->vdev);
> > > > + qvirtio_set_acknowledge(&qvirtio_mmio, &dev->vdev);
> > > > + qvirtio_set_driver(&qvirtio_mmio, &dev->vdev);
> > > > +
> > > > + capacity = qvirtio_config_readq(&qvirtio_mmio, &dev->vdev,
> > > > +
> > > > QVIRTIO_MMIO_DEVICE_SPECIFIC);
> > > > + g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
> > > > +
> > > > + features = qvirtio_get_features(&qvirtio_mmio, &dev->vdev);
> > > > + features = features & ~(QVIRTIO_F_RING_INDIRECT_DESC |
> > > > + QVIRTIO_F_RING_EVENT_IDX |
> > > > QVIRTIO_BLK_F_SCSI);
> > > > + qvirtio_set_features(&qvirtio_mmio, &dev->vdev, features);
> > > > +
> > > > + alloc = generic_alloc_init(MMIO_RAM_ADDR, MMIO_RAM_SIZE,
> > > > MMIO_PAGE_SIZE);
> > > > + vq = qvirtqueue_setup(&qvirtio_mmio, &dev->vdev, alloc, 0);
> > > > +
> > > > + qvirtio_set_driver_ok(&qvirtio_mmio, &dev->vdev);
> > > > +
> > > > + qmp("{ 'execute': 'block_resize', 'arguments': { 'device':
> > > > 'drive0', "
> > > > + " 'size':
> > > > %d } }", n_size); +
> > > > + qvirtio_wait_queue_isr(&qvirtio_mmio, &dev->vdev, vq,
> > > > + QVIRTIO_BLK_TIMEOUT_US);
> > > > +
> > > > + capacity = qvirtio_config_readq(&qvirtio_mmio, &dev->vdev,
> > > > +
> > > > QVIRTIO_MMIO_DEVICE_SPECIFIC);
> > > > + g_assert_cmpint(capacity, ==, n_size / 512);
> > > > +
> > > > + /* Write request */
> > > > + req.type = QVIRTIO_BLK_T_OUT;
> > > > + req.ioprio = 1;
> > > > + req.sector = 0;
> > > > + req.data = g_malloc0(512);
> > > > + strcpy(req.data, "TEST");
> > > > +
> > > > + req_addr = virtio_blk_request(alloc, &req, 512);
> > > > +
> > > > + g_free(req.data);
> > > > +
> > > > + free_head = qvirtqueue_add(vq, req_addr, 528, false, true);
> > > > + qvirtqueue_add(vq, req_addr + 528, 1, true, false);
> > > > + qvirtqueue_kick(&qvirtio_mmio, &dev->vdev, vq, free_head);
> > > > +
> > > > + qvirtio_wait_queue_isr(&qvirtio_mmio, &dev->vdev, vq,
> > > > + QVIRTIO_BLK_TIMEOUT_US);
> > > > + status = readb(req_addr + 528);
> > > > + g_assert_cmpint(status, ==, 0);
> > > > +
> > > > + guest_free(alloc, req_addr);
> > > > +
> > > > + /* Read request */
> > > > + req.type = QVIRTIO_BLK_T_IN;
> > > > + req.ioprio = 1;
> > > > + req.sector = 0;
> > > > + req.data = g_malloc0(512);
> > > > +
> > > > + req_addr = virtio_blk_request(alloc, &req, 512);
> > > > +
> > > > + g_free(req.data);
> > > > +
> > > > + free_head = qvirtqueue_add(vq, req_addr, 16, false, true);
> > > > + qvirtqueue_add(vq, req_addr + 16, 513, true, false);
> > > > +
> > > > + qvirtqueue_kick(&qvirtio_mmio, &dev->vdev, vq, free_head);
> > > > +
> > > > + qvirtio_wait_queue_isr(&qvirtio_mmio, &dev->vdev, vq,
> > > > + QVIRTIO_BLK_TIMEOUT_US);
> > > > + status = readb(req_addr + 528);
> > > > + g_assert_cmpint(status, ==, 0);
> > > > +
> > > > + data = g_malloc0(512);
> > > > + memread(req_addr + 16, data, 512);
> > > > + g_assert_cmpstr(data, ==, "TEST");
> > > > + g_free(data);
> > >
> > > There is a lot of code duplication here. Can the test logic but
> > > shared between PCI and MMIO?
> >
> > The code duplication that can be easily extracted and shared is
> > performing a simple write - read operation on the block device
> > (which is used in various test cases). Other places (for example,
> > checking the image size) use arch specific offsets. This could be
> > abstracted, but I think is a bit too complicated for a test case.
>
> Virtio config space is not transport-specific.
>
> The Linux virtio_blk driver does:
> virtio_cread(vdev, struct virtio_blk_config, capacity, &capacity);
>
> Why does this patch use:
> capacity = qvirtio_config_readq(&qvirtio_mmio, &dev->vdev,
> QVIRTIO_MMIO_DEVICE_SPECIFIC); ?
>
> I don't have the virtio spec open right now, but I guess
> qvirtio_config_readq() should always start at virtio configuration
> byte 0 and not rely on transport-specific offsets.
>
> Stefan
As usual, I arrived to wrong conclusions because I didn't look at it
carefully enough. Sorry.
What is variable is the position of the device-specific configuration
(in PCI depends of MSIX enabled or disabled and it can be 0x14 or 0x18,
and in MMIO is 0x100). But the capacity is always in the position 0 of
the device-specific configuration. With the base position, as well as an
allocator, a device and a bus, the basic tests can be performed (and
expanded later).
Thanks
Marc
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/5] libqos: Virtio MMIO driver
2014-11-01 17:02 [Qemu-devel] [PATCH v2 0/5] libqos: Virtio MMIO driver Marc Marí
` (4 preceding siblings ...)
2014-11-01 17:02 ` [Qemu-devel] [PATCH v2 5/5] libqos: Add virtio MMIO support Marc Marí
@ 2014-11-17 9:26 ` Marc Marí
5 siblings, 0 replies; 17+ messages in thread
From: Marc Marí @ 2014-11-17 9:26 UTC (permalink / raw)
To: Marc Marí; +Cc: qemu-devel, Stefan Hajnoczi, Andreas Färber
El Sat, 1 Nov 2014 18:02:25 +0100
Marc Marí <marc.mari.barcelo@gmail.com> escribió:
> Add virtio-mmio support to libqos and test case for virtio-blk.
Ping! It has been two weeks. Is it in somebody's todo list?
Thanks
Marc
^ permalink raw reply [flat|nested] 17+ messages in thread