[PATCH 0/2] s390x: ccw: A simple test device for virtio CCW

[PATCH 0/2] s390x: ccw: A simple test device for virtio CCW

[Pierre Morel]

Hello All,


This series presents a VIRTIO test device which receives data on its
input channel and sends back a simple checksum for the data it received
on its output channel.
 
The goal is to allow a simple VIRTIO device driver to check the VIRTIO
initialization and various data transfer.

For this I introduced a new device ID for the device and having no
Linux driver but a kvm-unit-test driver, I have the following
questions:

Is there another way to advertise new VIRTIO IDs but Linux?
If this QEMU test meet interest, should I write a Linux test program?

Regards,
Pierre


Pierre Morel (2):
  virtio: Linux: Update of virtio_ids
  s390x: ccw: A simple test device for virtio CCW

 hw/s390x/meson.build                        |   1 +
 hw/s390x/virtio-ccw-pong.c                  |  66 ++++++++
 hw/s390x/virtio-ccw.h                       |  13 ++
 hw/virtio/Kconfig                           |   5 +
 hw/virtio/meson.build                       |   1 +
 hw/virtio/virtio-pong.c                     | 161 ++++++++++++++++++++
 include/hw/virtio/virtio-pong.h             |  34 +++++
 include/standard-headers/linux/virtio_ids.h |   1 +
 8 files changed, 282 insertions(+)
 create mode 100644 hw/s390x/virtio-ccw-pong.c
 create mode 100644 hw/virtio/virtio-pong.c
 create mode 100644 include/hw/virtio/virtio-pong.h

-- 
2.25.1

[PATCH 1/2] virtio: Linux: Update of virtio_ids

[Pierre Morel]

The virtio IDs depends on Linux tree...

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 include/standard-headers/linux/virtio_ids.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/standard-headers/linux/virtio_ids.h b/include/standard-headers/linux/virtio_ids.h
index 4fe842c3a3..bf61801eeb 100644
--- a/include/standard-headers/linux/virtio_ids.h
+++ b/include/standard-headers/linux/virtio_ids.h
@@ -55,6 +55,7 @@
 #define VIRTIO_ID_FS			26 /* virtio filesystem */
 #define VIRTIO_ID_PMEM			27 /* virtio pmem */
 #define VIRTIO_ID_MAC80211_HWSIM	29 /* virtio mac80211-hwsim */
+#define VIRTIO_ID_PONG			30 /* virtio pong */
 #define VIRTIO_ID_BT			40 /* virtio bluetooth */
 
 #endif /* _LINUX_VIRTIO_IDS_H */
-- 
2.25.1

[PATCH 2/2] s390x: ccw: A simple test device for virtio CCW

[Pierre Morel]

This VIRTIO device receives data on its input channel
and emit a simple checksum for these data on its
output channel.

This allows a simple VIRTIO device driver to check the
VIRTIO initialization and various data transfer.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 hw/s390x/meson.build            |   1 +
 hw/s390x/virtio-ccw-pong.c      |  66 +++++++++++++
 hw/s390x/virtio-ccw.h           |  13 +++
 hw/virtio/Kconfig               |   5 +
 hw/virtio/meson.build           |   1 +
 hw/virtio/virtio-pong.c         | 161 ++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio-pong.h |  34 +++++++
 7 files changed, 281 insertions(+)
 create mode 100644 hw/s390x/virtio-ccw-pong.c
 create mode 100644 hw/virtio/virtio-pong.c
 create mode 100644 include/hw/virtio/virtio-pong.h

diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
index 74678861cf..e9edf1d196 100644
--- a/hw/s390x/meson.build
+++ b/hw/s390x/meson.build
@@ -18,6 +18,7 @@ s390x_ss.add(files(
   'sclpcpu.c',
   'sclpquiesce.c',
   'tod.c',
+  'virtio-ccw-pong.c',
 ))
 s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
   'tod-kvm.c',
