[1/2] s390x/ccs: add ccw-tester emulated device

Message ID 20170913132752.8484-2-pasic@linux.vnet.ibm.com
State New
Headers show
Series
  • tests for CCW IDA
Related show

Commit Message

Halil Pasic Sept. 13, 2017, 1:27 p.m.
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 <pasic@linux.vnet.ibm.com>
---

Depends on the series 'add CCW indirect data access support'

---
 hw/s390x/Makefile.objs |   1 +
 hw/s390x/ccw-tester.c  | 179 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 180 insertions(+)
 create mode 100644 hw/s390x/ccw-tester.c

Comments

Cornelia Huck Sept. 14, 2017, 2:26 p.m. | #1
On Wed, 13 Sep 2017 15:27:51 +0200
Halil Pasic <pasic@linux.vnet.ibm.com> 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 <pasic@linux.vnet.ibm.com>
> ---
> 
> Depends on the series 'add CCW indirect data access support'
> 
> ---
>  hw/s390x/Makefile.objs |   1 +
>  hw/s390x/ccw-tester.c  | 179 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 180 insertions(+)
>  create mode 100644 hw/s390x/ccw-tester.c
> 

> +static int  ccw_tester_write_fib(SubchDev *sch, CCW1 ccw)
> +{
> +    CcwTesterDevice *d = sch->driver_data;
> +    bool is_fib = true;
> +    uint32_t sum;
> +    int ret = 0;
> +
> +    ccw_dstream_init(&sch->cds, &ccw, &sch->orb);
> +    d->fib.next = 0;
> +    while (ccw_dstream_avail(&sch->cds) > 0) {
> +        ret = ccw_dstream_read(&sch->cds,
> +                               d->fib.ring[abs_to_ring(d->fib.next)]);
> +        if (ret) {
> +            error(0, -ret, "fib");
> +            break;
> +        }
> +        if (d->fib.next > 2) {
> +            sum = (d->fib.ring[abs_to_ring(d->fib.next - 1)]
> +                  + d->fib.ring[abs_to_ring(d->fib.next - 2)]);
> +            is_fib = sum ==  d->fib.ring[abs_to_ring(d->fib.next)];

This is not endian-safe (noticed while testing on my laptop). Trivial
to fix:

diff --git a/hw/s390x/ccw-tester.c b/hw/s390x/ccw-tester.c
index c8017818c4..a425daaa34 100644
--- a/hw/s390x/ccw-tester.c
+++ b/hw/s390x/ccw-tester.c
@@ -58,9 +58,9 @@ static int  ccw_tester_write_fib(SubchDev *sch, CCW1 ccw)
             break;
         }
         if (d->fib.next > 2) {
-            sum = (d->fib.ring[abs_to_ring(d->fib.next - 1)]
-                  + d->fib.ring[abs_to_ring(d->fib.next - 2)]);
-            is_fib = sum ==  d->fib.ring[abs_to_ring(d->fib.next)];
+            sum = be32_to_cpu(d->fib.ring[abs_to_ring(d->fib.next - 1)])
+                + be32_to_cpu(d->fib.ring[abs_to_ring(d->fib.next - 2)]);
+            is_fib = sum == be32_to_cpu(d->fib.ring[abs_to_ring(d->fib.next)]);
             if (!is_fib) {
                 break;
             }

> +            if (!is_fib) {
> +                break;
> +            }
> +        }
> +        ++(d->fib.next);
> +    }
> +    if (!is_fib) {
> +        sch->curr_status.scsw.ctrl &= ~SCSW_ACTL_START_PEND;
> +        sch->curr_status.scsw.ctrl |= SCSW_STCTL_PRIMARY |
> +                                      SCSW_STCTL_SECONDARY |
> +                                      SCSW_STCTL_ALERT |
> +                                      SCSW_STCTL_STATUS_PEND;
> +        sch->curr_status.scsw.count = ccw_dstream_residual_count(&sch->cds);
> +        sch->curr_status.scsw.cpa = sch->channel_prog + 8;
> +        sch->curr_status.scsw.dstat =  SCSW_DSTAT_UNIT_EXCEP;
> +        return -EIO;
> +    }
> +    return ret;
> +}
> +
(...)
> +static Property ccw_tester_properties[] = {
> +    DEFINE_PROP_UINT16("cu_type", CcwTesterDevice, cu_type,
> +                        0x3831),

0x4711 would be nice :)

