[PATCH v3 0/5] s390: Guest support for virtio-ccw.

[PATCH v3 0/5] s390: Guest support for virtio-ccw.

[Cornelia Huck]

Hi,

here are the guest support patches for virtio-ccw, rebased
against kvm/next.

Changes from v2:
- Adapted patch 4 to virtqueue interface changes.
- Added Marcelo's Reviewed-by:

I'd like to get these patches into kvm/next; patches 2 and 3 might
also show up in the s390 patches.

Cornelia Huck (5):
  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.
  KVM: s390: Split out early console code.

 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/early_printk.c |  42 ++
 drivers/s390/kvm/kvm_virtio.c   |  64 ++-
 drivers/s390/kvm/virtio_ccw.c   | 843 ++++++++++++++++++++++++++++++++++++++++
 10 files changed, 938 insertions(+), 37 deletions(-)
 create mode 100644 drivers/s390/kvm/early_printk.c
 create mode 100644 drivers/s390/kvm/virtio_ccw.c

-- 
1.7.12.4

[PATCH 1/5] 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 | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index 7dabef6..779e480 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,24 @@ static int __init kvm_devices_init(void)
 	if (!MACHINE_IS_KVM)
 		return -ENODEV;
 
+	if (test_devices_support(real_memory_size) < 0)
+		/* No error. */
+		return 0;
+
+	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/5] 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/5] 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

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

[Alexander Graf]

On 07.12.2012, at 13:29, Cornelia Huck wrote:

> 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)

If you make this

u32 ccw_device_get_schid(struct ccw_device *cdev)

and you return the u32 (architected) value of the schid, not the internal struct, then you can save yourself the struct export (patch 2/5) and the ugly cast in patch 4/5 to get the u32 value.


Alex

> +{
> +	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
> 

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

[Cornelia Huck]

On Sun, 9 Dec 2012 13:12:37 +0100
Alexander Graf <agraf@suse.de> wrote:

> 
> On 07.12.2012, at 13:29, Cornelia Huck wrote:
> 
> > 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)
> 
> If you make this
> 
> u32 ccw_device_get_schid(struct ccw_device *cdev)
> 
> and you return the u32 (architected) value of the schid, not the internal struct, then you can save yourself the struct export (patch 2/5) and the ugly cast in patch 4/5 to get the u32 value.

I really prefer using the structure instead.

Moreover, there's a patch based on this that switches non-kvm users to
this new interface (getting rid of an old, broken interface) already
queued in the linux-s390 tree AFAIK.

> 
> 
> Alex
> 
> > +{
> > +	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
> > 
> 

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

[Alexander Graf]

On 10.12.2012, at 10:03, Cornelia Huck wrote:

> On Sun, 9 Dec 2012 13:12:37 +0100
> Alexander Graf <agraf@suse.de> wrote:
> 
>> 
>> On 07.12.2012, at 13:29, Cornelia Huck wrote:
>> 
>>> 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)
>> 
>> If you make this
>> 
>> u32 ccw_device_get_schid(struct ccw_device *cdev)
>> 
>> and you return the u32 (architected) value of the schid, not the internal struct, then you can save yourself the struct export (patch 2/5) and the ugly cast in patch 4/5 to get the u32 value.
> 
> I really prefer using the structure instead.
> 
> Moreover, there's a patch based on this that switches non-kvm users to
> this new interface (getting rid of an old, broken interface) already
> queued in the linux-s390 tree AFAIK.

Well, then base on top of that patch and add another interface that allows you to receive a schid as integer value. Randomly casting structs to u32 in random code across the tree is just pure ugly.

An alternative would be to create a union around the struct and to return that one, so that you can access the u32 value or the struct value depending on the user's needs. That way the u32 cast is part of the API and not implicit, as it would be today.


Alex

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

[Cornelia Huck]

On Tue, 11 Dec 2012 11:09:55 +0100
Alexander Graf <agraf@suse.de> wrote:

> 
> On 10.12.2012, at 10:03, Cornelia Huck wrote:
> 
> > On Sun, 9 Dec 2012 13:12:37 +0100
> > Alexander Graf <agraf@suse.de> wrote:
> > 
> >> 
> >> On 07.12.2012, at 13:29, Cornelia Huck wrote:
> >> 
> >>> 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)
> >> 
> >> If you make this
> >> 
> >> u32 ccw_device_get_schid(struct ccw_device *cdev)
> >> 
> >> and you return the u32 (architected) value of the schid, not the internal struct, then you can save yourself the struct export (patch 2/5) and the ugly cast in patch 4/5 to get the u32 value.
> > 
> > I really prefer using the structure instead.
> > 
> > Moreover, there's a patch based on this that switches non-kvm users to
> > this new interface (getting rid of an old, broken interface) already
> > queued in the linux-s390 tree AFAIK.
> 
> Well, then base on top of that patch and add another interface that allows you to receive a schid as integer value. Randomly casting structs to u32 in random code across the tree is just pure ugly.

We seem to have different ideas of 'ugly' :)

I really prefer to have a function called ccw_device_get_schid()
provide a struct subchannel_id and not a u32 - a subchannel id is,
after all, what the caller asks for. The fact that a subchannel id may
fit into a u32 is an implementation detail, as well as the fact that an
instruction takes a 32 bit value containing a subchannel id. Passing
around a 32 bit value instead of a subchannel id basically discards the
information that we're dealing with a subchannel id and not with a
random 32 bit value.

> 
> An alternative would be to create a union around the struct and to return that one, so that you can access the u32 value or the struct value depending on the user's needs. That way the u32 cast is part of the API and not implicit, as it would be today.

What's the API here? I think we have two parts:

- The API to get the current subchannel id for a ccw device. For the
reasons above, I think this should use struct subchannel_id.

- The hardware/hypervisor API for addressing a subchannel, which
basically requires that a register contains a subchannel id. The easiest
way to do this is to cast to an integer. I don't see any need to play
games with unions here, after all we're using C and not Pascal ;)

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

[Alexander Graf]

On 11.12.2012, at 13:34, Cornelia Huck wrote:

> On Tue, 11 Dec 2012 11:09:55 +0100
> Alexander Graf <agraf@suse.de> wrote:
> 
>> 
>> On 10.12.2012, at 10:03, Cornelia Huck wrote:
>> 
>>> On Sun, 9 Dec 2012 13:12:37 +0100
>>> Alexander Graf <agraf@suse.de> wrote:
>>> 
>>>> 
>>>> On 07.12.2012, at 13:29, Cornelia Huck wrote:
>>>> 
>>>>> 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)
>>>> 
>>>> If you make this
>>>> 
>>>> u32 ccw_device_get_schid(struct ccw_device *cdev)
>>>> 
>>>> and you return the u32 (architected) value of the schid, not the internal struct, then you can save yourself the struct export (patch 2/5) and the ugly cast in patch 4/5 to get the u32 value.
>>> 
>>> I really prefer using the structure instead.
>>> 
>>> Moreover, there's a patch based on this that switches non-kvm users to
>>> this new interface (getting rid of an old, broken interface) already
>>> queued in the linux-s390 tree AFAIK.
>> 
>> Well, then base on top of that patch and add another interface that allows you to receive a schid as integer value. Randomly casting structs to u32 in random code across the tree is just pure ugly.
> 
> We seem to have different ideas of 'ugly' :)
> 
> I really prefer to have a function called ccw_device_get_schid()
> provide a struct subchannel_id and not a u32 - a subchannel id is,
> after all, what the caller asks for. The fact that a subchannel id may
> fit into a u32 is an implementation detail, as well as the fact that an
> instruction takes a 32 bit value containing a subchannel id. Passing
> around a 32 bit value instead of a subchannel id basically discards the
> information that we're dealing with a subchannel id and not with a
> random 32 bit value.
> 
>> 
>> An alternative would be to create a union around the struct and to return that one, so that you can access the u32 value or the struct value depending on the user's needs. That way the u32 cast is part of the API and not implicit, as it would be today.
> 
> What's the API here? I think we have two parts:
> 
> - The API to get the current subchannel id for a ccw device. For the
> reasons above, I think this should use struct subchannel_id.

Fine with me.

