[PATCH v4 0/4] s390: Guest support for virtio-ccw.

[PATCH v4 0/4] s390: Guest support for virtio-ccw.

[Cornelia Huck]

Hi,

here's the next iteration of the virtio-ccw guest support patches,
hopefully ready for kvm/next now.

Changes from v3:
- The virtio console early printk support remains confined to
  the old s390-virtio transport.
- virtio-ccw passes the subchannel id directly in the register
  instead of casting.

Cornelia Huck (4):
  KVM: s390: Handle hosts not supporting s390-virtio.
  s390: Move css limits from drivers/s390/cio/ to include/asm/.
  s390: Add a mechanism to get the subchannel id.
  KVM: s390: Add a channel I/O based virtio transport driver.

 arch/s390/include/asm/ccwdev.h |   5 +
 arch/s390/include/asm/cio.h    |   2 +
 arch/s390/include/asm/irq.h    |   1 +
 arch/s390/kernel/irq.c         |   1 +
 drivers/s390/cio/css.h         |   3 -
 drivers/s390/cio/device_ops.c  |  12 +
 drivers/s390/kvm/Makefile      |   2 +-
 drivers/s390/kvm/kvm_virtio.c  |  38 +-
 drivers/s390/kvm/virtio_ccw.c  | 854 +++++++++++++++++++++++++++++++++++++++++
 9 files changed, 906 insertions(+), 12 deletions(-)
 create mode 100644 drivers/s390/kvm/virtio_ccw.c

-- 
1.7.12.4

[PATCH 1/4] KVM: s390: Handle hosts not supporting s390-virtio.

[Cornelia Huck]

Running under a kvm host does not necessarily imply the presence of
a page mapped above the main memory with the virtio information;
however, the code includes a hard coded access to that page.

Instead, check for the presence of the page and exit gracefully
before we hit an addressing exception if it does not exist.

Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
cc: stable@vger.kernel.org
---
 drivers/s390/kvm/kvm_virtio.c | 38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index 7dabef6..b846b6c 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -422,6 +422,26 @@ static void kvm_extint_handler(struct ext_code ext_code,
 }
 
 /*
+ * For s390-virtio, we expect a page above main storage containing
+ * the virtio configuration. Try to actually load from this area
+ * in order to figure out if the host provides this page.
+ */
+static int __init test_devices_support(unsigned long addr)
+{
+	int ret = -EIO;
+
+	asm volatile(
+		"0:	lura	0,%1\n"
+		"1:	xgr	%0,%0\n"
+		"2:\n"
+		EX_TABLE(0b,2b)
+		EX_TABLE(1b,2b)
+		: "+d" (ret)
+		: "a" (addr)
+		: "0", "cc");
+	return ret;
+}
+/*
  * Init function for virtio
  * devices are in a single page above top of "normal" mem
  */
@@ -432,21 +452,23 @@ static int __init kvm_devices_init(void)
 	if (!MACHINE_IS_KVM)
 		return -ENODEV;
 
+	if (test_devices_support(real_memory_size) < 0)
+		return -ENODEV;
+
+	rc = vmem_add_mapping(real_memory_size, PAGE_SIZE);
+	if (rc)
+		return rc;
+
+	kvm_devices = (void *) real_memory_size;
+
 	kvm_root = root_device_register("kvm_s390");
 	if (IS_ERR(kvm_root)) {
 		rc = PTR_ERR(kvm_root);
 		printk(KERN_ERR "Could not register kvm_s390 root device");
+		vmem_remove_mapping(real_memory_size, PAGE_SIZE);
 		return rc;
 	}
 
-	rc = vmem_add_mapping(real_memory_size, PAGE_SIZE);
-	if (rc) {
-		root_device_unregister(kvm_root);
-		return rc;
-	}
-
-	kvm_devices = (void *) real_memory_size;
-
 	INIT_WORK(&hotplug_work, hotplug_devices);
 
 	service_subclass_irq_register();
-- 
1.7.12.4

[PATCH 2/4] s390: Move css limits from drivers/s390/cio/ to include/asm/.

[Cornelia Huck]

There's no need to keep __MAX_SUBCHANNEL and __MAX_SSID private to the
common I/O layer when __MAX_CSSID is usable by everybody.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 arch/s390/include/asm/cio.h | 2 ++
 drivers/s390/cio/css.h      | 3 ---
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/s390/include/asm/cio.h b/arch/s390/include/asm/cio.h
index 55bde60..ad2b924 100644
--- a/arch/s390/include/asm/cio.h
+++ b/arch/s390/include/asm/cio.h
@@ -9,6 +9,8 @@
 
 #define LPM_ANYPATH 0xff
 #define __MAX_CSSID 0
+#define __MAX_SUBCHANNEL 65535
+#define __MAX_SSID 3
 
 #include <asm/scsw.h>
 
diff --git a/drivers/s390/cio/css.h b/drivers/s390/cio/css.h
index 33bb4d8..4af3dfe 100644
--- a/drivers/s390/cio/css.h
+++ b/drivers/s390/cio/css.h
@@ -112,9 +112,6 @@ extern int for_each_subchannel(int(*fn)(struct subchannel_id, void *), void *);
 extern void css_reiterate_subchannels(void);
 void css_update_ssd_info(struct subchannel *sch);
 
-#define __MAX_SUBCHANNEL 65535
-#define __MAX_SSID 3
-
 struct channel_subsystem {
 	u8 cssid;
 	int valid;
-- 
1.7.12.4

[PATCH 3/4] s390: Add a mechanism to get the subchannel id.

[Cornelia Huck]

This will be needed by the new virtio-ccw transport.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 arch/s390/include/asm/ccwdev.h |  5 +++++
 drivers/s390/cio/device_ops.c  | 12 ++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/arch/s390/include/asm/ccwdev.h b/arch/s390/include/asm/ccwdev.h
index 1cb4bb3..9ad79f7 100644
--- a/arch/s390/include/asm/ccwdev.h
+++ b/arch/s390/include/asm/ccwdev.h
@@ -18,6 +18,9 @@ struct irb;
 struct ccw1;
 struct ccw_dev_id;
 
+/* from asm/schid.h */
+struct subchannel_id;
+
 /* simplified initializers for struct ccw_device:
  * CCW_DEVICE and CCW_DEVICE_DEVTYPE initialize one
  * entry in your MODULE_DEVICE_TABLE and set the match_flag correctly */
@@ -226,5 +229,7 @@ int ccw_device_siosl(struct ccw_device *);
 // FIXME: these have to go
 extern int _ccw_device_get_subchannel_number(struct ccw_device *);
 
+extern void ccw_device_get_schid(struct ccw_device *, struct subchannel_id *);
+
 extern void *ccw_device_get_chp_desc(struct ccw_device *, int);
 #endif /* _S390_CCWDEV_H_ */
diff --git a/drivers/s390/cio/device_ops.c b/drivers/s390/cio/device_ops.c
index ec7fb6d..2ad832f 100644
--- a/drivers/s390/cio/device_ops.c
+++ b/drivers/s390/cio/device_ops.c
@@ -763,6 +763,18 @@ _ccw_device_get_subchannel_number(struct ccw_device *cdev)
 	return cdev->private->schid.sch_no;
 }
 