If we want to follow up on that testdev idea (and I think we should),
it might make sense to have a proper type reserve to prevent accidental
clashes.

(Or is there already something reserved for "hypervisor use" or
whatever?)

> +    DEFINE_PROP_UINT8("chpid_type", CcwTesterDevice, chpid_type,
> +                       0x98),
> +    DEFINE_PROP_END_OF_LIST(),
> +};

IIUC, pci-testdev provides some unit tests to testers (like kvm-tests)
itself. This might be an idea to follow up on for ccw as well.

There's quite some potential in this. We may want to make this a
permanent addition.
Halil Pasic Sept. 14, 2017, 4:50 p.m. | #2
On 09/14/2017 04:26 PM, Cornelia Huck wrote:
> On Wed, 13 Sep 2017 15:27:51 +0200
> Halil Pasic <pasic@linux.vnet.ibm.com> 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 <pasic@linux.vnet.ibm.com>
>> ---
>>
>> Depends on the series 'add CCW indirect data access support'
>>
>> ---
>>  hw/s390x/Makefile.objs |   1 +
>>  hw/s390x/ccw-tester.c  | 179 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 180 insertions(+)
>>  create mode 100644 hw/s390x/ccw-tester.c
>>
> 
>> +static int  ccw_tester_write_fib(SubchDev *sch, CCW1 ccw)
>> +{
>> +    CcwTesterDevice *d = sch->driver_data;
>> +    bool is_fib = true;
>> +    uint32_t sum;
>> +    int ret = 0;
>> +
>> +    ccw_dstream_init(&sch->cds, &ccw, &sch->orb);
>> +    d->fib.next = 0;
>> +    while (ccw_dstream_avail(&sch->cds) > 0) {
>> +        ret = ccw_dstream_read(&sch->cds,
>> +                               d->fib.ring[abs_to_ring(d->fib.next)]);
>> +        if (ret) {
>> +            error(0, -ret, "fib");
>> +            break;
>> +        }
>> +        if (d->fib.next > 2) {
>> +            sum = (d->fib.ring[abs_to_ring(d->fib.next - 1)]
>> +                  + d->fib.ring[abs_to_ring(d->fib.next - 2)]);
>> +            is_fib = sum ==  d->fib.ring[abs_to_ring(d->fib.next)];
> 
> This is not endian-safe (noticed while testing on my laptop). Trivial
> to fix:
> 
> diff --git a/hw/s390x/ccw-tester.c b/hw/s390x/ccw-tester.c
> index c8017818c4..a425daaa34 100644
> --- a/hw/s390x/ccw-tester.c
> +++ b/hw/s390x/ccw-tester.c
> @@ -58,9 +58,9 @@ static int  ccw_tester_write_fib(SubchDev *sch, CCW1 ccw)
>              break;
>          }
>          if (d->fib.next > 2) {
> -            sum = (d->fib.ring[abs_to_ring(d->fib.next - 1)]
> -                  + d->fib.ring[abs_to_ring(d->fib.next - 2)]);
> -            is_fib = sum ==  d->fib.ring[abs_to_ring(d->fib.next)];
> +            sum = be32_to_cpu(d->fib.ring[abs_to_ring(d->fib.next - 1)])
> +                + be32_to_cpu(d->fib.ring[abs_to_ring(d->fib.next - 2)]);
> +            is_fib = sum == be32_to_cpu(d->fib.ring[abs_to_ring(d->fib.next)]);
>              if (!is_fib) {
>                  break;
>              }
> 

Nod. 

>> +            if (!is_fib) {
>> +                break;
>> +            }
>> +        }
>> +        ++(d->fib.next);
>> +    }
>> +    if (!is_fib) {
>> +        sch->curr_status.scsw.ctrl &= ~SCSW_ACTL_START_PEND;
>> +        sch->curr_status.scsw.ctrl |= SCSW_STCTL_PRIMARY |
>> +                                      SCSW_STCTL_SECONDARY |
>> +                                      SCSW_STCTL_ALERT |
>> +                                      SCSW_STCTL_STATUS_PEND;
>> +        sch->curr_status.scsw.count = ccw_dstream_residual_count(&sch->cds);
>> +        sch->curr_status.scsw.cpa = sch->channel_prog + 8;
>> +        sch->curr_status.scsw.dstat =  SCSW_DSTAT_UNIT_EXCEP;
>> +        return -EIO;
>> +    }
>> +    return ret;
>> +}
>> +
> (...)
>> +static Property ccw_tester_properties[] = {
>> +    DEFINE_PROP_UINT16("cu_type", CcwTesterDevice, cu_type,
>> +                        0x3831),
> 
> 0x4711 would be nice :)

