Re: [kvm-unit-tests PATCH v1 2/5] s390x: css: simplifications of the tests

Pierre Morel
2021-02-02T14:43:14Z

On 2/2/21 12:23 PM, Cornelia Huck wrote:
> On Fri, 29 Jan 2021 15:34:26 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
...snip...

>> +bool css_enabled(int schid)
>> +{
>> + struct pmcw *pmcw = &schib.pmcw;
>> + int cc;
>> +
>> + cc = stsch(schid, &schib);
>> + if (cc) {
>> + report_info("stsch: updating sch %08x failed with cc=%d",
>> + schid, cc);
>> + return false;
>> + }
>> +
>> + if (!(pmcw->flags & PMCW_ENABLE)) {
>> + report_info("stsch: sch %08x not enabled", schid);
>> + return 0;
>> + }
>> + return true;
>> +}
>> /*
>> * css_enable: enable the subchannel with the specified ISC
>> * @schid: Subchannel Identifier
>> @@ -167,18 +192,8 @@ retry:
>> /*
>> * Read the SCHIB again to verify the enablement
>> */
>> - cc = stsch(schid, &schib);
>> - if (cc) {
>> - report_info("stsch: updating sch %08x failed with cc=%d",
>> - schid, cc);
>> - return cc;
>> - }
>> -
>> - if ((pmcw->flags & flags) == flags) {
>> - report_info("stsch: sch %08x successfully modified after %d retries",
>> - schid, retry_count);
>> + if (css_enabled(schid))
>
> This is a slightly different test now. Previously, you also checked
> whether the ISC matched the requested one. Not sure how valuable that
> test was.

Yes, I do not think it can be anything else when the CSS accept to
enable the subchannel.

...
>>
>> - ret = css_enable(test_device_sid, IO_SCH_ISC);
>> - if (ret) {
>> - report(0, "Could not enable the subchannel: %08x",
>> - test_device_sid);
>> - return;
>> + if (!css_enabled(test_device_sid)) {
>> + report(0, "enabled subchannel: %08x", test_device_sid);
>
> Isn't that a _not_ enabled subchannel?

:) yes, may be:

s/enabled/enabling/

...snip...

>>
>> static void css_init(void)
>> @@ -146,8 +142,17 @@ static void css_init(void)
>> int ret;
>>
>> ret = get_chsc_scsc();
>> - if (!ret)
>> - report(1, " ");
>> + if (ret)
>> + return;
>
> Shouldn't you report a failure here?

Clearly yes.

Thanks for the comments,
regards,
Pierre

--
Pierre Morel
IBM Lab Boeblingen