diff --git a/hw/s390x/virtio-ccw-pong.c b/hw/s390x/virtio-ccw-pong.c
new file mode 100644
index 0000000000..c4e343b776
--- /dev/null
+++ b/hw/s390x/virtio-ccw-pong.c
@@ -0,0 +1,66 @@
+/*
+ * virtio ccw PONG device
+ *
+ * Copyright 2020, IBM Corp.
+ * Author(s): Pierre Morel <pmorel@linux.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev-properties.h"
+#include "hw/virtio/virtio.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+#include "virtio-ccw.h"
+
+static void virtio_ccw_pong_realize(VirtioCcwDevice *ccw_dev, Error **errp)
+{
+    VirtIOPONGCcw *dev = VIRTIO_PONG_CCW(ccw_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
+
+    if (!qdev_realize(vdev, BUS(&ccw_dev->bus), errp)) {
+        return;
+    }
+}
+
+static void virtio_ccw_pong_instance_init(Object *obj)
+{
+    VirtIOPONGCcw *dev = VIRTIO_PONG_CCW(obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_PONG);
+}
+
+static Property virtio_ccw_pong_properties[] = {
+    DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
+                       VIRTIO_CCW_MAX_REV),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_ccw_pong_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_CLASS(klass);
+
+    k->realize = virtio_ccw_pong_realize;
+    device_class_set_props(dc, virtio_ccw_pong_properties);
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static const TypeInfo virtio_ccw_pong = {
+    .name          = TYPE_VIRTIO_PONG_CCW,
+    .parent        = TYPE_VIRTIO_CCW_DEVICE,
+    .instance_size = sizeof(VirtIOPONGCcw),
+    .instance_init = virtio_ccw_pong_instance_init,
+    .class_init    = virtio_ccw_pong_class_init,
+};
+
+static void virtio_ccw_pong_register(void)
+{
+    type_register_static(&virtio_ccw_pong);
+}
+
+type_init(virtio_ccw_pong_register)
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 0168232e3b..f718ad32c2 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -31,6 +31,8 @@
 #include "hw/virtio/virtio-gpu.h"
 #include "hw/virtio/virtio-input.h"
 
+#include "hw/virtio/virtio-pong.h"
+
 #include "hw/s390x/s390_flic.h"
 #include "hw/s390x/css.h"
 #include "ccw-device.h"
@@ -176,6 +178,17 @@ struct VirtIORNGCcw {
     VirtIORNG vdev;
 };
 
+/* virtio-pong-ccw */
+
+#define TYPE_VIRTIO_PONG_CCW "virtio-pong-ccw"
+#define VIRTIO_PONG_CCW(obj) \
+        OBJECT_CHECK(VirtIOPONGCcw, (obj), TYPE_VIRTIO_PONG_CCW)
+
+typedef struct VirtIOPONGCcw {
+    VirtioCcwDevice parent_obj;
+    VirtIOPONG vdev;
+} VirtIOPONGCcw;
+
 /* virtio-crypto-ccw */
 
 #define TYPE_VIRTIO_CRYPTO_CCW "virtio-crypto-ccw"
diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index 35ab45e209..f73c87d953 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -4,6 +4,11 @@ config VHOST
 config VIRTIO
     bool
 
+config VIRTIO_PONG
+    bool
+    default y
+    depends on VIRTIO
+
 config VIRTIO_RNG
     bool
     default y
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index bc352a6009..1ba4dcc454 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -6,6 +6,7 @@ softmmu_virtio_ss.add(when: 'CONFIG_VHOST', if_false: files('vhost-stub.c'))
 
 softmmu_ss.add_all(when: 'CONFIG_VIRTIO', if_true: softmmu_virtio_ss)
 softmmu_ss.add(when: 'CONFIG_VIRTIO', if_false: files('vhost-stub.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO', if_true: files('virtio-pong.c'))
 
 softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('vhost-stub.c'))
 