+/**
+ * ccw_device_get_schid - obtain a subchannel id
+ * @cdev: device to obtain the id for
+ * @schid: where to fill in the values
+ */
+void ccw_device_get_schid(struct ccw_device *cdev, struct subchannel_id *schid)
+{
+	struct subchannel *sch = to_subchannel(cdev->dev.parent);
+
+	*schid = sch->schid;
+}
+EXPORT_SYMBOL_GPL(ccw_device_get_schid);
 
 MODULE_LICENSE("GPL");
 EXPORT_SYMBOL(ccw_device_set_options_mask);
-- 
1.7.12.4

[PATCH 4/4] KVM: s390: Add a channel I/O based virtio transport driver.

[Cornelia Huck]

Add a driver for kvm guests that matches virtual ccw devices provided
by the host as virtio bridge devices.

These virtio-ccw devices use a special set of channel commands in order
to perform virtio functions.

Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 arch/s390/include/asm/irq.h   |   1 +
 arch/s390/kernel/irq.c        |   1 +
 drivers/s390/kvm/Makefile     |   2 +-
 drivers/s390/kvm/virtio_ccw.c | 854 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 857 insertions(+), 1 deletion(-)
 create mode 100644 drivers/s390/kvm/virtio_ccw.c

diff --git a/arch/s390/include/asm/irq.h b/arch/s390/include/asm/irq.h
index 6703dd9..ad2ad6b 100644
--- a/arch/s390/include/asm/irq.h
+++ b/arch/s390/include/asm/irq.h
@@ -33,6 +33,7 @@ enum interruption_class {
 	IOINT_APB,
 	IOINT_ADM,
 	IOINT_CSC,
+	IOINT_VIR,
 	NMI_NMI,
 	NR_IRQS,
 };
diff --git a/arch/s390/kernel/irq.c b/arch/s390/kernel/irq.c
index 6cdc55b..97c171a 100644
--- a/arch/s390/kernel/irq.c
+++ b/arch/s390/kernel/irq.c
@@ -58,6 +58,7 @@ static const struct irq_class intrclass_names[] = {
 	[IOINT_APB]  = {.name = "APB", .desc = "[I/O] AP Bus"},
 	[IOINT_ADM]  = {.name = "ADM", .desc = "[I/O] EADM Subchannel"},
 	[IOINT_CSC]  = {.name = "CSC", .desc = "[I/O] CHSC Subchannel"},
+	[IOINT_VIR]  = {.name = "VIR", .desc = "[I/O] Virtual I/O Devices"},
 	[NMI_NMI]    = {.name = "NMI", .desc = "[NMI] Machine Check"},
 };
 
diff --git a/drivers/s390/kvm/Makefile b/drivers/s390/kvm/Makefile
index 0815690..241891a 100644
--- a/drivers/s390/kvm/Makefile
+++ b/drivers/s390/kvm/Makefile
@@ -6,4 +6,4 @@
 # it under the terms of the GNU General Public License (version 2 only)
 # as published by the Free Software Foundation.
 
