diff mbox series

[kvm-unit-tests,v3,7/9] s390x: css: msch, enable test

Message ID 1575649588-6127-8-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 Dec. 6, 2019, 4:26 p.m. UTC
A second step when testing the channel subsystem is to prepare a channel
for use.
This includes:
- Get the current SubCHannel Information Block (SCHIB) using STSCH
- Update it in memory to set the ENABLE bit
- Tell the CSS that the SCHIB has been modified using MSCH
- Get the SCHIB from the CSS again to verify that the subchannel is
  enabled.

This tests the success of the MSCH instruction by enabling a channel.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 s390x/css.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

Comments

Cornelia Huck Dec. 9, 2019, 4:54 p.m. UTC | #1
On Fri,  6 Dec 2019 17:26:26 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

> A second step when testing the channel subsystem is to prepare a channel
> for use.
> This includes:
> - Get the current SubCHannel Information Block (SCHIB) using STSCH
> - Update it in memory to set the ENABLE bit
> - Tell the CSS that the SCHIB has been modified using MSCH
> - Get the SCHIB from the CSS again to verify that the subchannel is
>   enabled.
> 
> This tests the success of the MSCH instruction by enabling a channel.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  s390x/css.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/s390x/css.c b/s390x/css.c
> index 3d4a986..4c0031c 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -58,11 +58,50 @@ static void test_enumerate(void)
>  	report("Tested %d devices, %d found", 1, scn, found);
>  }
>  
> +static void test_enable(void)
> +{
> +	struct pmcw *pmcw = &schib.pmcw;
> +	int cc;
> +
> +	if (!test_device_sid) {
> +		report_skip("No device");
> +		return;
> +	}
> +	/* Read the SCIB for this subchannel */

s/SCIB/SCHIB/

> +	cc = stsch(test_device_sid, &schib);
> +	if (cc) {
> +		report("stsch cc=%d", 0, cc);
> +		return;
> +	}
> +	/* Update the SCHIB to enable the channel */
> +	pmcw->flags |= PMCW_ENABLE;
> +
> +	/* Tell the CSS we want to modify the subchannel */
> +	cc = msch(test_device_sid, &schib);
> +	if (cc) {
> +		report("msch cc=%d", 0, cc);

So you expect the subchannel to be idle? Probably true, especially as
QEMU has no reason to post an unsolicited interrupt for a test device.

> +		return;
> +	}
> +
> +	/* Read the SCHIB again to verify the enablement */
> +	cc = stsch(test_device_sid, &schib);
> +	if (cc) {
> +		report("stsch cc=%d", 0, cc);
> +		return;
> +	}
> +	if (!(pmcw->flags & PMCW_ENABLE)) {
> +		report("Enable failed. pmcw: %x", 0, pmcw->flags);

This check is fine when running under KVM. If this test is modified to
run under z/VM in the future, you probably should retry here: I've seen
the enable bit 'stick' only after the second msch() there.

> +		return;
> +	}
> +	report("Tested", 1);
> +}
> +
>  static struct {
>  	const char *name;
>  	void (*func)(void);
>  } tests[] = {
>  	{ "enumerate (stsch)", test_enumerate },
> +	{ "enable (msch)", test_enable },
>  	{ NULL, NULL }
>  };
>
Pierre Morel Dec. 10, 2019, 9:01 a.m. UTC | #2
On 2019-12-09 17:54, Cornelia Huck wrote:
> On Fri,  6 Dec 2019 17:26:26 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> A second step when testing the channel subsystem is to prepare a channel
>> for use.
>> This includes:
>> - Get the current SubCHannel Information Block (SCHIB) using STSCH
>> - Update it in memory to set the ENABLE bit
>> - Tell the CSS that the SCHIB has been modified using MSCH
>> - Get the SCHIB from the CSS again to verify that the subchannel is
>>    enabled.
>>
>> This tests the success of the MSCH instruction by enabling a channel.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   s390x/css.c | 39 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 39 insertions(+)
>>
>> diff --git a/s390x/css.c b/s390x/css.c
>> index 3d4a986..4c0031c 100644
>> --- a/s390x/css.c
>> +++ b/s390x/css.c
>> @@ -58,11 +58,50 @@ static void test_enumerate(void)
>>   	report("Tested %d devices, %d found", 1, scn, found);
>>   }
>>   
>> +static void test_enable(void)
>> +{
>> +	struct pmcw *pmcw = &schib.pmcw;
>> +	int cc;
>> +
>> +	if (!test_device_sid) {
>> +		report_skip("No device");
>> +		return;
>> +	}
>> +	/* Read the SCIB for this subchannel */
> 
> s/SCIB/SCHIB/

yes

> 
>> +	cc = stsch(test_device_sid, &schib);
>> +	if (cc) {
>> +		report("stsch cc=%d", 0, cc);
>> +		return;
>> +	}
>> +	/* Update the SCHIB to enable the channel */
>> +	pmcw->flags |= PMCW_ENABLE;
>> +
>> +	/* Tell the CSS we want to modify the subchannel */
>> +	cc = msch(test_device_sid, &schib);
>> +	if (cc) {
>> +		report("msch cc=%d", 0, cc);
> 
> So you expect the subchannel to be idle? Probably true, especially as
> QEMU has no reason to post an unsolicited interrupt for a test device.
> 
>> +		return;
>> +	}
>> +
>> +	/* Read the SCHIB again to verify the enablement */
>> +	cc = stsch(test_device_sid, &schib);
>> +	if (cc) {
>> +		report("stsch cc=%d", 0, cc);
>> +		return;
>> +	}
>> +	if (!(pmcw->flags & PMCW_ENABLE)) {
>> +		report("Enable failed. pmcw: %x", 0, pmcw->flags);
> 
> This check is fine when running under KVM. If this test is modified to
> run under z/VM in the future, you probably should retry here: I've seen
> the enable bit 'stick' only after the second msch() there.

Oh. Thanks, may be I can loop with a delay and count.
If I need to do this may be I need to create dedicated sub-functions to 
include the sanity tests.

> 
>> +		return;
>> +	}
>> +	report("Tested", 1);
>> +}
>> +
>>   static struct {
>>   	const char *name;
>>   	void (*func)(void);
>>   } tests[] = {
>>   	{ "enumerate (stsch)", test_enumerate },
>> +	{ "enable (msch)", test_enable },
>>   	{ NULL, NULL }
>>   };
>>   
>
Cornelia Huck Dec. 10, 2019, 9:09 a.m. UTC | #3
On Tue, 10 Dec 2019 10:01:46 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

