diff mbox series

[kvm-unit-tests,v9,12/12] s390x: css: ssch/tsch with sense and interrupt

Message ID 1592213521-19390-13-git-send-email-pmorel@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: Testing the Channel Subsystem I/O | expand

Commit Message

Pierre Morel June 15, 2020, 9:32 a.m. UTC
After a channel is enabled we start a SENSE_ID command using
the SSCH instruction to recognize the control unit and device.

This tests the success of SSCH, the I/O interruption and the TSCH
instructions.

The SENSE_ID command response is tested to report 0xff inside
its reserved field and to report the same control unit type
as the cu_type kernel argument.

Without the cu_type kernel argument, the test expects a device
with a default control unit type of 0x3832, a.k.a virtio-net-ccw.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 lib/s390x/css.h     |  20 +++++++
 lib/s390x/css_lib.c |  46 +++++++++++++++
 s390x/css.c         | 140 +++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 205 insertions(+), 1 deletion(-)

Comments

Cornelia Huck June 17, 2020, 9:54 a.m. UTC | #1
On Mon, 15 Jun 2020 11:32:01 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:

> After a channel is enabled we start a SENSE_ID command using
> the SSCH instruction to recognize the control unit and device.
> 
> This tests the success of SSCH, the I/O interruption and the TSCH
> instructions.
> 
> The SENSE_ID command response is tested to report 0xff inside
> its reserved field and to report the same control unit type
> as the cu_type kernel argument.
> 
> Without the cu_type kernel argument, the test expects a device
> with a default control unit type of 0x3832, a.k.a virtio-net-ccw.

0x3832 is any virtio-ccw device; you could also test for the cu model
to make sure that it is a net device, but that probably doesn't add
much additional coverage.

> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/css.h     |  20 +++++++
>  lib/s390x/css_lib.c |  46 +++++++++++++++
>  s390x/css.c         | 140 +++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 205 insertions(+), 1 deletion(-)

(...)

> diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
> index 06a76db..c3d93d3 100644
> --- a/lib/s390x/css_lib.c
> +++ b/lib/s390x/css_lib.c
> @@ -128,3 +128,49 @@ retry:
>  		    schid, retry_count, pmcw->flags);
>  	return -1;
>  }
> +
> +int start_ccw1_chain(unsigned int sid, struct ccw1 *ccw)
> +{
> +	struct orb orb = {
> +		.intparm = sid,
> +		.ctrl = ORB_CTRL_ISIC|ORB_CTRL_FMT|ORB_LPM_DFLT,
> +		.cpa = (unsigned int) (unsigned long)ccw,
> +	};
> +
> +	return ssch(sid, &orb);
> +}
> +
> +/*
> + * In the next revisions we will implement the possibility to handle
> + * CCW chains doing this we will need to work with ccw1 pointers.

"In the future, we want to implement support for CCW chains; for that,
we will need to work with ccw1 pointers."

?

> + * For now we only need a unique CCW.
> + */
> +static struct ccw1 unique_ccw;
> +
> +int start_subchannel(unsigned int sid, int code, void *data, int count,
> +		     unsigned char flags)
> +{
> +	int cc;
> +	struct ccw1 *ccw = &unique_ccw;

Hm... it might better to call this function "start_single_ccw" or
something like that.

> +
> +	report_prefix_push("start_subchannel");
> +	/* Build the CCW chain with a single CCW */
> +	ccw->code = code;
> +	ccw->flags = flags; /* No flags need to be set */
> +	ccw->count = count;
> +	ccw->data_address = (int)(unsigned long)data;
> +
> +	cc = start_ccw1_chain(sid, ccw);
> +	if (cc) {
> +		report(0, "start_ccw_chain failed ret=%d", cc);
> +		report_prefix_pop();
> +		return cc;
> +	}
> +	report_prefix_pop();
> +	return 0;
> +}
> +
> +int sch_read_len(int sid)
> +{
> +	return unique_ccw.count;
> +}

This function is very odd... it takes a subchannel id as a parameter,
which it ignores, and instead returns the count field of the static ccw
used when starting I/O. What is the purpose of this function? Grab the
data length for the last I/O operation that was started on the
subchannel? If yes, it might be better to store that information along
with the sid? If it is the length for the last I/O operation that the
code _thinks_ it started, it might be better to reuse that information
from further up in the function instead.

