Re: [Qemu-devel] [PATCH RFC 07/11] dataplane: allow virtio-1 devices

2014-10-30 Thread Cornelia Huck 
On Tue, 28 Oct 2014 16:22:54 +0100
Greg Kurz gk...@linux.vnet.ibm.com wrote:

 On Tue,  7 Oct 2014 16:40:03 +0200
 Cornelia Huck cornelia.h...@de.ibm.com wrote:
 
  Handle endianness conversion for virtio-1 virtqueues correctly.
  
  Note that dataplane now needs to be built per-target.
  
 
 It also affects hw/virtio/virtio-pci.c:
 
 In file included from include/hw/virtio/dataplane/vring.h:23:0,
  from include/hw/virtio/virtio-scsi.h:21,
  from hw/virtio/virtio-pci.c:24:
 include/hw/virtio/virtio-access.h: In function ‘virtio_access_is_big_endian’:
 include/hw/virtio/virtio-access.h:28:15: error: attempt to use poisoned 
 TARGET_WORDS_BIGENDIAN
  #elif defined(TARGET_WORDS_BIGENDIAN)
^
 
 FWIW when I added endian ambivalent support to virtio, I remember *some 
 people*
 getting angry at the idea of turning common code into per-target... :)

Well, it probably can't be helped for something that is
endian-sensitive like virtio :( (Although we should try to keep it as
local as possible.)

 
 See comment below.
 
  Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
  ---
   hw/block/dataplane/virtio-blk.c |3 +-
   hw/scsi/virtio-scsi-dataplane.c |2 +-
   hw/virtio/Makefile.objs |2 +-
   hw/virtio/dataplane/Makefile.objs   |2 +-
   hw/virtio/dataplane/vring.c |   85 
  +++
   include/hw/virtio/dataplane/vring.h |   64 --
   6 files changed, 113 insertions(+), 45 deletions(-)
  

  diff --git a/include/hw/virtio/dataplane/vring.h 
  b/include/hw/virtio/dataplane/vring.h
  index d3e086a..fde15f3 100644
  --- a/ 
  +++ b/include/hw/virtio/dataplane/vring.h
  @@ -20,6 +20,7 @@
   #include qemu-common.h
   #include hw/virtio/virtio_ring.h
   #include hw/virtio/virtio.h
  +#include hw/virtio/virtio-access.h
  
 
 Since the following commit:
 
 commit 244e2898b7a7735b3da114c120abe206af56a167
 Author: Fam Zheng f...@redhat.com
 Date:   Wed Sep 24 15:21:41 2014 +0800
 
 virtio-scsi: Add VirtIOSCSIVring in VirtIOSCSIReq
 
 The include/hw/virtio/dataplane/vring.h header is indirectly included
 by hw/virtio/virtio-pci.c. Why don't you move all this target dependent
 helpers to another header ?

Ah, this seems to have come in after I hacked on that code - I'll take a
look at splitting off the accessors.

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Qemu-devel] [PATCH RFC 07/11] dataplane: allow virtio-1 devices

2014-10-28 Thread Greg Kurz 
On Tue,  7 Oct 2014 16:40:03 +0200
Cornelia Huck cornelia.h...@de.ibm.com wrote:

 Handle endianness conversion for virtio-1 virtqueues correctly.
 
 Note that dataplane now needs to be built per-target.
 

It also affects hw/virtio/virtio-pci.c:

In file included from include/hw/virtio/dataplane/vring.h:23:0,
 from include/hw/virtio/virtio-scsi.h:21,
 from hw/virtio/virtio-pci.c:24:
include/hw/virtio/virtio-access.h: In function ‘virtio_access_is_big_endian’:
include/hw/virtio/virtio-access.h:28:15: error: attempt to use poisoned 
TARGET_WORDS_BIGENDIAN
 #elif defined(TARGET_WORDS_BIGENDIAN)
   ^

FWIW when I added endian ambivalent support to virtio, I remember *some people*
getting angry at the idea of turning common code into per-target... :)