> On 2019-12-09 17:54, Cornelia Huck wrote:
> > On Fri,  6 Dec 2019 17:26:26 +0100
> > Pierre Morel <pmorel@linux.ibm.com> wrote:
> >   
> >> A second step when testing the channel subsystem is to prepare a channel
> >> for use.
> >> This includes:
> >> - Get the current SubCHannel Information Block (SCHIB) using STSCH
> >> - Update it in memory to set the ENABLE bit
> >> - Tell the CSS that the SCHIB has been modified using MSCH
> >> - Get the SCHIB from the CSS again to verify that the subchannel is
> >>    enabled.
> >>
> >> This tests the success of the MSCH instruction by enabling a channel.
> >>
> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> >> ---
> >>   s390x/css.c | 39 +++++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 39 insertions(+)
> >>
> >> diff --git a/s390x/css.c b/s390x/css.c
> >> index 3d4a986..4c0031c 100644
> >> --- a/s390x/css.c
> >> +++ b/s390x/css.c
> >> @@ -58,11 +58,50 @@ static void test_enumerate(void)
> >>   	report("Tested %d devices, %d found", 1, scn, found);
> >>   }
> >>   
> >> +static void test_enable(void)
> >> +{
> >> +	struct pmcw *pmcw = &schib.pmcw;
> >> +	int cc;
> >> +
> >> +	if (!test_device_sid) {
> >> +		report_skip("No device");
> >> +		return;
> >> +	}
> >> +	/* Read the SCIB for this subchannel */  
> > 
> > s/SCIB/SCHIB/  
> 
> yes
> 
> >   
> >> +	cc = stsch(test_device_sid, &schib);
> >> +	if (cc) {
> >> +		report("stsch cc=%d", 0, cc);
> >> +		return;
> >> +	}
> >> +	/* Update the SCHIB to enable the channel */
> >> +	pmcw->flags |= PMCW_ENABLE;
> >> +
> >> +	/* Tell the CSS we want to modify the subchannel */
> >> +	cc = msch(test_device_sid, &schib);
> >> +	if (cc) {
> >> +		report("msch cc=%d", 0, cc);  
> > 
> > So you expect the subchannel to be idle? Probably true, especially as
> > QEMU has no reason to post an unsolicited interrupt for a test device.
> >   
> >> +		return;
> >> +	}
> >> +
> >> +	/* Read the SCHIB again to verify the enablement */
> >> +	cc = stsch(test_device_sid, &schib);
> >> +	if (cc) {
> >> +		report("stsch cc=%d", 0, cc);
> >> +		return;
> >> +	}
> >> +	if (!(pmcw->flags & PMCW_ENABLE)) {
> >> +		report("Enable failed. pmcw: %x", 0, pmcw->flags);  
> > 
> > This check is fine when running under KVM. If this test is modified to
> > run under z/VM in the future, you probably should retry here: I've seen
> > the enable bit 'stick' only after the second msch() there.  
> 
> Oh. Thanks, may be I can loop with a delay and count.

FWIW, the Linux kernel code is trying 5 times.

> If I need to do this may be I need to create dedicated sub-functions to 
> include the sanity tests.

I'm not sure how worthwhile investing time here is, actually: If you
don't plan to run under anything but KVM, you won't need it. I'm not
sure if current versions of z/VM still display the same behaviour (it
has been some time...); on the other hand, it is compliant with the
architecture...

> 
> >   
> >> +		return;
> >> +	}
> >> +	report("Tested", 1);
> >> +}
> >> +
> >>   static struct {
> >>   	const char *name;
> >>   	void (*func)(void);
> >>   } tests[] = {
> >>   	{ "enumerate (stsch)", test_enumerate },
> >> +	{ "enable (msch)", test_enable },
> >>   	{ NULL, NULL }
> >>   };
> >>     
> >   
>
Pierre Morel Dec. 10, 2019, 5:35 p.m. UTC | #4
On 2019-12-10 10:09, Cornelia Huck wrote:
> On Tue, 10 Dec 2019 10:01:46 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> On 2019-12-09 17:54, Cornelia Huck wrote:
>>> On Fri,  6 Dec 2019 17:26:26 +0100
>>> Pierre Morel <pmorel@linux.ibm.com> wrote:
>>>    
>>>> A second step when testing the channel subsystem is to prepare a channel
>>>> for use.
>>>> This includes:
>>>> - Get the current SubCHannel Information Block (SCHIB) using STSCH
>>>> - Update it in memory to set the ENABLE bit
>>>> - Tell the CSS that the SCHIB has been modified using MSCH
>>>> - Get the SCHIB from the CSS again to verify that the subchannel is
>>>>     enabled.
>>>>
>>>> This tests the success of the MSCH instruction by enabling a channel.
>>>>
>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>> ---
>>>>    s390x/css.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 39 insertions(+)
>>>>
>>>> diff --git a/s390x/css.c b/s390x/css.c
>>>> index 3d4a986..4c0031c 100644
>>>> --- a/s390x/css.c
>>>> +++ b/s390x/css.c
>>>> @@ -58,11 +58,50 @@ static void test_enumerate(void)
>>>>    	report("Tested %d devices, %d found", 1, scn, found);
>>>>    }
>>>>    
>>>> +static void test_enable(void)
>>>> +{
>>>> +	struct pmcw *pmcw = &schib.pmcw;
>>>> +	int cc;
>>>> +
>>>> +	if (!test_device_sid) {
>>>> +		report_skip("No device");
>>>> +		return;
>>>> +	}
>>>> +	/* Read the SCIB for this subchannel */
>>>
>>> s/SCIB/SCHIB/
>>
>> yes
>>
>>>    
>>>> +	cc = stsch(test_device_sid, &schib);
>>>> +	if (cc) {
>>>> +		report("stsch cc=%d", 0, cc);
>>>> +		return;
>>>> +	}
>>>> +	/* Update the SCHIB to enable the channel */
>>>> +	pmcw->flags |= PMCW_ENABLE;
>>>> +
>>>> +	/* Tell the CSS we want to modify the subchannel */
>>>> +	cc = msch(test_device_sid, &schib);
>>>> +	if (cc) {
>>>> +		report("msch cc=%d", 0, cc);
>>>
>>> So you expect the subchannel to be idle? Probably true, especially as
>>> QEMU has no reason to post an unsolicited interrupt for a test device.
>>>    
>>>> +		return;
>>>> +	}
>>>> +
>>>> +	/* Read the SCHIB again to verify the enablement */
>>>> +	cc = stsch(test_device_sid, &schib);
>>>> +	if (cc) {
>>>> +		report("stsch cc=%d", 0, cc);
>>>> +		return;
>>>> +	}
>>>> +	if (!(pmcw->flags & PMCW_ENABLE)) {
>>>> +		report("Enable failed. pmcw: %x", 0, pmcw->flags);
>>>
>>> This check is fine when running under KVM. If this test is modified to
>>> run under z/VM in the future, you probably should retry here: I've seen
>>> the enable bit 'stick' only after the second msch() there.
>>
>> Oh. Thanks, may be I can loop with a delay and count.
> 
> FWIW, the Linux kernel code is trying 5 times.
> 
>> If I need to do this may be I need to create dedicated sub-functions to
>> include the sanity tests.
> 
> I'm not sure how worthwhile investing time here is, actually: If you
> don't plan to run under anything but KVM, you won't need it. I'm not
> sure if current versions of z/VM still display the same behaviour (it
> has been some time...); on the other hand, it is compliant with the
> architecture...

OK, I just insert the retry.

Thanks,
Pierre
diff mbox series

Patch

diff --git a/s390x/css.c b/s390x/css.c
index 3d4a986..4c0031c 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -58,11 +58,50 @@  static void test_enumerate(void)
 	report("Tested %d devices, %d found", 1, scn, found);
 }
 
+static void test_enable(void)
+{
+	struct pmcw *pmcw = &schib.pmcw;
+	int cc;
+
+	if (!test_device_sid) {
+		report_skip("No device");
+		return;
+	}
+	/* Read the SCIB for this subchannel */
+	cc = stsch(test_device_sid, &schib);
+	if (cc) {
+		report("stsch cc=%d", 0, cc);
+		return;
+	}
+	/* Update the SCHIB to enable the channel */
+	pmcw->flags |= PMCW_ENABLE;
+
+	/* Tell the CSS we want to modify the subchannel */
+	cc = msch(test_device_sid, &schib);
+	if (cc) {
+		report("msch cc=%d", 0, cc);
+		return;
+	}
+
+	/* Read the SCHIB again to verify the enablement */
+	cc = stsch(test_device_sid, &schib);
+	if (cc) {
+		report("stsch cc=%d", 0, cc);
+		return;
+	}
+	if (!(pmcw->flags & PMCW_ENABLE)) {
+		report("Enable failed. pmcw: %x", 0, pmcw->flags);
+		return;
+	}
+	report("Tested", 1);
+}
+
 static struct {
 	const char *name;
 	void (*func)(void);
 } tests[] = {
 	{ "enumerate (stsch)", test_enumerate },
+	{ "enable (msch)", test_enable },
 	{ NULL, NULL }
 };