But as you are mentioning above the fact that a subchannel id is a u32 internally is an implementation detail. Thus casting a struct schid * to a u32 * is violating that exact rule.

> - The hardware/hypervisor API for addressing a subchannel, which
> basically requires that a register contains a subchannel id. The easiest
> way to do this is to cast to an integer. I don't see any need to play
> games with unions here, after all we're using C and not Pascal ;)

Then just create a small inline function in the header file that converts a struct schid * to u32. I want to make sure we keep implementation details in files that deal with the implementation details, not in files that deal with using the interfaces.


Alex

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

[Cornelia Huck]

On Wed, 12 Dec 2012 01:33:46 +0100
Alexander Graf <agraf@suse.de> wrote:

> 
> On 11.12.2012, at 13:34, Cornelia Huck wrote:
> 
> > On Tue, 11 Dec 2012 11:09:55 +0100
> > Alexander Graf <agraf@suse.de> wrote:
> > 
> >> 
> >> On 10.12.2012, at 10:03, Cornelia Huck wrote:
> >> 
> >>> On Sun, 9 Dec 2012 13:12:37 +0100
> >>> Alexander Graf <agraf@suse.de> wrote:
> >>> 
> >>>> 
> >>>> On 07.12.2012, at 13:29, Cornelia Huck wrote:
> >>>> 
> >>>>> 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)
> >>>> 
> >>>> If you make this
> >>>> 
> >>>> u32 ccw_device_get_schid(struct ccw_device *cdev)
> >>>> 
> >>>> and you return the u32 (architected) value of the schid, not the internal struct, then you can save yourself the struct export (patch 2/5) and the ugly cast in patch 4/5 to get the u32 value.
> >>> 
> >>> I really prefer using the structure instead.
> >>> 
> >>> Moreover, there's a patch based on this that switches non-kvm users to
> >>> this new interface (getting rid of an old, broken interface) already
> >>> queued in the linux-s390 tree AFAIK.
> >> 
> >> Well, then base on top of that patch and add another interface that allows you to receive a schid as integer value. Randomly casting structs to u32 in random code across the tree is just pure ugly.
> > 
> > We seem to have different ideas of 'ugly' :)
> > 
> > I really prefer to have a function called ccw_device_get_schid()
> > provide a struct subchannel_id and not a u32 - a subchannel id is,
> > after all, what the caller asks for. The fact that a subchannel id may
> > fit into a u32 is an implementation detail, as well as the fact that an
> > instruction takes a 32 bit value containing a subchannel id. Passing
> > around a 32 bit value instead of a subchannel id basically discards the
> > information that we're dealing with a subchannel id and not with a
> > random 32 bit value.
> > 
> >> 
> >> An alternative would be to create a union around the struct and to return that one, so that you can access the u32 value or the struct value depending on the user's needs. That way the u32 cast is part of the API and not implicit, as it would be today.
> > 
> > What's the API here? I think we have two parts:
> > 
> > - The API to get the current subchannel id for a ccw device. For the
> > reasons above, I think this should use struct subchannel_id.
> 
> Fine with me.
> 
> But as you are mentioning above the fact that a subchannel id is a u32 internally is an implementation detail. Thus casting a struct schid * to a u32 * is violating that exact rule.
> 
> > - The hardware/hypervisor API for addressing a subchannel, which
> > basically requires that a register contains a subchannel id. The easiest
> > way to do this is to cast to an integer. I don't see any need to play
> > games with unions here, after all we're using C and not Pascal ;)
> 
> Then just create a small inline function in the header file that converts a struct schid * to u32. I want to make sure we keep implementation details in files that deal with the implementation details, not in files that deal with using the interfaces.
> 

I checked what the common I/O layer does for the I/O instructions in
drivers/s390/cio/ioasm.h and think that's really the best solution:
pass the subchannel id directly in the register. No information is
lost, no confusing casts, and I think the code looks nicer.

Updated guest patch set coming now.