I don't understand the joke/pun/whatever if there is one,
but I'm fine with changing this too.

> 
> If we want to follow up on that testdev idea (and I think we should),
> it might make sense to have a proper type reserve to prevent accidental
> clashes.

I agree. Although I would still keep the cu_type configurable,
because it might make sense to test a particular 'real' driver
(and not a test driver like here). I haven't really thought
this through, but it was an idea I had while agonizing over
not having a proper type reserved.

I suppose you did something like that for virtio, or? I'm in dark
when it comes to the question what process do we/I have to go to
get a type,for example 0x4711, reserved.

> 
> (Or is there already something reserved for "hypervisor use" or
> whatever?)

Not that I know. I can't recall encountering a list of reserved
types. Honestly I've hoped to leverage your experience (again
because of virtio-ccw).

> 
>> +    DEFINE_PROP_UINT8("chpid_type", CcwTesterDevice, chpid_type,
>> +                       0x98),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
> 
> IIUC, pci-testdev provides some unit tests to testers (like kvm-tests)
> itself. This might be an idea to follow up on for ccw as well.
> 

I've just had a first look at pci-testdev, and it does appear to be a similar
concept. 

> There's quite some potential in this. We may want to make this a
> permanent addition.
> 

I'm happy to contribute! I'm not sure how shall we proceed though.
Maybe with making a todo list?
Cornelia Huck Sept. 15, 2017, 7:27 a.m. | #3
On Thu, 14 Sep 2017 18:50:29 +0200
Halil Pasic <pasic@linux.vnet.ibm.com> wrote:

> On 09/14/2017 04:26 PM, Cornelia Huck wrote:
> > On Wed, 13 Sep 2017 15:27:51 +0200
> > Halil Pasic <pasic@linux.vnet.ibm.com> wrote:

> >> +static Property ccw_tester_properties[] = {
> >> +    DEFINE_PROP_UINT16("cu_type", CcwTesterDevice, cu_type,
> >> +                        0x3831),  
> > 
> > 0x4711 would be nice :)  
> 
> I don't understand the joke/pun/whatever if there is one,
> but I'm fine with changing this too.

https://en.wikipedia.org/wiki/4711

That's my default if I need a four-digit number :)

> 
> > 
> > If we want to follow up on that testdev idea (and I think we should),
> > it might make sense to have a proper type reserve to prevent accidental
> > clashes.  
> 
> I agree. Although I would still keep the cu_type configurable,
> because it might make sense to test a particular 'real' driver
> (and not a test driver like here). I haven't really thought
> this through, but it was an idea I had while agonizing over
> not having a proper type reserved.
> 
> I suppose you did something like that for virtio, or? I'm in dark
> when it comes to the question what process do we/I have to go to
> get a type,for example 0x4711, reserved.