diff --git a/hw/virtio/virtio-pong.c b/hw/virtio/virtio-pong.c
new file mode 100644
index 0000000000..c15100dd43
--- /dev/null
+++ b/hw/virtio/virtio-pong.c
@@ -0,0 +1,161 @@
+/*
+ * A virtio device implementing a PONG device
+ *
+ * Copyright 2020 IBM.
+ * Copyright 2020 Pierre Morel <pmorel@linux.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/iov.h"
+#include "qemu/module.h"
+#include "sysemu/runstate.h"
+#include "hw/virtio/virtio.h"
+#include "hw/qdev-properties.h"
+#include "standard-headers/linux/virtio_ids.h"
+#include "hw/virtio/virtio-pong.h"
+#include "qom/object_interfaces.h"
+#include "trace.h"
+#include "qemu/error-report.h"
+
+static char *buffer;
+static unsigned int cksum;
+
+static unsigned int simple_checksum(char *buf, unsigned long len)
+{
+    unsigned int sum = 0;
+
+    while (len--) {
+        sum += *buf * *buf + 7 * *buf + 3;
+        buf++;
+    }
+    return sum;
+}
+
+static void handle_output(VirtIODevice *vdev, VirtQueue *vq)
+{
+    VirtIOPONG *vpong = VIRTIO_PONG(vdev);
+    VirtQueueElement *elem;
+
+    if (!virtio_queue_ready(vq)) {
+        return;
+    }
+    if (virtio_queue_empty(vq)) {
+        return;
+    }
+
+    while ((elem = virtqueue_pop(vq, sizeof(*elem))) != NULL) {
+        buffer = g_malloc(elem->out_sg->iov_len);
+        iov_to_buf(elem->out_sg, elem->out_num, 0, buffer,
+                   elem->out_sg->iov_len);
+
+        if (vpong->cksum) {
+            cksum = simple_checksum(buffer, elem->out_sg->iov_len);
+        }
+        virtqueue_push(vq, elem, 0);
+        g_free(buffer);
+        g_free(elem);
+    }
+
+    virtio_notify(vdev, vq);
+}
+
+static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
+{
+    VirtQueueElement *elem;
+
+    if (!virtio_queue_ready(vq)) {
+        return;
+    }
+    if (virtio_queue_empty(vq)) {
+        return;
+    }
+
+    while ((elem = virtqueue_pop(vq, sizeof(*elem))) != NULL) {
+        int len = 0;
+
+        len = iov_from_buf(elem->out_sg, elem->out_num,
+                         0, &cksum, sizeof(cksum));
+
+        virtqueue_push(vq, elem, len);
+        g_free(elem);
+    }
+
+    virtio_notify(vdev, vq);
+
+}
+
+static uint64_t get_features(VirtIODevice *vdev, uint64_t f, Error **errp)
+{
+    VirtIOPONG *vpong = VIRTIO_PONG(vdev);
+
+    if (vpong->cksum) {
+        f |= 1ull << VIRTIO_PONG_F_CKSUM;
+    }
+    return f;
+}
+
+static void virtio_pong_set_status(VirtIODevice *vdev, uint8_t status)
+{
+    if (!vdev->vm_running) {
+        return;
+    }
+    vdev->status = status;
+}
+
+static void virtio_pong_device_realize(DeviceState *dev, Error **errp)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOPONG *vpong = VIRTIO_PONG(dev);
+
+    virtio_init(vdev, "virtio-pong", VIRTIO_ID_PONG, 0);
+
+    vpong->vq_in = virtio_add_queue(vdev, 8, handle_input);
+    vpong->vq_out = virtio_add_queue(vdev, 8, handle_output);
+}
+
+static void virtio_pong_device_unrealize(DeviceState *dev)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOPONG *vpong = VIRTIO_PONG(dev);
+
+    qemu_del_vm_change_state_handler(vpong->vmstate);
+    virtio_del_queue(vdev, 0);
+    virtio_cleanup(vdev);
+}
+
+static Property virtio_pong_properties[] = {
+    DEFINE_PROP_UINT64("cksum", VirtIOPONG, cksum, 1),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_pong_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+
+    device_class_set_props(dc, virtio_pong_properties);
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+    vdc->realize = virtio_pong_device_realize;
+    vdc->unrealize = virtio_pong_device_unrealize;
+    vdc->get_features = get_features;
+    vdc->set_status = virtio_pong_set_status;
+}
+
+static const TypeInfo virtio_pong_info = {
+    .name = TYPE_VIRTIO_PONG,
+    .parent = TYPE_VIRTIO_DEVICE,
+    .instance_size = sizeof(VirtIOPONG),
+    .class_init = virtio_pong_class_init,
+};
+
+static void virtio_register_types(void)
+{
+    type_register_static(&virtio_pong_info);
+}
+
+type_init(virtio_register_types)
diff --git a/include/hw/virtio/virtio-pong.h b/include/hw/virtio/virtio-pong.h
new file mode 100644
index 0000000000..ff44f2fead
--- /dev/null
+++ b/include/hw/virtio/virtio-pong.h
@@ -0,0 +1,34 @@
+/*
+ * Virtio PONG Support
+ *
+ * Copyright IBM 2020
+ * Copyright Pierre Morel <pmorel@linux.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#ifndef QEMU_VIRTIO_PONG_H
+#define QEMU_VIRTIO_PONG_H
+
+#include "hw/virtio/virtio.h"
+
+#define TYPE_VIRTIO_PONG "virtio-pong-device"
+#define VIRTIO_PONG(obj) \
+        OBJECT_CHECK(VirtIOPONG, (obj), TYPE_VIRTIO_PONG)
+#define VIRTIO_PONG_GET_PARENT_CLASS(obj) \
+        OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_PONG)
+
+typedef struct VirtIOPONG {
+    VirtIODevice parent_obj;
+    VirtQueue *vq_in;
+    VirtQueue *vq_out;
+    VMChangeStateEntry *vmstate;
+    uint64_t cksum;
+} VirtIOPONG;
+
+/* Feature bits */
+#define VIRTIO_PONG_F_CKSUM    1       /* Indicates pong using checksum */
+
+#endif
-- 
2.25.1

