From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33172) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xssn5-0001J2-KH for qemu-devel@nongnu.org; Mon, 24 Nov 2014 07:31:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xssms-0003pG-Gw for qemu-devel@nongnu.org; Mon, 24 Nov 2014 07:30:51 -0500 Received: from mail-wg0-x22e.google.com ([2a00:1450:400c:c00::22e]:52297) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xssms-0003p1-5z for qemu-devel@nongnu.org; Mon, 24 Nov 2014 07:30:38 -0500 Received: by mail-wg0-f46.google.com with SMTP id x12so12173831wgg.33 for ; Mon, 24 Nov 2014 04:30:37 -0800 (PST) Date: Mon, 24 Nov 2014 13:30:32 +0100 From: Marc =?UTF-8?B?TWFyw60=?= Message-ID: <20141124133032.61327b37@crunchbang> In-Reply-To: <20141124115919.GE4596@stefanha-thinkpad.redhat.com> References: <1414861350-13099-1-git-send-email-marc.mari.barcelo@gmail.com> <1414861350-13099-6-git-send-email-marc.mari.barcelo@gmail.com> <20141117154809.GI16192@stefanha-thinkpad.redhat.com> <20141123124125.7cbe61ea@crunchbang> <20141124115919.GE4596@stefanha-thinkpad.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 5/5] libqos: Add virtio MMIO support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Stefan Hajnoczi , qemu-devel@nongnu.org, Andreas =?UTF-8?B?RsOkcmJlcg==?= El Mon, 24 Nov 2014 11:59:19 +0000 Stefan Hajnoczi escribi=C3=B3: > On Sun, Nov 23, 2014 at 12:41:25PM +0100, Marc Mar=C3=AD wrote: > > El Mon, 17 Nov 2014 15:48:09 +0000 > > Stefan Hajnoczi escribi=C3=B3: > > > On Sat, Nov 01, 2014 at 06:02:30PM +0100, Marc Mar=C3=AD wrote: > > >=20 > > > > +static void mmio_basic(void) > > > > +{ > > > > + QVirtioMMIODevice *dev; > > > > + QVirtQueue *vq; > > > > + QGuestAllocator *alloc; > > > > + QVirtioBlkReq req; > > > > + int n_size =3D 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 =3D qvirtio_mmio_init_device(MMIO_DEV_BASE_ADDR, > > > > MMIO_PAGE_SIZE); > > > > + g_assert(dev !=3D NULL); > > > > + g_assert_cmphex(dev->vdev.device_type, =3D=3D, > > > > 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 =3D qvirtio_config_readq(&qvirtio_mmio, &dev->vdev, > > > > + > > > > QVIRTIO_MMIO_DEVICE_SPECIFIC); > > > > + g_assert_cmpint(capacity, =3D=3D, TEST_IMAGE_SIZE / 512); > > > > + > > > > + features =3D qvirtio_get_features(&qvirtio_mmio, &dev->vdev); > > > > + features =3D features & ~(QVIRTIO_F_RING_INDIRECT_DESC | > > > > + QVIRTIO_F_RING_EVENT_IDX | > > > > QVIRTIO_BLK_F_SCSI); > > > > + qvirtio_set_features(&qvirtio_mmio, &dev->vdev, features); > > > > + > > > > + alloc =3D generic_alloc_init(MMIO_RAM_ADDR, MMIO_RAM_SIZE, > > > > MMIO_PAGE_SIZE); > > > > + vq =3D 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 =3D qvirtio_config_readq(&qvirtio_mmio, &dev->vdev, > > > > + > > > > QVIRTIO_MMIO_DEVICE_SPECIFIC); > > > > + g_assert_cmpint(capacity, =3D=3D, n_size / 512); > > > > + > > > > + /* Write request */ > > > > + req.type =3D QVIRTIO_BLK_T_OUT; > > > > + req.ioprio =3D 1; > > > > + req.sector =3D 0; > > > > + req.data =3D g_malloc0(512); > > > > + strcpy(req.data, "TEST"); > > > > + > > > > + req_addr =3D virtio_blk_request(alloc, &req, 512); > > > > + > > > > + g_free(req.data); > > > > + > > > > + free_head =3D 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 =3D readb(req_addr + 528); > > > > + g_assert_cmpint(status, =3D=3D, 0); > > > > + > > > > + guest_free(alloc, req_addr); > > > > + > > > > + /* Read request */ > > > > + req.type =3D QVIRTIO_BLK_T_IN; > > > > + req.ioprio =3D 1; > > > > + req.sector =3D 0; > > > > + req.data =3D g_malloc0(512); > > > > + > > > > + req_addr =3D virtio_blk_request(alloc, &req, 512); > > > > + > > > > + g_free(req.data); > > > > + > > > > + free_head =3D 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 =3D readb(req_addr + 528); > > > > + g_assert_cmpint(status, =3D=3D, 0); > > > > + > > > > + data =3D g_malloc0(512); > > > > + memread(req_addr + 16, data, 512); > > > > + g_assert_cmpstr(data, =3D=3D, "TEST"); > > > > + g_free(data); > > >=20 > > > There is a lot of code duplication here. Can the test logic but > > > shared between PCI and MMIO? > >=20 > > 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. >=20 > Virtio config space is not transport-specific. >=20 > The Linux virtio_blk driver does: > virtio_cread(vdev, struct virtio_blk_config, capacity, &capacity); >=20 > Why does this patch use: > capacity =3D qvirtio_config_readq(&qvirtio_mmio, &dev->vdev, > QVIRTIO_MMIO_DEVICE_SPECIFIC); ? >=20 > 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. >=20 > 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