4711 is more a joke :) It might be worth trying the same channels as
for virtio-ccw.

Christian should know more about that.

> 
> > 
> > (Or is there already something reserved for "hypervisor use" or
> > whatever?)  
> 
> Not that I know. I can't recall encountering a list of reserved
> types. Honestly I've hoped to leverage your experience (again
> because of virtio-ccw).

My thought was that the z/VM folks already might have something
(type-wise) that we could use as well. There's a surprising amount of
values that are reserved for one use or the other. But obviously, I
can't find out about that.

> 
> >   
> >> +    DEFINE_PROP_UINT8("chpid_type", CcwTesterDevice, chpid_type,
> >> +                       0x98),

This might also need re-evaluation - we should not really need a new
chpid type.

> >> +    DEFINE_PROP_END_OF_LIST(),
> >> +};  
> > 
> > IIUC, pci-testdev provides some unit tests to testers (like kvm-tests)
> > itself. This might be an idea to follow up on for ccw as well.
> >   
> 
> I've just had a first look at pci-testdev, and it does appear to be a similar
> concept. 
> 
> > There's quite some potential in this. We may want to make this a
> > permanent addition.
> >   
> 
> I'm happy to contribute! I'm not sure how shall we proceed though.
> Maybe with making a todo list?

I think the first step would be to figure out the ids so we don't step
on anyone's toes. Then maybe refactor a bit so that other testers can
be added easily.

For ideas about things to be tested, maybe put a list into the wiki?
Cornelia Huck Sept. 15, 2017, 5:01 p.m. | #4
On Fri, 15 Sep 2017 09:27:58 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Thu, 14 Sep 2017 18:50:29 +0200
> Halil Pasic <pasic@linux.vnet.ibm.com> wrote:
> 
> > On 09/14/2017 04:26 PM, Cornelia Huck wrote:  

> > > There's quite some potential in this. We may want to make this a
> > > permanent addition.
> > >     
> > 
> > I'm happy to contribute! I'm not sure how shall we proceed though.
> > Maybe with making a todo list?  
> 
> I think the first step would be to figure out the ids so we don't step
> on anyone's toes. Then maybe refactor a bit so that other testers can
> be added easily.
> 
> For ideas about things to be tested, maybe put a list into the wiki?

OK, I've created https://wiki.qemu.org/Testing/CCWTestDevice - let me
know if you need an account.
Christian Borntraeger Sept. 18, 2017, 8:30 a.m. | #5
On 09/15/2017 09:27 AM, Cornelia Huck wrote:
> On Thu, 14 Sep 2017 18:50:29 +0200
> Halil Pasic <pasic@linux.vnet.ibm.com> wrote:
> 
>> On 09/14/2017 04:26 PM, Cornelia Huck wrote:
>>> On Wed, 13 Sep 2017 15:27:51 +0200
>>> Halil Pasic <pasic@linux.vnet.ibm.com> wrote:
> 
>>>> +static Property ccw_tester_properties[] = {
>>>> +    DEFINE_PROP_UINT16("cu_type", CcwTesterDevice, cu_type,
>>>> +                        0x3831),  
>>>
>>> 0x4711 would be nice :)  
>>
>> I don't understand the joke/pun/whatever if there is one,
>> but I'm fine with changing this too.
> 
> https://en.wikipedia.org/wiki/4711
> 
> That's my default if I need a four-digit number :)
> 
>>
>>>
>>> If we want to follow up on that testdev idea (and I think we should),
>>> it might make sense to have a proper type reserve to prevent accidental
>>> clashes.  
>>
>> I agree. Although I would still keep the cu_type configurable,
>> because it might make sense to test a particular 'real' driver
>> (and not a test driver like here). I haven't really thought
>> this through, but it was an idea I had while agonizing over
>> not having a proper type reserved.
>>
>> I suppose you did something like that for virtio, or? I'm in dark
>> when it comes to the question what process do we/I have to go to
>> get a type,for example 0x4711, reserved.
> 
> 4711 is more a joke :) It might be worth trying the same channels as
> for virtio-ccw.
> 
> Christian should know more about that.