> diff --git a/s390x/css.c b/s390x/css.c
> index 6948d73..6b618a1 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -16,10 +16,18 @@
>  #include <string.h>
>  #include <interrupt.h>
>  #include <asm/arch_def.h>
> +#include <kernel-args.h>
>  
>  #include <css.h>
>  
> +#define DEFAULT_CU_TYPE		0x3832

Maybe append /* virtio-ccw */

> +static unsigned long cu_type = DEFAULT_CU_TYPE;
> +
> +struct lowcore *lowcore = (void *)0x0;
> +
>  static int test_device_sid;
> +static struct irb irb;
> +static struct senseid senseid;
>  
>  static void test_enumerate(void)
>  {
> @@ -45,20 +53,150 @@ static void test_enable(void)
>  	report(cc == 0, "Enable subchannel %08x", test_device_sid);
>  }
>  
> +static void enable_io_isc(void)
> +{
> +	/* Let's enable all ISCs for I/O interrupt */
> +	lctlg(6, 0x00000000ff000000);
> +}
> +
> +static void irq_io(void)
> +{
> +	int ret = 0;
> +	char *flags;
> +	int sid;
> +
> +	report_prefix_push("Interrupt");
> +	/* Lowlevel set the SID as interrupt parameter. */
> +	if (lowcore->io_int_param != test_device_sid) {
> +		report(0,
> +		       "Bad io_int_param: %x expected %x",
> +		       lowcore->io_int_param, test_device_sid);
> +		goto pop;
> +	}
> +	report_prefix_pop();
> +
> +	report_prefix_push("tsch");
> +	sid = lowcore->subsys_id_word;
> +	ret = tsch(sid, &irb);
> +	switch (ret) {
> +	case 1:
> +		dump_irb(&irb);
> +		flags = dump_scsw_flags(irb.scsw.ctrl);
> +		report(0,
> +		       "I/O interrupt, CC 1 but tsch reporting sch %08x as not status pending: %s",
> +		       sid, flags);
> +		break;
> +	case 2:
> +		report(0, "tsch returns unexpected CC 2");
> +		break;
> +	case 3:
> +		report(0, "tsch reporting sch %08x as not operational", sid);
> +		break;
> +	case 0:
> +		/* Stay humble on success */
> +		break;
> +	}
> +pop:
> +	report_prefix_pop();
> +	lowcore->io_old_psw.mask &= ~PSW_MASK_WAIT;
> +}
> +
> +/*
> + * test_sense
> + * Pre-requisits:
> + * - We need the test device as the first recognized
> + *   device by the enumeration.
> + */
> +static void test_sense(void)
> +{
> +	int ret;
> +
> +	if (!test_device_sid) {
> +		report_skip("No device");
> +		return;
> +	}
> +
> +	ret = css_enable(test_device_sid);
> +	if (ret) {
> +		report(0,
> +		       "Could not enable the subchannel: %08x",
> +		       test_device_sid);
> +		return;
> +	}
> +
> +	ret = register_io_int_func(irq_io);
> +	if (ret) {
> +		report(0, "Could not register IRQ handler");
> +		goto unreg_cb;
> +	}
> +
> +	lowcore->io_int_param = 0;
> +
> +	memset(&senseid, 0, sizeof(senseid));
> +	ret = start_subchannel(test_device_sid, CCW_CMD_SENSE_ID,
> +			       &senseid, sizeof(senseid), CCW_F_SLI);
> +	if (ret) {
> +		report(0, "ssch failed for SENSE ID on sch %08x with cc %d",
> +		       test_device_sid, ret);
> +		goto unreg_cb;
> +	}
> +
> +	wait_for_interrupt(PSW_MASK_IO);
> +
> +	ret = sch_read_len(test_device_sid);
> +	if (ret < CSS_SENSEID_COMMON_LEN) {
> +		report(0,
> +		       "ssch succeeded for SENSE ID but report a too short length: %d",
> +		       ret);
> +		goto unreg_cb;
> +	}

Oh, so you want to check something even different: You know what you
put in the request, and you expect a certain minimal length back. But
that length is contained in the scsw, not in the started ccw, isn't it?

> +
> +	if (senseid.reserved != 0xff) {
> +		report(0,
> +		       "ssch succeeded for SENSE ID but reports garbage: %x",
> +		       senseid.reserved);
> +		goto unreg_cb;
> +	}
> +
> +	if (lowcore->io_int_param != test_device_sid)
> +		goto unreg_cb;

You probably want to check this further up? But doesn't irq_io()
already check this?

> +
> +	report_info("senseid length read: %d", ret);
> +	report_info("reserved %02x cu_type %04x cu_model %02x dev_type %04x dev_model %02x",
> +		    senseid.reserved, senseid.cu_type, senseid.cu_model,
> +		    senseid.dev_type, senseid.dev_model);
> +
> +	report(senseid.cu_type == cu_type, "cu_type: expect 0x%04x got 0x%04x",
> +	       (uint16_t) cu_type, senseid.cu_type);
> +
> +unreg_cb:
> +	unregister_io_int_func(irq_io);
> +}
> +
>  static struct {
>  	const char *name;
>  	void (*func)(void);
>  } tests[] = {
>  	{ "enumerate (stsch)", test_enumerate },
>  	{ "enable (msch)", test_enable },
> +	{ "sense (ssch/tsch)", test_sense },
>  	{ NULL, NULL }
>  };
>  
> +static unsigned long value;
> +
>  int main(int argc, char *argv[])
>  {
> -	int i;
> +	int i, ret;
> +
> +	ret = kernel_arg(argc, argv, "cu_type=", &value);
> +	if (!ret)
> +		cu_type = (uint16_t)value;
> +	else
> +		report_info("Using cu_type default value: 0x%04lx", cu_type);
>  
>  	report_prefix_push("Channel Subsystem");
> +	enable_io_isc();
>  	for (i = 0; tests[i].name; i++) {
>  		report_prefix_push(tests[i].name);
>  		tests[i].func();
Pierre Morel June 17, 2020, 11:55 a.m. UTC | #2
On 2020-06-17 11:54, Cornelia Huck wrote:
> On Mon, 15 Jun 2020 11:32:01 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> After a channel is enabled we start a SENSE_ID command using
>> the SSCH instruction to recognize the control unit and device.
>>
>> This tests the success of SSCH, the I/O interruption and the TSCH
>> instructions.
>>
>> The SENSE_ID command response is tested to report 0xff inside
>> its reserved field and to report the same control unit type
>> as the cu_type kernel argument.
>>
>> Without the cu_type kernel argument, the test expects a device
>> with a default control unit type of 0x3832, a.k.a virtio-net-ccw.
> 
> 0x3832 is any virtio-ccw device; you could also test for the cu model
> to make sure that it is a net device, but that probably doesn't add
> much additional coverage.
> 
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/css.h     |  20 +++++++
>>   lib/s390x/css_lib.c |  46 +++++++++++++++
>>   s390x/css.c         | 140 +++++++++++++++++++++++++++++++++++++++++++-
>>   3 files changed, 205 insertions(+), 1 deletion(-)
> 
...snip...

>> +/*
>> + * In the next revisions we will implement the possibility to handle
>> + * CCW chains doing this we will need to work with ccw1 pointers.
> 
> "In the future, we want to implement support for CCW chains; for that,
> we will need to work with ccw1 pointers."
> 
> ?

yes, better, thanks.

> 
>> + * For now we only need a unique CCW.
>> + */
>> +static struct ccw1 unique_ccw;
>> +
>> +int start_subchannel(unsigned int sid, int code, void *data, int count,
>> +		     unsigned char flags)
>> +{
>> +	int cc;
>> +	struct ccw1 *ccw = &unique_ccw;
> 
> Hm... it might better to call this function "start_single_ccw" or
> something like that.

You are right.
I will rework this.
What about differentiating this badly named "start_subchannel()" into:

ccw_setup_ccw(ccw, data, cnt, flgs);
ccw_setup_orb(orb, ccw, flgs)
ccw_start_request(schid, orb);

would be much clearer I think.

> 
>> +
>> +	report_prefix_push("start_subchannel");
>> +	/* Build the CCW chain with a single CCW */
>> +	ccw->code = code;
>> +	ccw->flags = flags; /* No flags need to be set */
>> +	ccw->count = count;
>> +	ccw->data_address = (int)(unsigned long)data;
>> +
>> +	cc = start_ccw1_chain(sid, ccw);
>> +	if (cc) {
>> +		report(0, "start_ccw_chain failed ret=%d", cc);
>> +		report_prefix_pop();
>> +		return cc;
>> +	}
>> +	report_prefix_pop();
>> +	return 0;
>> +}
>> +
>> +int sch_read_len(int sid)
>> +{
>> +	return unique_ccw.count;
>> +}
> 
> This function is very odd... it takes a subchannel id as a parameter,
> which it ignores, and instead returns the count field of the static ccw
> used when starting I/O. What is the purpose of this function? Grab the
> data length for the last I/O operation that was started on the
> subchannel? If yes, it might be better to store that information along
> with the sid? If it is the length for the last I/O operation that the
> code _thinks_ it started, it might be better to reuse that information
> from further up in the function instead.

agreed, I forgot to update this, totally confused.
will rework this.


> 
>> diff --git a/s390x/css.c b/s390x/css.c
>> index 6948d73..6b618a1 100644
>> --- a/s390x/css.c
>> +++ b/s390x/css.c
>> @@ -16,10 +16,18 @@
>>   #include <string.h>
>>   #include <interrupt.h>
>>   #include <asm/arch_def.h>
>> +#include <kernel-args.h>
>>   
>>   #include <css.h>
>>   
>> +#define DEFAULT_CU_TYPE		0x3832
> 
> Maybe append /* virtio-ccw */

yes, thanks

> 
>> +static unsigned long cu_type = DEFAULT_CU_TYPE;
>> +
>> +struct lowcore *lowcore = (void *)0x0;
>> +
>>   static int test_device_sid;
>> +static struct irb irb;
>> +static struct senseid senseid;
>>   
>>   static void test_enumerate(void)
>>   {
>> @@ -45,20 +53,150 @@ static void test_enable(void)
>>   	report(cc == 0, "Enable subchannel %08x", test_device_sid);
>>   }
>>   
>> +static void enable_io_isc(void)
>> +{
>> +	/* Let's enable all ISCs for I/O interrupt */
>> +	lctlg(6, 0x00000000ff000000);
>> +}
>> +
>> +static void irq_io(void)
>> +{
>> +	int ret = 0;
>> +	char *flags;
>> +	int sid;
>> +
>> +	report_prefix_push("Interrupt");
>> +	/* Lowlevel set the SID as interrupt parameter. */
>> +	if (lowcore->io_int_param != test_device_sid) {
>> +		report(0,
>> +		       "Bad io_int_param: %x expected %x",
>> +		       lowcore->io_int_param, test_device_sid);
>> +		goto pop;
>> +	}
>> +	report_prefix_pop();
>> +
>> +	report_prefix_push("tsch");
>> +	sid = lowcore->subsys_id_word;
>> +	ret = tsch(sid, &irb);
>> +	switch (ret) {
>> +	case 1:
>> +		dump_irb(&irb);
>> +		flags = dump_scsw_flags(irb.scsw.ctrl);
>> +		report(0,
>> +		       "I/O interrupt, CC 1 but tsch reporting sch %08x as not status pending: %s",
>> +		       sid, flags);
>> +		break;
>> +	case 2:
>> +		report(0, "tsch returns unexpected CC 2");
>> +		break;
>> +	case 3:
>> +		report(0, "tsch reporting sch %08x as not operational", sid);
>> +		break;
>> +	case 0:
>> +		/* Stay humble on success */
>> +		break;
>> +	}
>> +pop:
>> +	report_prefix_pop();
>> +	lowcore->io_old_psw.mask &= ~PSW_MASK_WAIT;
>> +}
>> +
>> +/*
>> + * test_sense
>> + * Pre-requisits:
>> + * - We need the test device as the first recognized
>> + *   device by the enumeration.
>> + */
>> +static void test_sense(void)
>> +{
>> +	int ret;
>> +
>> +	if (!test_device_sid) {
>> +		report_skip("No device");
>> +		return;
>> +	}
>> +
>> +	ret = css_enable(test_device_sid);
>> +	if (ret) {
>> +		report(0,
>> +		       "Could not enable the subchannel: %08x",
>> +		       test_device_sid);
>> +		return;
>> +	}
>> +
>> +	ret = register_io_int_func(irq_io);
>> +	if (ret) {
>> +		report(0, "Could not register IRQ handler");
>> +		goto unreg_cb;
>> +	}
>> +
>> +	lowcore->io_int_param = 0;
>> +
>> +	memset(&senseid, 0, sizeof(senseid));
>> +	ret = start_subchannel(test_device_sid, CCW_CMD_SENSE_ID,
>> +			       &senseid, sizeof(senseid), CCW_F_SLI);
>> +	if (ret) {
>> +		report(0, "ssch failed for SENSE ID on sch %08x with cc %d",
>> +		       test_device_sid, ret);
>> +		goto unreg_cb;
>> +	}
>> +
>> +	wait_for_interrupt(PSW_MASK_IO);
>> +
>> +	ret = sch_read_len(test_device_sid);
>> +	if (ret < CSS_SENSEID_COMMON_LEN) {
>> +		report(0,
>> +		       "ssch succeeded for SENSE ID but report a too short length: %d",
>> +		       ret);
>> +		goto unreg_cb;
>> +	}
> 
> Oh, so you want to check something even different: You know what you
> put in the request, and you expect a certain minimal length back. But
> that length is contained in the scsw, not in the started ccw, isn't it?

yes it is.

> 
>> +
>> +	if (senseid.reserved != 0xff) {
>> +		report(0,
>> +		       "ssch succeeded for SENSE ID but reports garbage: %x",
>> +		       senseid.reserved);
>> +		goto unreg_cb;
>> +	}
>> +
>> +	if (lowcore->io_int_param != test_device_sid)
>> +		goto unreg_cb;
> 
> You probably want to check this further up? But doesn't irq_io()
> already check this?

yes it does

Thanks for the comments,

I will rework this.

- rework the start_subchannel()
- rework the read_len() if we ever need this

Also thinking to put the irq_io routine inside the library, it will be 
reused by other tests.

Regards,
Pierre
Cornelia Huck June 19, 2020, 6:57 a.m. UTC | #3
On Wed, 17 Jun 2020 13:55:52 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:

> On 2020-06-17 11:54, Cornelia Huck wrote:
> > On Mon, 15 Jun 2020 11:32:01 +0200
> > Pierre Morel <pmorel@linux.ibm.com> wrote:

(...)

> >> +int start_subchannel(unsigned int sid, int code, void *data, int count,
> >> +		     unsigned char flags)
> >> +{
> >> +	int cc;
> >> +	struct ccw1 *ccw = &unique_ccw;  
> > 
> > Hm... it might better to call this function "start_single_ccw" or
> > something like that.  
> 
> You are right.
> I will rework this.
> What about differentiating this badly named "start_subchannel()" into:
> 
> ccw_setup_ccw(ccw, data, cnt, flgs);
> ccw_setup_orb(orb, ccw, flgs)
> ccw_start_request(schid, orb);
> 
> would be much clearer I think.

Not sure about ccw_setup_ccw; might get a bit non-obvious if you're
trying to build a chain.

Let's see how this turns out.

(...)

> I will rework this.
> 
> - rework the start_subchannel()
> - rework the read_len() if we ever need this

I think checking the count after the request concluded is actually a
good idea. In the future, we could also add a check that it matches the
requested length for a request where SLI was not specified.

> 
> Also thinking to put the irq_io routine inside the library, it will be 
> reused by other tests.

Yes, that probably makes sense as well.
Pierre Morel July 2, 2020, 12:56 p.m. UTC | #4
On 2020-06-19 08:57, Cornelia Huck wrote:
> On Wed, 17 Jun 2020 13:55:52 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> On 2020-06-17 11:54, Cornelia Huck wrote:
>>> On Mon, 15 Jun 2020 11:32:01 +0200
>>> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
> (...)
> 
>>>> +int start_subchannel(unsigned int sid, int code, void *data, int count,
>>>> +		     unsigned char flags)
>>>> +{
>>>> +	int cc;
>>>> +	struct ccw1 *ccw = &unique_ccw;
>>>
>>> Hm... it might better to call this function "start_single_ccw" or
>>> something like that.
>>
>> You are right.
>> I will rework this.
>> What about differentiating this badly named "start_subchannel()" into:
>>
>> ccw_setup_ccw(ccw, data, cnt, flgs);
>> ccw_setup_orb(orb, ccw, flgs)
>> ccw_start_request(schid, orb);
>>
>> would be much clearer I think.
> 
> Not sure about ccw_setup_ccw; might get a bit non-obvious if you're
> trying to build a chain.
> 
> Let's see how this turns out.
> 
> (...)
> 
>> I will rework this.
>>
>> - rework the start_subchannel()
>> - rework the read_len() if we ever need this
> 
> I think checking the count after the request concluded is actually a
> good idea. In the future, we could also add a check that it matches the
> requested length for a request where SLI was not specified.
> 
>>
>> Also thinking to put the irq_io routine inside the library, it will be
>> reused by other tests.
> 
> Yes, that probably makes sense as well.
> 

Thanks,
I respin soon.

Regards,
Pierre
diff mbox series

Patch

diff --git a/lib/s390x/css.h b/lib/s390x/css.h
index 0ddceb1..3091d63 100644
--- a/lib/s390x/css.h
+++ b/lib/s390x/css.h
@@ -100,6 +100,19 @@  struct irb {
 	uint32_t emw[8];
 } __attribute__ ((aligned(4)));
 
+#define CCW_CMD_SENSE_ID	0xe4
+#define CSS_SENSEID_COMMON_LEN	8
+struct senseid {
+	/* common part */
+	uint8_t reserved;        /* always 0x'FF' */
+	uint16_t cu_type;        /* control unit type */
+	uint8_t cu_model;        /* control unit model */
+	uint16_t dev_type;       /* device type */
+	uint8_t dev_model;       /* device model */
+	uint8_t unused;          /* padding byte */
+	uint8_t padding[256 - 10]; /* Extra padding for CCW */
+} __attribute__ ((aligned(4))) __attribute__ ((packed));
+
 /* CSS low level access functions */
 
 static inline int ssch(unsigned long schid, struct orb *addr)
@@ -253,4 +266,11 @@  int css_enumerate(void);
 #define MAX_ENABLE_RETRIES      5
 int css_enable(int schid);
 
+
+/* Library functions */
+int start_ccw1_chain(unsigned int sid, struct ccw1 *ccw);
+int start_subchannel(unsigned int sid, int code, void *data, int count,
+		     unsigned char flags);
+int sch_read_len(int sid);
+
 #endif
diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
index 06a76db..c3d93d3 100644
--- a/lib/s390x/css_lib.c
+++ b/lib/s390x/css_lib.c
@@ -128,3 +128,49 @@  retry:
 		    schid, retry_count, pmcw->flags);
 	return -1;
 }
+
+int start_ccw1_chain(unsigned int sid, struct ccw1 *ccw)
+{
+	struct orb orb = {
+		.intparm = sid,
+		.ctrl = ORB_CTRL_ISIC|ORB_CTRL_FMT|ORB_LPM_DFLT,
+		.cpa = (unsigned int) (unsigned long)ccw,
+	};
+
+	return ssch(sid, &orb);
+}
+
+/*
+ * In the next revisions we will implement the possibility to handle
+ * CCW chains doing this we will need to work with ccw1 pointers.
+ * For now we only need a unique CCW.
+ */
+static struct ccw1 unique_ccw;
+
+int start_subchannel(unsigned int sid, int code, void *data, int count,
+		     unsigned char flags)
+{
+	int cc;
+	struct ccw1 *ccw = &unique_ccw;
+
+	report_prefix_push("start_subchannel");
+	/* Build the CCW chain with a single CCW */
+	ccw->code = code;
+	ccw->flags = flags; /* No flags need to be set */
+	ccw->count = count;
+	ccw->data_address = (int)(unsigned long)data;
+
+	cc = start_ccw1_chain(sid, ccw);
+	if (cc) {
+		report(0, "start_ccw_chain failed ret=%d", cc);
+		report_prefix_pop();
+		return cc;
+	}
+	report_prefix_pop();
+	return 0;
+}
+
+int sch_read_len(int sid)
+{
+	return unique_ccw.count;
+}
diff --git a/s390x/css.c b/s390x/css.c
index 6948d73..6b618a1 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -16,10 +16,18 @@ 
 #include <string.h>
 #include <interrupt.h>
 #include <asm/arch_def.h>
+#include <kernel-args.h>
 
 #include <css.h>
 
+#define DEFAULT_CU_TYPE		0x3832
+static unsigned long cu_type = DEFAULT_CU_TYPE;
+
+struct lowcore *lowcore = (void *)0x0;
+
 static int test_device_sid;
+static struct irb irb;
+static struct senseid senseid;
 
 static void test_enumerate(void)
 {
@@ -45,20 +53,150 @@  static void test_enable(void)
 	report(cc == 0, "Enable subchannel %08x", test_device_sid);
 }
 
+static void enable_io_isc(void)
+{
+	/* Let's enable all ISCs for I/O interrupt */
+	lctlg(6, 0x00000000ff000000);
+}
+
+static void irq_io(void)
+{
+	int ret = 0;
+	char *flags;
+	int sid;
+
+	report_prefix_push("Interrupt");
+	/* Lowlevel set the SID as interrupt parameter. */
+	if (lowcore->io_int_param != test_device_sid) {
+		report(0,
+		       "Bad io_int_param: %x expected %x",
+		       lowcore->io_int_param, test_device_sid);
+		goto pop;
+	}
+	report_prefix_pop();
+
+	report_prefix_push("tsch");
+	sid = lowcore->subsys_id_word;
+	ret = tsch(sid, &irb);
+	switch (ret) {
+	case 1:
+		dump_irb(&irb);
+		flags = dump_scsw_flags(irb.scsw.ctrl);
+		report(0,
+		       "I/O interrupt, CC 1 but tsch reporting sch %08x as not status pending: %s",
+		       sid, flags);
+		break;
+	case 2:
+		report(0, "tsch returns unexpected CC 2");
+		break;
+	case 3:
+		report(0, "tsch reporting sch %08x as not operational", sid);
+		break;
+	case 0:
+		/* Stay humble on success */
+		break;
+	}
+pop:
+	report_prefix_pop();
+	lowcore->io_old_psw.mask &= ~PSW_MASK_WAIT;
+}
+
+/*
+ * test_sense
+ * Pre-requisits:
+ * - We need the test device as the first recognized
+ *   device by the enumeration.
+ */
+static void test_sense(void)
+{
+	int ret;
+
+	if (!test_device_sid) {
+		report_skip("No device");
+		return;
+	}
+
+	ret = css_enable(test_device_sid);
+	if (ret) {
+		report(0,
+		       "Could not enable the subchannel: %08x",
+		       test_device_sid);
+		return;
+	}
+
+	ret = register_io_int_func(irq_io);
+	if (ret) {
+		report(0, "Could not register IRQ handler");
+		goto unreg_cb;
+	}
+
+	lowcore->io_int_param = 0;
+
+	memset(&senseid, 0, sizeof(senseid));
+	ret = start_subchannel(test_device_sid, CCW_CMD_SENSE_ID,
+			       &senseid, sizeof(senseid), CCW_F_SLI);
+	if (ret) {
+		report(0, "ssch failed for SENSE ID on sch %08x with cc %d",
+		       test_device_sid, ret);
+		goto unreg_cb;
+	}
+
+	wait_for_interrupt(PSW_MASK_IO);
+
+	ret = sch_read_len(test_device_sid);
+	if (ret < CSS_SENSEID_COMMON_LEN) {
+		report(0,
+		       "ssch succeeded for SENSE ID but report a too short length: %d",
+		       ret);
+		goto unreg_cb;
+	}
+
+	if (senseid.reserved != 0xff) {
+		report(0,
+		       "ssch succeeded for SENSE ID but reports garbage: %x",
+		       senseid.reserved);
+		goto unreg_cb;
+	}
+
+	if (lowcore->io_int_param != test_device_sid)
+		goto unreg_cb;
+
+	report_info("senseid length read: %d", ret);
+	report_info("reserved %02x cu_type %04x cu_model %02x dev_type %04x dev_model %02x",
+		    senseid.reserved, senseid.cu_type, senseid.cu_model,
+		    senseid.dev_type, senseid.dev_model);
+
+	report(senseid.cu_type == cu_type, "cu_type: expect 0x%04x got 0x%04x",
+	       (uint16_t) cu_type, senseid.cu_type);
+
+unreg_cb:
+	unregister_io_int_func(irq_io);
+}
+
 static struct {
 	const char *name;
 	void (*func)(void);
 } tests[] = {
 	{ "enumerate (stsch)", test_enumerate },
 	{ "enable (msch)", test_enable },
+	{ "sense (ssch/tsch)", test_sense },
 	{ NULL, NULL }
 };
 
+static unsigned long value;
+
 int main(int argc, char *argv[])
 {
-	int i;
+	int i, ret;
+
+	ret = kernel_arg(argc, argv, "cu_type=", &value);
+	if (!ret)
+		cu_type = (uint16_t)value;
+	else
+		report_info("Using cu_type default value: 0x%04lx", cu_type);
 
 	report_prefix_push("Channel Subsystem");
+	enable_io_isc();
 	for (i = 0; tests[i].name; i++) {
 		report_prefix_push(tests[i].name);
 		tests[i].func();