Re: [Qemu-devel] [PATCH 5/5] s390x/ccs: add ccw-tester emulated device

[Halil Pasic]

On 09/06/2017 05:20 PM, Cornelia Huck wrote:
> On Wed, 6 Sep 2017 16:24:13 +0200
> Halil Pasic <address@hidden> wrote:
> 
>> On 09/06/2017 03:18 PM, Cornelia Huck wrote:
>>> On Tue,  5 Sep 2017 13:16:45 +0200
>>> Halil Pasic <address@hidden> wrote:
>>>   
>>>> Add a fake device meant for testing the correctness of our css emulation.
>>>>
>>>> What we currently have is writing a Fibonacci sequence of uint32_t to the
>>>> device via ccw write. The write is going to fail if it ain't a Fibonacci
>>>> and indicate a device exception in scsw together with the proper residual
>>>> count.
>>>>
>>>> Of course lot's of invalid inputs (besides basic data processing) can be
>>>> tested with that as well.
>>>>
>>>> Usage:
>>>> 1) fire up a qemu with something like -device ccw-tester,devno=fe.0.0001
>>>>    on the command line
>>>> 2) exercise the device from the guest
>>>>
>>>> Signed-off-by: Halil Pasic <address@hidden>
>>>> ---
>>>>
>>>> It may not make sense to merge this work in the current form, as it is
>>>> solely for test purposes.
>>>> ---
>>>>  hw/s390x/Makefile.objs |   1 +
>>>>  hw/s390x/ccw-tester.c  | 179 
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 180 insertions(+)
>>>>  create mode 100644 hw/s390x/ccw-tester.c  
>>>
>>> The main problem here is that you want to exercise a middle layer (the
>>> css code) and need to write boilerplate code on both host and guest
>>> side in order to be able to do so.
>>>   
>>
>> Nod.
>>
>>> In general, a device that accepts arbitrary channel programs looks
>>> useful for testing purposes. I would split out processing of expected
>>> responses out, though, so that it can be more easily reused for
>>> different use cases.
>>>   
>>
>> I'm not sure what do you mean here. Could you clarify please?
> 
> Maybe doing things like logging received ccws and responses etc. and
> and then comparing against an expected outcome. You should be able to
> write test cases for different sets of things to be tested. I know this
> is awfully vague :)

Yes it does sound awfully vague :). In my case different tests are
actually done in the kernel module implementing a driver for this
device. I compare the expected outcome and the actual outcome there.
This device is just a back-end lending itself to experimentation.

> 
>>
>>> (I dimly recall other test devices...)
>>>   
>>
>> What's on your mind? How do these relate to our problem? Can you
>> give me some pointers?
> 
> I was thinking of precedent for test devices... but as said, I only
> recall this very dimly.
> 
>>
>>> For the guest tester: Can that be done via the qtest infrastructure
>>> somehow?
>>>   
>>
>> Well, for now I have the out-of-tree Linux kernel module provided in the
>> cover letter of the series (you did not comment on that one yet).
> 
> Yes, I saw that, but have not yet looked at it. If there is a way to do
> it without too many extra parts, I'd prefer that.
> 

Well, I think the module is almost as economical with extra parts as it
can get: it uses the kernel infrastructure and does not do a whole lot
of things on top.

I think it's worth a look. I hope it will also give answers to some
of the implicit questions I see above. Yes, tests done in the driver
are currently very few. Both with and without indirect data access
we first let a device consume a Fibonacci sequence and verify that
the IO has succeeded. Then we deliberately change an element in the
sequence so it ain't the next Fibonacci number. And check for unit
exception and proper residual count. With that we are sure that
the data was properly consumed up until the given element. For IDA
we the shorten the sequence so it does not contain the 'broken'
element, and expect completion again. As you see this is a sufficient
test for the good path for single CCW programs.

Extending to testing chaining (different concern), or responses
to broken channel programs (e.g. ORB flags, bad addresses, tic
rules) should be quite straightforward.

>>
>> I think for building trust regarding my IDA implementation it should be
>> able to do the job. Don't you agree?
> 
> There's nothing wrong with your test. But we really should try to move
> to some kind of unified testing that is hopefully self-contained so
> that interested people can just play with it within the context of qemu.
> 
>>
>> Just a couple of hours ago Janosch (cc-ing Janosch) came to my office,
>> and told be that he is working on CCW-tests for zonk (a minimal kernel
>> for testing -- internal tool AFAIR).
>>
>> By qtest you mean libqtest/libqos? I'm not familiar with that and have no
>> idea what do we have for s390x. I see on libqos-s390x file in test/libqos
>> for starters.
> 
> Yes, that was what I was thinking about. Integrating into what is
> already there is what we should aim for.
> 
> [For things that are done via kvm, kvm unit tests are good. But I think
> we can do css tests completely within qemu.]
> 

I agree fully, the point is somebody needs to make it happen. I would
be very sad if forced to make it happen for the sake of this patch set.

That said. I could not identify any required actions on this front (testing
IDA) . If I made a mistake please point it out. I'm gonna try and speak
with the folks here about our testing strategy. IMHO libqtest/libqos could
very well be a part of it, but I can't tell anything for sure for now.

If somebody volunteers to transform what I have in this patch and the
cover letter to something better integrated, it will make me more than
happy.

Regards,
Halil