Getting a new number was very easy (because it is attached to a machine type
number). I I remember correctly, only numerical values are uses, so maybe
we can use ffff as there will never be such a real value?
Cornelia Huck Sept. 18, 2017, 8:42 a.m. | #6
On Mon, 18 Sep 2017 10:30:32 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 09/15/2017 09:27 AM, Cornelia Huck wrote:
> > On Thu, 14 Sep 2017 18:50:29 +0200
> > Halil Pasic <pasic@linux.vnet.ibm.com> wrote:
> >   
> >> On 09/14/2017 04:26 PM, Cornelia Huck wrote:  
> >>> On Wed, 13 Sep 2017 15:27:51 +0200
> >>> Halil Pasic <pasic@linux.vnet.ibm.com> wrote:  
> >   
> >>>> +static Property ccw_tester_properties[] = {
> >>>> +    DEFINE_PROP_UINT16("cu_type", CcwTesterDevice, cu_type,
> >>>> +                        0x3831),    
> >>>
> >>> 0x4711 would be nice :)    
> >>
> >> I don't understand the joke/pun/whatever if there is one,
> >> but I'm fine with changing this too.  
> > 
> > https://en.wikipedia.org/wiki/4711
> > 
> > That's my default if I need a four-digit number :)
> >   
> >>  
> >>>
> >>> If we want to follow up on that testdev idea (and I think we should),
> >>> it might make sense to have a proper type reserve to prevent accidental
> >>> clashes.    
> >>
> >> I agree. Although I would still keep the cu_type configurable,
> >> because it might make sense to test a particular 'real' driver
> >> (and not a test driver like here). I haven't really thought
> >> this through, but it was an idea I had while agonizing over
> >> not having a proper type reserved.
> >>
> >> I suppose you did something like that for virtio, or? I'm in dark
> >> when it comes to the question what process do we/I have to go to
> >> get a type,for example 0x4711, reserved.  
> > 
> > 4711 is more a joke :) It might be worth trying the same channels as
> > for virtio-ccw.
> > 
> > Christian should know more about that.  
> 
> Getting a new number was very easy (because it is attached to a machine type
> number). I I remember correctly, only numerical values are uses, so maybe
> we can use ffff as there will never be such a real value?
> 

Yes, that sounds like the easiest way to do that.
Pierre Morel Sept. 19, 2017, 2:20 p.m. | #7
On 15/09/2017 19:01, Cornelia Huck wrote:
> On Fri, 15 Sep 2017 09:27:58 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
> 
>> On Thu, 14 Sep 2017 18:50:29 +0200
>> Halil Pasic <pasic@linux.vnet.ibm.com> wrote:
>>
>>> On 09/14/2017 04:26 PM, Cornelia Huck wrote:
> 
>>>> There's quite some potential in this. We may want to make this a
>>>> permanent addition.
>>>>      
>>>
>>> I'm happy to contribute! I'm not sure how shall we proceed though.
>>> Maybe with making a todo list?
>>
>> I think the first step would be to figure out the ids so we don't step
>> on anyone's toes. Then maybe refactor a bit so that other testers can
>> be added easily.
>>
>> For ideas about things to be tested, maybe put a list into the wiki?
> 
> OK, I've created https://wiki.qemu.org/Testing/CCWTestDevice - let me
> know if you need an account.
> 

Great!
I just saw your email, it was waiting in my inbox.
strange things happens some time. A good serie. strange things.
neverless...

Seriously.
Yes, I definitively need an account!


