[PATCH v3 0/5] kvm: Make ioeventfd usable on s390.

[PATCH v3 0/5] kvm: Make ioeventfd usable on s390.

[Cornelia Huck]

Here's the latest version of my patch series enabling ioeventfds
on s390, again against kvm-next.

Patches 1 and 2 (cleaning up initialization and exporting the virtio-ccw
api) would make sense even independent of the ioeventfd enhancements.

Patches 3-5 are concerned with adding a new type of ioeventfds for
virtio-ccw notifications on s390. The naming is now hopefully clearer.
We won't add ioeventfd support for the legacy s390-virtio transport.

Please consider applying.

v2 -> v3:
- Added a patch exporting the virtio-ccw api and use it for the
  diagnose implementation.
- Better naming: We're dealing with virtio-ccw notifications only.
v1 -> v2:
- Move irqfd initialization from a module init function to kvm_init,
  eliminating the need for a second module for kvm/s390.
- Use kvm_io_device for s390 css devices.

Cornelia Huck (5):
  KVM: Initialize irqfd from kvm_init().
  KVM: s390: Export virtio-ccw api.
  KVM: Introduce KVM_VIRTIO_CCW_NOTIFY_BUS.
  KVM: ioeventfd for virtio-ccw devices.
  KVM: s390: Wire up ioeventfd.

 Documentation/virtual/kvm/api.txt       |  8 ++++++++
 arch/s390/include/uapi/asm/Kbuild       |  1 +
 arch/s390/include/uapi/asm/virtio-ccw.h | 21 +++++++++++++++++++++
 arch/s390/kvm/Kconfig                   |  1 +
 arch/s390/kvm/Makefile                  |  2 +-
 arch/s390/kvm/diag.c                    | 26 ++++++++++++++++++++++++++
 arch/s390/kvm/kvm-s390.c                |  1 +
 drivers/s390/kvm/virtio_ccw.c           |  5 +----
 include/linux/kvm_host.h                | 14 ++++++++++++++
 include/uapi/linux/kvm.h                |  3 +++
 virt/kvm/eventfd.c                      | 21 ++++++++++++++-------
 virt/kvm/kvm_main.c                     |  6 ++++++
 12 files changed, 97 insertions(+), 12 deletions(-)
 create mode 100644 arch/s390/include/uapi/asm/virtio-ccw.h

-- 
1.7.12.4

[PATCH v3 1/5] KVM: Initialize irqfd from kvm_init().

[Cornelia Huck]

Currently, eventfd introduces module_init/module_exit functions
to initialize/cleanup the irqfd workqueue. This only works, however,
if no other module_init/module_exit functions are built into the
same module.

Let's just move the initialization and cleanup to kvm_init and kvm_exit.
This way, it is also clearer where kvm startup may fail.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 include/linux/kvm_host.h | 13 +++++++++++++
 virt/kvm/eventfd.c       |  7 ++-----
 virt/kvm/kvm_main.c      |  6 ++++++
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 722cae7..3b768ef 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -423,6 +423,19 @@ void kvm_vcpu_uninit(struct kvm_vcpu *vcpu);
 int __must_check vcpu_load(struct kvm_vcpu *vcpu);
 void vcpu_put(struct kvm_vcpu *vcpu);
 