Re: [PATCH 0/2] s390x: ccw: A simple test device for virtio CCW

[Christian Borntraeger]

On 27.08.21 12:50, Pierre Morel wrote:
> Hello All,
> 
> 
> This series presents a VIRTIO test device which receives data on its
> input channel and sends back a simple checksum for the data it received
> on its output channel.
>   
> The goal is to allow a simple VIRTIO device driver to check the VIRTIO
> initialization and various data transfer.
> 
> For this I introduced a new device ID for the device and having no
> Linux driver but a kvm-unit-test driver, I have the following
> questions:

I think we should reserve an ID in the official virtio spec then for such a device?
Maybe also add mst for such things.
  

> Is there another way to advertise new VIRTIO IDs but Linux?
> If this QEMU test meet interest, should I write a Linux test program?
> 
> Regards,
> Pierre
> 
> 
> Pierre Morel (2):
>    virtio: Linux: Update of virtio_ids
>    s390x: ccw: A simple test device for virtio CCW
> 
>   hw/s390x/meson.build                        |   1 +
>   hw/s390x/virtio-ccw-pong.c                  |  66 ++++++++
>   hw/s390x/virtio-ccw.h                       |  13 ++
>   hw/virtio/Kconfig                           |   5 +
>   hw/virtio/meson.build                       |   1 +
>   hw/virtio/virtio-pong.c                     | 161 ++++++++++++++++++++
>   include/hw/virtio/virtio-pong.h             |  34 +++++
>   include/standard-headers/linux/virtio_ids.h |   1 +
>   8 files changed, 282 insertions(+)
>   create mode 100644 hw/s390x/virtio-ccw-pong.c
>   create mode 100644 hw/virtio/virtio-pong.c
>   create mode 100644 include/hw/virtio/virtio-pong.h
> 

Re: [PATCH 0/2] s390x: ccw: A simple test device for virtio CCW

[Pierre Morel]

On 8/30/21 11:51 AM, Christian Borntraeger wrote:
> 
> 
> On 27.08.21 12:50, Pierre Morel wrote:
>> Hello All,
>>
>>
>> This series presents a VIRTIO test device which receives data on its
>> input channel and sends back a simple checksum for the data it received
>> on its output channel.
>> The goal is to allow a simple VIRTIO device driver to check the VIRTIO
>> initialization and various data transfer.
>>
>> For this I introduced a new device ID for the device and having no
>> Linux driver but a kvm-unit-test driver, I have the following
>> questions:
> 
> I think we should reserve an ID in the official virtio spec then for 
> such a device?

Yes, you are right, I think we should.

> Maybe also add mst for such things.

Yes, I did.

Thanks,
Pierre



-- 
Pierre Morel
IBM Lab Boeblingen

Re: [PATCH 0/2] s390x: ccw: A simple test device for virtio CCW

[Halil Pasic]

On Mon, 30 Aug 2021 11:51:51 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 27.08.21 12:50, Pierre Morel wrote:
> > Hello All,
> > 
> > 
> > This series presents a VIRTIO test device which receives data on its
> > input channel and sends back a simple checksum for the data it received
> > on its output channel.
> >   
> > The goal is to allow a simple VIRTIO device driver to check the VIRTIO
> > initialization and various data transfer.