-obj-$(CONFIG_S390_GUEST) += kvm_virtio.o
+obj-$(CONFIG_S390_GUEST) += kvm_virtio.o virtio_ccw.o
diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
new file mode 100644
index 0000000..7f1a0c6
--- /dev/null
+++ b/drivers/s390/kvm/virtio_ccw.c
@@ -0,0 +1,854 @@
+/*
+ * ccw based virtio transport
+ *
+ * Copyright IBM Corp. 2012
+ *
+ * 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>
+ */
+
+#include <linux/kernel_stat.h>
+#include <linux/init.h>
+#include <linux/bootmem.h>
+#include <linux/err.h>
+#include <linux/virtio.h>
+#include <linux/virtio_config.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/virtio_ring.h>
+#include <linux/pfn.h>
+#include <linux/async.h>
+#include <linux/wait.h>
+#include <linux/list.h>
+#include <linux/bitops.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/kvm_para.h>
+#include <asm/setup.h>
+#include <asm/irq.h>
+#include <asm/cio.h>
+#include <asm/ccwdev.h>
+#include <asm/schid.h>
+
+/*
+ * virtio related functions
+ */
+
+struct vq_config_block {
+	__u16 index;
+	__u16 num;
+} __packed;
+
+#define VIRTIO_CCW_CONFIG_SIZE 0x100
+/* same as PCI config space size, should be enough for all drivers */
+
+struct virtio_ccw_device {
+	struct virtio_device vdev;
+	__u8 status;
+	__u8 config[VIRTIO_CCW_CONFIG_SIZE];
+	struct ccw_device *cdev;
+	struct ccw1 *ccw;
+	__u32 area;
+	__u32 curr_io;
+	int err;
+	wait_queue_head_t wait_q;
+	spinlock_t lock;
+	struct list_head virtqueues;
+	unsigned long indicators;
+	unsigned long indicators2;
+	struct vq_config_block *config_block;
+};
+
+struct vq_info_block {
+	__u64 queue;
+	__u32 align;
+	__u16 index;
+	__u16 num;
+} __packed;
+
+struct virtio_feature_desc {
+	__u32 features;
+	__u8 index;
+} __packed;
+
+struct virtio_ccw_vq_info {
+	struct virtqueue *vq;
+	int num;
+	void *queue;
+	struct vq_info_block *info_block;
+	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
+#define CCW_CMD_SET_CONF_IND 0x53
+#define CCW_CMD_READ_FEAT 0x12
+#define CCW_CMD_WRITE_FEAT 0x11
+#define CCW_CMD_READ_CONF 0x22
+#define CCW_CMD_WRITE_CONF 0x21
+#define CCW_CMD_WRITE_STATUS 0x31
+#define CCW_CMD_READ_VQ_CONF 0x32
+
+#define VIRTIO_CCW_DOING_SET_VQ 0x00010000
+#define VIRTIO_CCW_DOING_RESET 0x00040000
+#define VIRTIO_CCW_DOING_READ_FEAT 0x00080000
+#define VIRTIO_CCW_DOING_WRITE_FEAT 0x00100000
+#define VIRTIO_CCW_DOING_READ_CONFIG 0x00200000
+#define VIRTIO_CCW_DOING_WRITE_CONFIG 0x00400000
+#define VIRTIO_CCW_DOING_WRITE_STATUS 0x00800000
+#define VIRTIO_CCW_DOING_SET_IND 0x01000000
+#define VIRTIO_CCW_DOING_READ_VQ_CONF 0x02000000
+#define VIRTIO_CCW_DOING_SET_CONF_IND 0x04000000
+#define VIRTIO_CCW_INTPARM_MASK 0xffff0000
+
+static struct virtio_ccw_device *to_vc_device(struct virtio_device *vdev)
+{
+	return container_of(vdev, struct virtio_ccw_device, vdev);
+}
+
+static int doing_io(struct virtio_ccw_device *vcdev, __u32 flag)
+{
+	unsigned long flags;
+	__u32 ret;
+
+	spin_lock_irqsave(get_ccwdev_lock(vcdev->cdev), flags);
+	if (vcdev->err)
+		ret = 0;
+	else
+		ret = vcdev->curr_io & flag;
+	spin_unlock_irqrestore(get_ccwdev_lock(vcdev->cdev), flags);
+	return ret;
+}
+
+static int ccw_io_helper(struct virtio_ccw_device *vcdev, __u32 intparm)
+{
+	int ret;
+	unsigned long flags;
+	int flag = intparm & VIRTIO_CCW_INTPARM_MASK;
+
+	spin_lock_irqsave(get_ccwdev_lock(vcdev->cdev), flags);
+	ret = ccw_device_start(vcdev->cdev, vcdev->ccw, intparm, 0, 0);
+	if (!ret)
+		vcdev->curr_io |= flag;
+	spin_unlock_irqrestore(get_ccwdev_lock(vcdev->cdev), flags);
+	wait_event(vcdev->wait_q, doing_io(vcdev, flag) == 0);
+	return ret ? ret : vcdev->err;
+}
+
+static inline long do_kvm_notify(struct subchannel_id schid,
+				 unsigned long queue_index)
+{
+	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");
+
+	asm volatile ("diag 2,4,0x500\n"
+		      : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index)
+		      : "memory", "cc");
+	return __rc;
+}
+
+static void virtio_ccw_kvm_notify(struct virtqueue *vq)
+{
+	struct virtio_ccw_vq_info *info = vq->priv;
+	struct virtio_ccw_device *vcdev;
+	struct subchannel_id schid;
+
+	vcdev = to_vc_device(info->vq->vdev);
+	ccw_device_get_schid(vcdev->cdev, &schid);
+	do_kvm_notify(schid, virtqueue_get_queue_index(vq));
+}
+
+static int virtio_ccw_read_vq_conf(struct virtio_ccw_device *vcdev, int index)
+{
+	vcdev->config_block->index = index;
+	vcdev->ccw->cmd_code = CCW_CMD_READ_VQ_CONF;
+	vcdev->ccw->flags = 0;
+	vcdev->ccw->count = sizeof(struct vq_config_block);
+	vcdev->ccw->cda = (__u32)(unsigned long)(vcdev->config_block);
+	ccw_io_helper(vcdev, VIRTIO_CCW_DOING_READ_VQ_CONF);
+	return vcdev->config_block->num;
+}
+
+static void virtio_ccw_del_vq(struct virtqueue *vq)
+{
+	struct virtio_ccw_device *vcdev = to_vc_device(vq->vdev);
+	struct virtio_ccw_vq_info *info = vq->priv;
+	unsigned long flags;
+	unsigned long size;
+	int ret;
+	unsigned int index = virtqueue_get_queue_index(vq);
+
+	/* Remove from our list. */
+	spin_lock_irqsave(&vcdev->lock, flags);
+	list_del(&info->node);
+	spin_unlock_irqrestore(&vcdev->lock, flags);
+
+	/* Release from host. */
+	info->info_block->queue = 0;
+	info->info_block->align = 0;
+	info->info_block->index = index;
+	info->info_block->num = 0;
+	vcdev->ccw->cmd_code = CCW_CMD_SET_VQ;
+	vcdev->ccw->flags = 0;
+	vcdev->ccw->count = sizeof(*info->info_block);
+	vcdev->ccw->cda = (__u32)(unsigned long)(info->info_block);
+	ret = ccw_io_helper(vcdev, VIRTIO_CCW_DOING_SET_VQ | index);
+	/*
+	 * -ENODEV isn't considered an error: The device is gone anyway.
+	 * This may happen on device detach.
+	 */
+	if (ret && (ret != -ENODEV))
+		dev_warn(&vq->vdev->dev, "Error %d while deleting queue %d",
+			 ret, index);
+
+	vring_del_virtqueue(vq);
+	size = PAGE_ALIGN(vring_size(info->num, KVM_VIRTIO_CCW_RING_ALIGN));
+	free_pages_exact(info->queue, size);
+	kfree(info->info_block);
+	kfree(info);
+}
+
+static void virtio_ccw_del_vqs(struct virtio_device *vdev)
+{
+	struct virtqueue *vq, *n;
+
+	list_for_each_entry_safe(vq, n, &vdev->vqs, list)
+		virtio_ccw_del_vq(vq);
+}
+
+static struct virtqueue *virtio_ccw_setup_vq(struct virtio_device *vdev,
+					     int i, vq_callback_t *callback,
+					     const char *name)
+{
+	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
+	int err;
+	struct virtqueue *vq;
+	struct virtio_ccw_vq_info *info;
+	unsigned long size;
+	unsigned long flags;
+
+	/* Allocate queue. */
+	info = kzalloc(sizeof(struct virtio_ccw_vq_info), GFP_KERNEL);
+	if (!info) {
+		dev_warn(&vcdev->cdev->dev, "no info\n");
+		err = -ENOMEM;
+		goto out_err;
+	}
+	info->info_block = kzalloc(sizeof(*info->info_block),
+				   GFP_DMA | GFP_KERNEL);
+	if (!info->info_block) {
+		dev_warn(&vcdev->cdev->dev, "no info block\n");
+		err = -ENOMEM;
+		goto out_err;
+	}
+	info->num = virtio_ccw_read_vq_conf(vcdev, i);
+	size = PAGE_ALIGN(vring_size(info->num, KVM_VIRTIO_CCW_RING_ALIGN));
+	info->queue = alloc_pages_exact(size, GFP_KERNEL | __GFP_ZERO);
+	if (info->queue == NULL) {
+		dev_warn(&vcdev->cdev->dev, "no queue\n");
+		err = -ENOMEM;
+		goto out_err;
+	}
+
+	vq = vring_new_virtqueue(i, info->num, KVM_VIRTIO_CCW_RING_ALIGN, vdev,
+				 true, info->queue, virtio_ccw_kvm_notify,
+				 callback, name);
+	if (!vq) {
+		/* For now, we fail if we can't get the requested size. */
+		dev_warn(&vcdev->cdev->dev, "no vq\n");
+		err = -ENOMEM;
+		free_pages_exact(info->queue, size);
+		goto out_err;
+	}
+	info->vq = vq;
+	vq->priv = info;
+
+	/* Register it with the host. */
+	info->info_block->queue = (__u64)info->queue;
+	info->info_block->align = KVM_VIRTIO_CCW_RING_ALIGN;
+	info->info_block->index = i;
+	info->info_block->num = info->num;
+	vcdev->ccw->cmd_code = CCW_CMD_SET_VQ;
+	vcdev->ccw->flags = 0;
+	vcdev->ccw->count = sizeof(*info->info_block);
+	vcdev->ccw->cda = (__u32)(unsigned long)(info->info_block);
+	err = ccw_io_helper(vcdev, VIRTIO_CCW_DOING_SET_VQ | i);
+	if (err) {
+		dev_warn(&vcdev->cdev->dev, "SET_VQ failed\n");
+		free_pages_exact(info->queue, size);
+		info->vq = NULL;
+		vq->priv = NULL;
+		goto out_err;
+	}
+
+	/* Save it to our list. */
+	spin_lock_irqsave(&vcdev->lock, flags);
+	list_add(&info->node, &vcdev->virtqueues);
+	spin_unlock_irqrestore(&vcdev->lock, flags);
+
+	return vq;
+
+out_err:
+	if (info)
+		kfree(info->info_block);
+	kfree(info);
+	return ERR_PTR(err);
+}
+
+static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs,
+			       struct virtqueue *vqs[],
+			       vq_callback_t *callbacks[],
+			       const char *names[])
+{
+	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
+	unsigned long *indicatorp = NULL;
+	int ret, i;
+
+	for (i = 0; i < nvqs; ++i) {
+		vqs[i] = virtio_ccw_setup_vq(vdev, i, callbacks[i], names[i]);
+		if (IS_ERR(vqs[i])) {
+			ret = PTR_ERR(vqs[i]);
+			vqs[i] = NULL;
+			goto out;
+		}
+	}
+	ret = -ENOMEM;
+	/* We need a data area under 2G to communicate. */
+	indicatorp = kmalloc(sizeof(&vcdev->indicators), GFP_DMA | GFP_KERNEL);
+	if (!indicatorp)
+		goto out;
+	*indicatorp = (unsigned long) &vcdev->indicators;
+	/* Register queue indicators with host. */
+	vcdev->indicators = 0;
+	vcdev->ccw->cmd_code = CCW_CMD_SET_IND;
+	vcdev->ccw->flags = 0;
+	vcdev->ccw->count = sizeof(vcdev->indicators);
+	vcdev->ccw->cda = (__u32)(unsigned long) indicatorp;
+	ret = ccw_io_helper(vcdev, VIRTIO_CCW_DOING_SET_IND);
+	if (ret)
+		goto out;
+	/* Register indicators2 with host for config changes */
+	*indicatorp = (unsigned long) &vcdev->indicators2;
+	vcdev->indicators2 = 0;
+	vcdev->ccw->cmd_code = CCW_CMD_SET_CONF_IND;
+	vcdev->ccw->flags = 0;
+	vcdev->ccw->count = sizeof(vcdev->indicators2);
+	vcdev->ccw->cda = (__u32)(unsigned long) indicatorp;
+	ret = ccw_io_helper(vcdev, VIRTIO_CCW_DOING_SET_CONF_IND);
+	if (ret)
+		goto out;
+
+	kfree(indicatorp);
+	return 0;
+out:
+	kfree(indicatorp);
+	virtio_ccw_del_vqs(vdev);
+	return ret;
+}
+
+static void virtio_ccw_reset(struct virtio_device *vdev)
+{
+	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
+
+	/* Zero status bits. */
+	vcdev->status = 0;
+
+	/* Send a reset ccw on device. */
+	vcdev->ccw->cmd_code = CCW_CMD_VDEV_RESET;
+	vcdev->ccw->flags = 0;
+	vcdev->ccw->count = 0;
+	vcdev->ccw->cda = 0;
+	ccw_io_helper(vcdev, VIRTIO_CCW_DOING_RESET);
+}
+
+static u32 virtio_ccw_get_features(struct virtio_device *vdev)
+{
+	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
+	struct virtio_feature_desc features;
+	int ret;
+
+	/* Read the feature bits from the host. */
+	/* TODO: Features > 32 bits */
+	features.index = 0;
+	vcdev->ccw->cmd_code = CCW_CMD_READ_FEAT;
+	vcdev->ccw->flags = 0;
+	vcdev->ccw->count = sizeof(features);
+	vcdev->ccw->cda = vcdev->area;
+	ret = ccw_io_helper(vcdev, VIRTIO_CCW_DOING_READ_FEAT);
+	if (ret)
+		return 0;
+
+	memcpy(&features, (void *)(unsigned long)vcdev->area,
+	       sizeof(features));
+	return le32_to_cpu(features.features);
+}
+
+static void virtio_ccw_finalize_features(struct virtio_device *vdev)
+{
+	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
+	struct virtio_feature_desc features;
+	int i;
+
+	/* Give virtio_ring a chance to accept features. */
+	vring_transport_features(vdev);
+
+	for (i = 0; i < sizeof(*vdev->features) / sizeof(features.features);
+	     i++) {
+		int highbits = i % 2 ? 32 : 0;
+		features.index = i;
+		features.features = cpu_to_le32(vdev->features[i / 2]
+						>> highbits);
+		memcpy((void *)(unsigned long)vcdev->area, &features,
+		       sizeof(features));
+		/* Write the feature bits to the host. */
+		vcdev->ccw->cmd_code = CCW_CMD_WRITE_FEAT;
+		vcdev->ccw->flags = 0;
+		vcdev->ccw->count = sizeof(features);
+		vcdev->ccw->cda = vcdev->area;
+		ccw_io_helper(vcdev, VIRTIO_CCW_DOING_WRITE_FEAT);
+	}
+}
+
+static void virtio_ccw_get_config(struct virtio_device *vdev,
+				  unsigned int offset, void *buf, unsigned len)
+{
+	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
+	int ret;
+
+	/* Read the config area from the host. */
+	vcdev->ccw->cmd_code = CCW_CMD_READ_CONF;
+	vcdev->ccw->flags = 0;
+	vcdev->ccw->count = offset + len;
+	vcdev->ccw->cda = vcdev->area;
+	ret = ccw_io_helper(vcdev, VIRTIO_CCW_DOING_READ_CONFIG);
+	if (ret)
+		return;
+
+	memcpy(vcdev->config, (void *)(unsigned long)vcdev->area,
+	       sizeof(vcdev->config));
+	memcpy(buf, &vcdev->config[offset], len);
+}
+
+static void virtio_ccw_set_config(struct virtio_device *vdev,
+				  unsigned int offset, const void *buf,
+				  unsigned len)
+{
+	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
+
+	memcpy(&vcdev->config[offset], buf, len);
+	/* Write the config area to the host. */
+	memcpy((void *)(unsigned long)vcdev->area, vcdev->config,
+	       sizeof(vcdev->config));
+	vcdev->ccw->cmd_code = CCW_CMD_WRITE_CONF;
+	vcdev->ccw->flags = 0;
+	vcdev->ccw->count = offset + len;
+	vcdev->ccw->cda = vcdev->area;
+	ccw_io_helper(vcdev, VIRTIO_CCW_DOING_WRITE_CONFIG);
+}
+
+static u8 virtio_ccw_get_status(struct virtio_device *vdev)
+{
+	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
+
+	return vcdev->status;
+}
+
+static void virtio_ccw_set_status(struct virtio_device *vdev, u8 status)
+{
+	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
+
+	/* Write the status to the host. */
+	vcdev->status = status;
+	memcpy((void *)(unsigned long)vcdev->area, &status, sizeof(status));
+	vcdev->ccw->cmd_code = CCW_CMD_WRITE_STATUS;
+	vcdev->ccw->flags = 0;
+	vcdev->ccw->count = sizeof(status);
+	vcdev->ccw->cda = vcdev->area;
+	ccw_io_helper(vcdev, VIRTIO_CCW_DOING_WRITE_STATUS);
+}
+
+static struct virtio_config_ops virtio_ccw_config_ops = {
+	.get_features = virtio_ccw_get_features,
+	.finalize_features = virtio_ccw_finalize_features,
+	.get = virtio_ccw_get_config,
+	.set = virtio_ccw_set_config,
+	.get_status = virtio_ccw_get_status,
+	.set_status = virtio_ccw_set_status,
+	.reset = virtio_ccw_reset,
+	.find_vqs = virtio_ccw_find_vqs,
+	.del_vqs = virtio_ccw_del_vqs,
+};
+
+
+/*
+ * ccw bus driver related functions
+ */
+
+static void virtio_ccw_release_dev(struct device *_d)
+{
+	struct virtio_device *dev = container_of(_d, struct virtio_device,
+						 dev);
+	struct virtio_ccw_device *vcdev = to_vc_device(dev);
+
+	kfree((void *)(unsigned long)vcdev->area);
+	kfree(vcdev->config_block);
+	kfree(vcdev->ccw);
+	kfree(vcdev);
+}
+
+static int irb_is_error(struct irb *irb)
+{
+	if (scsw_cstat(&irb->scsw) != 0)
+		return 1;
+	if (scsw_dstat(&irb->scsw) & ~(DEV_STAT_CHN_END | DEV_STAT_DEV_END))
+		return 1;
+	if (scsw_cc(&irb->scsw) != 0)
+		return 1;
+	return 0;
+}
+
+static struct virtqueue *virtio_ccw_vq_by_ind(struct virtio_ccw_device *vcdev,
+					      int index)
+{
+	struct virtio_ccw_vq_info *info;
+	unsigned long flags;
+	struct virtqueue *vq;
+
+	vq = NULL;
+	spin_lock_irqsave(&vcdev->lock, flags);
+	list_for_each_entry(info, &vcdev->virtqueues, node) {
+		if (virtqueue_get_queue_index(info->vq) == index) {
+			vq = info->vq;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&vcdev->lock, flags);
+	return vq;
+}
+
+static void virtio_ccw_int_handler(struct ccw_device *cdev,
+				   unsigned long intparm,
+				   struct irb *irb)
+{
+	__u32 activity = intparm & VIRTIO_CCW_INTPARM_MASK;
+	struct virtio_ccw_device *vcdev = dev_get_drvdata(&cdev->dev);
+	int i;
+	struct virtqueue *vq;
+	struct virtio_driver *drv;
+
+	/* Check if it's a notification from the host. */
+	if ((intparm == 0) &&
+	    (scsw_stctl(&irb->scsw) ==
+	     (SCSW_STCTL_ALERT_STATUS | SCSW_STCTL_STATUS_PEND))) {
+		/* OK */
+	}
+	if (irb_is_error(irb))
+		vcdev->err = -EIO; /* XXX - use real error */
+	if (vcdev->curr_io & activity) {
+		switch (activity) {
+		case VIRTIO_CCW_DOING_READ_FEAT:
+		case VIRTIO_CCW_DOING_WRITE_FEAT:
+		case VIRTIO_CCW_DOING_READ_CONFIG:
+		case VIRTIO_CCW_DOING_WRITE_CONFIG:
+		case VIRTIO_CCW_DOING_WRITE_STATUS:
+		case VIRTIO_CCW_DOING_SET_VQ:
+		case VIRTIO_CCW_DOING_SET_IND:
+		case VIRTIO_CCW_DOING_SET_CONF_IND:
+		case VIRTIO_CCW_DOING_RESET:
+		case VIRTIO_CCW_DOING_READ_VQ_CONF:
+			vcdev->curr_io &= ~activity;
+			wake_up(&vcdev->wait_q);
+			break;
+		default:
+			/* don't know what to do... */
+			dev_warn(&cdev->dev, "Suspicious activity '%08x'\n",
+				 activity);
+			WARN_ON(1);
+			break;
+		}
+	}
+	for_each_set_bit(i, &vcdev->indicators,
+			 sizeof(vcdev->indicators) * BITS_PER_BYTE) {
+		/* The bit clear must happen before the vring kick. */
+		clear_bit(i, &vcdev->indicators);
+		barrier();
+		vq = virtio_ccw_vq_by_ind(vcdev, i);
+		vring_interrupt(0, vq);
+	}
+	if (test_bit(0, &vcdev->indicators2)) {
+		drv = container_of(vcdev->vdev.dev.driver,
+				   struct virtio_driver, driver);
+
+		if (drv && drv->config_changed)
+			drv->config_changed(&vcdev->vdev);
+		clear_bit(0, &vcdev->indicators2);
+	}
+}
+
+/*
+ * We usually want to autoonline all devices, but give the admin
+ * a way to exempt devices from this.
+ */
+#define __DEV_WORDS ((__MAX_SUBCHANNEL + (8*sizeof(long) - 1)) / \
+		     (8*sizeof(long)))
+static unsigned long devs_no_auto[__MAX_SSID + 1][__DEV_WORDS];
+
+static char *no_auto = "";
+
+module_param(no_auto, charp, 0444);
+MODULE_PARM_DESC(no_auto, "list of ccw bus id ranges not to be auto-onlined");
+
+static int virtio_ccw_check_autoonline(struct ccw_device *cdev)
+{
+	struct ccw_dev_id id;
+
+	ccw_device_get_id(cdev, &id);
+	if (test_bit(id.devno, devs_no_auto[id.ssid]))
+		return 0;
+	return 1;
+}
+
+static void virtio_ccw_auto_online(void *data, async_cookie_t cookie)
+{
+	struct ccw_device *cdev = data;
+	int ret;
+
+	ret = ccw_device_set_online(cdev);
+	if (ret)
+		dev_warn(&cdev->dev, "Failed to set online: %d\n", ret);
+}
+
+static int virtio_ccw_probe(struct ccw_device *cdev)
+{
+	cdev->handler = virtio_ccw_int_handler;
+
+	if (virtio_ccw_check_autoonline(cdev))
+		async_schedule(virtio_ccw_auto_online, cdev);
+	return 0;
+}
+
+static void virtio_ccw_remove(struct ccw_device *cdev)
+{
+	struct virtio_ccw_device *vcdev = dev_get_drvdata(&cdev->dev);
+
+	if (cdev->online) {
+		unregister_virtio_device(&vcdev->vdev);
+		dev_set_drvdata(&cdev->dev, NULL);
+	}
+	cdev->handler = NULL;
+}
+
+static int virtio_ccw_offline(struct ccw_device *cdev)
+{
+	struct virtio_ccw_device *vcdev = dev_get_drvdata(&cdev->dev);
+
+	unregister_virtio_device(&vcdev->vdev);
+	dev_set_drvdata(&cdev->dev, NULL);
+	return 0;
+}
+
+
+/* Area needs to be big enough to fit status, features or configuration. */
+#define VIRTIO_AREA_SIZE VIRTIO_CCW_CONFIG_SIZE /* biggest possible use */
+
+static int virtio_ccw_online(struct ccw_device *cdev)
+{
+	int ret;
+	struct virtio_ccw_device *vcdev;
+
+	vcdev = kzalloc(sizeof(*vcdev), GFP_KERNEL);
+	if (!vcdev) {
+		dev_warn(&cdev->dev, "Could not get memory for virtio\n");
+		ret = -ENOMEM;
+		goto out_free;
+	}
+	vcdev->area = (__u32)(unsigned long)kzalloc(VIRTIO_AREA_SIZE,
+						    GFP_DMA | GFP_KERNEL);
+	if (!vcdev->area) {
+		dev_warn(&cdev->dev, "Cound not get memory for virtio\n");
+		ret = -ENOMEM;
+		goto out_free;
+	}
+	vcdev->config_block = kzalloc(sizeof(*vcdev->config_block),
+				   GFP_DMA | GFP_KERNEL);
+	if (!vcdev->config_block) {
+		ret = -ENOMEM;
+		goto out_free;
+	}
+	vcdev->ccw = kzalloc(sizeof(*vcdev->ccw), GFP_DMA | GFP_KERNEL);
+	if (!vcdev->ccw) {
+		ret = -ENOMEM;
+		goto out_free;
+	}
+
+	vcdev->vdev.dev.parent = &cdev->dev;
+	vcdev->vdev.dev.release = virtio_ccw_release_dev;
+	vcdev->vdev.config = &virtio_ccw_config_ops;
+	vcdev->cdev = cdev;
+	init_waitqueue_head(&vcdev->wait_q);
+	INIT_LIST_HEAD(&vcdev->virtqueues);
+	spin_lock_init(&vcdev->lock);
+
+	dev_set_drvdata(&cdev->dev, vcdev);
+	vcdev->vdev.id.vendor = cdev->id.cu_type;
+	vcdev->vdev.id.device = cdev->id.cu_model;
+	ret = register_virtio_device(&vcdev->vdev);
+	if (ret) {
+		dev_warn(&cdev->dev, "Failed to register virtio device: %d\n",
+			 ret);
+		goto out_put;
+	}
+	return 0;
+out_put:
+	dev_set_drvdata(&cdev->dev, NULL);
+	put_device(&vcdev->vdev.dev);
+	return ret;
+out_free:
+	if (vcdev) {
+		kfree((void *)(unsigned long)vcdev->area);
+		kfree(vcdev->config_block);
+		kfree(vcdev->ccw);
+	}
+	kfree(vcdev);
+	return ret;
+}
+
+static int virtio_ccw_cio_notify(struct ccw_device *cdev, int event)
+{
+	/* TODO: Check whether we need special handling here. */
+	return 0;
+}
+
+static struct ccw_device_id virtio_ids[] = {
+	{ CCW_DEVICE(0x3832, 0) },
+	{},
+};
+MODULE_DEVICE_TABLE(ccw, virtio_ids);
+
+static struct ccw_driver virtio_ccw_driver = {
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = "virtio_ccw",
+	},
+	.ids = virtio_ids,
+	.probe = virtio_ccw_probe,
+	.remove = virtio_ccw_remove,
+	.set_offline = virtio_ccw_offline,
+	.set_online = virtio_ccw_online,
+	.notify = virtio_ccw_cio_notify,
+	.int_class = IOINT_VIR,
+};
+
+static int __init pure_hex(char **cp, unsigned int *val, int min_digit,
+			   int max_digit, int max_val)
+{
+	int diff;
+
+	diff = 0;
+	*val = 0;
+
+	while (diff <= max_digit) {
+		int value = hex_to_bin(**cp);
+
+		if (value < 0)
+			break;
+		*val = *val * 16 + value;
+		(*cp)++;
+		diff++;
+	}
+
+	if ((diff < min_digit) || (diff > max_digit) || (*val > max_val))
+		return 1;
+
+	return 0;
+}
+
+static int __init parse_busid(char *str, unsigned int *cssid,
+			      unsigned int *ssid, unsigned int *devno)
+{
+	char *str_work;
+	int rc, ret;
+
+	rc = 1;
+
+	if (*str == '\0')
+		goto out;
+
+	str_work = str;
+	ret = pure_hex(&str_work, cssid, 1, 2, __MAX_CSSID);
+	if (ret || (str_work[0] != '.'))
+		goto out;
+	str_work++;
+	ret = pure_hex(&str_work, ssid, 1, 1, __MAX_SSID);
+	if (ret || (str_work[0] != '.'))
+		goto out;
+	str_work++;
+	ret = pure_hex(&str_work, devno, 4, 4, __MAX_SUBCHANNEL);
+	if (ret || (str_work[0] != '\0'))
+		goto out;
+
+	rc = 0;
+out:
+	return rc;
+}
+
+static void __init no_auto_parse(void)
+{
+	unsigned int from_cssid, to_cssid, from_ssid, to_ssid, from, to;
+	char *parm, *str;
+	int rc;
+
+	str = no_auto;
+	while ((parm = strsep(&str, ","))) {
+		rc = parse_busid(strsep(&parm, "-"), &from_cssid,
+				 &from_ssid, &from);
+		if (rc)
+			continue;
+		if (parm != NULL) {
+			rc = parse_busid(parm, &to_cssid,
+					 &to_ssid, &to);
+			if ((from_ssid > to_ssid) ||
+			    ((from_ssid == to_ssid) && (from > to)))
+				rc = -EINVAL;
+		} else {
+			to_cssid = from_cssid;
+			to_ssid = from_ssid;
+			to = from;
+		}
+		if (rc)
+			continue;
+		while ((from_ssid < to_ssid) ||
+		       ((from_ssid == to_ssid) && (from <= to))) {
+			set_bit(from, devs_no_auto[from_ssid]);
+			from++;
+			if (from > __MAX_SUBCHANNEL) {
+				from_ssid++;
+				from = 0;
+			}
+		}
+	}
+}
+
+static int __init virtio_ccw_init(void)
+{
+	/* parse no_auto string before we do anything further */
+	no_auto_parse();
+	return ccw_driver_register(&virtio_ccw_driver);
+}
+module_init(virtio_ccw_init);
+
+static void __exit virtio_ccw_exit(void)
+{
+	ccw_driver_unregister(&virtio_ccw_driver);
+}
+module_exit(virtio_ccw_exit);
-- 
1.7.12.4

