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

Pierre Morel
2020-07-02T12:56:31Z

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

--
Pierre Morel
IBM Lab Boeblingen