diff mbox series

[kvm-unit-tests,v3,6/8] s390x: Add more tests for STSCH

Message ID 20220223132940.2765217-7-nrb@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: Extend instruction interception tests | expand

Commit Message

Nico Boehr Feb. 23, 2022, 1:29 p.m. UTC
css_lib extensively uses STSCH, but two more cases deserve their own
tests:

- unaligned address for SCHIB. We check for misalignment by 1 and 2
  bytes.
- channel not operational
- bit 47 in SID not set
- bit 5 of PMCW flags.
  As per the principles of operation, bit 5 of the PMCW flags shall be
  ignored by msch and always stored as zero by stsch.

  Older QEMU versions require this bit to always be zero on msch,
  which is why this test may fail. A fix is available in QEMU master
  commit 2df59b73e086 ("s390x/css: fix PMCW invalid mask").

Here's the QEMU PMCW invalid mask fix: https://lists.nongnu.org/archive/html/qemu-s390x/2021-12/msg00100.html

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
---
 s390x/css.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

Comments

Claudio Imbrenda Feb. 23, 2022, 3:16 p.m. UTC | #1
On Wed, 23 Feb 2022 14:29:38 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:

> css_lib extensively uses STSCH, but two more cases deserve their own
> tests:
> 
> - unaligned address for SCHIB. We check for misalignment by 1 and 2
>   bytes.
> - channel not operational
> - bit 47 in SID not set
> - bit 5 of PMCW flags.
>   As per the principles of operation, bit 5 of the PMCW flags shall be
>   ignored by msch and always stored as zero by stsch.
> 
>   Older QEMU versions require this bit to always be zero on msch,
>   which is why this test may fail. A fix is available in QEMU master
>   commit 2df59b73e086 ("s390x/css: fix PMCW invalid mask").
> 
> Here's the QEMU PMCW invalid mask fix: https://lists.nongnu.org/archive/html/qemu-s390x/2021-12/msg00100.html
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  s390x/css.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
> 
> diff --git a/s390x/css.c b/s390x/css.c
> index a90a0cd64e2b..021eb12573c0 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -496,6 +496,78 @@ static void test_ssch(void)
>  	report_prefix_pop();
>  }
>  
> +static void test_stsch(void)
> +{
> +	const int align_to = 4;
> +	struct schib schib;
> +	int cc;
> +
> +	if (!test_device_sid) {
> +		report_skip("No device");
> +		return;
> +	}
> +
> +	report_prefix_push("Unaligned");
> +	for (int i = 1; i < align_to; i *= 2) {
> +		report_prefix_pushf("%d", i);
> +
> +		expect_pgm_int();
> +		stsch(test_device_sid, (struct schib *)(alignment_test_page + i));
> +		check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +
> +		report_prefix_pop();
> +	}
> +	report_prefix_pop();
> +
> +	report_prefix_push("Invalid subchannel number");
> +	cc = stsch(0x0001ffff, &schib);
> +	report(cc == 3, "Channel not operational");
> +	report_prefix_pop();
> +
> +	report_prefix_push("Bit 47 in SID is zero");
> +	expect_pgm_int();
> +	stsch(0x0000ffff, &schib);
> +	check_pgm_int_code(PGM_INT_CODE_OPERAND);
> +	report_prefix_pop();
> +}
> +
> +static void test_pmcw_bit5(void)
> +{
> +	int cc;
> +	uint16_t old_pmcw_flags;
> +
> +	cc = stsch(test_device_sid, &schib);
> +	if (cc) {
> +		report_fail("stsch: sch %08x failed with cc=%d", test_device_sid, cc);
> +		return;
> +	}
> +	old_pmcw_flags = schib.pmcw.flags;
> +
> +	report_prefix_push("Bit 5 set");
> +
> +	schib.pmcw.flags = old_pmcw_flags | BIT(15 - 5);
> +	cc = msch(test_device_sid, &schib);
> +	report(!cc, "MSCH cc == 0");
> +
> +	cc = stsch(test_device_sid, &schib);
> +	report(!cc, "STSCH cc == 0");
> +	report(!(schib.pmcw.flags & BIT(15 - 5)), "STSCH PMCW Bit 5 is clear");
> +
> +	report_prefix_pop();
> +
> +	report_prefix_push("Bit 5 clear");
> +
> +	schib.pmcw.flags = old_pmcw_flags & ~BIT(15 - 5);
> +	cc = msch(test_device_sid, &schib);
> +	report(!cc, "MSCH cc == 0");
> +
> +	cc = stsch(test_device_sid, &schib);
> +	report(!cc, "STSCH cc == 0");
> +	report(!(schib.pmcw.flags & BIT(15 - 5)), "STSCH PMCW Bit 5 is clear");
> +
> +	report_prefix_pop();
> +}
> +
>  static struct {
>  	const char *name;
>  	void (*func)(void);
> @@ -511,6 +583,8 @@ static struct {
>  	{ "msch", test_msch },
>  	{ "stcrw", test_stcrw },
>  	{ "ssch", test_ssch },
> +	{ "stsch", test_stsch },
> +	{ "pmcw bit 5 ignored", test_pmcw_bit5 },
>  	{ NULL, NULL }
>  };
>
Janosch Frank Feb. 23, 2022, 3:39 p.m. UTC | #2
On 2/23/22 14:29, Nico Boehr wrote:
> css_lib extensively uses STSCH, but two more cases deserve their own
> tests:
> 
> - unaligned address for SCHIB. We check for misalignment by 1 and 2
>    bytes.
> - channel not operational
> - bit 47 in SID not set
> - bit 5 of PMCW flags.
>    As per the principles of operation, bit 5 of the PMCW flags shall be
>    ignored by msch and always stored as zero by stsch.
> 
>    Older QEMU versions require this bit to always be zero on msch,
>    which is why this test may fail. A fix is available in QEMU master
>    commit 2df59b73e086 ("s390x/css: fix PMCW invalid mask"). >
> Here's the QEMU PMCW invalid mask fix: https://lists.nongnu.org/archive/html/qemu-s390x/2021-12/msg00100.html
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> ---
>   s390x/css.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 74 insertions(+)
> 
> diff --git a/s390x/css.c b/s390x/css.c
> index a90a0cd64e2b..021eb12573c0 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -496,6 +496,78 @@ static void test_ssch(void)
>   	report_prefix_pop();
>   }
>   
> +static void test_stsch(void)
> +{
> +	const int align_to = 4;
> +	struct schib schib;
> +	int cc;
> +
> +	if (!test_device_sid) {
> +		report_skip("No device");
> +		return;
> +	}
> +
> +	report_prefix_push("Unaligned");
> +	for (int i = 1; i < align_to; i *= 2) {
> +		report_prefix_pushf("%d", i);
> +
> +		expect_pgm_int();
> +		stsch(test_device_sid, (struct schib *)(alignment_test_page + i));
> +		check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +
> +		report_prefix_pop();
> +	}
> +	report_prefix_pop();
> +
> +	report_prefix_push("Invalid subchannel number");
> +	cc = stsch(0x0001ffff, &schib);
> +	report(cc == 3, "Channel not operational");
> +	report_prefix_pop();
> +
> +	report_prefix_push("Bit 47 in SID is zero");
> +	expect_pgm_int();
> +	stsch(0x0000ffff, &schib);
> +	check_pgm_int_code(PGM_INT_CODE_OPERAND);
> +	report_prefix_pop();

Add a comment:
No matter if the multiple-subchannel-set facility is installed or not, 
bit 47 always needs to be 1.

Do we have the MSS facility?
If yes, could we disable it to test the 32-47 == 0x0001 case?

> +}
> +
> +static void test_pmcw_bit5(void)
> +{
> +	int cc;
> +	uint16_t old_pmcw_flags;

I need a comment here for further reference since that behavior is 
documented at the description of the schib and not where STSCH is described:
According to architecture MSCH does ignore bit 5 of the second word but 
STSCH will store bit 5 as zero.


We could check if bits 0,1 and 6,7 are also zero but I'm not sure if 
that's interesting since MSCH does not ignore those bits and should 
result in an operand exception when trying to set them.

@Halil, @Pierre: Any opinions?

> +
> +	cc = stsch(test_device_sid, &schib);
> +	if (cc) {
> +		report_fail("stsch: sch %08x failed with cc=%d", test_device_sid, cc);
> +		return;
> +	}
> +	old_pmcw_flags = schib.pmcw.flags;
> +
> +	report_prefix_push("Bit 5 set");
> +
> +	schib.pmcw.flags = old_pmcw_flags | BIT(15 - 5);
> +	cc = msch(test_device_sid, &schib);
> +	report(!cc, "MSCH cc == 0");
> +
> +	cc = stsch(test_device_sid, &schib);
> +	report(!cc, "STSCH cc == 0");
> +	report(!(schib.pmcw.flags & BIT(15 - 5)), "STSCH PMCW Bit 5 is clear");
> +
> +	report_prefix_pop();
> +
> +	report_prefix_push("Bit 5 clear");
> +
> +	schib.pmcw.flags = old_pmcw_flags & ~BIT(15 - 5);
> +	cc = msch(test_device_sid, &schib);
> +	report(!cc, "MSCH cc == 0");
> +
> +	cc = stsch(test_device_sid, &schib);
> +	report(!cc, "STSCH cc == 0");
> +	report(!(schib.pmcw.flags & BIT(15 - 5)), "STSCH PMCW Bit 5 is clear");
> +
> +	report_prefix_pop();
> +}
> +
>   static struct {
>   	const char *name;
>   	void (*func)(void);
> @@ -511,6 +583,8 @@ static struct {
>   	{ "msch", test_msch },
>   	{ "stcrw", test_stcrw },
>   	{ "ssch", test_ssch },
> +	{ "stsch", test_stsch },
> +	{ "pmcw bit 5 ignored", test_pmcw_bit5 },
>   	{ NULL, NULL }
>   };
>
Nico Boehr Feb. 23, 2022, 5:33 p.m. UTC | #3
On Wed, 2022-02-23 at 16:39 +0100, Janosch Frank wrote:
> On 2/23/22 14:29, Nico Boehr wrote:
> > 
[...]
> >   
> > +static void test_stsch(void)
> > +{
> > 
[...]
> > +       report_prefix_push("Bit 47 in SID is zero");
> > +       expect_pgm_int();
> > +       stsch(0x0000ffff, &schib);
> > +       check_pgm_int_code(PGM_INT_CODE_OPERAND);
> > +       report_prefix_pop();
> 
> Add a comment:
> No matter if the multiple-subchannel-set facility is installed or
> not, 
> bit 47 always needs to be 1.

Will do.

> Do we have the MSS facility?

Not an IO expert, but it seems like it's enabled by QEMU in pc-
bios/s390-ccw/main.c, css_setup(). The comment suggests it's always
there.

> If yes, could we disable it to test the 32-47 == 0x0001 case?

I see ioinst_handle_chsc_sda() in QEMU to enable it. Disabling only
works with a full reset of the CSS (see css_reset()) which can be
triggered from a subsystem_reset(), which basically means we need to
IPL. I think that's not really viable or do you see any other way?

Halil, Pierre, can you confirm?

> 
> > +}
> > +
> > +static void test_pmcw_bit5(void)
> > +{
> > +       int cc;
> > +       uint16_t old_pmcw_flags;
> 
> I need a comment here for further reference since that behavior is 
> documented at the description of the schib and not where STSCH is
> described:
> According to architecture MSCH does ignore bit 5 of the second word
> but 
> STSCH will store bit 5 as zero.

Will add the comment above the function, OK?

> We could check if bits 0,1 and 6,7 are also zero but I'm not sure if 
> that's interesting since MSCH does not ignore those bits and should 
> result in an operand exception when trying to set them.

I already have a test in MSCH which checks for the operand exception.
It's simple enough to extend it to do a STSCH after the MSCH and check
the respective bits is clear. Will be added in v4.
Halil Pasic Feb. 24, 2022, 12:13 a.m. UTC | #4
On Wed, 23 Feb 2022 18:33:17 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:

> On Wed, 2022-02-23 at 16:39 +0100, Janosch Frank wrote:
> > On 2/23/22 14:29, Nico Boehr wrote:  
> > >   
> [...]
> > >   
> > > +static void test_stsch(void)
> > > +{
> > >   
> [...]
> > > +       report_prefix_push("Bit 47 in SID is zero");
> > > +       expect_pgm_int();
> > > +       stsch(0x0000ffff, &schib);
> > > +       check_pgm_int_code(PGM_INT_CODE_OPERAND);
> > > +       report_prefix_pop();  
> > 
> > Add a comment:
> > No matter if the multiple-subchannel-set facility is installed or
> > not, 
> > bit 47 always needs to be 1.  
> 
> Will do.
> 
> > Do we have the MSS facility?  
> 
> Not an IO expert, but it seems like it's enabled by QEMU in pc-
> bios/s390-ccw/main.c, css_setup(). The comment suggests it's always
> there.
> 

AFAIR. The MSS facility is unconditionally implemented by QEMU thus
it is always indicated as installed, but lies dormant per default
and needs to be enabled.

The idea is that a non-enlightened OS would not enable the facility,
and thus effectively end up specifying zeros and using the
subchannel-set 0, and would observe no changes whatsoever compared
to running on a machine that does not have MSS facility installed.

Whether MSS is installed in some configuration or not can be detected
via the facility bit 47 of Store Channel-Subsystem Characteristics.

> > If yes, could we disable it to test the 32-47 == 0x0001 case?  
> 
> I see ioinst_handle_chsc_sda() in QEMU to enable it. Disabling only
> works with a full reset of the CSS (see css_reset()) which can be
> triggered from a subsystem_reset(), which basically means we need to
> IPL. I think that's not really viable or do you see any other way?
> 
> Halil, Pierre, can you confirm?

I don't think there is an other way, and there is usually no good reason
to attempt that. If your OS enables it is enlightened, and it won't
become non-enlightened. It is basically an opt-in. Eventually you may
want to IPL something different, and you are covered by the subsystem
reset.

The best way to test this would be to not enable the facility. I have
no idea if there is a way for a kvm-unit-test to accomplish that.

Regards,
Halil
Pierre Morel Feb. 24, 2022, 10:27 a.m. UTC | #5
On 2/23/22 16:39, Janosch Frank wrote:
> On 2/23/22 14:29, Nico Boehr wrote:
>> css_lib extensively uses STSCH, but two more cases deserve their own
>> tests:
>>
>> - unaligned address for SCHIB. We check for misalignment by 1 and 2
>>    bytes.
>> - channel not operational
>> - bit 47 in SID not set
>> - bit 5 of PMCW flags.
>>    As per the principles of operation, bit 5 of the PMCW flags shall be
>>    ignored by msch and always stored as zero by stsch.
>>
>>    Older QEMU versions require this bit to always be zero on msch,
>>    which is why this test may fail. A fix is available in QEMU master
>>    commit 2df59b73e086 ("s390x/css: fix PMCW invalid mask"). >
>> Here's the QEMU PMCW invalid mask fix: 
>> https://lists.nongnu.org/archive/html/qemu-s390x/2021-12/msg00100.html
>>
>> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
>> ---
>>   s390x/css.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 74 insertions(+)
>>
>> diff --git a/s390x/css.c b/s390x/css.c
>> index a90a0cd64e2b..021eb12573c0 100644
>> --- a/s390x/css.c
>> +++ b/s390x/css.c
>> @@ -496,6 +496,78 @@ static void test_ssch(void)
>>       report_prefix_pop();
>>   }
>> +static void test_stsch(void)
>> +{
>> +    const int align_to = 4;
>> +    struct schib schib;
>> +    int cc;
>> +
>> +    if (!test_device_sid) {
>> +        report_skip("No device");
>> +        return;
>> +    }
>> +
>> +    report_prefix_push("Unaligned");
>> +    for (int i = 1; i < align_to; i *= 2) {
>> +        report_prefix_pushf("%d", i);
>> +
>> +        expect_pgm_int();
>> +        stsch(test_device_sid, (struct schib *)(alignment_test_page + 
>> i));
>> +        check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
>> +
>> +        report_prefix_pop();
>> +    }
>> +    report_prefix_pop();
>> +
>> +    report_prefix_push("Invalid subchannel number");
>> +    cc = stsch(0x0001ffff, &schib);
>> +    report(cc == 3, "Channel not operational");
>> +    report_prefix_pop();
>> +
>> +    report_prefix_push("Bit 47 in SID is zero");
>> +    expect_pgm_int();
>> +    stsch(0x0000ffff, &schib);
>> +    check_pgm_int_code(PGM_INT_CODE_OPERAND);
>> +    report_prefix_pop();
> 
> Add a comment:
> No matter if the multiple-subchannel-set facility is installed or not, 
> bit 47 always needs to be 1.
> 
> Do we have the MSS facility?

yes

> If yes, could we disable it to test the 32-47 == 0x0001 case?

AFAIK it is not enabled in the KVM unit tests
We are able to enable it, it could be done with CHSC tests.

> 
>> +}
>> +
>> +static void test_pmcw_bit5(void)
>> +{
>> +    int cc;
>> +    uint16_t old_pmcw_flags;
> 
> I need a comment here for further reference since that behavior is 
> documented at the description of the schib and not where STSCH is 
> described:
> According to architecture MSCH does ignore bit 5 of the second word but 
> STSCH will store bit 5 as zero.
> 
> 
> We could check if bits 0,1 and 6,7 are also zero but I'm not sure if 
> that's interesting since MSCH does not ignore those bits and should 
> result in an operand exception when trying to set them.
> 
> @Halil, @Pierre: Any opinions?


Yes we should check this.
We often do STSCH/MSCH in a row so I think it is interesting to check it.

> 
>> +
>> +    cc = stsch(test_device_sid, &schib);
>> +    if (cc) {
>> +        report_fail("stsch: sch %08x failed with cc=%d", 
>> test_device_sid, cc);
>> +        return;
>> +    }
>> +    old_pmcw_flags = schib.pmcw.flags;
>> +
>> +    report_prefix_push("Bit 5 set");
>> +
>> +    schib.pmcw.flags = old_pmcw_flags | BIT(15 - 5);
>> +    cc = msch(test_device_sid, &schib);
>> +    report(!cc, "MSCH cc == 0");
>> +
>> +    cc = stsch(test_device_sid, &schib);
>> +    report(!cc, "STSCH cc == 0");
>> +    report(!(schib.pmcw.flags & BIT(15 - 5)), "STSCH PMCW Bit 5 is 
>> clear");
>> +
>> +    report_prefix_pop();
>> +
>> +    report_prefix_push("Bit 5 clear");
>> +
>> +    schib.pmcw.flags = old_pmcw_flags & ~BIT(15 - 5);
>> +    cc = msch(test_device_sid, &schib);
>> +    report(!cc, "MSCH cc == 0");
>> +
>> +    cc = stsch(test_device_sid, &schib);
>> +    report(!cc, "STSCH cc == 0");
>> +    report(!(schib.pmcw.flags & BIT(15 - 5)), "STSCH PMCW Bit 5 is 
>> clear");
>> +
>> +    report_prefix_pop();
>> +}
>> +
>>   static struct {
>>       const char *name;
>>       void (*func)(void);
>> @@ -511,6 +583,8 @@ static struct {
>>       { "msch", test_msch },
>>       { "stcrw", test_stcrw },
>>       { "ssch", test_ssch },
>> +    { "stsch", test_stsch },
>> +    { "pmcw bit 5 ignored", test_pmcw_bit5 },
>>       { NULL, NULL }
>>   };
>
Pierre Morel Feb. 24, 2022, 10:28 a.m. UTC | #6
On 2/24/22 11:27, Pierre Morel wrote:
> 
> 
> On 2/23/22 16:39, Janosch Frank wrote:
>> On 2/23/22 14:29, Nico Boehr wrote:
>>> css_lib extensively uses STSCH, but two more cases deserve their own
>>> tests:
>>>
>>> - unaligned address for SCHIB. We check for misalignment by 1 and 2
>>>    bytes.
>>> - channel not operational
>>> - bit 47 in SID not set
>>> - bit 5 of PMCW flags.
>>>    As per the principles of operation, bit 5 of the PMCW flags shall be
>>>    ignored by msch and always stored as zero by stsch.
>>>
>>>    Older QEMU versions require this bit to always be zero on msch,
>>>    which is why this test may fail. A fix is available in QEMU master
>>>    commit 2df59b73e086 ("s390x/css: fix PMCW invalid mask"). >
>>> Here's the QEMU PMCW invalid mask fix: 
>>> https://lists.nongnu.org/archive/html/qemu-s390x/2021-12/msg00100.html
>>>
>>> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
>>> ---
>>>   s390x/css.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 74 insertions(+)
>>>
>>> diff --git a/s390x/css.c b/s390x/css.c
>>> index a90a0cd64e2b..021eb12573c0 100644
>>> --- a/s390x/css.c
>>> +++ b/s390x/css.c
>>> @@ -496,6 +496,78 @@ static void test_ssch(void)
>>>       report_prefix_pop();
>>>   }
>>> +static void test_stsch(void)
>>> +{
>>> +    const int align_to = 4;
>>> +    struct schib schib;
>>> +    int cc;
>>> +
>>> +    if (!test_device_sid) {
>>> +        report_skip("No device");
>>> +        return;
>>> +    }
>>> +
>>> +    report_prefix_push("Unaligned");
>>> +    for (int i = 1; i < align_to; i *= 2) {
>>> +        report_prefix_pushf("%d", i);
>>> +
>>> +        expect_pgm_int();
>>> +        stsch(test_device_sid, (struct schib *)(alignment_test_page 
>>> + i));
>>> +        check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
>>> +
>>> +        report_prefix_pop();
>>> +    }
>>> +    report_prefix_pop();
>>> +
>>> +    report_prefix_push("Invalid subchannel number");
>>> +    cc = stsch(0x0001ffff, &schib);
>>> +    report(cc == 3, "Channel not operational");
>>> +    report_prefix_pop();
>>> +
>>> +    report_prefix_push("Bit 47 in SID is zero");
>>> +    expect_pgm_int();
>>> +    stsch(0x0000ffff, &schib);
>>> +    check_pgm_int_code(PGM_INT_CODE_OPERAND);
>>> +    report_prefix_pop();
>>
>> Add a comment:
>> No matter if the multiple-subchannel-set facility is installed or not, 
>> bit 47 always needs to be 1.
>>
>> Do we have the MSS facility?
> 
> yes
> 
>> If yes, could we disable it to test the 32-47 == 0x0001 case?
> 
> AFAIK it is not enabled in the KVM unit tests
> We are able to enable it, it could be done with CHSC tests.

I mean to check if it is enabled.

> 
>>
>>> +}
>>> +
>>> +static void test_pmcw_bit5(void)
>>> +{
>>> +    int cc;
>>> +    uint16_t old_pmcw_flags;
>>
>> I need a comment here for further reference since that behavior is 
>> documented at the description of the schib and not where STSCH is 
>> described:
>> According to architecture MSCH does ignore bit 5 of the second word 
>> but STSCH will store bit 5 as zero.
>>
>>
>> We could check if bits 0,1 and 6,7 are also zero but I'm not sure if 
>> that's interesting since MSCH does not ignore those bits and should 
>> result in an operand exception when trying to set them.
>>
>> @Halil, @Pierre: Any opinions?
> 
> 
> Yes we should check this.
> We often do STSCH/MSCH in a row so I think it is interesting to check it.
> 
>>
>>> +
>>> +    cc = stsch(test_device_sid, &schib);
>>> +    if (cc) {
>>> +        report_fail("stsch: sch %08x failed with cc=%d", 
>>> test_device_sid, cc);
>>> +        return;
>>> +    }
>>> +    old_pmcw_flags = schib.pmcw.flags;
>>> +
>>> +    report_prefix_push("Bit 5 set");
>>> +
>>> +    schib.pmcw.flags = old_pmcw_flags | BIT(15 - 5);
>>> +    cc = msch(test_device_sid, &schib);
>>> +    report(!cc, "MSCH cc == 0");
>>> +
>>> +    cc = stsch(test_device_sid, &schib);
>>> +    report(!cc, "STSCH cc == 0");
>>> +    report(!(schib.pmcw.flags & BIT(15 - 5)), "STSCH PMCW Bit 5 is 
>>> clear");
>>> +
>>> +    report_prefix_pop();
>>> +
>>> +    report_prefix_push("Bit 5 clear");
>>> +
>>> +    schib.pmcw.flags = old_pmcw_flags & ~BIT(15 - 5);
>>> +    cc = msch(test_device_sid, &schib);
>>> +    report(!cc, "MSCH cc == 0");
>>> +
>>> +    cc = stsch(test_device_sid, &schib);
>>> +    report(!cc, "STSCH cc == 0");
>>> +    report(!(schib.pmcw.flags & BIT(15 - 5)), "STSCH PMCW Bit 5 is 
>>> clear");
>>> +
>>> +    report_prefix_pop();
>>> +}
>>> +
>>>   static struct {
>>>       const char *name;
>>>       void (*func)(void);
>>> @@ -511,6 +583,8 @@ static struct {
>>>       { "msch", test_msch },
>>>       { "stcrw", test_stcrw },
>>>       { "ssch", test_ssch },
>>> +    { "stsch", test_stsch },
>>> +    { "pmcw bit 5 ignored", test_pmcw_bit5 },
>>>       { NULL, NULL }
>>>   };
>>
>
Halil Pasic Feb. 24, 2022, 2:08 p.m. UTC | #7
On Wed, 23 Feb 2022 16:39:07 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> We could check if bits 0,1 and 6,7 are also zero but I'm not sure if 
> that's interesting since MSCH does not ignore those bits and should 
> result in an operand exception when trying to set them.
> 
> @Halil, @Pierre: Any opinions?

IMHO more testing doesn't hurt. But I don't have clarity on some aspect
of how the architecture is extended. What I'm trying to say is: I'm not
100% certain these bits must stay 0 and no-semantics-defined forever.

Regards,
Halil
diff mbox series

Patch

diff --git a/s390x/css.c b/s390x/css.c
index a90a0cd64e2b..021eb12573c0 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -496,6 +496,78 @@  static void test_ssch(void)
 	report_prefix_pop();
 }
 
+static void test_stsch(void)
+{
+	const int align_to = 4;
+	struct schib schib;
+	int cc;
+
+	if (!test_device_sid) {
+		report_skip("No device");
+		return;
+	}
+
+	report_prefix_push("Unaligned");
+	for (int i = 1; i < align_to; i *= 2) {
+		report_prefix_pushf("%d", i);
+
+		expect_pgm_int();
+		stsch(test_device_sid, (struct schib *)(alignment_test_page + i));
+		check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
+
+		report_prefix_pop();
+	}
+	report_prefix_pop();
+
+	report_prefix_push("Invalid subchannel number");
+	cc = stsch(0x0001ffff, &schib);
+	report(cc == 3, "Channel not operational");
+	report_prefix_pop();
+
+	report_prefix_push("Bit 47 in SID is zero");
+	expect_pgm_int();
+	stsch(0x0000ffff, &schib);
+	check_pgm_int_code(PGM_INT_CODE_OPERAND);
+	report_prefix_pop();
+}
+
+static void test_pmcw_bit5(void)
+{
+	int cc;
+	uint16_t old_pmcw_flags;
+
+	cc = stsch(test_device_sid, &schib);
+	if (cc) {
+		report_fail("stsch: sch %08x failed with cc=%d", test_device_sid, cc);
+		return;
+	}
+	old_pmcw_flags = schib.pmcw.flags;
+
+	report_prefix_push("Bit 5 set");
+
+	schib.pmcw.flags = old_pmcw_flags | BIT(15 - 5);
+	cc = msch(test_device_sid, &schib);
+	report(!cc, "MSCH cc == 0");
+
+	cc = stsch(test_device_sid, &schib);
+	report(!cc, "STSCH cc == 0");
+	report(!(schib.pmcw.flags & BIT(15 - 5)), "STSCH PMCW Bit 5 is clear");
+
+	report_prefix_pop();
+
+	report_prefix_push("Bit 5 clear");
+
+	schib.pmcw.flags = old_pmcw_flags & ~BIT(15 - 5);
+	cc = msch(test_device_sid, &schib);
+	report(!cc, "MSCH cc == 0");
+
+	cc = stsch(test_device_sid, &schib);
+	report(!cc, "STSCH cc == 0");
+	report(!(schib.pmcw.flags & BIT(15 - 5)), "STSCH PMCW Bit 5 is clear");
+
+	report_prefix_pop();
+}
+
 static struct {
 	const char *name;
 	void (*func)(void);
@@ -511,6 +583,8 @@  static struct {
 	{ "msch", test_msch },
 	{ "stcrw", test_stcrw },
 	{ "ssch", test_ssch },
+	{ "stsch", test_stsch },
+	{ "pmcw bit 5 ignored", test_pmcw_bit5 },
 	{ NULL, NULL }
 };