[PATCH 4/5] 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 | 844 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 847 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..0e98b523a
--- /dev/null
+++ b/drivers/s390/kvm/virtio_ccw.c
@@ -0,0 +1,844 @@
+/*
+ * 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/virtio_console.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 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;
+	__u32 reg2;
+
+	vcdev = to_vc_device(info->vq->vdev);
+	ccw_device_get_schid(vcdev->cdev, &schid);
+	reg2 = *(__u32 *)&schid;
+	kvm_hypercall2(KVM_S390_VIRTIO_CCW_NOTIFY, reg2,
+		       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

[PATCH 5/5] KVM: s390: Split out early console code.

[Cornelia Huck]

This code is transport agnostic and can be used by both the legacy
virtio code and virtio_ccw.

Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 drivers/s390/kvm/Makefile       |  2 +-
 drivers/s390/kvm/early_printk.c | 42 +++++++++++++++++++++++++++++++++++++++++
 drivers/s390/kvm/kvm_virtio.c   | 29 ++--------------------------
 drivers/s390/kvm/virtio_ccw.c   |  1 -
 4 files changed, 45 insertions(+), 29 deletions(-)
 create mode 100644 drivers/s390/kvm/early_printk.c

diff --git a/drivers/s390/kvm/Makefile b/drivers/s390/kvm/Makefile
index 241891a..a3c8fc4 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 virtio_ccw.o
+obj-$(CONFIG_S390_GUEST) += kvm_virtio.o early_printk.o virtio_ccw.o
diff --git a/drivers/s390/kvm/early_printk.c b/drivers/s390/kvm/early_printk.c
new file mode 100644
index 0000000..915d687
--- /dev/null
+++ b/drivers/s390/kvm/early_printk.c
@@ -0,0 +1,42 @@
+/*
+ * early_printk.c - code for early console output with virtio_console
+ * split off from kvm_virtio.c
+ *
+ * Copyright IBM Corp. 2008
+ *
+ * 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): Christian Borntraeger <borntraeger@de.ibm.com>
+ */
+
+#include <linux/kernel_stat.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/virtio_console.h>
+#include <linux/kvm_para.h>
+#include <asm/kvm_virtio.h>
+#include <asm/setup.h>
+#include <asm/sclp.h>
+
+static __init int early_put_chars(u32 vtermno, const char *buf, int count)
+{
+	char scratch[17];
+	unsigned int len = count;
+
+	if (len > sizeof(scratch) - 1)
+		len = sizeof(scratch) - 1;
+	scratch[len] = '\0';
+	memcpy(scratch, buf, len);
+	kvm_hypercall1(KVM_S390_VIRTIO_NOTIFY, __pa(scratch));
+	return len;
+}
+
+static int __init s390_virtio_console_init(void)
+{
+	if (sclp_has_vt220() || sclp_has_linemode())
+		return -ENODEV;
+	return virtio_cons_early_init(early_put_chars);
+}
+console_initcall(s390_virtio_console_init);
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index 779e480..6ca06ed 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -17,7 +17,6 @@
 #include <linux/virtio.h>
 #include <linux/virtio_config.h>
 #include <linux/slab.h>
-#include <linux/virtio_console.h>
 #include <linux/interrupt.h>
 #include <linux/virtio_ring.h>
 #include <linux/export.h>
@@ -25,9 +24,9 @@
 #include <asm/io.h>
 #include <asm/kvm_para.h>
 #include <asm/kvm_virtio.h>
-#include <asm/sclp.h>
 #include <asm/setup.h>
 #include <asm/irq.h>
+#include <asm/sclp.h>
 
 #define VIRTIO_SUBCODE_64 0x0D00
 
@@ -453,8 +452,7 @@ static int __init kvm_devices_init(void)
 		return -ENODEV;
 
 	if (test_devices_support(real_memory_size) < 0)
-		/* No error. */
-		return 0;
+		return -ENODEV;
 
 	rc = vmem_add_mapping(real_memory_size, PAGE_SIZE);
 	if (rc)
@@ -479,29 +477,6 @@ static int __init kvm_devices_init(void)
 	return 0;
 }
 