See comment below.

 Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
 ---
  hw/block/dataplane/virtio-blk.c |3 +-
  hw/scsi/virtio-scsi-dataplane.c |2 +-
  hw/virtio/Makefile.objs |2 +-
  hw/virtio/dataplane/Makefile.objs   |2 +-
  hw/virtio/dataplane/vring.c |   85 
 +++
  include/hw/virtio/dataplane/vring.h |   64 --
  6 files changed, 113 insertions(+), 45 deletions(-)
 
 diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
 index 5458f9d..eb45a3d 100644
 --- a/hw/block/dataplane/virtio-blk.c
 +++ b/hw/block/dataplane/virtio-blk.c
 @@ -16,6 +16,7 @@
  #include qemu/iov.h
  #include qemu/thread.h
  #include qemu/error-report.h
 +#include hw/virtio/virtio-access.h
  #include hw/virtio/dataplane/vring.h
  #include block/block.h
  #include hw/virtio/virtio-blk.h
 @@ -75,7 +76,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 b778e05..3e2b706 100644
 --- a/hw/scsi/virtio-scsi-dataplane.c
 +++ b/hw/scsi/virtio-scsi-dataplane.c
 @@ -81,7 +81,7 @@ VirtIOSCSIReq *virtio_scsi_pop_req_vring(VirtIOSCSI *s,
 
  void virtio_scsi_vring_push_notify(VirtIOSCSIReq *req)
  {
 -vring_push(req-vring-vring, req-elem,
 +vring_push((VirtIODevice *)req-dev, req-vring-vring, req-elem,
 req-qsgl.size + req-resp_iov.size);
  event_notifier_set(req-vring-guest_notifier);
  }
 diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
 index d21c397..19b224a 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-$(CONFIG_VIRTIO) += dataplane/
 
  obj-y += virtio.o virtio-balloon.o 
  obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
 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 b84957f..4624521 100644
 --- a/hw/virtio/dataplane/vring.c
 +++ b/hw/virtio/dataplane/vring.c
 @@ -18,6 +18,7 @@
  #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
 
 @@ -83,7 +84,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 = vring_get_used_idx(vdev, vring);
  vring-signalled_used = 0;
  vring-signalled_used_valid = false;
 
 @@ -104,7 +105,7 @@ void vring_teardown(Vring *vring, VirtIODevice *vdev, int 
 n)
  void vring_disable_notification(VirtIODevice *vdev, Vring *vring)
  {
  if (!(vdev-guest_features[0]  (1  VIRTIO_RING_F_EVENT_IDX))) {
 -vring-vr.used-flags |= VRING_USED_F_NO_NOTIFY;
 +vring_set_used_flags(vdev, vring, VRING_USED_F_NO_NOTIFY);
  }
  }
 
 @@ -117,10 +118,10 @@ bool vring_enable_notification(VirtIODevice *vdev, 
 Vring *vring)
  if (vdev-guest_features[0]  (1  VIRTIO_RING_F_EVENT_IDX)) {
  vring_avail_event(vring-vr) = vring-vr.avail-idx;
  } else {
 -vring-vr.used-flags = ~VRING_USED_F_NO_NOTIFY;
 +vring_clear_used_flags(vdev, vring, VRING_USED_F_NO_NOTIFY);
  }
  smp_mb(); /* ensure update is seen before

[PATCH RFC 07/11] dataplane: allow virtio-1 devices

2014-10-07 Thread Cornelia Huck 
Handle endianness conversion for virtio-1 virtqueues correctly.

Note that dataplane now needs to be built per-target.

Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
---
 hw/block/dataplane/virtio-blk.c |3 +-
 hw/scsi/virtio-scsi-dataplane.c |2 +-
 hw/virtio/Makefile.objs |2 +-
 hw/virtio/dataplane/Makefile.objs   |2 +-
 hw/virtio/dataplane/vring.c |   85 +++
 include/hw/virtio/dataplane/vring.h |   64 --
 6 files changed, 113 insertions(+), 45 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 5458f9d..eb45a3d 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -16,6 +16,7 @@
 #include qemu/iov.h
 #include qemu/thread.h
 #include qemu/error-report.h
+#include hw/virtio/virtio-access.h
 #include hw/virtio/dataplane/vring.h
 #include block/block.h
 #include hw/virtio/virtio-blk.h
@@ -75,7 +76,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 b778e05..3e2b706 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -81,7 +81,7 @@ VirtIOSCSIReq *virtio_scsi_pop_req_vring(VirtIOSCSI *s,
 
 void virtio_scsi_vring_push_notify(VirtIOSCSIReq *req)
 {
-vring_push(req-vring-vring, req-elem,
+vring_push((VirtIODevice *)req-dev, req-vring-vring, req-elem,
req-qsgl.size + req-resp_iov.size);
 event_notifier_set(req-vring-guest_notifier);
 }
diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index d21c397..19b224a 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-$(CONFIG_VIRTIO) += dataplane/
 
 obj-y += virtio.o virtio-balloon.o 
 obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
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 b84957f..4624521 100644
--- a/hw/virtio/dataplane/vring.c
+++ b/hw/virtio/dataplane/vring.c
@@ -18,6 +18,7 @@
 #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
 
@@ -83,7 +84,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 = vring_get_used_idx(vdev, vring);
 vring-signalled_used = 0;
 vring-signalled_used_valid = false;
 
@@ -104,7 +105,7 @@ void vring_teardown(Vring *vring, VirtIODevice *vdev, int n)
 void vring_disable_notification(VirtIODevice *vdev, Vring *vring)
 {
 if (!(vdev-guest_features[0]  (1  VIRTIO_RING_F_EVENT_IDX))) {
-vring-vr.used-flags |= VRING_USED_F_NO_NOTIFY;
+vring_set_used_flags(vdev, vring, VRING_USED_F_NO_NOTIFY);
 }
 }
 
@@ -117,10 +118,10 @@ bool vring_enable_notification(VirtIODevice *vdev, Vring 
*vring)
 if (vdev-guest_features[0]  (1  VIRTIO_RING_F_EVENT_IDX)) {
 vring_avail_event(vring-vr) = vring-vr.avail-idx;
 } else {
-vring-vr.used-flags = ~VRING_USED_F_NO_NOTIFY;
+vring_clear_used_flags(vdev, vring, 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() */
@@ -134,12 +135,13 @@ bool vring_should_notify(VirtIODevice *vdev, Vring *vring)
 smp_mb();
 
 if ((vdev-guest_features[0]  VIRTIO_F_NOTIFY_ON_EMPTY) 
-unlikely(vring-vr.avail-idx == vring-last_avail_idx)) {
+unlikely(!vring_more_avail(vdev, vring))) {
 return true;
 }
 
 if (!(vdev-guest_features[0]  VIRTIO_RING_F_EVENT_IDX)) {
-return !(vring-vr.avail-flags  VRING_AVAIL_F_NO_INTERRUPT);
+return !(vring_get_avail_flags(vdev, vring) 
+ VRING_AVAIL_F_NO_INTERRUPT);
 }
 old = vring-signalled_used;
 v = vring-signalled_used_valid;
@@