Pierre
Pierre Morel Sept. 19, 2017, 2:26 p.m. | #8
On 14/09/2017 16:26, Cornelia Huck wrote:
> On Wed, 13 Sep 2017 15:27:51 +0200
> Halil Pasic <pasic@linux.vnet.ibm.com> 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 <pasic@linux.vnet.ibm.com>
>> ---
>>
>> Depends on the series 'add CCW indirect data access support'
>>
>> ---
>>   hw/s390x/Makefile.objs |   1 +
>>   hw/s390x/ccw-tester.c  | 179 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 180 insertions(+)
>>   create mode 100644 hw/s390x/ccw-tester.c
>>
> 
>> +static int  ccw_tester_write_fib(SubchDev *sch, CCW1 ccw)
>> +{
>> +    CcwTesterDevice *d = sch->driver_data;
>> +    bool is_fib = true;
>> +    uint32_t sum;
>> +    int ret = 0;
>> +
>> +    ccw_dstream_init(&sch->cds, &ccw, &sch->orb);
>> +    d->fib.next = 0;
>> +    while (ccw_dstream_avail(&sch->cds) > 0) {
>> +        ret = ccw_dstream_read(&sch->cds,
>> +                               d->fib.ring[abs_to_ring(d->fib.next)]);
>> +        if (ret) {
>> +            error(0, -ret, "fib");
>> +            break;
>> +        }
>> +        if (d->fib.next > 2) {
>> +            sum = (d->fib.ring[abs_to_ring(d->fib.next - 1)]
>> +                  + d->fib.ring[abs_to_ring(d->fib.next - 2)]);
>> +            is_fib = sum ==  d->fib.ring[abs_to_ring(d->fib.next)];
> 
> This is not endian-safe (noticed while testing on my laptop). Trivial
> to fix:
> 
> diff --git a/hw/s390x/ccw-tester.c b/hw/s390x/ccw-tester.c
> index c8017818c4..a425daaa34 100644
> --- a/hw/s390x/ccw-tester.c
> +++ b/hw/s390x/ccw-tester.c
> @@ -58,9 +58,9 @@ static int  ccw_tester_write_fib(SubchDev *sch, CCW1 ccw)
>               break;
>           }
>           if (d->fib.next > 2) {
> -            sum = (d->fib.ring[abs_to_ring(d->fib.next - 1)]
> -                  + d->fib.ring[abs_to_ring(d->fib.next - 2)]);
> -            is_fib = sum ==  d->fib.ring[abs_to_ring(d->fib.next)];
> +            sum = be32_to_cpu(d->fib.ring[abs_to_ring(d->fib.next - 1)])
> +                + be32_to_cpu(d->fib.ring[abs_to_ring(d->fib.next - 2)]);
> +            is_fib = sum == be32_to_cpu(d->fib.ring[abs_to_ring(d->fib.next)]);
>               if (!is_fib) {
>                   break;
>               }
> 
>> +            if (!is_fib) {
>> +                break;
>> +            }
>> +        }
>> +        ++(d->fib.next);
>> +    }
>> +    if (!is_fib) {
>> +        sch->curr_status.scsw.ctrl &= ~SCSW_ACTL_START_PEND;
>> +        sch->curr_status.scsw.ctrl |= SCSW_STCTL_PRIMARY |
>> +                                      SCSW_STCTL_SECONDARY |
>> +                                      SCSW_STCTL_ALERT |
>> +                                      SCSW_STCTL_STATUS_PEND;
>> +        sch->curr_status.scsw.count = ccw_dstream_residual_count(&sch->cds);
>> +        sch->curr_status.scsw.cpa = sch->channel_prog + 8;
>> +        sch->curr_status.scsw.dstat =  SCSW_DSTAT_UNIT_EXCEP;
>> +        return -EIO;
>> +    }
>> +    return ret;
>> +}
>> +
> (...)
>> +static Property ccw_tester_properties[] = {
>> +    DEFINE_PROP_UINT16("cu_type", CcwTesterDevice, cu_type,
>> +                        0x3831),
> 
> 0x4711 would be nice :)