-/* code for early console output with virtio_console */
-static __init int early_put_chars(u32 vtermno, const char *buf, int count)
-{
-	char scratch[17];
-	unsigned int len = count;
-
-	if (len > sizeof(scratch) - 1)
-		len = sizeof(scratch) - 1;
-	scratch[len] = '\0';
-	memcpy(scratch, buf, len);
-	kvm_hypercall1(KVM_S390_VIRTIO_NOTIFY, __pa(scratch));
-	return len;
-}
-
-static int __init s390_virtio_console_init(void)
-{
-	if (sclp_has_vt220() || sclp_has_linemode())
-		return -ENODEV;
-	return virtio_cons_early_init(early_put_chars);
-}
-console_initcall(s390_virtio_console_init);
-
-
 /*
  * We do this after core stuff, but before the drivers.
  */
diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
index 0e98b523a..1350391 100644
--- a/drivers/s390/kvm/virtio_ccw.c
+++ b/drivers/s390/kvm/virtio_ccw.c
@@ -17,7 +17,6 @@
 #include <linux/virtio.h>
 #include <linux/virtio_config.h>
 #include <linux/slab.h>
-#include <linux/virtio_console.h>
 #include <linux/interrupt.h>
 #include <linux/virtio_ring.h>
 #include <linux/pfn.h>
-- 
1.7.12.4

Re: [PATCH 5/5] KVM: s390: Split out early console code.

[Alexander Graf]

On 07.12.2012, at 13:29, Cornelia Huck wrote:

> This code is transport agnostic and can be used by both the legacy
> virtio code and virtio_ccw.

This code should die. It's a nightmare to implement in QEMU, because it ends up being a separate char device. I'd prefer if we don't carry it over to the virtio-ccw world :).


Alex

> 
> Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
> drivers/s390/kvm/Makefile       |  2 +-
> drivers/s390/kvm/early_printk.c | 42 +++++++++++++++++++++++++++++++++++++++++
> drivers/s390/kvm/kvm_virtio.c   | 29 ++--------------------------
> drivers/s390/kvm/virtio_ccw.c   |  1 -
> 4 files changed, 45 insertions(+), 29 deletions(-)
> create mode 100644 drivers/s390/kvm/early_printk.c
> 
> diff --git a/drivers/s390/kvm/Makefile b/drivers/s390/kvm/Makefile
> index 241891a..a3c8fc4 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 virtio_ccw.o
> +obj-$(CONFIG_S390_GUEST) += kvm_virtio.o early_printk.o virtio_ccw.o
> diff --git a/drivers/s390/kvm/early_printk.c b/drivers/s390/kvm/early_printk.c
> new file mode 100644
> index 0000000..915d687
> --- /dev/null
> +++ b/drivers/s390/kvm/early_printk.c
> @@ -0,0 +1,42 @@
> +/*
> + * early_printk.c - code for early console output with virtio_console
> + * split off from kvm_virtio.c
> + *
> + * Copyright IBM Corp. 2008
> + *
> + * 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): Christian Borntraeger <borntraeger@de.ibm.com>
> + */
> +
> +#include <linux/kernel_stat.h>
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/virtio_console.h>
> +#include <linux/kvm_para.h>
> +#include <asm/kvm_virtio.h>
> +#include <asm/setup.h>
> +#include <asm/sclp.h>
> +
> +static __init int early_put_chars(u32 vtermno, const char *buf, int count)
> +{
> +	char scratch[17];
> +	unsigned int len = count;
> +
> +	if (len > sizeof(scratch) - 1)
> +		len = sizeof(scratch) - 1;
> +	scratch[len] = '\0';
> +	memcpy(scratch, buf, len);
> +	kvm_hypercall1(KVM_S390_VIRTIO_NOTIFY, __pa(scratch));
> +	return len;
> +}
> +
> +static int __init s390_virtio_console_init(void)
> +{
> +	if (sclp_has_vt220() || sclp_has_linemode())
> +		return -ENODEV;
> +	return virtio_cons_early_init(early_put_chars);
> +}
> +console_initcall(s390_virtio_console_init);
> diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
> index 779e480..6ca06ed 100644
> --- a/drivers/s390/kvm/kvm_virtio.c
> +++ b/drivers/s390/kvm/kvm_virtio.c
> @@ -17,7 +17,6 @@
> #include <linux/virtio.h>
> #include <linux/virtio_config.h>
> #include <linux/slab.h>
> -#include <linux/virtio_console.h>
> #include <linux/interrupt.h>
> #include <linux/virtio_ring.h>
> #include <linux/export.h>
> @@ -25,9 +24,9 @@
> #include <asm/io.h>
> #include <asm/kvm_para.h>
> #include <asm/kvm_virtio.h>
> -#include <asm/sclp.h>
> #include <asm/setup.h>
> #include <asm/irq.h>
> +#include <asm/sclp.h>
> 
> #define VIRTIO_SUBCODE_64 0x0D00
> 
> @@ -453,8 +452,7 @@ static int __init kvm_devices_init(void)
> 		return -ENODEV;
> 
> 	if (test_devices_support(real_memory_size) < 0)
> -		/* No error. */
> -		return 0;
> +		return -ENODEV;
> 
> 	rc = vmem_add_mapping(real_memory_size, PAGE_SIZE);
> 	if (rc)
> @@ -479,29 +477,6 @@ static int __init kvm_devices_init(void)
> 	return 0;
> }
> 
> -/* code for early console output with virtio_console */
> -static __init int early_put_chars(u32 vtermno, const char *buf, int count)
> -{
> -	char scratch[17];
> -	unsigned int len = count;
> -
> -	if (len > sizeof(scratch) - 1)
> -		len = sizeof(scratch) - 1;
> -	scratch[len] = '\0';
> -	memcpy(scratch, buf, len);
> -	kvm_hypercall1(KVM_S390_VIRTIO_NOTIFY, __pa(scratch));
> -	return len;
> -}
> -
> -static int __init s390_virtio_console_init(void)
> -{
> -	if (sclp_has_vt220() || sclp_has_linemode())
> -		return -ENODEV;
> -	return virtio_cons_early_init(early_put_chars);
> -}
> -console_initcall(s390_virtio_console_init);
> -
> -
> /*
>  * We do this after core stuff, but before the drivers.
>  */
> diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
> index 0e98b523a..1350391 100644
> --- a/drivers/s390/kvm/virtio_ccw.c
> +++ b/drivers/s390/kvm/virtio_ccw.c
> @@ -17,7 +17,6 @@
> #include <linux/virtio.h>
> #include <linux/virtio_config.h>
> #include <linux/slab.h>
> -#include <linux/virtio_console.h>
> #include <linux/interrupt.h>
> #include <linux/virtio_ring.h>
> #include <linux/pfn.h>
> -- 
> 1.7.12.4
> 