+#ifdef __KVM_HAVE_IOAPIC
+int kvm_irqfd_init(void);
+void kvm_irqfd_exit(void);
+#else
+static inline int kvm_irqfd_init(void)
+{
+	return 0;
+}
+
+static inline void kvm_irqfd_exit(void)
+{
+}
+#endif
 int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
 		  struct module *module);
 void kvm_exit(void);
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index b6eea5c..f0ced1a 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -544,7 +544,7 @@ void kvm_irq_routing_update(struct kvm *kvm,
  * aggregated from all vm* instances. We need our own isolated single-thread
  * queue to prevent deadlock against flushing the normal work-queue.
  */
-static int __init irqfd_module_init(void)
+int kvm_irqfd_init(void)
 {
 	irqfd_cleanup_wq = create_singlethread_workqueue("kvm-irqfd-cleanup");
 	if (!irqfd_cleanup_wq)
@@ -553,13 +553,10 @@ static int __init irqfd_module_init(void)
 	return 0;
 }
 
-static void __exit irqfd_module_exit(void)
+void kvm_irqfd_exit(void)
 {
 	destroy_workqueue(irqfd_cleanup_wq);
 }
-
-module_init(irqfd_module_init);
-module_exit(irqfd_module_exit);
 #endif
 
 /*
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index adc68fe..7c188a3 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2920,6 +2920,9 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
 	int r;
 	int cpu;
 
+	r = kvm_irqfd_init();
+	if (r)
+		goto out_irqfd;
 	r = kvm_arch_init(opaque);
 	if (r)
 		goto out_fail;
@@ -3000,6 +3003,8 @@ out_free_0a:
 out_free_0:
 	kvm_arch_exit();
 out_fail:
+	kvm_irqfd_exit();
+out_irqfd:
 	return r;
 }
 EXPORT_SYMBOL_GPL(kvm_init);
@@ -3016,6 +3021,7 @@ void kvm_exit(void)
 	on_each_cpu(hardware_disable_nolock, NULL, 1);
 	kvm_arch_hardware_unsetup();
 	kvm_arch_exit();
+	kvm_irqfd_exit();
 	free_cpumask_var(cpus_hardware_enabled);
 }
 EXPORT_SYMBOL_GPL(kvm_exit);
-- 
1.7.12.4

[PATCH v3 2/5] KVM: s390: Export virtio-ccw api.

[Cornelia Huck]

Export the virtio-ccw api in a header for usage by other code.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 arch/s390/include/uapi/asm/Kbuild       |  1 +
 arch/s390/include/uapi/asm/virtio-ccw.h | 21 +++++++++++++++++++++
 drivers/s390/kvm/virtio_ccw.c           |  5 +----
 3 files changed, 23 insertions(+), 4 deletions(-)
 create mode 100644 arch/s390/include/uapi/asm/virtio-ccw.h

diff --git a/arch/s390/include/uapi/asm/Kbuild b/arch/s390/include/uapi/asm/Kbuild
index 7bf68ff..9ccd190 100644
--- a/arch/s390/include/uapi/asm/Kbuild
+++ b/arch/s390/include/uapi/asm/Kbuild
@@ -44,5 +44,6 @@ header-y += termios.h
 header-y += types.h
 header-y += ucontext.h
 header-y += unistd.h
+header-y += virtio-ccw.h
 header-y += vtoc.h
 header-y += zcrypt.h
diff --git a/arch/s390/include/uapi/asm/virtio-ccw.h b/arch/s390/include/uapi/asm/virtio-ccw.h
new file mode 100644
index 0000000..a9a4ebf
--- /dev/null
+++ b/arch/s390/include/uapi/asm/virtio-ccw.h
@@ -0,0 +1,21 @@
+/*
+ * Definitions for virtio-ccw devices.
+ *
+ * Copyright IBM Corp. 2013
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License (version 2 only)
+ * as published by the Free Software Foundation.
+ *
+ *  Author(s): Cornelia Huck <cornelia.huck@de.ibm.com>
+ */
+#ifndef __KVM_VIRTIO_CCW_H
+#define __KVM_VIRTIO_CCW_H
+
+/* Alignment of vring buffers. */
+#define KVM_VIRTIO_CCW_RING_ALIGN 4096
+
+/* Subcode for diagnose 500 (virtio hypercall). */
+#define KVM_S390_VIRTIO_CCW_NOTIFY 3
+
+#endif
diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
index 3217dfe..c5532b3 100644
--- a/drivers/s390/kvm/virtio_ccw.c
+++ b/drivers/s390/kvm/virtio_ccw.c
@@ -31,6 +31,7 @@
 #include <asm/irq.h>
 #include <asm/cio.h>
 #include <asm/ccwdev.h>
+#include <asm/virtio-ccw.h>
 
 /*
  * virtio related functions
@@ -79,10 +80,6 @@ struct virtio_ccw_vq_info {
 	struct list_head node;
 };
 
-#define KVM_VIRTIO_CCW_RING_ALIGN 4096
-
-#define KVM_S390_VIRTIO_CCW_NOTIFY 3
-
 #define CCW_CMD_SET_VQ 0x13
 #define CCW_CMD_VDEV_RESET 0x33
 #define CCW_CMD_SET_IND 0x43
-- 
1.7.12.4

[PATCH v3 3/5] KVM: Introduce KVM_VIRTIO_CCW_NOTIFY_BUS.

[Cornelia Huck]

Add a new bus type for virtio-ccw devices on s390.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 include/linux/kvm_host.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3b768ef..206247f 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -148,6 +148,7 @@ struct kvm_io_bus {
 enum kvm_bus {
 	KVM_MMIO_BUS,
 	KVM_PIO_BUS,
+	KVM_VIRTIO_CCW_NOTIFY_BUS,
 	KVM_NR_BUSES
 };
 
-- 
1.7.12.4

[PATCH v3 4/5] KVM: ioeventfd for virtio-ccw devices.

[Cornelia Huck]

Enhance KVM_IOEVENTFD with a new flag that allows to attach to virtio-ccw
devices on s390 via the KVM_VIRTIO_CCW_NOTIFY_BUS.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 Documentation/virtual/kvm/api.txt |  8 ++++++++
 include/uapi/linux/kvm.h          |  3 +++
 virt/kvm/eventfd.c                | 14 ++++++++++++--
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index c2534c3..86232d6 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1468,15 +1468,23 @@ struct kvm_ioeventfd {
 	__u8  pad[36];
 };
 
+For the special case of virtio-ccw devices on s390, the ioevent is matched
+to a subchannel/virtqueue tuple instead.
+
 The following flags are defined:
 
 #define KVM_IOEVENTFD_FLAG_DATAMATCH (1 << kvm_ioeventfd_flag_nr_datamatch)
 #define KVM_IOEVENTFD_FLAG_PIO       (1 << kvm_ioeventfd_flag_nr_pio)
 #define KVM_IOEVENTFD_FLAG_DEASSIGN  (1 << kvm_ioeventfd_flag_nr_deassign)
+#define KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY \
+	(1 << kvm_ioeventfd_flag_nr_virtio_ccw_notify)
 
 If datamatch flag is set, the event will be signaled only if the written value
 to the registered address is equal to datamatch in struct kvm_ioeventfd.
 
+For virtio-ccw devices, addr contains the subchannel id and datamatch the
+virtqueue index.
+
 
 4.60 KVM_DIRTY_TLB
 
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 9a2db57..8f3e5ae 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -448,12 +448,15 @@ enum {
 	kvm_ioeventfd_flag_nr_datamatch,
 	kvm_ioeventfd_flag_nr_pio,
 	kvm_ioeventfd_flag_nr_deassign,
+	kvm_ioeventfd_flag_nr_virtio_ccw_notify,
 	kvm_ioeventfd_flag_nr_max,
 };
 
 #define KVM_IOEVENTFD_FLAG_DATAMATCH (1 << kvm_ioeventfd_flag_nr_datamatch)
 #define KVM_IOEVENTFD_FLAG_PIO       (1 << kvm_ioeventfd_flag_nr_pio)
 #define KVM_IOEVENTFD_FLAG_DEASSIGN  (1 << kvm_ioeventfd_flag_nr_deassign)
+#define KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY \
+	(1 << kvm_ioeventfd_flag_nr_virtio_ccw_notify)
 
 #define KVM_IOEVENTFD_VALID_FLAG_MASK  ((1 << kvm_ioeventfd_flag_nr_max) - 1)
 
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index f0ced1a..8de3cd7 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -679,11 +679,16 @@ static int
 kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
 {
 	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
-	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
+	int                       ccw;
+	enum kvm_bus              bus_idx;
 	struct _ioeventfd        *p;
 	struct eventfd_ctx       *eventfd;
 	int                       ret;
 
+	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
+	bus_idx = pio ? KVM_PIO_BUS :
+		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
+		KVM_MMIO_BUS;
 	/* must be natural-word sized */
 	switch (args->len) {
 	case 1:
@@ -759,11 +764,16 @@ static int
 kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
 {
 	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
-	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
+	int                       ccw;
+	enum kvm_bus              bus_idx;
 	struct _ioeventfd        *p, *tmp;
 	struct eventfd_ctx       *eventfd;
 	int                       ret = -ENOENT;
 
+	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
+	bus_idx = pio ? KVM_PIO_BUS :
+		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
+		KVM_MMIO_BUS;
 	eventfd = eventfd_ctx_fdget(args->fd);
 	if (IS_ERR(eventfd))
 		return PTR_ERR(eventfd);
-- 
1.7.12.4

Re: [PATCH v3 4/5] KVM: ioeventfd for virtio-ccw devices.

[Michael S. Tsirkin]

On Mon, Feb 25, 2013 at 04:27:49PM +0100, Cornelia Huck wrote:
> Enhance KVM_IOEVENTFD with a new flag that allows to attach to virtio-ccw
> devices on s390 via the KVM_VIRTIO_CCW_NOTIFY_BUS.
> 
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  Documentation/virtual/kvm/api.txt |  8 ++++++++
>  include/uapi/linux/kvm.h          |  3 +++
>  virt/kvm/eventfd.c                | 14 ++++++++++++--
>  3 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index c2534c3..86232d6 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -1468,15 +1468,23 @@ struct kvm_ioeventfd {
>  	__u8  pad[36];
>  };
>  
> +For the special case of virtio-ccw devices on s390, the ioevent is matched
> +to a subchannel/virtqueue tuple instead.
> +
>  The following flags are defined:
>  
>  #define KVM_IOEVENTFD_FLAG_DATAMATCH (1 << kvm_ioeventfd_flag_nr_datamatch)
>  #define KVM_IOEVENTFD_FLAG_PIO       (1 << kvm_ioeventfd_flag_nr_pio)
>  #define KVM_IOEVENTFD_FLAG_DEASSIGN  (1 << kvm_ioeventfd_flag_nr_deassign)
> +#define KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY \
> +	(1 << kvm_ioeventfd_flag_nr_virtio_ccw_notify)
>  
>  If datamatch flag is set, the event will be signaled only if the written value
>  to the registered address is equal to datamatch in struct kvm_ioeventfd.
>  
> +For virtio-ccw devices, addr contains the subchannel id and datamatch the
> +virtqueue index.
> +
>  
>  4.60 KVM_DIRTY_TLB
>  
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 9a2db57..8f3e5ae 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -448,12 +448,15 @@ enum {
>  	kvm_ioeventfd_flag_nr_datamatch,
>  	kvm_ioeventfd_flag_nr_pio,
>  	kvm_ioeventfd_flag_nr_deassign,
> +	kvm_ioeventfd_flag_nr_virtio_ccw_notify,
>  	kvm_ioeventfd_flag_nr_max,
>  };
>  
>  #define KVM_IOEVENTFD_FLAG_DATAMATCH (1 << kvm_ioeventfd_flag_nr_datamatch)
>  #define KVM_IOEVENTFD_FLAG_PIO       (1 << kvm_ioeventfd_flag_nr_pio)
>  #define KVM_IOEVENTFD_FLAG_DEASSIGN  (1 << kvm_ioeventfd_flag_nr_deassign)
> +#define KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY \
> +	(1 << kvm_ioeventfd_flag_nr_virtio_ccw_notify)
>  
>  #define KVM_IOEVENTFD_VALID_FLAG_MASK  ((1 << kvm_ioeventfd_flag_nr_max) - 1)
>  
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index f0ced1a..8de3cd7 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -679,11 +679,16 @@ static int
>  kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
>  {
>  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> +	int                       ccw;
> +	enum kvm_bus              bus_idx;
>  	struct _ioeventfd        *p;
>  	struct eventfd_ctx       *eventfd;
>  	int                       ret;
>  
> +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> +	bus_idx = pio ? KVM_PIO_BUS :
> +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> +		KVM_MMIO_BUS;

May be better to rewrite using if/else.

>  	/* must be natural-word sized */
>  	switch (args->len) {
>  	case 1:
> @@ -759,11 +764,16 @@ static int
>  kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
>  {
>  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> +	int                       ccw;
> +	enum kvm_bus              bus_idx;
>  	struct _ioeventfd        *p, *tmp;
>  	struct eventfd_ctx       *eventfd;
>  	int                       ret = -ENOENT;
>  
> +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> +	bus_idx = pio ? KVM_PIO_BUS :
> +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> +		KVM_MMIO_BUS;
>  	eventfd = eventfd_ctx_fdget(args->fd);
>  	if (IS_ERR(eventfd))
>  		return PTR_ERR(eventfd);
> -- 
> 1.7.12.4

Re: [PATCH v3 4/5] KVM: ioeventfd for virtio-ccw devices.

[Cornelia Huck]

On Tue, 26 Feb 2013 12:55:36 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Feb 25, 2013 at 04:27:49PM +0100, Cornelia Huck wrote:
> > Enhance KVM_IOEVENTFD with a new flag that allows to attach to virtio-ccw
> > devices on s390 via the KVM_VIRTIO_CCW_NOTIFY_BUS.
> > 
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > ---
> >  Documentation/virtual/kvm/api.txt |  8 ++++++++
> >  include/uapi/linux/kvm.h          |  3 +++
> >  virt/kvm/eventfd.c                | 14 ++++++++++++--
> >  3 files changed, 23 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> > index c2534c3..86232d6 100644
> > --- a/Documentation/virtual/kvm/api.txt
> > +++ b/Documentation/virtual/kvm/api.txt
> > @@ -1468,15 +1468,23 @@ struct kvm_ioeventfd {
> >  	__u8  pad[36];
> >  };
> >  
> > +For the special case of virtio-ccw devices on s390, the ioevent is matched
> > +to a subchannel/virtqueue tuple instead.
> > +
> >  The following flags are defined:
> >  
> >  #define KVM_IOEVENTFD_FLAG_DATAMATCH (1 << kvm_ioeventfd_flag_nr_datamatch)
> >  #define KVM_IOEVENTFD_FLAG_PIO       (1 << kvm_ioeventfd_flag_nr_pio)
> >  #define KVM_IOEVENTFD_FLAG_DEASSIGN  (1 << kvm_ioeventfd_flag_nr_deassign)
> > +#define KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY \
> > +	(1 << kvm_ioeventfd_flag_nr_virtio_ccw_notify)
> >  
> >  If datamatch flag is set, the event will be signaled only if the written value
> >  to the registered address is equal to datamatch in struct kvm_ioeventfd.
> >  
> > +For virtio-ccw devices, addr contains the subchannel id and datamatch the
> > +virtqueue index.
> > +
> >  
> >  4.60 KVM_DIRTY_TLB
> >  
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 9a2db57..8f3e5ae 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -448,12 +448,15 @@ enum {
> >  	kvm_ioeventfd_flag_nr_datamatch,
> >  	kvm_ioeventfd_flag_nr_pio,
> >  	kvm_ioeventfd_flag_nr_deassign,
> > +	kvm_ioeventfd_flag_nr_virtio_ccw_notify,
> >  	kvm_ioeventfd_flag_nr_max,
> >  };
> >  
> >  #define KVM_IOEVENTFD_FLAG_DATAMATCH (1 << kvm_ioeventfd_flag_nr_datamatch)
> >  #define KVM_IOEVENTFD_FLAG_PIO       (1 << kvm_ioeventfd_flag_nr_pio)
> >  #define KVM_IOEVENTFD_FLAG_DEASSIGN  (1 << kvm_ioeventfd_flag_nr_deassign)
> > +#define KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY \
> > +	(1 << kvm_ioeventfd_flag_nr_virtio_ccw_notify)
> >  
> >  #define KVM_IOEVENTFD_VALID_FLAG_MASK  ((1 << kvm_ioeventfd_flag_nr_max) - 1)
> >  
> > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> > index f0ced1a..8de3cd7 100644
> > --- a/virt/kvm/eventfd.c
> > +++ b/virt/kvm/eventfd.c
> > @@ -679,11 +679,16 @@ static int
> >  kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
> >  {
> >  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> > -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> > +	int                       ccw;
> > +	enum kvm_bus              bus_idx;
> >  	struct _ioeventfd        *p;
> >  	struct eventfd_ctx       *eventfd;
> >  	int                       ret;
> >  
> > +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> > +	bus_idx = pio ? KVM_PIO_BUS :
> > +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> > +		KVM_MMIO_BUS;
> 
> May be better to rewrite using if/else.

Matter of taste, I guess. But I can do that.

> 
> >  	/* must be natural-word sized */
> >  	switch (args->len) {
> >  	case 1:
> > @@ -759,11 +764,16 @@ static int
> >  kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
> >  {
> >  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> > -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> > +	int                       ccw;
> > +	enum kvm_bus              bus_idx;
> >  	struct _ioeventfd        *p, *tmp;
> >  	struct eventfd_ctx       *eventfd;
> >  	int                       ret = -ENOENT;
> >  
> > +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> > +	bus_idx = pio ? KVM_PIO_BUS :
> > +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> > +		KVM_MMIO_BUS;
> >  	eventfd = eventfd_ctx_fdget(args->fd);
> >  	if (IS_ERR(eventfd))
> >  		return PTR_ERR(eventfd);
> > -- 
> > 1.7.12.4
> 

Re: [Qemu-devel] [PATCH v3 4/5] KVM: ioeventfd for virtio-ccw devices.

[Stefan Hajnoczi]

On Tue, Feb 26, 2013 at 12:55:36PM +0200, Michael S. Tsirkin wrote:
> On Mon, Feb 25, 2013 at 04:27:49PM +0100, Cornelia Huck wrote:
> > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> > index f0ced1a..8de3cd7 100644
> > --- a/virt/kvm/eventfd.c
> > +++ b/virt/kvm/eventfd.c
> > @@ -679,11 +679,16 @@ static int
> >  kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
> >  {
> >  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> > -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> > +	int                       ccw;
> > +	enum kvm_bus              bus_idx;
> >  	struct _ioeventfd        *p;
> >  	struct eventfd_ctx       *eventfd;
> >  	int                       ret;
> >  
> > +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> > +	bus_idx = pio ? KVM_PIO_BUS :
> > +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> > +		KVM_MMIO_BUS;
> 
> May be better to rewrite using if/else.

Saw this after sending my comment.  I agree with Michael, an if
statement allows you to drop the locals and capture the bus_idx
conversion in a single place (it could even be a static function to save
duplicating the code in both functions that use it).

Stefan

Re: [PATCH v3 4/5] KVM: ioeventfd for virtio-ccw devices.

[Cornelia Huck]

On Tue, 26 Feb 2013 15:20:52 +0100
Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Tue, Feb 26, 2013 at 12:55:36PM +0200, Michael S. Tsirkin wrote:
> > On Mon, Feb 25, 2013 at 04:27:49PM +0100, Cornelia Huck wrote:
> > > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> > > index f0ced1a..8de3cd7 100644
> > > --- a/virt/kvm/eventfd.c
> > > +++ b/virt/kvm/eventfd.c
> > > @@ -679,11 +679,16 @@ static int
> > >  kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
> > >  {
> > >  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> > > -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> > > +	int                       ccw;
> > > +	enum kvm_bus              bus_idx;
> > >  	struct _ioeventfd        *p;
> > >  	struct eventfd_ctx       *eventfd;
> > >  	int                       ret;
> > >  
> > > +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> > > +	bus_idx = pio ? KVM_PIO_BUS :
> > > +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> > > +		KVM_MMIO_BUS;
> > 
> > May be better to rewrite using if/else.
> 
> Saw this after sending my comment.  I agree with Michael, an if
> statement allows you to drop the locals and capture the bus_idx
> conversion in a single place (it could even be a static function to save
> duplicating the code in both functions that use it).
> 
> Stefan
> 

Introducing a helper function sounds reasonable.

Re: [PATCH v3 4/5] KVM: ioeventfd for virtio-ccw devices.

[Stefan Hajnoczi]

On Mon, Feb 25, 2013 at 04:27:49PM +0100, Cornelia Huck wrote:
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index f0ced1a..8de3cd7 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -679,11 +679,16 @@ static int
>  kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
>  {
>  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> +	int                       ccw;
> +	enum kvm_bus              bus_idx;
>  	struct _ioeventfd        *p;
>  	struct eventfd_ctx       *eventfd;
>  	int                       ret;
>  
> +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> +	bus_idx = pio ? KVM_PIO_BUS :
> +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> +		KVM_MMIO_BUS;
>  	/* must be natural-word sized */
>  	switch (args->len) {
>  	case 1:
> @@ -759,11 +764,16 @@ static int
>  kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
>  {
>  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> +	int                       ccw;
> +	enum kvm_bus              bus_idx;
>  	struct _ioeventfd        *p, *tmp;
>  	struct eventfd_ctx       *eventfd;
>  	int                       ret = -ENOENT;
>  
> +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> +	bus_idx = pio ? KVM_PIO_BUS :
> +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> +		KVM_MMIO_BUS;

This is getting pretty convoluted.  Drop of pio and ccw local variables,
replace ?: with an if statement:

if (args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY)
	bus_idx = KVM_VIRTIO_CCW_NOTIFY_BUS;
else if (args->flags & KVM_IOEVENTFD_FLAG_PIO)
	bus_idx = KVM_PIO_BUS;
else
	bus_idx = KVM_MMIO_BUS;

[PATCH v3 5/5] KVM: s390: Wire up ioeventfd.

[Cornelia Huck]

Enable ioeventfd support on s390 and hook up diagnose 500 virtio-ccw
notifications.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 arch/s390/kvm/Kconfig    |  1 +
 arch/s390/kvm/Makefile   |  2 +-
 arch/s390/kvm/diag.c     | 26 ++++++++++++++++++++++++++
 arch/s390/kvm/kvm-s390.c |  1 +
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kvm/Kconfig b/arch/s390/kvm/Kconfig
index b58dd86..3c43e30 100644
--- a/arch/s390/kvm/Kconfig
+++ b/arch/s390/kvm/Kconfig
@@ -22,6 +22,7 @@ config KVM
 	select PREEMPT_NOTIFIERS
 	select ANON_INODES
 	select HAVE_KVM_CPU_RELAX_INTERCEPT
+	select HAVE_KVM_EVENTFD
 	---help---
 	  Support hosting paravirtualized guest machines using the SIE
 	  virtualization capability on the mainframe. This should work
diff --git a/arch/s390/kvm/Makefile b/arch/s390/kvm/Makefile
index 3975722..8fe9d65 100644
--- a/arch/s390/kvm/Makefile
+++ b/arch/s390/kvm/Makefile
@@ -6,7 +6,7 @@
 # it under the terms of the GNU General Public License (version 2 only)
 # as published by the Free Software Foundation.
 
-common-objs = $(addprefix ../../../virt/kvm/, kvm_main.o)
+common-objs = $(addprefix ../../../virt/kvm/, kvm_main.o eventfd.o)
 
 ccflags-y := -Ivirt/kvm -Iarch/s390/kvm
 
diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
index a390687..96907f2 100644
--- a/arch/s390/kvm/diag.c
+++ b/arch/s390/kvm/diag.c
@@ -13,6 +13,7 @@
 
 #include <linux/kvm.h>
 #include <linux/kvm_host.h>
+#include <asm/virtio-ccw.h>
 #include "kvm-s390.h"
 #include "trace.h"
 #include "trace-s390.h"
@@ -104,6 +105,29 @@ static int __diag_ipl_functions(struct kvm_vcpu *vcpu)
 	return -EREMOTE;
 }
 
+static int __diag_virtio_hypercall(struct kvm_vcpu *vcpu)
+{
+	int ret, idx;
+
+	/* No virtio-ccw notification? Get out quickly. */
+	if (!vcpu->kvm->arch.css_support ||
+	    (vcpu->run->s.regs.gprs[1] != KVM_S390_VIRTIO_CCW_NOTIFY))
+		return -EOPNOTSUPP;
+
+	idx = srcu_read_lock(&vcpu->kvm->srcu);
+	/*
+	 * The layout is as follows:
+	 * - gpr 2 contains the subchannel id (passed as addr)
+	 * - gpr 3 contains the virtqueue index (passed as datamatch)
+	 */
+	ret = kvm_io_bus_write(vcpu->kvm, KVM_VIRTIO_CCW_NOTIFY_BUS,
+				vcpu->run->s.regs.gprs[2],
+				8, &vcpu->run->s.regs.gprs[3]);
+	srcu_read_unlock(&vcpu->kvm->srcu, idx);
+	/* kvm_io_bus_write returns -EOPNOTSUPP if it found no match. */
+	return ret;
+}
+
 int kvm_s390_handle_diag(struct kvm_vcpu *vcpu)
 {
 	int code = (vcpu->arch.sie_block->ipb & 0xfff0000) >> 16;
@@ -118,6 +142,8 @@ int kvm_s390_handle_diag(struct kvm_vcpu *vcpu)
 		return __diag_time_slice_end_directed(vcpu);
 	case 0x308:
 		return __diag_ipl_functions(vcpu);
+	case 0x500:
+		return __diag_virtio_hypercall(vcpu);
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index f822d36..04d2454 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -142,6 +142,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_ONE_REG:
 	case KVM_CAP_ENABLE_CAP:
 	case KVM_CAP_S390_CSS_SUPPORT:
+	case KVM_CAP_IOEVENTFD:
 		r = 1;
 		break;
 	case KVM_CAP_NR_VCPUS:
-- 
1.7.12.4

Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.

[Michael S. Tsirkin]

On Mon, Feb 25, 2013 at 04:27:45PM +0100, Cornelia Huck wrote:
> Here's the latest version of my patch series enabling ioeventfds
> on s390, again against kvm-next.
> 
> Patches 1 and 2 (cleaning up initialization and exporting the virtio-ccw
> api) would make sense even independent of the ioeventfd enhancements.
> 
> Patches 3-5 are concerned with adding a new type of ioeventfds for
> virtio-ccw notifications on s390. The naming is now hopefully clearer.
> We won't add ioeventfd support for the legacy s390-virtio transport.
> 
> Please consider applying.

I just had a thought: this makes us lookup the device on the bus
for each notification. It would be better to simply get the
device index from guest instead.

We could validate that it matches the correct device,
if not - fallback to the current linear scan.

We could return the index to guest for the next call.

I know this needs guest changes but it's still not too late to
fix this for 3.9 guests so that we won't need to worry
about compatibility going forward.

Hmm?

> v2 -> v3:
> - Added a patch exporting the virtio-ccw api and use it for the
>   diagnose implementation.
> - Better naming: We're dealing with virtio-ccw notifications only.
> v1 -> v2:
> - Move irqfd initialization from a module init function to kvm_init,
>   eliminating the need for a second module for kvm/s390.
> - Use kvm_io_device for s390 css devices.
> 
> Cornelia Huck (5):
>   KVM: Initialize irqfd from kvm_init().
>   KVM: s390: Export virtio-ccw api.
>   KVM: Introduce KVM_VIRTIO_CCW_NOTIFY_BUS.
>   KVM: ioeventfd for virtio-ccw devices.
>   KVM: s390: Wire up ioeventfd.
> 
>  Documentation/virtual/kvm/api.txt       |  8 ++++++++
>  arch/s390/include/uapi/asm/Kbuild       |  1 +
>  arch/s390/include/uapi/asm/virtio-ccw.h | 21 +++++++++++++++++++++
>  arch/s390/kvm/Kconfig                   |  1 +
>  arch/s390/kvm/Makefile                  |  2 +-
>  arch/s390/kvm/diag.c                    | 26 ++++++++++++++++++++++++++
>  arch/s390/kvm/kvm-s390.c                |  1 +
>  drivers/s390/kvm/virtio_ccw.c           |  5 +----
>  include/linux/kvm_host.h                | 14 ++++++++++++++
>  include/uapi/linux/kvm.h                |  3 +++
>  virt/kvm/eventfd.c                      | 21 ++++++++++++++-------
>  virt/kvm/kvm_main.c                     |  6 ++++++
>  12 files changed, 97 insertions(+), 12 deletions(-)
>  create mode 100644 arch/s390/include/uapi/asm/virtio-ccw.h
> 
> -- 
> 1.7.12.4

Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.

[Michael S. Tsirkin]

On Tue, Feb 26, 2013 at 01:04:21PM +0200, Michael S. Tsirkin wrote:
> On Mon, Feb 25, 2013 at 04:27:45PM +0100, Cornelia Huck wrote:
> > Here's the latest version of my patch series enabling ioeventfds
> > on s390, again against kvm-next.
> > 
> > Patches 1 and 2 (cleaning up initialization and exporting the virtio-ccw
> > api) would make sense even independent of the ioeventfd enhancements.
> > 
> > Patches 3-5 are concerned with adding a new type of ioeventfds for
> > virtio-ccw notifications on s390. The naming is now hopefully clearer.
> > We won't add ioeventfd support for the legacy s390-virtio transport.
> > 
> > Please consider applying.
> 
> I just had a thought: this makes us lookup the device on the bus
> for each notification. It would be better to simply get the
> device index from guest instead.
> 
> We could validate that it matches the correct device,
> if not - fallback to the current linear scan.
> 
> We could return the index to guest for the next call.
> 
> I know this needs guest changes but it's still not too late to
> fix this for 3.9 guests so that we won't need to worry
> about compatibility going forward.
> 
> Hmm?

And just to clarify, here's what I mean (BTW, why doesn't
this code use the interfaces from kvm_para.h?)
I think it's a good idea to merge this before 3.9 so we don't
need to worry about legacy going forward.

Completely untested, just to give you the idea.

--->
virtio_ccw: pass a cookie value to kvm hypercall

Lookups by channel/vq pair on host during virtio notifications might be
expensive.  Interpret hypercall return value as a cookie which host can
use to do device lookups for the next notification more efficiently.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---

diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
index 2029b6c..1054f3a 100644
--- a/drivers/s390/kvm/virtio_ccw.c
+++ b/drivers/s390/kvm/virtio_ccw.c
@@ -77,6 +77,7 @@ struct virtio_ccw_vq_info {
 	void *queue;
 	struct vq_info_block *info_block;
 	struct list_head node;
+	long cookie;
 };
 
 #define KVM_VIRTIO_CCW_RING_ALIGN 4096
@@ -145,15 +146,18 @@ static int ccw_io_helper(struct virtio_ccw_device *vcdev,
 }
 
 static inline long do_kvm_notify(struct subchannel_id schid,
-				 unsigned long queue_index)
+				 unsigned long queue_index,
+				 long cookie)
 {
 	register unsigned long __nr asm("1") = KVM_S390_VIRTIO_CCW_NOTIFY;
 	register struct subchannel_id __schid asm("2") = schid;
 	register unsigned long __index asm("3") = queue_index;
 	register long __rc asm("2");
+	register long __cookie asm("4") = cookie;
 
 	asm volatile ("diag 2,4,0x500\n"
-		      : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index)
+		      : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index),
+		      "d"(__cookie)
 		      : "memory", "cc");
 	return __rc;
 }
@@ -166,7 +170,7 @@ static void virtio_ccw_kvm_notify(struct virtqueue *vq)
 
 	vcdev = to_vc_device(info->vq->vdev);
 	ccw_device_get_schid(vcdev->cdev, &schid);
-	do_kvm_notify(schid, virtqueue_get_queue_index(vq));
+	info->cookie = do_kvm_notify(schid, virtqueue_get_queue_index(vq), info->cookie);
 }
 
 static int virtio_ccw_read_vq_conf(struct virtio_ccw_device *vcdev,

Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.

[Christian Borntraeger]

On 26/02/13 12:18, Michael S. Tsirkin wrote:
> On Tue, Feb 26, 2013 at 01:04:21PM +0200, Michael S. Tsirkin wrote:
>> On Mon, Feb 25, 2013 at 04:27:45PM +0100, Cornelia Huck wrote:
>>> Here's the latest version of my patch series enabling ioeventfds
>>> on s390, again against kvm-next.
>>>
>>> Patches 1 and 2 (cleaning up initialization and exporting the virtio-ccw
>>> api) would make sense even independent of the ioeventfd enhancements.
>>>
>>> Patches 3-5 are concerned with adding a new type of ioeventfds for
>>> virtio-ccw notifications on s390. The naming is now hopefully clearer.
>>> We won't add ioeventfd support for the legacy s390-virtio transport.
>>>
>>> Please consider applying.
>>
>> I just had a thought: this makes us lookup the device on the bus
>> for each notification. It would be better to simply get the
>> device index from guest instead.
>>
>> We could validate that it matches the correct device,
>> if not - fallback to the current linear scan.
>>
>> We could return the index to guest for the next call.
>>
>> I know this needs guest changes but it's still not too late to
>> fix this for 3.9 guests so that we won't need to worry
>> about compatibility going forward.
>>
>> Hmm?
> 
> And just to clarify, here's what I mean (BTW, why doesn't
> this code use the interfaces from kvm_para.h?)
> I think it's a good idea to merge this before 3.9 so we don't
> need to worry about legacy going forward.
> 
> Completely untested, just to give you the idea.

Thinking more about it: Isnt the index on the kvm bus just an implementation
detail? In other words, what happens if we want to re-arrange the kvm io bus 
to a tree like structure. Then the index becomes pretty much useless. Do we 
really want to put such an internal thing into an interface?

CHristian

Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.

[Christian Borntraeger]

On 26/02/13 12:18, Michael S. Tsirkin wrote:
> virtio_ccw: pass a cookie value to kvm hypercall
> 
> Lookups by channel/vq pair on host during virtio notifications might be
> expensive.  Interpret hypercall return value as a cookie which host can
> use to do device lookups for the next notification more efficiently.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ---
> 
> diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
> index 2029b6c..1054f3a 100644
> --- a/drivers/s390/kvm/virtio_ccw.c
> +++ b/drivers/s390/kvm/virtio_ccw.c
> @@ -77,6 +77,7 @@ struct virtio_ccw_vq_info {
>  	void *queue;
>  	struct vq_info_block *info_block;
>  	struct list_head node;
> +	long cookie;
>  };
> 
>  #define KVM_VIRTIO_CCW_RING_ALIGN 4096
> @@ -145,15 +146,18 @@ static int ccw_io_helper(struct virtio_ccw_device *vcdev,
>  }
> 
>  static inline long do_kvm_notify(struct subchannel_id schid,
> -				 unsigned long queue_index)
> +				 unsigned long queue_index,
> +				 long cookie)
>  {
>  	register unsigned long __nr asm("1") = KVM_S390_VIRTIO_CCW_NOTIFY;
>  	register struct subchannel_id __schid asm("2") = schid;
>  	register unsigned long __index asm("3") = queue_index;
>  	register long __rc asm("2");
> +	register long __cookie asm("4") = cookie;
> 
>  	asm volatile ("diag 2,4,0x500\n"
> -		      : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index)
> +		      : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index),
> +		      "d"(__cookie)
>  		      : "memory", "cc");
>  	return __rc;
>  }
> @@ -166,7 +170,7 @@ static void virtio_ccw_kvm_notify(struct virtqueue *vq)
> 
>  	vcdev = to_vc_device(info->vq->vdev);
>  	ccw_device_get_schid(vcdev->cdev, &schid);
> -	do_kvm_notify(schid, virtqueue_get_queue_index(vq));
> +	info->cookie = do_kvm_notify(schid, virtqueue_get_queue_index(vq), info->cookie);
>  }
> 
>  static int virtio_ccw_read_vq_conf(struct virtio_ccw_device *vcdev,


Hmmm, forget my last mail. This actually could be even forward and backward compatible.
In the virtio spec we will not define the cookie format (just 64bit int). That will allow
qemu or future kernels to use that for other things (as long as a validity check is 
possible) if we dont have a kvm bus.

Now:

old guest, old host: 
works.

old guest, new host: 
the cookie from the guest contains junk, the host needs to detect that the cookie is 
junk and ignores it. It will return the new cookie anyway. 

new guest, old host:
The guest will get a junk cookie and pass it back to the host. But the host will ignore
it anyway.

new guest, new host:
works.

So...
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>

Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.

[Cornelia Huck]

On Tue, 26 Feb 2013 13:13:39 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 26/02/13 12:18, Michael S. Tsirkin wrote:
> > virtio_ccw: pass a cookie value to kvm hypercall
> > 
> > Lookups by channel/vq pair on host during virtio notifications might be
> > expensive.  Interpret hypercall return value as a cookie which host can
> > use to do device lookups for the next notification more efficiently.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > ---
> > 
> > diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
> > index 2029b6c..1054f3a 100644
> > --- a/drivers/s390/kvm/virtio_ccw.c
> > +++ b/drivers/s390/kvm/virtio_ccw.c
> > @@ -77,6 +77,7 @@ struct virtio_ccw_vq_info {
> >  	void *queue;
> >  	struct vq_info_block *info_block;
> >  	struct list_head node;
> > +	long cookie;
> >  };
> > 
> >  #define KVM_VIRTIO_CCW_RING_ALIGN 4096
> > @@ -145,15 +146,18 @@ static int ccw_io_helper(struct virtio_ccw_device *vcdev,
> >  }
> > 
> >  static inline long do_kvm_notify(struct subchannel_id schid,
> > -				 unsigned long queue_index)
> > +				 unsigned long queue_index,
> > +				 long cookie)
> >  {
> >  	register unsigned long __nr asm("1") = KVM_S390_VIRTIO_CCW_NOTIFY;
> >  	register struct subchannel_id __schid asm("2") = schid;
> >  	register unsigned long __index asm("3") = queue_index;
> >  	register long __rc asm("2");
> > +	register long __cookie asm("4") = cookie;
> > 
> >  	asm volatile ("diag 2,4,0x500\n"
> > -		      : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index)
> > +		      : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index),
> > +		      "d"(__cookie)
> >  		      : "memory", "cc");
> >  	return __rc;
> >  }
> > @@ -166,7 +170,7 @@ static void virtio_ccw_kvm_notify(struct virtqueue *vq)
> > 
> >  	vcdev = to_vc_device(info->vq->vdev);
> >  	ccw_device_get_schid(vcdev->cdev, &schid);
> > -	do_kvm_notify(schid, virtqueue_get_queue_index(vq));
> > +	info->cookie = do_kvm_notify(schid, virtqueue_get_queue_index(vq), info->cookie);
> >  }
> > 
> >  static int virtio_ccw_read_vq_conf(struct virtio_ccw_device *vcdev,
> 
> 
> Hmmm, forget my last mail. This actually could be even forward and backward compatible.
> In the virtio spec we will not define the cookie format (just 64bit int). That will allow
> qemu or future kernels to use that for other things (as long as a validity check is 
> possible) if we dont have a kvm bus.
> 
> Now:
> 
> old guest, old host: 
> works.
> 
> old guest, new host: 
> the cookie from the guest contains junk, the host needs to detect that the cookie is 
> junk and ignores it. It will return the new cookie anyway. 
> 
> new guest, old host:
> The guest will get a junk cookie and pass it back to the host. But the host will ignore
> it anyway.
> 
> new guest, new host:
> works.
> 
> So...
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>

Yes, that sounds sane; I'll give it a try later.

However, I'd rather not want to rush this; I'd prefer to get the
initial version in first.

I'll do a v4 later.

Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.

[Michael S. Tsirkin]

On Tue, Feb 26, 2013 at 02:29:07PM +0100, Cornelia Huck wrote:
> On Tue, 26 Feb 2013 13:13:39 +0100
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
> > On 26/02/13 12:18, Michael S. Tsirkin wrote:
> > > virtio_ccw: pass a cookie value to kvm hypercall
> > > 
> > > Lookups by channel/vq pair on host during virtio notifications might be
> > > expensive.  Interpret hypercall return value as a cookie which host can
> > > use to do device lookups for the next notification more efficiently.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > 
> > > ---
> > > 
> > > diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
> > > index 2029b6c..1054f3a 100644
> > > --- a/drivers/s390/kvm/virtio_ccw.c
> > > +++ b/drivers/s390/kvm/virtio_ccw.c
> > > @@ -77,6 +77,7 @@ struct virtio_ccw_vq_info {
> > >  	void *queue;
> > >  	struct vq_info_block *info_block;
> > >  	struct list_head node;
> > > +	long cookie;
> > >  };
> > > 
> > >  #define KVM_VIRTIO_CCW_RING_ALIGN 4096
> > > @@ -145,15 +146,18 @@ static int ccw_io_helper(struct virtio_ccw_device *vcdev,
> > >  }
> > > 
> > >  static inline long do_kvm_notify(struct subchannel_id schid,
> > > -				 unsigned long queue_index)
> > > +				 unsigned long queue_index,
> > > +				 long cookie)
> > >  {
> > >  	register unsigned long __nr asm("1") = KVM_S390_VIRTIO_CCW_NOTIFY;
> > >  	register struct subchannel_id __schid asm("2") = schid;
> > >  	register unsigned long __index asm("3") = queue_index;
> > >  	register long __rc asm("2");
> > > +	register long __cookie asm("4") = cookie;
> > > 
> > >  	asm volatile ("diag 2,4,0x500\n"
> > > -		      : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index)
> > > +		      : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index),
> > > +		      "d"(__cookie)
> > >  		      : "memory", "cc");
> > >  	return __rc;
> > >  }
> > > @@ -166,7 +170,7 @@ static void virtio_ccw_kvm_notify(struct virtqueue *vq)
> > > 
> > >  	vcdev = to_vc_device(info->vq->vdev);
> > >  	ccw_device_get_schid(vcdev->cdev, &schid);
> > > -	do_kvm_notify(schid, virtqueue_get_queue_index(vq));
> > > +	info->cookie = do_kvm_notify(schid, virtqueue_get_queue_index(vq), info->cookie);
> > >  }
> > > 
> > >  static int virtio_ccw_read_vq_conf(struct virtio_ccw_device *vcdev,
> > 
> > 
> > Hmmm, forget my last mail. This actually could be even forward and backward compatible.
> > In the virtio spec we will not define the cookie format (just 64bit int). That will allow
> > qemu or future kernels to use that for other things (as long as a validity check is 
> > possible) if we dont have a kvm bus.
> > 
> > Now:
> > 
> > old guest, old host: 
> > works.
> > 
> > old guest, new host: 
> > the cookie from the guest contains junk, the host needs to detect that the cookie is 
> > junk and ignores it. It will return the new cookie anyway. 
> > 
> > new guest, old host:
> > The guest will get a junk cookie and pass it back to the host. But the host will ignore
> > it anyway.
> > 
> > new guest, new host:
> > works.
> > 
> > So...
> > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> 
> Yes, that sounds sane; I'll give it a try later.
> 
> However, I'd rather not want to rush this; I'd prefer to get the
> initial version in first.

Well planning to obsolete an interface from the start sounds wrong
somehow. We could always drop ccw in 3.9 if we feel we need more
time, but to me, this looks like a minor enough change to do even after
the merge window closed.

Want me to write you a spec patch too?

> I'll do a v4 later.

Right, just return 0 and it'll work.

-- 
MST

Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.

[Cornelia Huck]

On Tue, 26 Feb 2013 15:56:43 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Feb 26, 2013 at 02:29:07PM +0100, Cornelia Huck wrote:
> > On Tue, 26 Feb 2013 13:13:39 +0100
> > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> > 
> > > On 26/02/13 12:18, Michael S. Tsirkin wrote:
> > > > virtio_ccw: pass a cookie value to kvm hypercall
> > > > 
> > > > Lookups by channel/vq pair on host during virtio notifications might be
> > > > expensive.  Interpret hypercall return value as a cookie which host can
> > > > use to do device lookups for the next notification more efficiently.
> > > > 
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > 
> > > > ---
> > > > 
> > > > diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
> > > > index 2029b6c..1054f3a 100644
> > > > --- a/drivers/s390/kvm/virtio_ccw.c
> > > > +++ b/drivers/s390/kvm/virtio_ccw.c
> > > > @@ -77,6 +77,7 @@ struct virtio_ccw_vq_info {
> > > >  	void *queue;
> > > >  	struct vq_info_block *info_block;
> > > >  	struct list_head node;
> > > > +	long cookie;
> > > >  };
> > > > 
> > > >  #define KVM_VIRTIO_CCW_RING_ALIGN 4096
> > > > @@ -145,15 +146,18 @@ static int ccw_io_helper(struct virtio_ccw_device *vcdev,
> > > >  }
> > > > 
> > > >  static inline long do_kvm_notify(struct subchannel_id schid,
> > > > -				 unsigned long queue_index)
> > > > +				 unsigned long queue_index,
> > > > +				 long cookie)
> > > >  {
> > > >  	register unsigned long __nr asm("1") = KVM_S390_VIRTIO_CCW_NOTIFY;
> > > >  	register struct subchannel_id __schid asm("2") = schid;
> > > >  	register unsigned long __index asm("3") = queue_index;
> > > >  	register long __rc asm("2");
> > > > +	register long __cookie asm("4") = cookie;
> > > > 
> > > >  	asm volatile ("diag 2,4,0x500\n"
> > > > -		      : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index)
> > > > +		      : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index),
> > > > +		      "d"(__cookie)
> > > >  		      : "memory", "cc");
> > > >  	return __rc;
> > > >  }
> > > > @@ -166,7 +170,7 @@ static void virtio_ccw_kvm_notify(struct virtqueue *vq)
> > > > 
> > > >  	vcdev = to_vc_device(info->vq->vdev);
> > > >  	ccw_device_get_schid(vcdev->cdev, &schid);
> > > > -	do_kvm_notify(schid, virtqueue_get_queue_index(vq));
> > > > +	info->cookie = do_kvm_notify(schid, virtqueue_get_queue_index(vq), info->cookie);
> > > >  }
> > > > 
> > > >  static int virtio_ccw_read_vq_conf(struct virtio_ccw_device *vcdev,
> > > 
> > > 
> > > Hmmm, forget my last mail. This actually could be even forward and backward compatible.
> > > In the virtio spec we will not define the cookie format (just 64bit int). That will allow
> > > qemu or future kernels to use that for other things (as long as a validity check is 
> > > possible) if we dont have a kvm bus.
> > > 
> > > Now:
> > > 
> > > old guest, old host: 
> > > works.
> > > 
> > > old guest, new host: 
> > > the cookie from the guest contains junk, the host needs to detect that the cookie is 
> > > junk and ignores it. It will return the new cookie anyway. 
> > > 
> > > new guest, old host:
> > > The guest will get a junk cookie and pass it back to the host. But the host will ignore
> > > it anyway.
> > > 
> > > new guest, new host:
> > > works.
> > > 
> > > So...
> > > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > 
> > Yes, that sounds sane; I'll give it a try later.
> > 
> > However, I'd rather not want to rush this; I'd prefer to get the
> > initial version in first.
> 
> Well planning to obsolete an interface from the start sounds wrong
> somehow. We could always drop ccw in 3.9 if we feel we need more
> time, but to me, this looks like a minor enough change to do even after
> the merge window closed.

This was aimed at the exploitation; I'm fine with this patch for 3.9
after we let it mature for a day or two.

> 
> Want me to write you a spec patch too?

That would be great.

> 
> > I'll do a v4 later.
> 
> Right, just return 0 and it'll work.
> 

Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.

[Michael S. Tsirkin]

On Tue, Feb 26, 2013 at 01:13:39PM +0100, Christian Borntraeger wrote:
> On 26/02/13 12:18, Michael S. Tsirkin wrote:
> > virtio_ccw: pass a cookie value to kvm hypercall
> > 
> > Lookups by channel/vq pair on host during virtio notifications might be
> > expensive.  Interpret hypercall return value as a cookie which host can
> > use to do device lookups for the next notification more efficiently.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > ---
> > 
> > diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
> > index 2029b6c..1054f3a 100644
> > --- a/drivers/s390/kvm/virtio_ccw.c
> > +++ b/drivers/s390/kvm/virtio_ccw.c
> > @@ -77,6 +77,7 @@ struct virtio_ccw_vq_info {
> >  	void *queue;
> >  	struct vq_info_block *info_block;
> >  	struct list_head node;
> > +	long cookie;
> >  };
> > 
> >  #define KVM_VIRTIO_CCW_RING_ALIGN 4096
> > @@ -145,15 +146,18 @@ static int ccw_io_helper(struct virtio_ccw_device *vcdev,
> >  }
> > 
> >  static inline long do_kvm_notify(struct subchannel_id schid,
> > -				 unsigned long queue_index)
> > +				 unsigned long queue_index,
> > +				 long cookie)
> >  {
> >  	register unsigned long __nr asm("1") = KVM_S390_VIRTIO_CCW_NOTIFY;
> >  	register struct subchannel_id __schid asm("2") = schid;
> >  	register unsigned long __index asm("3") = queue_index;
> >  	register long __rc asm("2");
> > +	register long __cookie asm("4") = cookie;
> > 
> >  	asm volatile ("diag 2,4,0x500\n"
> > -		      : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index)
> > +		      : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index),
> > +		      "d"(__cookie)
> >  		      : "memory", "cc");
> >  	return __rc;
> >  }
> > @@ -166,7 +170,7 @@ static void virtio_ccw_kvm_notify(struct virtqueue *vq)
> > 
> >  	vcdev = to_vc_device(info->vq->vdev);
> >  	ccw_device_get_schid(vcdev->cdev, &schid);
> > -	do_kvm_notify(schid, virtqueue_get_queue_index(vq));
> > +	info->cookie = do_kvm_notify(schid, virtqueue_get_queue_index(vq), info->cookie);
> >  }
> > 
> >  static int virtio_ccw_read_vq_conf(struct virtio_ccw_device *vcdev,
> 
> 
> Hmmm, forget my last mail. This actually could be even forward and backward compatible.
> In the virtio spec we will not define the cookie format (just 64bit int). That will allow
> qemu or future kernels to use that for other things (as long as a validity check is 
> possible) if we dont have a kvm bus.
> 
> Now:
> 
> old guest, old host: 
> works.
> 
> old guest, new host: 
> the cookie from the guest contains junk, the host needs to detect that the cookie is 
> junk and ignores it. It will return the new cookie anyway. 
> 
> new guest, old host:
> The guest will get a junk cookie and pass it back to the host. But the host will ignore
> it anyway.
> 
> new guest, new host:
> works.
> 
> So...
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>

So let's apply the patch for 3.9 and avoid caring about
"old guests" much?

-- 
MST

Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.

[Christian Borntraeger]

On 26/02/13 14:41, Michael S. Tsirkin wrote:
>> So...
>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> 
> So let's apply the patch for 3.9 and

Give us 1 or 2 days testing for regression and then this can go for 3.9.
The host changes can then be deferred to a later point in time.

> avoid caring about "old guests" much?

Well caring about old guests is related to caring about malicious guests.
Maybe we should make it explicit in the spec that the token can be wrong
or omitted. Just to avoid that anybody starts to optimize things too
aggressive. 

Ok?

Christian

Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.

[Michael S. Tsirkin]

On Tue, Feb 26, 2013 at 02:48:38PM +0100, Christian Borntraeger wrote:
> On 26/02/13 14:41, Michael S. Tsirkin wrote:
> >> So...
> >> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > 
> > So let's apply the patch for 3.9 and
> 
> Give us 1 or 2 days testing for regression and then this can go for 3.9.
> The host changes can then be deferred to a later point in time.

Nod.

> > avoid caring about "old guests" much?
> 
> Well caring about old guests is related to caring about malicious guests.
> Maybe we should make it explicit in the spec that the token can be wrong
> or omitted. Just to avoid that anybody starts to optimize things too
> aggressive. 
> 
> Ok?
> 
> Christian

Absolutely, we can't trust the guest anyway. It's just an optimization
hint, must validate and if it fails do a linear lookup.

-- 
MST

Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.

[Christian Borntraeger]

On 26/02/13 12:18, Michael S. Tsirkin wrote:

> virtio_ccw: pass a cookie value to kvm hypercall
> 
> Lookups by channel/vq pair on host during virtio notifications might be
> expensive.  Interpret hypercall return value as a cookie which host can
> use to do device lookups for the next notification more efficiently.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Seems to work fine. (as expected).
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>

> 
> ---
> 
> diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
> index 2029b6c..1054f3a 100644
> --- a/drivers/s390/kvm/virtio_ccw.c
> +++ b/drivers/s390/kvm/virtio_ccw.c
> @@ -77,6 +77,7 @@ struct virtio_ccw_vq_info {
>  	void *queue;
>  	struct vq_info_block *info_block;
>  	struct list_head node;
> +	long cookie;
>  };
> 
>  #define KVM_VIRTIO_CCW_RING_ALIGN 4096
> @@ -145,15 +146,18 @@ static int ccw_io_helper(struct virtio_ccw_device *vcdev,
>  }
> 
>  static inline long do_kvm_notify(struct subchannel_id schid,
> -				 unsigned long queue_index)
> +				 unsigned long queue_index,
> +				 long cookie)
>  {
>  	register unsigned long __nr asm("1") = KVM_S390_VIRTIO_CCW_NOTIFY;
>  	register struct subchannel_id __schid asm("2") = schid;
>  	register unsigned long __index asm("3") = queue_index;
>  	register long __rc asm("2");
> +	register long __cookie asm("4") = cookie;
> 
>  	asm volatile ("diag 2,4,0x500\n"
> -		      : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index)
> +		      : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index),
> +		      "d"(__cookie)
>  		      : "memory", "cc");
>  	return __rc;
>  }
> @@ -166,7 +170,7 @@ static void virtio_ccw_kvm_notify(struct virtqueue *vq)
> 
>  	vcdev = to_vc_device(info->vq->vdev);
>  	ccw_device_get_schid(vcdev->cdev, &schid);
> -	do_kvm_notify(schid, virtqueue_get_queue_index(vq));
> +	info->cookie = do_kvm_notify(schid, virtqueue_get_queue_index(vq), info->cookie);
>  }
> 
>  static int virtio_ccw_read_vq_conf(struct virtio_ccw_device *vcdev,
> 

virtio-s390: document GPR4/GPR2 cookie values

[Michael S. Tsirkin]

virtio-s390 on kvm can use a cookie value passed to guest
to optimize channel/VQ lookups.
Document this.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---
diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index 72d956c..91aed06 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -10627,7 +10626,252 @@ Guest->Host Notification
 For notifying the host of virtqueue buffers, the guest unfortunately can't
  use a channel command (the asynchronous characteristics of channel I/O
  interact badly with the host block I/O backend).
- Instead, it uses a diagnose 0x500 call with subcode 3 specifying the queue.
+ Instead, it uses a diagnose 0x500 call with subcode 3 specifying the queue
+\change_inserted 1986246365 1362677938
+, as follows:
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 1986246365 1362677973
+\begin_inset Tabular
+<lyxtabular version="3" rows="5" columns="3">
+<features tabularvalignment="middle">
+<column alignment="center" valignment="top" width="0">
+<column alignment="center" valignment="top" width="0">
+<column alignment="center" valignment="top" width="0">
+<row>
+<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362677978
+GPR
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678629
+Input Value
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" rightline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678627
+Output Value
+\end_layout
+
+\end_inset
+</cell>
+</row>
+<row>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362677991
+1
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678208
+0x3
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" rightline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678620
+
+\end_layout
+
+\end_inset
+</cell>
+</row>
+<row>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678226
+2
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678498
+Subchannel ID
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" rightline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678642
+Host Cookie
+\end_layout
+
+\end_inset
+</cell>
+</row>
+<row>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678500
+3
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678540
+Virtqueue number
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" rightline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678620
+
+\end_layout
+
+\end_inset
+</cell>
+</row>
+<row>
+<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678529
+4
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678644
+Host Cookie
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" rightline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678620
+
+\end_layout
+
+\end_inset
+</cell>
+</row>
+</lyxtabular>
+
+\end_inset
+
+
+\change_deleted 1986246365 1362677938
+.
+\change_inserted 1986246365 1362678646
+
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 1986246365 1362679010
+Host cookie is an optional per-virtqueue 64 bit value that can be used by
+ the hypervisor to speed up the notification execution.
+ For each notification, the output value is returned in GPR2 and should
+ be passed in GPR4 for the next notification:
+\end_layout
+
+\begin_layout LyX-Code
+
+\change_inserted 1986246365 1362679011
+\begin_inset listings
+inline false
+status open
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362679033
+
+info->cookie = do_notify(schid, virtqueue_get_queue_index(vq), info->cookie);
+\end_layout
+
+\end_inset
+
+
 \end_layout
 
 \begin_layout Subsubsection*

Re: virtio-s390: document GPR4/GPR2 cookie values

[Cornelia Huck]

On Thu, 7 Mar 2013 20:02:21 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> virtio-s390 on kvm can use a cookie value passed to guest

s/virtio-s390/virtio-ccw/ (to avoid confusion with s390-virtio, which
was never specced)

> to optimize channel/VQ lookups.
> Document this.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Otherwise:
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

Rusty, could you please apply?

> 
> ---
> diff --git a/virtio-spec.lyx b/virtio-spec.lyx
> index 72d956c..91aed06 100644
> --- a/virtio-spec.lyx
> +++ b/virtio-spec.lyx
> @@ -10627,7 +10626,252 @@ Guest->Host Notification
>  For notifying the host of virtqueue buffers, the guest unfortunately can't
>   use a channel command (the asynchronous characteristics of channel I/O
>   interact badly with the host block I/O backend).
> - Instead, it uses a diagnose 0x500 call with subcode 3 specifying the queue.
> + Instead, it uses a diagnose 0x500 call with subcode 3 specifying the queue
> +\change_inserted 1986246365 1362677938
> +, as follows:
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 1986246365 1362677973
> +\begin_inset Tabular
> +<lyxtabular version="3" rows="5" columns="3">
> +<features tabularvalignment="middle">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<row>
> +<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362677978
> +GPR
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678629
> +Input Value
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" rightline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678627
> +Output Value
> +\end_layout
> +
> +\end_inset
> +</cell>
> +</row>
> +<row>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362677991
> +1
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678208
> +0x3
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" rightline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678620
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +</row>
> +<row>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678226
> +2
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678498
> +Subchannel ID
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" rightline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678642
> +Host Cookie
> +\end_layout
> +
> +\end_inset
> +</cell>
> +</row>
> +<row>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678500
> +3
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678540
> +Virtqueue number
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" rightline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678620
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +</row>
> +<row>
> +<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678529
> +4
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678644
> +Host Cookie
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" rightline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678620
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +</row>
> +</lyxtabular>
> +
> +\end_inset
> +
> +
> +\change_deleted 1986246365 1362677938
> +.
> +\change_inserted 1986246365 1362678646
> +
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 1986246365 1362679010
> +Host cookie is an optional per-virtqueue 64 bit value that can be used by
> + the hypervisor to speed up the notification execution.
> + For each notification, the output value is returned in GPR2 and should
> + be passed in GPR4 for the next notification:
> +\end_layout
> +
> +\begin_layout LyX-Code
> +
> +\change_inserted 1986246365 1362679011
> +\begin_inset listings
> +inline false
> +status open
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362679033
> +
> +info->cookie = do_notify(schid, virtqueue_get_queue_index(vq), info->cookie);
> +\end_layout
> +
> +\end_inset
> +
> +
>  \end_layout
> 
>  \begin_layout Subsubsection*
> 

Re: virtio-s390: document GPR4/GPR2 cookie values

[Rusty Russell]

Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> On Thu, 7 Mar 2013 20:02:21 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
>> virtio-s390 on kvm can use a cookie value passed to guest
>
> s/virtio-s390/virtio-ccw/ (to avoid confusion with s390-virtio, which
> was never specced)
>
>> to optimize channel/VQ lookups.
>> Document this.
>> 
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Otherwise:
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>
> Rusty, could you please apply?

Modified and committed.

Thanks!
Rusty.

Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.

[Christian Borntraeger]

On 26/02/13 12:04, Michael S. Tsirkin wrote:
> On Mon, Feb 25, 2013 at 04:27:45PM +0100, Cornelia Huck wrote:
>> Here's the latest version of my patch series enabling ioeventfds
>> on s390, again against kvm-next.
>>
>> Patches 1 and 2 (cleaning up initialization and exporting the virtio-ccw
>> api) would make sense even independent of the ioeventfd enhancements.
>>
>> Patches 3-5 are concerned with adding a new type of ioeventfds for
>> virtio-ccw notifications on s390. The naming is now hopefully clearer.
>> We won't add ioeventfd support for the legacy s390-virtio transport.
>>
>> Please consider applying.
> 
> I just had a thought: this makes us lookup the device on the bus
> for each notification. It would be better to simply get the
> device index from guest instead.
> 
> We could validate that it matches the correct device,
> if not - fallback to the current linear scan.
> 
> We could return the index to guest for the next call.
> 
> I know this needs guest changes but it's still not too late to
> fix this for 3.9 guests so that we won't need to worry
> about compatibility going forward.
> 

Hmm, this would certainly have the best scalability, but such
a change would require adotions to the virtio spec and getting this
fully tested till 3.9 seems somewhat dangerous. 
So I would prefer to actually improve the lookup (e.g. with a tree or hash)
in kvm_io_bus_write or something like that as a first step.

We also have some guests in the wild  (sles11sp3 beta) that already
use the current interface, so compatibility is already a an interesting
aspect (does being a beta counts?)

Just thinking loud here, we might be able to do this in a compatible way.
The vq number that we pass is 64bit, but we dont need such a big range. The
guest could use the upper 32 bit as a cookie (old code will have 0 ---> list
traversal). We must have a query cookie function, that will return 0 on
older qemus, though. Such an optimization could be added later on without
needing to rush.

Ideas, opinions?

Christian