Re: [PATCH 4/4] KVM: s390: Add a channel I/O based virtio transport driver.

[Alexander Graf]

On 12/12/2012 05:36 PM, Cornelia Huck wrote:
> Add a driver for kvm guests that matches virtual ccw devices provided
> by the host as virtio bridge devices.
>
> These virtio-ccw devices use a special set of channel commands in order
> to perform virtio functions.
>
> Reviewed-by: Marcelo Tosatti<mtosatti@redhat.com>
> Signed-off-by: Cornelia Huck<cornelia.huck@de.ibm.com>
> ---
>   arch/s390/include/asm/irq.h   |   1 +
>   arch/s390/kernel/irq.c        |   1 +
>   drivers/s390/kvm/Makefile     |   2 +-
>   drivers/s390/kvm/virtio_ccw.c | 854 ++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 857 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/s390/kvm/virtio_ccw.c
>
> diff --git a/arch/s390/include/asm/irq.h b/arch/s390/include/asm/irq.h
> index 6703dd9..ad2ad6b 100644
> --- a/arch/s390/include/asm/irq.h
> +++ b/arch/s390/include/asm/irq.h
> @@ -33,6 +33,7 @@ enum interruption_class {
>   	IOINT_APB,
>   	IOINT_ADM,
>   	IOINT_CSC,
> +	IOINT_VIR,
>   	NMI_NMI,
>   	NR_IRQS,
>   };
> diff --git a/arch/s390/kernel/irq.c b/arch/s390/kernel/irq.c
> index 6cdc55b..97c171a 100644
> --- a/arch/s390/kernel/irq.c
> +++ b/arch/s390/kernel/irq.c
> @@ -58,6 +58,7 @@ static const struct irq_class intrclass_names[] = {
>   	[IOINT_APB]  = {.name = "APB", .desc = "[I/O] AP Bus"},
>   	[IOINT_ADM]  = {.name = "ADM", .desc = "[I/O] EADM Subchannel"},
>   	[IOINT_CSC]  = {.name = "CSC", .desc = "[I/O] CHSC Subchannel"},
> +	[IOINT_VIR]  = {.name = "VIR", .desc = "[I/O] Virtual I/O Devices"},
>   	[NMI_NMI]    = {.name = "NMI", .desc = "[NMI] Machine Check"},
>   };
>
> diff --git a/drivers/s390/kvm/Makefile b/drivers/s390/kvm/Makefile
> index 0815690..241891a 100644
> --- a/drivers/s390/kvm/Makefile
> +++ b/drivers/s390/kvm/Makefile
> @@ -6,4 +6,4 @@
>   # it under the terms of the GNU General Public License (version 2 only)
>   # as published by the Free Software Foundation.
>
> -obj-$(CONFIG_S390_GUEST) += kvm_virtio.o
> +obj-$(CONFIG_S390_GUEST) += kvm_virtio.o virtio_ccw.o
> diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
> new file mode 100644
> index 0000000..7f1a0c6
> --- /dev/null
> +++ b/drivers/s390/kvm/virtio_ccw.c
> @@ -0,0 +1,854 @@
> +/*
> + * ccw based virtio transport
> + *
> + * Copyright IBM Corp. 2012
> + *
> + * 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>
> + */
> +
> +#include<linux/kernel_stat.h>
> +#include<linux/init.h>
> +#include<linux/bootmem.h>
> +#include<linux/err.h>
> +#include<linux/virtio.h>
> +#include<linux/virtio_config.h>
> +#include<linux/slab.h>
> +#include<linux/interrupt.h>
> +#include<linux/virtio_ring.h>
> +#include<linux/pfn.h>
> +#include<linux/async.h>
> +#include<linux/wait.h>
> +#include<linux/list.h>
> +#include<linux/bitops.h>
> +#include<linux/module.h>
> +#include<linux/io.h>
> +#include<linux/kvm_para.h>
> +#include<asm/setup.h>
> +#include<asm/irq.h>
> +#include<asm/cio.h>
> +#include<asm/ccwdev.h>
> +#include<asm/schid.h>
> +
> +/*
> + * virtio related functions
> + */
> +
> +struct vq_config_block {
> +	__u16 index;
> +	__u16 num;
> +} __packed;
> +
> +#define VIRTIO_CCW_CONFIG_SIZE 0x100
> +/* same as PCI config space size, should be enough for all drivers */
> +
> +struct virtio_ccw_device {
> +	struct virtio_device vdev;
> +	__u8 status;
> +	__u8 config[VIRTIO_CCW_CONFIG_SIZE];
> +	struct ccw_device *cdev;
> +	struct ccw1 *ccw;
> +	__u32 area;
> +	__u32 curr_io;
> +	int err;
> +	wait_queue_head_t wait_q;
> +	spinlock_t lock;
> +	struct list_head virtqueues;
> +	unsigned long indicators;
> +	unsigned long indicators2;
> +	struct vq_config_block *config_block;
> +};
> +
> +struct vq_info_block {
> +	__u64 queue;
> +	__u32 align;
> +	__u16 index;
> +	__u16 num;
> +} __packed;
> +
> +struct virtio_feature_desc {
> +	__u32 features;
> +	__u8 index;
> +} __packed;
> +
> +struct virtio_ccw_vq_info {
> +	struct virtqueue *vq;
> +	int num;
> +	void *queue;
> +	struct vq_info_block *info_block;
> +	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
> +#define CCW_CMD_SET_CONF_IND 0x53
> +#define CCW_CMD_READ_FEAT 0x12
> +#define CCW_CMD_WRITE_FEAT 0x11
> +#define CCW_CMD_READ_CONF 0x22
> +#define CCW_CMD_WRITE_CONF 0x21
> +#define CCW_CMD_WRITE_STATUS 0x31
> +#define CCW_CMD_READ_VQ_CONF 0x32
> +
> +#define VIRTIO_CCW_DOING_SET_VQ 0x00010000
> +#define VIRTIO_CCW_DOING_RESET 0x00040000
> +#define VIRTIO_CCW_DOING_READ_FEAT 0x00080000
> +#define VIRTIO_CCW_DOING_WRITE_FEAT 0x00100000
> +#define VIRTIO_CCW_DOING_READ_CONFIG 0x00200000
> +#define VIRTIO_CCW_DOING_WRITE_CONFIG 0x00400000
> +#define VIRTIO_CCW_DOING_WRITE_STATUS 0x00800000
> +#define VIRTIO_CCW_DOING_SET_IND 0x01000000
> +#define VIRTIO_CCW_DOING_READ_VQ_CONF 0x02000000
> +#define VIRTIO_CCW_DOING_SET_CONF_IND 0x04000000
> +#define VIRTIO_CCW_INTPARM_MASK 0xffff0000
> +
> +static struct virtio_ccw_device *to_vc_device(struct virtio_device *vdev)
> +{
> +	return container_of(vdev, struct virtio_ccw_device, vdev);
> +}
> +
> +static int doing_io(struct virtio_ccw_device *vcdev, __u32 flag)
> +{
> +	unsigned long flags;
> +	__u32 ret;
> +
> +	spin_lock_irqsave(get_ccwdev_lock(vcdev->cdev), flags);
> +	if (vcdev->err)
> +		ret = 0;
> +	else
> +		ret = vcdev->curr_io&  flag;
> +	spin_unlock_irqrestore(get_ccwdev_lock(vcdev->cdev), flags);
> +	return ret;
> +}
> +
> +static int ccw_io_helper(struct virtio_ccw_device *vcdev, __u32 intparm)
> +{
> +	int ret;
> +	unsigned long flags;
> +	int flag = intparm&  VIRTIO_CCW_INTPARM_MASK;
> +
> +	spin_lock_irqsave(get_ccwdev_lock(vcdev->cdev), flags);
> +	ret = ccw_device_start(vcdev->cdev, vcdev->ccw, intparm, 0, 0);
> +	if (!ret)
> +		vcdev->curr_io |= flag;
> +	spin_unlock_irqrestore(get_ccwdev_lock(vcdev->cdev), flags);
> +	wait_event(vcdev->wait_q, doing_io(vcdev, flag) == 0);
> +	return ret ? ret : vcdev->err;
> +}
> +
> +static inline long do_kvm_notify(struct subchannel_id schid,
> +				 unsigned long queue_index)
> +{
> +	register unsigned long __nr asm("1") = KVM_S390_VIRTIO_CCW_NOTIFY;
> +	register struct subchannel_id __schid asm("2") = schid;

How does the compiler know that the struct fits within a register?


Alex

Re: [PATCH 4/4] KVM: s390: Add a channel I/O based virtio transport driver.

[Christian Borntraeger]

On 12/12/12 19:31, Alexander Graf wrote:
 +static inline long do_kvm_notify(struct subchannel_id schid,
>> +                 unsigned long queue_index)
>> +{
>> +    register unsigned long __nr asm("1") = KVM_S390_VIRTIO_CCW_NOTIFY;
>> +    register struct subchannel_id __schid asm("2") = schid;
> 
> How does the compiler know that the struct fits within a register?
> 

Well it knows that sizeof struct subchannel_id is 4.

then we have 
"ELF Application Binary Interface Supplement Linux on zSeries"

[...]
Parameter passing
[...]
- Structures with a size of 1, 2, 4, or 8 bytes are passed as integral values.  <=============
- All other structures are passed by reference. If needed, the called function makes
  a copy of the value.
[...]

while not being a perfect match it gives a good indication that this will work.

Furthermore, we know it is working at least since 
commit 94c12cc7d196bab34aaa98d38521549fa1e5ef76
Author: Martin Schwidefsky <schwidefsky@de.ibm.com>
Date:   Thu Sep 28 16:56:43 2006 +0200
    [S390] Inline assembly cleanup.

in 
drivers/s390/cio/ioasm.h

[...]
94c12cc7 (Martin Schwidefsky  2006-09-28 16:56:43 +0200  30)    register struct subchannel_id reg1 asm ("1") = schid;
94c12cc7 (Martin Schwidefsky  2006-09-28 16:56:43 +0200  31)    int ccode = -EIO;
fb6958a5 (Cornelia Huck       2006-01-06 00:19:25 -0800  32) 
94c12cc7 (Martin Schwidefsky  2006-09-28 16:56:43 +0200  33)    asm volatile(
[...]

So this complies with the coding style of the s390 maintainer :-)

Christian

Re: [PATCH 4/4] KVM: s390: Add a channel I/O based virtio transport driver.

[Alexander Graf]

On 12.12.2012, at 21:12, Christian Borntraeger wrote:

> On 12/12/12 19:31, Alexander Graf wrote:
> +static inline long do_kvm_notify(struct subchannel_id schid,
>>> +                 unsigned long queue_index)
>>> +{
>>> +    register unsigned long __nr asm("1") = KVM_S390_VIRTIO_CCW_NOTIFY;
>>> +    register struct subchannel_id __schid asm("2") = schid;
>> 
>> How does the compiler know that the struct fits within a register?
>> 
> 
> Well it knows that sizeof struct subchannel_id is 4.

How so? The asm/ccwdev.h header simply says

  struct subchannel_id;

with the actual implementation of that struct hidden in asm/schid.h. So either patch 3/4 exports the struct needlessly and should rather do an #include <asm/schid.h> or the compiler can do magic :).


Alex

> 
> then we have 
> "ELF Application Binary Interface Supplement Linux on zSeries"
> 
> [...]
> Parameter passing
> [...]
> - Structures with a size of 1, 2, 4, or 8 bytes are passed as integral values.  <=============
> - All other structures are passed by reference. If needed, the called function makes
>  a copy of the value.
> [...]
> 
> while not being a perfect match it gives a good indication that this will work.
> 
> Furthermore, we know it is working at least since 
> commit 94c12cc7d196bab34aaa98d38521549fa1e5ef76
> Author: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Date:   Thu Sep 28 16:56:43 2006 +0200
>    [S390] Inline assembly cleanup.
> 
> in 
> drivers/s390/cio/ioasm.h
> 
> [...]
> 94c12cc7 (Martin Schwidefsky  2006-09-28 16:56:43 +0200  30)    register struct subchannel_id reg1 asm ("1") = schid;
> 94c12cc7 (Martin Schwidefsky  2006-09-28 16:56:43 +0200  31)    int ccode = -EIO;
> fb6958a5 (Cornelia Huck       2006-01-06 00:19:25 -0800  32) 
> 94c12cc7 (Martin Schwidefsky  2006-09-28 16:56:43 +0200  33)    asm volatile(
> [...]
> 
> So this complies with the coding style of the s390 maintainer :-)
> 
> Christian
> 

Re: [PATCH 4/4] KVM: s390: Add a channel I/O based virtio transport driver.

[Christian Borntraeger]

On 13/12/12 12:06, Alexander Graf wrote:
> 
> On 12.12.2012, at 21:12, Christian Borntraeger wrote:
> 
>> On 12/12/12 19:31, Alexander Graf wrote:
>> +static inline long do_kvm_notify(struct subchannel_id schid,
>>>> +                 unsigned long queue_index)
>>>> +{
>>>> +    register unsigned long __nr asm("1") = KVM_S390_VIRTIO_CCW_NOTIFY;
>>>> +    register struct subchannel_id __schid asm("2") = schid;
>>>
>>> How does the compiler know that the struct fits within a register?
>>>
>>
>> Well it knows that sizeof struct subchannel_id is 4.
> 
> How so? The asm/ccwdev.h header simply says
> 
>   struct subchannel_id;

see original patch:

>>> +#include <asm/cio.h>
>>> +#include <asm/ccwdev.h>
>>> +#include <asm/schid.h>
>>> +
>>> +/*
>>> + * virtio related functions
>>> + */
>>> +

asm/schid.h includes uapi/asm/schid.h and that contains the definition.

Re: [PATCH 4/4] KVM: s390: Add a channel I/O based virtio transport driver.

[Alexander Graf]

On 13.12.2012, at 12:13, Christian Borntraeger wrote:

> On 13/12/12 12:06, Alexander Graf wrote:
>> 
>> On 12.12.2012, at 21:12, Christian Borntraeger wrote:
>> 
>>> On 12/12/12 19:31, Alexander Graf wrote:
>>> +static inline long do_kvm_notify(struct subchannel_id schid,
>>>>> +                 unsigned long queue_index)
>>>>> +{
>>>>> +    register unsigned long __nr asm("1") = KVM_S390_VIRTIO_CCW_NOTIFY;
>>>>> +    register struct subchannel_id __schid asm("2") = schid;
>>>> 
>>>> How does the compiler know that the struct fits within a register?
>>>> 
>>> 
>>> Well it knows that sizeof struct subchannel_id is 4.
>> 
>> How so? The asm/ccwdev.h header simply says
>> 
>>  struct subchannel_id;
> 
> see original patch:
> 
>>>> +#include <asm/cio.h>
>>>> +#include <asm/ccwdev.h>
>>>> +#include <asm/schid.h>
>>>> +
>>>> +/*
>>>> + * virtio related functions
>>>> + */
>>>> +
> 
> asm/schid.h includes uapi/asm/schid.h and that contains the definition.

My point is that the asm/schid.h include should be in asm/ccwdev.h then, not in virtio_ccw.c. Because semantically you are creating a dependency due to the new function that returns a struct schid.


Alex

Re: [PATCH 4/4] KVM: s390: Add a channel I/O based virtio transport driver.

[Christian Borntraeger]

On 13/12/12 12:19, Alexander Graf wrote:
> 
> On 13.12.2012, at 12:13, Christian Borntraeger wrote:
> 
>> On 13/12/12 12:06, Alexander Graf wrote:
>>>
>>> On 12.12.2012, at 21:12, Christian Borntraeger wrote:
>>>
>>>> On 12/12/12 19:31, Alexander Graf wrote:
>>>> +static inline long do_kvm_notify(struct subchannel_id schid,
>>>>>> +                 unsigned long queue_index)
>>>>>> +{
>>>>>> +    register unsigned long __nr asm("1") = KVM_S390_VIRTIO_CCW_NOTIFY;
>>>>>> +    register struct subchannel_id __schid asm("2") = schid;
>>>>>
>>>>> How does the compiler know that the struct fits within a register?
>>>>>
>>>>
>>>> Well it knows that sizeof struct subchannel_id is 4.
>>>
>>> How so? The asm/ccwdev.h header simply says
>>>
>>>  struct subchannel_id;
>>
>> see original patch:
>>
>>>>> +#include <asm/cio.h>
>>>>> +#include <asm/ccwdev.h>
>>>>> +#include <asm/schid.h>
>>>>> +
>>>>> +/*
>>>>> + * virtio related functions
>>>>> + */
>>>>> +
>>
>> asm/schid.h includes uapi/asm/schid.h and that contains the definition.
> 
> My point is that the asm/schid.h include should be in asm/ccwdev.h then, not in virtio_ccw.c. Because semantically you are creating a dependency due to the new function that returns a struct schid.

Ok, so you actually want to have patch 3 slightly modified as described above.
Yes makes sense.

Christian