Re: [PATCH 5/5] KVM: s390: Split out early console code.

[Cornelia Huck]

On Sun, 9 Dec 2012 13:45:47 +0100
Alexander Graf <agraf@suse.de> wrote:

> 
> On 07.12.2012, at 13:29, Cornelia Huck wrote:
> 
> > This code is transport agnostic and can be used by both the legacy
> > virtio code and virtio_ccw.
> 
> This code should die. It's a nightmare to implement in QEMU, because it ends up being a separate char device. I'd prefer if we don't carry it over to the virtio-ccw world :).

I don't really want to kill an interface in a patchset adding a new
transport. We may kill this later on, though...

> 
> 
> Alex
> 
> > 
> > Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > ---
> > drivers/s390/kvm/Makefile       |  2 +-
> > drivers/s390/kvm/early_printk.c | 42 +++++++++++++++++++++++++++++++++++++++++
> > drivers/s390/kvm/kvm_virtio.c   | 29 ++--------------------------
> > drivers/s390/kvm/virtio_ccw.c   |  1 -
> > 4 files changed, 45 insertions(+), 29 deletions(-)
> > create mode 100644 drivers/s390/kvm/early_printk.c
> > 
> > diff --git a/drivers/s390/kvm/Makefile b/drivers/s390/kvm/Makefile
> > index 241891a..a3c8fc4 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 virtio_ccw.o
> > +obj-$(CONFIG_S390_GUEST) += kvm_virtio.o early_printk.o virtio_ccw.o
> > diff --git a/drivers/s390/kvm/early_printk.c b/drivers/s390/kvm/early_printk.c
> > new file mode 100644
> > index 0000000..915d687
> > --- /dev/null
> > +++ b/drivers/s390/kvm/early_printk.c
> > @@ -0,0 +1,42 @@
> > +/*
> > + * early_printk.c - code for early console output with virtio_console
> > + * split off from kvm_virtio.c
> > + *
> > + * Copyright IBM Corp. 2008
> > + *
> > + * 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): Christian Borntraeger <borntraeger@de.ibm.com>
> > + */
> > +
> > +#include <linux/kernel_stat.h>
> > +#include <linux/init.h>
> > +#include <linux/err.h>
> > +#include <linux/virtio_console.h>
> > +#include <linux/kvm_para.h>
> > +#include <asm/kvm_virtio.h>
> > +#include <asm/setup.h>
> > +#include <asm/sclp.h>
> > +
> > +static __init int early_put_chars(u32 vtermno, const char *buf, int count)
> > +{
> > +	char scratch[17];
> > +	unsigned int len = count;
> > +
> > +	if (len > sizeof(scratch) - 1)
> > +		len = sizeof(scratch) - 1;
> > +	scratch[len] = '\0';
> > +	memcpy(scratch, buf, len);
> > +	kvm_hypercall1(KVM_S390_VIRTIO_NOTIFY, __pa(scratch));
> > +	return len;
> > +}
> > +
> > +static int __init s390_virtio_console_init(void)
> > +{
> > +	if (sclp_has_vt220() || sclp_has_linemode())
> > +		return -ENODEV;
> > +	return virtio_cons_early_init(early_put_chars);
> > +}
> > +console_initcall(s390_virtio_console_init);
> > diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
> > index 779e480..6ca06ed 100644
> > --- a/drivers/s390/kvm/kvm_virtio.c
> > +++ b/drivers/s390/kvm/kvm_virtio.c
> > @@ -17,7 +17,6 @@
> > #include <linux/virtio.h>
> > #include <linux/virtio_config.h>
> > #include <linux/slab.h>
> > -#include <linux/virtio_console.h>
> > #include <linux/interrupt.h>
> > #include <linux/virtio_ring.h>
> > #include <linux/export.h>
> > @@ -25,9 +24,9 @@
> > #include <asm/io.h>
> > #include <asm/kvm_para.h>
> > #include <asm/kvm_virtio.h>
> > -#include <asm/sclp.h>
> > #include <asm/setup.h>
> > #include <asm/irq.h>
> > +#include <asm/sclp.h>
> > 
> > #define VIRTIO_SUBCODE_64 0x0D00
> > 
> > @@ -453,8 +452,7 @@ static int __init kvm_devices_init(void)
> > 		return -ENODEV;
> > 
> > 	if (test_devices_support(real_memory_size) < 0)
> > -		/* No error. */
> > -		return 0;
> > +		return -ENODEV;
> > 
> > 	rc = vmem_add_mapping(real_memory_size, PAGE_SIZE);
> > 	if (rc)
> > @@ -479,29 +477,6 @@ static int __init kvm_devices_init(void)
> > 	return 0;
> > }
> > 
> > -/* code for early console output with virtio_console */
> > -static __init int early_put_chars(u32 vtermno, const char *buf, int count)
> > -{
> > -	char scratch[17];
> > -	unsigned int len = count;
> > -
> > -	if (len > sizeof(scratch) - 1)
> > -		len = sizeof(scratch) - 1;
> > -	scratch[len] = '\0';
> > -	memcpy(scratch, buf, len);
> > -	kvm_hypercall1(KVM_S390_VIRTIO_NOTIFY, __pa(scratch));
> > -	return len;
> > -}
> > -
> > -static int __init s390_virtio_console_init(void)
> > -{
> > -	if (sclp_has_vt220() || sclp_has_linemode())
> > -		return -ENODEV;
> > -	return virtio_cons_early_init(early_put_chars);
> > -}
> > -console_initcall(s390_virtio_console_init);
> > -
> > -
> > /*
> >  * We do this after core stuff, but before the drivers.
> >  */
> > diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
> > index 0e98b523a..1350391 100644
> > --- a/drivers/s390/kvm/virtio_ccw.c
> > +++ b/drivers/s390/kvm/virtio_ccw.c
> > @@ -17,7 +17,6 @@
> > #include <linux/virtio.h>
> > #include <linux/virtio_config.h>
> > #include <linux/slab.h>
> > -#include <linux/virtio_console.h>
> > #include <linux/interrupt.h>
> > #include <linux/virtio_ring.h>
> > #include <linux/pfn.h>
> > -- 
> > 1.7.12.4
> > 
> 

