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

[Pierre Morel <pmorel@linux.ibm.com>]

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
IBM Lab Boeblingen