Can you please elaborate a little on the objectives.

> > 
> > For this I introduced a new device ID for the device and having no
> > Linux driver but a kvm-unit-test driver, I have the following
> > questions:  
> 
> I think we should reserve an ID in the official virtio spec then for such a device?
> Maybe also add mst for such things.

I agree having ID reserved is a good idea. But then if we are going to
introduce an official test device, I believe we should write a
specification for it as well. Yes having the guarantee that test devices
and real devices won't mix is a value in itself, but if we had a
standardized test device, whoever does work with it would not have to
ask themselves is this test device compatible with this test device
driver.

>   
> 
> > Is there another way to advertise new VIRTIO IDs but Linux?
> > If this QEMU test meet interest, should I write a Linux test program?
> > 

You may not simply claim and advertise a VIRTIO ID. The virtio ids
are allocated by the virtio standardisation body, and the list of the
IDs reserved in the v1.1-cs01 incarnation of the spec can be found here:
https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-1930005

For how to contribute to the virtio specification please take look at
this:
https://github.com/oasis-tcs/virtio-admin/blob/master/README.md

> > Regards,
> > Pierre
> > 
> > 
> > Pierre Morel (2):
> >    virtio: Linux: Update of virtio_ids
> >    s390x: ccw: A simple test device for virtio CCW
> > 
> >   hw/s390x/meson.build                        |   1 +
> >   hw/s390x/virtio-ccw-pong.c                  |  66 ++++++++
> >   hw/s390x/virtio-ccw.h                       |  13 ++
> >   hw/virtio/Kconfig                           |   5 +
> >   hw/virtio/meson.build                       |   1 +
> >   hw/virtio/virtio-pong.c                     | 161 ++++++++++++++++++++
> >   include/hw/virtio/virtio-pong.h             |  34 +++++
> >   include/standard-headers/linux/virtio_ids.h |   1 +
> >   8 files changed, 282 insertions(+)
> >   create mode 100644 hw/s390x/virtio-ccw-pong.c
> >   create mode 100644 hw/virtio/virtio-pong.c
> >   create mode 100644 include/hw/virtio/virtio-pong.h
> >   

Re: [PATCH 0/2] s390x: ccw: A simple test device for virtio CCW

[Pierre Morel]

On 8/30/21 10:42 PM, Halil Pasic wrote:
> On Mon, 30 Aug 2021 11:51:51 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> On 27.08.21 12:50, Pierre Morel wrote:
>>> Hello All,
>>>
>>>
>>> This series presents a VIRTIO test device which receives data on its
>>> input channel and sends back a simple checksum for the data it received
>>> on its output channel.
>>>    
>>> The goal is to allow a simple VIRTIO device driver to check the VIRTIO
>>> initialization and various data transfer.
> 
> Can you please elaborate a little on the objectives.

Yes I will, but I must think a lot more about it, I think doing the 
specifications you speak about later in this response is the right way 
to do it.


> 
>>>
>>> For this I introduced a new device ID for the device and having no
>>> Linux driver but a kvm-unit-test driver, I have the following
>>> questions:
>>
>> I think we should reserve an ID in the official virtio spec then for such a device?
>> Maybe also add mst for such things.
> 
> I agree having ID reserved is a good idea. But then if we are going to
> introduce an official test device, I believe we should write a
> specification for it as well. Yes having the guarantee that test devices
> and real devices won't mix is a value in itself, but if we had a
> standardized test device, whoever does work with it would not have to
> ask themselves is this test device compatible with this test device
> driver.

Yes right.

> 
>>    
>>
>>> Is there another way to advertise new VIRTIO IDs but Linux?
>>> If this QEMU test meet interest, should I write a Linux test program?
>>>
> 
> You may not simply claim and advertise a VIRTIO ID. The virtio ids
> are allocated by the virtio standardisation body, and the list of the
> IDs reserved in the v1.1-cs01 incarnation of the spec can be found here:
> https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-1930005
> 
> For how to contribute to the virtio specification please take look at
> this:
> https://github.com/oasis-tcs/virtio-admin/blob/master/README.md

Thanks, I will go this way.


Thanks for these constructive answers,

Regards,
Pierre



-- 
Pierre Morel
IBM Lab Boeblingen