Re: [PATCH 5/5] KVM: s390: Split out early console code.

[Alexander Graf]

On 10.12.2012, at 10:06, Cornelia Huck wrote:

> On Sun, 9 Dec 2012 13:45:47 +0100
> Alexander Graf <agraf@suse.de> wrote:
> 
>> 
>> On 07.12.2012, at 13:29, Cornelia Huck wrote:
>> 
>>> This code is transport agnostic and can be used by both the legacy
>>> virtio code and virtio_ccw.
>> 
>> This code should die. It's a nightmare to implement in QEMU, because it ends up being a separate char device. I'd prefer if we don't carry it over to the virtio-ccw world :).
> 
> I don't really want to kill an interface in a patchset adding a new
> transport. We may kill this later on, though...

I fully agree :). What I was trying to say is "leave this to die in the old code and don't bother about it in the new world order". Removing support for the old kvm virtio machine is nothing for today.


Alex

Re: [PATCH 5/5] KVM: s390: Split out early console code.

[Cornelia Huck]

On Tue, 11 Dec 2012 11:10:39 +0100
Alexander Graf <agraf@suse.de> wrote:

> 
> On 10.12.2012, at 10:06, Cornelia Huck wrote:
> 
> > On Sun, 9 Dec 2012 13:45:47 +0100
> > Alexander Graf <agraf@suse.de> wrote:
> > 
> >> 
> >> On 07.12.2012, at 13:29, Cornelia Huck wrote:
> >> 
> >>> This code is transport agnostic and can be used by both the legacy
> >>> virtio code and virtio_ccw.
> >> 
> >> This code should die. It's a nightmare to implement in QEMU, because it ends up being a separate char device. I'd prefer if we don't carry it over to the virtio-ccw world :).
> > 
> > I don't really want to kill an interface in a patchset adding a new
> > transport. We may kill this later on, though...
> 
> I fully agree :). What I was trying to say is "leave this to die in the old code and don't bother about it in the new world order". Removing support for the old kvm virtio machine is nothing for today.

