[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 5/5] libqos: Add virtio MMIO support
From: |
Marc Marí |
Subject: |
Re: [Qemu-devel] [PATCH v2 5/5] libqos: Add virtio MMIO support |
Date: |
Mon, 24 Nov 2014 13:30:32 +0100 |
El Mon, 24 Nov 2014 11:59:19 +0000
Stefan Hajnoczi <address@hidden> 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 <address@hidden> 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
- Re: [Qemu-devel] [PATCH v2 1/5] libqos: Change use of pointers to uint64_t in virtio, (continued)
Re: [Qemu-devel] [PATCH v2 0/5] libqos: Virtio MMIO driver, Marc Marí, 2014/11/17