From patchwork Mon Jan 19 17:04:37 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Hajnoczi X-Patchwork-Id: 430614 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 79FEC1401F0 for ; Tue, 20 Jan 2015 04:06:49 +1100 (AEDT) Received: from localhost ([::1]:38727 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDFmo-0006cu-MP for incoming@patchwork.ozlabs.org; Mon, 19 Jan 2015 12:06:46 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47076) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDFky-0003wK-1A for qemu-devel@nongnu.org; Mon, 19 Jan 2015 12:04:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YDFkv-000567-SF for qemu-devel@nongnu.org; Mon, 19 Jan 2015 12:04:51 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58427) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDFkv-00055k-Ir for qemu-devel@nongnu.org; Mon, 19 Jan 2015 12:04:49 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t0JH4ixS028458 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Mon, 19 Jan 2015 12:04:44 -0500 Received: from localhost (ovpn-112-65.ams2.redhat.com [10.36.112.65]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t0JH4h2q004691; Mon, 19 Jan 2015 12:04:43 -0500 From: Stefan Hajnoczi To: qemu-devel@nongnu.org Date: Mon, 19 Jan 2015 17:04:37 +0000 Message-Id: <1421687077-9025-3-git-send-email-stefanha@redhat.com> In-Reply-To: <1421687077-9025-1-git-send-email-stefanha@redhat.com> References: <1421687077-9025-1-git-send-email-stefanha@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Paolo Bonzini , David Gibson , Fam Zheng , Stefan Hajnoczi Subject: [Qemu-devel] [PATCH 2/2] dataplane: use virtio_ld/st_p() for endian-aware memory access X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org The vring.c code was written under the assumption that virtio devices have the same endianness as the host. This is wrong when emulating targets that differ in endianness from the host. It is also wrong when emulating bi-endian targets like POWER 8 processors, which support both little- and big-endian at run-time. In this case the virtio device knows which endianness to use. This change requires the virtio-access.h APIs and therefore builds vring.o for each target (obj-y instead of common-obj-y). Note that $(CONFIG_VIRTIO) for dataplane/ was dropped in hw/virtio/Makefile.objs because hw/Makefile.objs conditionally includes hw/virtio/ on $(CONFIG_VIRTIO) already. Only a small change is needed to the vring.h interface: vring_push() now takes a VirtIODevice *vdev argument. Cc: Paolo Bonzini Cc: Fam Zheng Signed-off-by: Stefan Hajnoczi Reviewed-by: David Gibson Tested-by: David Gibson --- hw/block/dataplane/virtio-blk.c | 2 +- hw/scsi/virtio-scsi-dataplane.c | 2 +- hw/virtio/Makefile.objs | 2 +- hw/virtio/dataplane/Makefile.objs | 2 +- hw/virtio/dataplane/vring.c | 67 +++++++++++++++++++++++++------------ include/hw/virtio/dataplane/vring.h | 3 +- 6 files changed, 51 insertions(+), 27 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 39c5d71..aca2a8d 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -75,7 +75,7 @@ static void complete_request_vring(VirtIOBlockReq *req, unsigned char status) VirtIOBlockDataPlane *s = req->dev->dataplane; stb_p(&req->in->status, status); - vring_push(&req->dev->dataplane->vring, &req->elem, + vring_push(s->vdev, &req->dev->dataplane->vring, &req->elem, req->qiov.size + sizeof(*req->in)); /* Suppress notification to guest by BH and its scheduled diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c index 03a1e8c..418d73b 100644 --- a/hw/scsi/virtio-scsi-dataplane.c +++ b/hw/scsi/virtio-scsi-dataplane.c @@ -94,7 +94,7 @@ void virtio_scsi_vring_push_notify(VirtIOSCSIReq *req) { VirtIODevice *vdev = VIRTIO_DEVICE(req->vring->parent); - vring_push(&req->vring->vring, &req->elem, + vring_push(vdev, &req->vring->vring, &req->elem, req->qsgl.size + req->resp_iov.size); if (vring_should_notify(vdev, &req->vring->vring)) { diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs index d21c397..3e93a3a 100644 --- a/hw/virtio/Makefile.objs +++ b/hw/virtio/Makefile.objs @@ -2,7 +2,7 @@ common-obj-y += virtio-rng.o common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o common-obj-y += virtio-bus.o common-obj-y += virtio-mmio.o -common-obj-$(CONFIG_VIRTIO) += dataplane/ obj-y += virtio.o virtio-balloon.o obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o +obj-y += dataplane/ diff --git a/hw/virtio/dataplane/Makefile.objs b/hw/virtio/dataplane/Makefile.objs index 9a8cfc0..753a9ca 100644 --- a/hw/virtio/dataplane/Makefile.objs +++ b/hw/virtio/dataplane/Makefile.objs @@ -1 +1 @@ -common-obj-y += vring.o +obj-y += vring.o diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c index cb31d7f..a36dc19 100644 --- a/hw/virtio/dataplane/vring.c +++ b/hw/virtio/dataplane/vring.c @@ -18,13 +18,14 @@ #include "hw/hw.h" #include "exec/memory.h" #include "exec/address-spaces.h" +#include "hw/virtio/virtio-access.h" #include "hw/virtio/dataplane/vring.h" #include "qemu/error-report.h" /* Are there more descriptors available? */ -static inline bool vring_more_avail(Vring *vring) +static inline bool vring_more_avail(VirtIODevice *vdev, Vring *vring) { - return vring->vr.avail->idx != vring->last_avail_idx; + return virtio_lduw_p(vdev, &vring->vr.avail->idx) != vring->last_avail_idx; } /* vring_map can be coupled with vring_unmap or (if you still have the @@ -89,7 +90,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n) vring_init(&vring->vr, virtio_queue_get_num(vdev, n), vring_ptr, 4096); vring->last_avail_idx = virtio_queue_get_last_avail_idx(vdev, n); - vring->last_used_idx = vring->vr.used->idx; + vring->last_used_idx = virtio_lduw_p(vdev, &vring->vr.used->idx); vring->signalled_used = 0; vring->signalled_used_valid = false; @@ -110,7 +111,9 @@ void vring_teardown(Vring *vring, VirtIODevice *vdev, int n) void vring_disable_notification(VirtIODevice *vdev, Vring *vring) { if (!(vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX))) { - vring->vr.used->flags |= VRING_USED_F_NO_NOTIFY; + uint16_t flags = virtio_lduw_p(vdev, &vring->vr.used->flags); + virtio_stw_p(vdev, &vring->vr.used->flags, + flags | VRING_USED_F_NO_NOTIFY); } } @@ -121,18 +124,21 @@ void vring_disable_notification(VirtIODevice *vdev, Vring *vring) bool vring_enable_notification(VirtIODevice *vdev, Vring *vring) { if (vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) { - vring_avail_event(&vring->vr) = vring->vr.avail->idx; + virtio_stw_p(vdev, &vring_avail_event(&vring->vr), + virtio_lduw_p(vdev, &vring->vr.avail->idx)); } else { - vring->vr.used->flags &= ~VRING_USED_F_NO_NOTIFY; + uint16_t flags = virtio_lduw_p(vdev, &vring->vr.used->flags); + virtio_stw_p(vdev, &vring->vr.used->flags, + flags & ~VRING_USED_F_NO_NOTIFY); } smp_mb(); /* ensure update is seen before reading avail_idx */ - return !vring_more_avail(vring); + return !vring_more_avail(vdev, vring); } /* This is stolen from linux/drivers/vhost/vhost.c:vhost_notify() */ bool vring_should_notify(VirtIODevice *vdev, Vring *vring) { - uint16_t old, new; + uint16_t used_event_idx, old, new; bool v; /* Flush out used index updates. This is paired * with the barrier that the Guest executes when enabling @@ -140,12 +146,14 @@ bool vring_should_notify(VirtIODevice *vdev, Vring *vring) smp_mb(); if ((vdev->guest_features & VIRTIO_F_NOTIFY_ON_EMPTY) && - unlikely(vring->vr.avail->idx == vring->last_avail_idx)) { + unlikely(virtio_lduw_p(vdev, &vring->vr.avail->idx) == + vring->last_avail_idx)) { return true; } if (!(vdev->guest_features & VIRTIO_RING_F_EVENT_IDX)) { - return !(vring->vr.avail->flags & VRING_AVAIL_F_NO_INTERRUPT); + uint16_t flags = virtio_lduw_p(vdev, &vring->vr.avail->flags); + return !(flags & VRING_AVAIL_F_NO_INTERRUPT); } old = vring->signalled_used; v = vring->signalled_used_valid; @@ -156,7 +164,8 @@ bool vring_should_notify(VirtIODevice *vdev, Vring *vring) return true; } - return vring_need_event(vring_used_event(&vring->vr), new, old); + used_event_idx = virtio_lduw_p(vdev, &vring_used_event(&vring->vr)); + return vring_need_event(used_event_idx, new, old); } @@ -208,8 +217,19 @@ static int get_desc(Vring *vring, VirtQueueElement *elem, return 0; } +static void copy_in_vring_desc(VirtIODevice *vdev, + const struct vring_desc *guest, + struct vring_desc *host) +{ + host->addr = virtio_ldq_p(vdev, &guest->addr); + host->len = virtio_ldl_p(vdev, &guest->len); + host->flags = virtio_lduw_p(vdev, &guest->flags); + host->next = virtio_lduw_p(vdev, &guest->next); +} + /* This is stolen from linux/drivers/vhost/vhost.c. */ -static int get_indirect(Vring *vring, VirtQueueElement *elem, +static int get_indirect(VirtIODevice *vdev, Vring *vring, + VirtQueueElement *elem, struct vring_desc *indirect) { struct vring_desc desc; @@ -250,7 +270,7 @@ static int get_indirect(Vring *vring, VirtQueueElement *elem, vring->broken = true; return -EFAULT; } - desc = *desc_ptr; + copy_in_vring_desc(vdev, desc_ptr, &desc); memory_region_unref(mr); /* Ensure descriptor has been loaded before accessing fields */ @@ -326,7 +346,7 @@ int vring_pop(VirtIODevice *vdev, Vring *vring, /* Check it isn't doing very strange things with descriptor numbers. */ last_avail_idx = vring->last_avail_idx; - avail_idx = vring->vr.avail->idx; + avail_idx = virtio_lduw_p(vdev, &vring->vr.avail->idx); barrier(); /* load indices now and not again later */ if (unlikely((uint16_t)(avail_idx - last_avail_idx) > num)) { @@ -347,7 +367,7 @@ int vring_pop(VirtIODevice *vdev, Vring *vring, /* Grab the next descriptor number they're advertising, and increment * the index we've seen. */ - head = vring->vr.avail->ring[last_avail_idx % num]; + head = virtio_lduw_p(vdev, &vring->vr.avail->ring[last_avail_idx % num]); elem->index = head; @@ -371,13 +391,13 @@ int vring_pop(VirtIODevice *vdev, Vring *vring, ret = -EFAULT; goto out; } - desc = vring->vr.desc[i]; + copy_in_vring_desc(vdev, &vring->vr.desc[i], &desc); /* Ensure descriptor is loaded before accessing fields */ barrier(); if (desc.flags & VRING_DESC_F_INDIRECT) { - ret = get_indirect(vring, elem, &desc); + ret = get_indirect(vdev, vring, elem, &desc); if (ret < 0) { goto out; } @@ -395,7 +415,8 @@ int vring_pop(VirtIODevice *vdev, Vring *vring, /* On success, increment avail index. */ vring->last_avail_idx++; if (vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) { - vring_avail_event(&vring->vr) = vring->last_avail_idx; + virtio_stw_p(vdev, &vring_avail_event(&vring->vr), + vring->last_avail_idx); } return head; @@ -413,7 +434,8 @@ out: * * Stolen from linux/drivers/vhost/vhost.c. */ -void vring_push(Vring *vring, VirtQueueElement *elem, int len) +void vring_push(VirtIODevice *vdev, Vring *vring, + VirtQueueElement *elem, int len) { struct vring_used_elem *used; unsigned int head = elem->index; @@ -429,13 +451,14 @@ void vring_push(Vring *vring, VirtQueueElement *elem, int len) /* The virtqueue contains a ring of used buffers. Get a pointer to the * next entry in that used ring. */ used = &vring->vr.used->ring[vring->last_used_idx % vring->vr.num]; - used->id = head; - used->len = len; + virtio_stl_p(vdev, &used->id, head); + virtio_stl_p(vdev, &used->len, len); /* Make sure buffer is written before we update index. */ smp_wmb(); - new = vring->vr.used->idx = ++vring->last_used_idx; + new = ++vring->last_used_idx; + virtio_stw_p(vdev, &vring->vr.used->idx, new); if (unlikely((int16_t)(new - vring->signalled_used) < (uint16_t)1)) { vring->signalled_used_valid = false; } diff --git a/include/hw/virtio/dataplane/vring.h b/include/hw/virtio/dataplane/vring.h index 1e871e6..38e581b 100644 --- a/include/hw/virtio/dataplane/vring.h +++ b/include/hw/virtio/dataplane/vring.h @@ -48,6 +48,7 @@ void vring_disable_notification(VirtIODevice *vdev, Vring *vring); bool vring_enable_notification(VirtIODevice *vdev, Vring *vring); bool vring_should_notify(VirtIODevice *vdev, Vring *vring); int vring_pop(VirtIODevice *vdev, Vring *vring, VirtQueueElement *elem); -void vring_push(Vring *vring, VirtQueueElement *elem, int len); +void vring_push(VirtIODevice *vdev, Vring *vring, + VirtQueueElement *elem, int len); #endif /* VRING_H */