Ah, OK. I'll merge the return -ENODEV into the first patch then and
drop this one.

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

[Cornelia Huck]

Avi, Marcelo,

I'd like to propose inclusion of the guest support patches for
virtio-ccw into 3.8.

I'm confident that the host <-> guest interface for virtio-ccw
is fine now, and the patches have been extensively tested by our
internal test team.

Patch 1 might conceivably be 3.7 material, though I fear it's a
bit late for that.

Patch 2 has been moved over from the host-support patchset since
the limits are needed by the guest driver as well.

Patch 4 has seen some further bugfixes (feature bits, 2G and 4G
problems, device detach handling) and is working well in our
internal environment.

Cornelia Huck (5):
  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.
  KVM: s390: Split out early console code.

 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/early_printk.c |  42 ++
 drivers/s390/kvm/kvm_virtio.c   |  64 ++-
 drivers/s390/kvm/virtio_ccw.c   | 841 ++++++++++++++++++++++++++++++++++++++++
 10 files changed, 936 insertions(+), 37 deletions(-)
 create mode 100644 drivers/s390/kvm/early_printk.c
 create mode 100644 drivers/s390/kvm/virtio_ccw.c

-- 
1.7.12.4

[PATCH 1/5] 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.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 drivers/s390/kvm/kvm_virtio.c | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index 47cccd5..76b95f3 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -419,6 +419,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
  */
@@ -429,21 +449,24 @@ static int __init kvm_devices_init(void)
 	if (!MACHINE_IS_KVM)
 		return -ENODEV;
 
+	if (test_devices_support(real_memory_size) < 0)
+		/* No error. */
+		return 0;
+
+	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