The C0C0 channel would be nice too.
Sorry.

> 
> If we want to follow up on that testdev idea (and I think we should),
> it might make sense to have a proper type reserve to prevent accidental
> clashes.
> 
> (Or is there already something reserved for "hypervisor use" or
> whatever?)
> 
>> +    DEFINE_PROP_UINT8("chpid_type", CcwTesterDevice, chpid_type,
>> +                       0x98),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
> 
> IIUC, pci-testdev provides some unit tests to testers (like kvm-tests)
> itself. This might be an idea to follow up on for ccw as well.
> 
> There's quite some potential in this. We may want to make this a
> permanent addition.
>

Patch

diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index 7ee19d3abc..28eb58a3cb 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -18,3 +18,4 @@  obj-y += s390-stattrib.o
 obj-$(CONFIG_KVM) += s390-skeys-kvm.o
 obj-$(CONFIG_KVM) += s390-stattrib-kvm.o
 obj-y += s390-ccw.o
+obj-y += ccw-tester.o
diff --git a/hw/s390x/ccw-tester.c b/hw/s390x/ccw-tester.c
new file mode 100644
index 0000000000..c8017818c4
--- /dev/null
+++ b/hw/s390x/ccw-tester.c
@@ -0,0 +1,179 @@ 
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+#include "cpu.h"
+#include "hw/s390x/css.h"
+#include "hw/s390x/css-bridge.h"
+#include "hw/s390x/3270-ccw.h"
+#include "exec/address-spaces.h"
+#include <error.h>
+
+typedef struct CcwTesterDevice {
+    CcwDevice parent_obj;
+    uint16_t cu_type;
+    uint8_t chpid_type;
+    struct {
+        uint32_t ring[4];
+        unsigned int next;
+    } fib;
+} CcwTesterDevice;
+
+
+typedef struct CcwTesterClass {
+    CCWDeviceClass parent_class;
+    DeviceRealize parent_realize;
+} CcwTesterClass;
+
+#define TYPE_CCW_TESTER "ccw-tester"
+
+#define CCW_TESTER(obj) \
+     OBJECT_CHECK(CcwTesterDevice, (obj), TYPE_CCW_TESTER)
+#define CCW_TESTER_CLASS(klass) \
+     OBJECT_CLASS_CHECK(CcwTesterClass, (klass), TYPE_CCW_TESTER)
+#define CCW_TESTER_GET_CLASS(obj) \
+     OBJECT_GET_CLASS(CcwTesterClass, (obj), TYPE_CCW_TESTER)
+
+#define CCW_CMD_READ 0x01U
+#define CCW_CMD_WRITE 0x02U
+
+static unsigned int abs_to_ring(unsigned int i)
+{
+    return i & 0x3;
+}
+
+static int  ccw_tester_write_fib(SubchDev *sch, CCW1 ccw)
+{
+    CcwTesterDevice *d = sch->driver_data;
+    bool is_fib = true;
+    uint32_t sum;
+    int ret = 0;
+
+    ccw_dstream_init(&sch->cds, &ccw, &sch->orb);
+    d->fib.next = 0;
+    while (ccw_dstream_avail(&sch->cds) > 0) {
+        ret = ccw_dstream_read(&sch->cds,
+                               d->fib.ring[abs_to_ring(d->fib.next)]);
+        if (ret) {
+            error(0, -ret, "fib");
+            break;
+        }
+        if (d->fib.next > 2) {
+            sum = (d->fib.ring[abs_to_ring(d->fib.next - 1)]
+                  + d->fib.ring[abs_to_ring(d->fib.next - 2)]);
+            is_fib = sum ==  d->fib.ring[abs_to_ring(d->fib.next)];
+            if (!is_fib) {
+                break;
+            }
+        }
+        ++(d->fib.next);
+    }
+    if (!is_fib) {
+        sch->curr_status.scsw.ctrl &= ~SCSW_ACTL_START_PEND;
+        sch->curr_status.scsw.ctrl |= SCSW_STCTL_PRIMARY |
+                                      SCSW_STCTL_SECONDARY |
+                                      SCSW_STCTL_ALERT |
+                                      SCSW_STCTL_STATUS_PEND;
+        sch->curr_status.scsw.count = ccw_dstream_residual_count(&sch->cds);
+        sch->curr_status.scsw.cpa = sch->channel_prog + 8;
+        sch->curr_status.scsw.dstat =  SCSW_DSTAT_UNIT_EXCEP;
+        return -EIO;
+    }
+    return ret;
+}
+
+static int ccw_tester_ccw_cb_impl(SubchDev *sch, CCW1 ccw)
+{
+    switch (ccw.cmd_code) {
+    case CCW_CMD_READ:
+        break;
+    case CCW_CMD_WRITE:
+        return ccw_tester_write_fib(sch, ccw);
+    default:
+        return -EINVAL;
+    }
+    return 0;
+}
+
+static void ccw_tester_realize(DeviceState *ds, Error **errp)
+{
+    uint16_t chpid;
+    CcwTesterDevice *dev = CCW_TESTER(ds);
+    CcwTesterClass *ctc = CCW_TESTER_GET_CLASS(dev);
+    CcwDevice *cdev = CCW_DEVICE(ds);
+    BusState *qbus = qdev_get_parent_bus(ds);
+    VirtualCssBus *cbus = VIRTUAL_CSS_BUS(qbus);
+    SubchDev *sch;
+    Error *err = NULL;
+
+    sch = css_create_sch(cdev->devno, true, cbus->squash_mcss, errp);
+    if (!sch) {
+        return;
+    }
+
+    sch->driver_data = dev;
+    cdev->sch = sch;
+    chpid = css_find_free_chpid(sch->cssid);
+
+    if (chpid > MAX_CHPID) {
+        error_setg(&err, "No available chpid to use.");
+        goto out_err;
+    }
+
+    sch->id.reserved = 0xff;
+    sch->id.cu_type = dev->cu_type;
+    css_sch_build_virtual_schib(sch, (uint8_t)chpid,
+                                dev->chpid_type);
+    sch->ccw_cb = ccw_tester_ccw_cb_impl;
+    sch->do_subchannel_work = do_subchannel_work_virtual;
+
+
+    ctc->parent_realize(ds, &err);
+    if (err) {
+        goto out_err;
+    }
+
+    css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,
+                          ds->hotplugged, 1);
+    return;
+
+out_err:
+    error_propagate(errp, err);
+    css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL);
+    cdev->sch = NULL;
+    g_free(sch);
+}
+
+static Property ccw_tester_properties[] = {
+    DEFINE_PROP_UINT16("cu_type", CcwTesterDevice, cu_type,
+                        0x3831),
+    DEFINE_PROP_UINT8("chpid_type", CcwTesterDevice, chpid_type,
+                       0x98),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void ccw_tester_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    CcwTesterClass *ctc = CCW_TESTER_CLASS(klass);
+
+    dc->props = ccw_tester_properties;
+    dc->bus_type = TYPE_VIRTUAL_CSS_BUS;
+    ctc->parent_realize = dc->realize;
+    dc->realize = ccw_tester_realize;
+    dc->hotpluggable = false;
+}
+
+static const TypeInfo ccw_tester_info = {
+    .name = TYPE_CCW_TESTER,
+    .parent = TYPE_CCW_DEVICE,
+    .instance_size = sizeof(CcwTesterDevice),
+    .class_init = ccw_tester_class_init,
+    .class_size = sizeof(CcwTesterClass),
+};
+
+static void ccw_tester_register(void)
+{
+    type_register_static(&ccw_tester_info);
+}
+
+type_init(ccw_tester_register)