[PATCH 0/2] virtio: Add vhost-user-gpio device's support

Viresh Kumar posted 2 patches 2 years, 3 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/cover.1641987128.git.viresh.kumar@linaro.org
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>
hw/virtio/Kconfig                   |   5 +
hw/virtio/meson.build               |   2 +
hw/virtio/vhost-user-gpio-pci.c     |  69 ++++++
hw/virtio/vhost-user-gpio.c         | 343 ++++++++++++++++++++++++++++
include/hw/virtio/vhost-user-gpio.h |  35 +++
5 files changed, 454 insertions(+)
create mode 100644 hw/virtio/vhost-user-gpio-pci.c
create mode 100644 hw/virtio/vhost-user-gpio.c
create mode 100644 include/hw/virtio/vhost-user-gpio.h
[PATCH 0/2] virtio: Add vhost-user-gpio device's support
Posted by Viresh Kumar 2 years, 3 months ago
Hello,

This patchset adds vhost-user-gpio device's support in Qemu. The support for the
same has already been added to virtio specification and Linux Kernel.

A Rust based backend is also in progress and is tested against this patchset:

https://github.com/rust-vmm/vhost-device/pull/76

--
Viresh

Viresh Kumar (2):
  hw/virtio: add boilerplate for vhost-user-gpio device
  hw/virtio: add vhost-user-gpio-pci boilerplate

 hw/virtio/Kconfig                   |   5 +
 hw/virtio/meson.build               |   2 +
 hw/virtio/vhost-user-gpio-pci.c     |  69 ++++++
 hw/virtio/vhost-user-gpio.c         | 343 ++++++++++++++++++++++++++++
 include/hw/virtio/vhost-user-gpio.h |  35 +++
 5 files changed, 454 insertions(+)
 create mode 100644 hw/virtio/vhost-user-gpio-pci.c
 create mode 100644 hw/virtio/vhost-user-gpio.c
 create mode 100644 include/hw/virtio/vhost-user-gpio.h

-- 
2.31.1.272.g89b43f80a514


Re: [PATCH 0/2] virtio: Add vhost-user-gpio device's support
Posted by Michael S. Tsirkin 2 years, 3 months ago
On Wed, Jan 12, 2022 at 05:04:57PM +0530, Viresh Kumar wrote:
> Hello,
> 
> This patchset adds vhost-user-gpio device's support in Qemu. The support for the
> same has already been added to virtio specification and Linux Kernel.
> 
> A Rust based backend is also in progress and is tested against this patchset:
> 
> https://github.com/rust-vmm/vhost-device/pull/76


I'm reluctant to add this with no tests in tree.
Want to write a minimal libhost-user based backend?
We also need some maintainers to step up.


> --
> Viresh
> 
> Viresh Kumar (2):
>   hw/virtio: add boilerplate for vhost-user-gpio device
>   hw/virtio: add vhost-user-gpio-pci boilerplate
> 
>  hw/virtio/Kconfig                   |   5 +
>  hw/virtio/meson.build               |   2 +
>  hw/virtio/vhost-user-gpio-pci.c     |  69 ++++++
>  hw/virtio/vhost-user-gpio.c         | 343 ++++++++++++++++++++++++++++
>  include/hw/virtio/vhost-user-gpio.h |  35 +++
>  5 files changed, 454 insertions(+)
>  create mode 100644 hw/virtio/vhost-user-gpio-pci.c
>  create mode 100644 hw/virtio/vhost-user-gpio.c
>  create mode 100644 include/hw/virtio/vhost-user-gpio.h
> 
> -- 
> 2.31.1.272.g89b43f80a514


Re: [PATCH 0/2] virtio: Add vhost-user-gpio device's support
Posted by Alex Bennée 2 years, 3 months ago
"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Wed, Jan 12, 2022 at 05:04:57PM +0530, Viresh Kumar wrote:
>> Hello,
>> 
>> This patchset adds vhost-user-gpio device's support in Qemu. The support for the
>> same has already been added to virtio specification and Linux Kernel.
>> 
>> A Rust based backend is also in progress and is tested against this patchset:
>> 
>> https://github.com/rust-vmm/vhost-device/pull/76
>
>
> I'm reluctant to add this with no tests in tree.
> Want to write a minimal libhost-user based backend?

This is going to be a problem going forward as we have more out-of-tree
backends written as a first preference. While the first couple of vhost
devices have C implementations in contrib before we worked on the rust
version I think we are getting to the point of skipping a first C
version for future devices.

However I notice we have qtest/vhost-user-test.c so would that be enough
to ensure we can instantiate the device and a basic vhost-user
initialisation sequence doesn't cause it to crap out. This obviously
won't be exercising the virtq processing itself but does that really
exercise any of QEMU's boilerplate anyway?

> We also need some maintainers to step up.

You mean more reviewers for the vhost and virtio sections of QEMU's
maintainers?

>
>
>> --
>> Viresh
>> 
>> Viresh Kumar (2):
>>   hw/virtio: add boilerplate for vhost-user-gpio device
>>   hw/virtio: add vhost-user-gpio-pci boilerplate
>> 
>>  hw/virtio/Kconfig                   |   5 +
>>  hw/virtio/meson.build               |   2 +
>>  hw/virtio/vhost-user-gpio-pci.c     |  69 ++++++
>>  hw/virtio/vhost-user-gpio.c         | 343 ++++++++++++++++++++++++++++
>>  include/hw/virtio/vhost-user-gpio.h |  35 +++
>>  5 files changed, 454 insertions(+)
>>  create mode 100644 hw/virtio/vhost-user-gpio-pci.c
>>  create mode 100644 hw/virtio/vhost-user-gpio.c
>>  create mode 100644 include/hw/virtio/vhost-user-gpio.h
>> 
>> -- 
>> 2.31.1.272.g89b43f80a514


-- 
Alex Bennée

Re: [PATCH 0/2] virtio: Add vhost-user-gpio device's support
Posted by Viresh Kumar 2 years, 3 months ago
On 17-01-22, 10:11, Alex Bennée wrote:
> 
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Wed, Jan 12, 2022 at 05:04:57PM +0530, Viresh Kumar wrote:
> >> Hello,
> >> 
> >> This patchset adds vhost-user-gpio device's support in Qemu. The support for the
> >> same has already been added to virtio specification and Linux Kernel.
> >> 
> >> A Rust based backend is also in progress and is tested against this patchset:
> >> 
> >> https://github.com/rust-vmm/vhost-device/pull/76
> >
> >
> > I'm reluctant to add this with no tests in tree.
> > Want to write a minimal libhost-user based backend?

I actually have one already, that I wrote before attempting the Rust
counterpart, but never upstreamed as I am not sure if anyone is ever
going to use it, as I am not. And I thought what's the point of
merging code which I will never end up using.

I am not sure what test I can add here to make sure this doesn't
breaks in future though.

> This is going to be a problem going forward as we have more out-of-tree
> backends written as a first preference. While the first couple of vhost
> devices have C implementations in contrib before we worked on the rust
> version I think we are getting to the point of skipping a first C
> version for future devices.
> 
> However I notice we have qtest/vhost-user-test.c so would that be enough
> to ensure we can instantiate the device and a basic vhost-user
> initialisation sequence doesn't cause it to crap out. This obviously
> won't be exercising the virtq processing itself but does that really
> exercise any of QEMU's boilerplate anyway?
> 
> > We also need some maintainers to step up.
> 
> You mean more reviewers for the vhost and virtio sections of QEMU's
> maintainers?

And I too was waiting for replies on these. I can surely write
something up if you guys feel there is a need. I just want to
understand it better.

-- 
viresh

Re: [PATCH 0/2] virtio: Add vhost-user-gpio device's support
Posted by Michael S. Tsirkin 2 years, 3 months ago
On Thu, Jan 20, 2022 at 09:32:34AM +0530, Viresh Kumar wrote:
> On 17-01-22, 10:11, Alex Bennée wrote:
> > 
> > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > 
> > > On Wed, Jan 12, 2022 at 05:04:57PM +0530, Viresh Kumar wrote:
> > >> Hello,
> > >> 
> > >> This patchset adds vhost-user-gpio device's support in Qemu. The support for the
> > >> same has already been added to virtio specification and Linux Kernel.
> > >> 
> > >> A Rust based backend is also in progress and is tested against this patchset:
> > >> 
> > >> https://github.com/rust-vmm/vhost-device/pull/76
> > >
> > >
> > > I'm reluctant to add this with no tests in tree.
> > > Want to write a minimal libhost-user based backend?
> 
> I actually have one already, that I wrote before attempting the Rust
> counterpart, but never upstreamed as I am not sure if anyone is ever
> going to use it, as I am not. And I thought what's the point of
> merging code which I will never end up using.
> 
> I am not sure what test I can add here to make sure this doesn't
> breaks in future though.

something that executes with make check.


> > This is going to be a problem going forward as we have more out-of-tree
> > backends written as a first preference. While the first couple of vhost
> > devices have C implementations in contrib before we worked on the rust
> > version I think we are getting to the point of skipping a first C
> > version for future devices.
> > 
> > However I notice we have qtest/vhost-user-test.c so would that be enough
> > to ensure we can instantiate the device and a basic vhost-user
> > initialisation sequence doesn't cause it to crap out. This obviously
> > won't be exercising the virtq processing itself but does that really
> > exercise any of QEMU's boilerplate anyway?
> > 
> > > We also need some maintainers to step up.
> > 
> > You mean more reviewers for the vhost and virtio sections of QEMU's
> > maintainers?
> 
> And I too was waiting for replies on these. I can surely write
> something up if you guys feel there is a need. I just want to
> understand it better.
> 
> -- 
> viresh


Re: [PATCH 0/2] virtio: Add vhost-user-gpio device's support
Posted by Alex Bennée 2 years, 3 months ago
"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Jan 20, 2022 at 09:32:34AM +0530, Viresh Kumar wrote:
>> On 17-01-22, 10:11, Alex Bennée wrote:
>> > 
>> > "Michael S. Tsirkin" <mst@redhat.com> writes:
>> > 
>> > > On Wed, Jan 12, 2022 at 05:04:57PM +0530, Viresh Kumar wrote:
>> > >> Hello,
>> > >> 
>> > >> This patchset adds vhost-user-gpio device's support in Qemu. The support for the
>> > >> same has already been added to virtio specification and Linux Kernel.
>> > >> 
>> > >> A Rust based backend is also in progress and is tested against this patchset:
>> > >> 
>> > >> https://github.com/rust-vmm/vhost-device/pull/76
>> > >
>> > >
>> > > I'm reluctant to add this with no tests in tree.
>> > > Want to write a minimal libhost-user based backend?
>> 
>> I actually have one already, that I wrote before attempting the Rust
>> counterpart, but never upstreamed as I am not sure if anyone is ever
>> going to use it, as I am not. And I thought what's the point of
>> merging code which I will never end up using.
>> 
>> I am not sure what test I can add here to make sure this doesn't
>> breaks in future though.
>
> something that executes with make check.

With Eric's latest series
(20220118203833.316741-1-eric.auger@redhat.com) we are already
exercising most of the PCI code:

  ➜  gcov ./libqemu-aarch64-softmmu.fa.p/hw_virtio_vhost-user-gpio.c.gcda
  File '../../hw/virtio/vhost-user-gpio.c'
  Lines executed:10.43% of 163
  Creating 'vhost-user-gpio.c.gcov'

  File '/home/alex/lsrc/qemu.git/include/qemu/bitops.h'
  Lines executed:100.00% of 4
  Creating 'bitops.h.gcov'

  File '/home/alex/lsrc/qemu.git/include/hw/virtio/virtio.h'
  Lines executed:12.50% of 8
  Creating 'virtio.h.gcov'

  File '/home/alex/lsrc/qemu.git/include/hw/virtio/vhost-user-gpio.h'
  Lines executed:0.00% of 1
  Creating 'vhost-user-gpio.h.gcov'

  File '/home/alex/lsrc/qemu.git/include/hw/virtio/virtio-bus.h'
  Lines executed:0.00% of 1
  Creating 'virtio-bus.h.gcov'

  File '/home/alex/lsrc/qemu.git/include/hw/qdev-core.h'
  Lines executed:50.00% of 2
  Creating 'qdev-core.h.gcov'

  🕙15:40:35 alex@zen:qemu.git/builds/gprof  on  review/virtio-gpio-boilerplate [$?] 
  ➜  gcov ./libqemu-aarch64-softmmu.fa.p/hw_virtio_vhost-user-gpio-pci.c.gcda
  File '../../hw/virtio/vhost-user-gpio-pci.c'
  Lines executed:76.92% of 26
  Creating 'vhost-user-gpio-pci.c.gcov'

  File '/home/alex/lsrc/qemu.git/include/qemu/bitops.h'
  Lines executed:100.00% of 4
  Creating 'bitops.h.gcov'

  File '../../hw/virtio/virtio-pci.h'
  Lines executed:100.00% of 1
  Creating 'virtio-pci.h.gcov'

  File '/home/alex/lsrc/qemu.git/include/hw/pci/pci.h'
  Lines executed:100.00% of 1
  Creating 'pci.h.gcov'

  File '/home/alex/lsrc/qemu.git/include/hw/qdev-core.h'
  Lines executed:50.00% of 2
  Creating 'qdev-core.h.gcov'

So I think if we add the runes to ensure we instantiate both types
explicitly I think we will have enough coverage.

-- 
Alex Bennée

[RFC PATCH] tests/qtest: attempt to enable tests for virtio-gpio (!working)
Posted by Alex Bennée 2 years, 3 months ago
We don't have a virtio-gpio implementation in QEMU and only
support a vhost-user backend. The QEMU side of the code is minimal so
it should be enough to instantiate the device and pass some vhost-user
messages over the control socket. To do this we hook into the existing
vhost-user-test code and just add the bits required for gpio.

Based-on: 20220118203833.316741-1-eric.auger@redhat.com
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>

---

This goes as far as to add things to the QOS tree but so far it's
failing to properly start QEMU with the chardev socket needed to
communicate between the mock vhost-user daemon and QEMU itself.
---
 tests/qtest/libqos/virtio-gpio.h | 34 +++++++++++
 tests/qtest/libqos/virtio-gpio.c | 98 ++++++++++++++++++++++++++++++++
 tests/qtest/vhost-user-test.c    | 34 +++++++++++
 tests/qtest/libqos/meson.build   |  1 +
 4 files changed, 167 insertions(+)
 create mode 100644 tests/qtest/libqos/virtio-gpio.h
 create mode 100644 tests/qtest/libqos/virtio-gpio.c

diff --git a/tests/qtest/libqos/virtio-gpio.h b/tests/qtest/libqos/virtio-gpio.h
new file mode 100644
index 0000000000..abe6967ae9
--- /dev/null
+++ b/tests/qtest/libqos/virtio-gpio.h
@@ -0,0 +1,34 @@
+/*
+ * virtio-gpio structures
+ *
+ * Copyright (c) 2022 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef TESTS_LIBQOS_VIRTIO_GPIO_H
+#define TESTS_LIBQOS_VIRTIO_GPIO_H
+
+#include "qgraph.h"
+#include "virtio.h"
+#include "virtio-pci.h"
+
+typedef struct QVhostUserGPIO QVhostUserGPIO;
+typedef struct QVhostUserGPIOPCI QVhostUserGPIOPCI;
+typedef struct QVhostUserGPIODevice QVhostUserGPIODevice;
+
+struct QVhostUserGPIO {
+    QVirtioDevice *vdev;
+};
+
+struct QVhostUserGPIOPCI {
+    QVirtioPCIDevice pci_vdev;
+    QVhostUserGPIO gpio;
+};
+
+struct QVhostUserGPIODevice {
+    QOSGraphObject obj;
+    QVhostUserGPIO gpio;
+};
+
+#endif
diff --git a/tests/qtest/libqos/virtio-gpio.c b/tests/qtest/libqos/virtio-gpio.c
new file mode 100644
index 0000000000..62c8074777
--- /dev/null
+++ b/tests/qtest/libqos/virtio-gpio.c
@@ -0,0 +1,98 @@
+/*
+ * virtio-gpio nodes for testing
+ *
+ * Copyright (c) 2022 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qemu/module.h"
+#include "qgraph.h"
+#include "virtio-gpio.h"
+
+static void *qvirtio_gpio_get_driver(QVhostUserGPIO *v_gpio,
+                                       const char *interface)
+{
+    if (!g_strcmp0(interface, "vhost-user-gpio")) {
+        return v_gpio;
+    }
+    if (!g_strcmp0(interface, "virtio")) {
+        return v_gpio->vdev;
+    }
+
+    fprintf(stderr, "%s not present in virtio-gpio-device\n", interface);
+    g_assert_not_reached();
+}
+
+static void *qvirtio_gpio_device_get_driver(void *object,
+                                              const char *interface)
+{
+    QVhostUserGPIODevice *v_gpio = object;
+    return qvirtio_gpio_get_driver(&v_gpio->gpio, interface);
+}
+
+static void *virtio_gpio_device_create(void *virtio_dev,
+                                         QGuestAllocator *t_alloc,
+                                         void *addr)
+{
+    QVhostUserGPIODevice *virtio_device = g_new0(QVhostUserGPIODevice, 1);
+    QVhostUserGPIO *interface = &virtio_device->gpio;
+
+    interface->vdev = virtio_dev;
+
+    virtio_device->obj.get_driver = qvirtio_gpio_device_get_driver;
+
+    return &virtio_device->obj;
+}
+
+/* virtio-gpio-pci */
+static void *qvirtio_gpio_pci_get_driver(void *object, const char *interface)
+{
+    QVhostUserGPIOPCI *v_gpio = object;
+    if (!g_strcmp0(interface, "pci-device")) {
+        return v_gpio->pci_vdev.pdev;
+    }
+    return qvirtio_gpio_get_driver(&v_gpio->gpio, interface);
+}
+
+static void *virtio_gpio_pci_create(void *pci_bus, QGuestAllocator *t_alloc,
+                                      void *addr)
+{
+    QVhostUserGPIOPCI *virtio_spci = g_new0(QVhostUserGPIOPCI, 1);
+    QVhostUserGPIO *interface = &virtio_spci->gpio;
+    QOSGraphObject *obj = &virtio_spci->pci_vdev.obj;
+
+    virtio_pci_init(&virtio_spci->pci_vdev, pci_bus, addr);
+    interface->vdev = &virtio_spci->pci_vdev.vdev;
+
+    obj->get_driver = qvirtio_gpio_pci_get_driver;
+
+    return obj;
+}
+
+static void virtio_gpio_register_nodes(void)
+{
+    QPCIAddress addr = {
+        .devfn = QPCI_DEVFN(4, 0),
+    };
+
+    QOSGraphEdgeOptions edge_opts = { };
+
+    /* vhost-user-gpio-device */
+    edge_opts.extra_device_opts = "id=gpio0,chardev=vhgpio0";
+    qos_node_create_driver("vhost-user-gpio-device",
+                            virtio_gpio_device_create);
+    qos_node_consumes("vhost-user-gpio-device", "virtio-bus", &edge_opts);
+    qos_node_produces("vhost-user-gpio-device", "vhost-user-gpio");
+
+    /* virtio-gpio-pci */
+    edge_opts.extra_device_opts = "id=gpio0,addr=04.0,chardev=vhgpio0";
+    add_qpci_address(&edge_opts, &addr);
+    qos_node_create_driver("vhost-user-gpio-pci", virtio_gpio_pci_create);
+    qos_node_consumes("vhost-user-gpio-pci", "pci-bus", &edge_opts);
+    qos_node_produces("vhost-user-gpio-pci", "vhost-user-gpio");
+}
+
+libqos_init(virtio_gpio_register_nodes);
diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index ee30f54796..4f8789424b 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -43,6 +43,7 @@
                         " -numa node,memdev=mem"
 #define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s%s"
 #define QEMU_CMD_NETDEV " -netdev vhost-user,id=hs0,chardev=%s,vhostforce=on"
+#define QEMU_CMD_GPIO   " -device vhost-user-gpio-pci,id=gpio0,chardev=%s,vhostforce=on"
 
 #define HUGETLBFS_MAGIC       0x958458f6
 
@@ -137,6 +138,7 @@ enum {
 
 enum {
     VHOST_USER_NET,
+    VHOST_USER_GPIO,
 };
 
 typedef struct TestServer {
@@ -194,6 +196,14 @@ static void append_vhost_net_opts(TestServer *s, GString *cmd_line,
                            chr_opts, s->chr_name);
 }
 
+static void append_vhost_gpio_opts(TestServer *s, GString *cmd_line,
+                             const char *chr_opts)
+{
+    g_string_append_printf(cmd_line, QEMU_CMD_CHR QEMU_CMD_GPIO,
+                           s->chr_name, s->socket_path,
+                           chr_opts, s->chr_name);
+}
+
 static void append_mem_opts(TestServer *server, GString *cmd_line,
                             int size, enum test_memfd memfd)
 {
@@ -1014,3 +1024,27 @@ static void register_vhost_user_test(void)
                  test_multiqueue, &opts);
 }
 libqos_init(register_vhost_user_test);
+
+static struct vhost_user_ops g_vu_gpio_ops = {
+    .type = VHOST_USER_GPIO,
+
+    .append_opts = append_vhost_gpio_opts,
+
+    .set_features = vu_net_set_features,
+    .get_protocol_features = vu_net_get_protocol_features,
+};
+
+static void register_vhost_gpio_test(void)
+{
+    QOSGraphTestOptions opts = {
+        .before = vhost_user_test_setup,
+        .subprocess = true,
+        .arg = &g_vu_gpio_ops,
+    };
+
+    qemu_add_opts(&qemu_chardev_opts);
+
+    qos_add_test("read-guest-mem/memfile",
+                 "vhost-user-gpio", test_read_guest_mem, &opts);
+}
+libqos_init(register_vhost_gpio_test);
diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build
index 8c8ee15553..f581330d8b 100644
--- a/tests/qtest/libqos/meson.build
+++ b/tests/qtest/libqos/meson.build
@@ -42,6 +42,7 @@ libqos_srcs = files('../libqtest.c',
         'virtio-scsi.c',
         'virtio-serial.c',
         'virtio-iommu.c',
+        'virtio-gpio.c',
         'generic-pcihost.c',
 
         # qgraph machines:
-- 
2.30.2


Re: [RFC PATCH] tests/qtest: attempt to enable tests for virtio-gpio (!working)
Posted by Alex Bennée 2 years ago
(expanding the CC list for help, anyone have a better idea about how
vhost-user qtests should work/see obvious issues with this patch?)

Alex Bennée <alex.bennee@linaro.org> writes:

> We don't have a virtio-gpio implementation in QEMU and only
> support a vhost-user backend. The QEMU side of the code is minimal so
> it should be enough to instantiate the device and pass some vhost-user
> messages over the control socket. To do this we hook into the existing
> vhost-user-test code and just add the bits required for gpio.
>
> Based-on: 20220118203833.316741-1-eric.auger@redhat.com
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
>
> ---
>
> This goes as far as to add things to the QOS tree but so far it's
> failing to properly start QEMU with the chardev socket needed to
> communicate between the mock vhost-user daemon and QEMU itself.
> ---
>  tests/qtest/libqos/virtio-gpio.h | 34 +++++++++++
>  tests/qtest/libqos/virtio-gpio.c | 98 ++++++++++++++++++++++++++++++++
>  tests/qtest/vhost-user-test.c    | 34 +++++++++++
>  tests/qtest/libqos/meson.build   |  1 +
>  4 files changed, 167 insertions(+)
>  create mode 100644 tests/qtest/libqos/virtio-gpio.h
>  create mode 100644 tests/qtest/libqos/virtio-gpio.c
>
> diff --git a/tests/qtest/libqos/virtio-gpio.h b/tests/qtest/libqos/virtio-gpio.h
> new file mode 100644
> index 0000000000..abe6967ae9
> --- /dev/null
> +++ b/tests/qtest/libqos/virtio-gpio.h
> @@ -0,0 +1,34 @@
> +/*
> + * virtio-gpio structures
> + *
> + * Copyright (c) 2022 Linaro Ltd
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef TESTS_LIBQOS_VIRTIO_GPIO_H
> +#define TESTS_LIBQOS_VIRTIO_GPIO_H
> +
> +#include "qgraph.h"
> +#include "virtio.h"
> +#include "virtio-pci.h"
> +
> +typedef struct QVhostUserGPIO QVhostUserGPIO;
> +typedef struct QVhostUserGPIOPCI QVhostUserGPIOPCI;
> +typedef struct QVhostUserGPIODevice QVhostUserGPIODevice;
> +
> +struct QVhostUserGPIO {
> +    QVirtioDevice *vdev;
> +};
> +
> +struct QVhostUserGPIOPCI {
> +    QVirtioPCIDevice pci_vdev;
> +    QVhostUserGPIO gpio;
> +};
> +
> +struct QVhostUserGPIODevice {
> +    QOSGraphObject obj;
> +    QVhostUserGPIO gpio;
> +};
> +
> +#endif
> diff --git a/tests/qtest/libqos/virtio-gpio.c b/tests/qtest/libqos/virtio-gpio.c
> new file mode 100644
> index 0000000000..62c8074777
> --- /dev/null
> +++ b/tests/qtest/libqos/virtio-gpio.c
> @@ -0,0 +1,98 @@
> +/*
> + * virtio-gpio nodes for testing
> + *
> + * Copyright (c) 2022 Linaro Ltd
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqtest.h"
> +#include "qemu/module.h"
> +#include "qgraph.h"
> +#include "virtio-gpio.h"
> +
> +static void *qvirtio_gpio_get_driver(QVhostUserGPIO *v_gpio,
> +                                       const char *interface)
> +{
> +    if (!g_strcmp0(interface, "vhost-user-gpio")) {
> +        return v_gpio;
> +    }
> +    if (!g_strcmp0(interface, "virtio")) {
> +        return v_gpio->vdev;
> +    }
> +
> +    fprintf(stderr, "%s not present in virtio-gpio-device\n", interface);
> +    g_assert_not_reached();
> +}
> +
> +static void *qvirtio_gpio_device_get_driver(void *object,
> +                                              const char *interface)
> +{
> +    QVhostUserGPIODevice *v_gpio = object;
> +    return qvirtio_gpio_get_driver(&v_gpio->gpio, interface);
> +}
> +
> +static void *virtio_gpio_device_create(void *virtio_dev,
> +                                         QGuestAllocator *t_alloc,
> +                                         void *addr)
> +{
> +    QVhostUserGPIODevice *virtio_device = g_new0(QVhostUserGPIODevice, 1);
> +    QVhostUserGPIO *interface = &virtio_device->gpio;
> +
> +    interface->vdev = virtio_dev;
> +
> +    virtio_device->obj.get_driver = qvirtio_gpio_device_get_driver;
> +
> +    return &virtio_device->obj;
> +}
> +
> +/* virtio-gpio-pci */
> +static void *qvirtio_gpio_pci_get_driver(void *object, const char *interface)
> +{
> +    QVhostUserGPIOPCI *v_gpio = object;
> +    if (!g_strcmp0(interface, "pci-device")) {
> +        return v_gpio->pci_vdev.pdev;
> +    }
> +    return qvirtio_gpio_get_driver(&v_gpio->gpio, interface);
> +}
> +
> +static void *virtio_gpio_pci_create(void *pci_bus, QGuestAllocator *t_alloc,
> +                                      void *addr)
> +{
> +    QVhostUserGPIOPCI *virtio_spci = g_new0(QVhostUserGPIOPCI, 1);
> +    QVhostUserGPIO *interface = &virtio_spci->gpio;
> +    QOSGraphObject *obj = &virtio_spci->pci_vdev.obj;
> +
> +    virtio_pci_init(&virtio_spci->pci_vdev, pci_bus, addr);
> +    interface->vdev = &virtio_spci->pci_vdev.vdev;
> +
> +    obj->get_driver = qvirtio_gpio_pci_get_driver;
> +
> +    return obj;
> +}
> +
> +static void virtio_gpio_register_nodes(void)
> +{
> +    QPCIAddress addr = {
> +        .devfn = QPCI_DEVFN(4, 0),
> +    };
> +
> +    QOSGraphEdgeOptions edge_opts = { };
> +
> +    /* vhost-user-gpio-device */
> +    edge_opts.extra_device_opts = "id=gpio0,chardev=vhgpio0";
> +    qos_node_create_driver("vhost-user-gpio-device",
> +                            virtio_gpio_device_create);
> +    qos_node_consumes("vhost-user-gpio-device", "virtio-bus", &edge_opts);
> +    qos_node_produces("vhost-user-gpio-device", "vhost-user-gpio");
> +
> +    /* virtio-gpio-pci */
> +    edge_opts.extra_device_opts = "id=gpio0,addr=04.0,chardev=vhgpio0";
> +    add_qpci_address(&edge_opts, &addr);
> +    qos_node_create_driver("vhost-user-gpio-pci", virtio_gpio_pci_create);
> +    qos_node_consumes("vhost-user-gpio-pci", "pci-bus", &edge_opts);
> +    qos_node_produces("vhost-user-gpio-pci", "vhost-user-gpio");
> +}
> +
> +libqos_init(virtio_gpio_register_nodes);
> diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
> index ee30f54796..4f8789424b 100644
> --- a/tests/qtest/vhost-user-test.c
> +++ b/tests/qtest/vhost-user-test.c
> @@ -43,6 +43,7 @@
>                          " -numa node,memdev=mem"
>  #define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s%s"
>  #define QEMU_CMD_NETDEV " -netdev vhost-user,id=hs0,chardev=%s,vhostforce=on"
> +#define QEMU_CMD_GPIO   " -device vhost-user-gpio-pci,id=gpio0,chardev=%s,vhostforce=on"
>  
>  #define HUGETLBFS_MAGIC       0x958458f6
>  
> @@ -137,6 +138,7 @@ enum {
>  
>  enum {
>      VHOST_USER_NET,
> +    VHOST_USER_GPIO,
>  };
>  
>  typedef struct TestServer {
> @@ -194,6 +196,14 @@ static void append_vhost_net_opts(TestServer *s, GString *cmd_line,
>                             chr_opts, s->chr_name);
>  }
>  
> +static void append_vhost_gpio_opts(TestServer *s, GString *cmd_line,
> +                             const char *chr_opts)
> +{
> +    g_string_append_printf(cmd_line, QEMU_CMD_CHR QEMU_CMD_GPIO,
> +                           s->chr_name, s->socket_path,
> +                           chr_opts, s->chr_name);
> +}
> +
>  static void append_mem_opts(TestServer *server, GString *cmd_line,
>                              int size, enum test_memfd memfd)
>  {
> @@ -1014,3 +1024,27 @@ static void register_vhost_user_test(void)
>                   test_multiqueue, &opts);
>  }
>  libqos_init(register_vhost_user_test);
> +
> +static struct vhost_user_ops g_vu_gpio_ops = {
> +    .type = VHOST_USER_GPIO,
> +
> +    .append_opts = append_vhost_gpio_opts,
> +
> +    .set_features = vu_net_set_features,
> +    .get_protocol_features = vu_net_get_protocol_features,
> +};
> +
> +static void register_vhost_gpio_test(void)
> +{
> +    QOSGraphTestOptions opts = {
> +        .before = vhost_user_test_setup,
> +        .subprocess = true,
> +        .arg = &g_vu_gpio_ops,
> +    };
> +
> +    qemu_add_opts(&qemu_chardev_opts);
> +
> +    qos_add_test("read-guest-mem/memfile",
> +                 "vhost-user-gpio", test_read_guest_mem, &opts);
> +}
> +libqos_init(register_vhost_gpio_test);
> diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build
> index 8c8ee15553..f581330d8b 100644
> --- a/tests/qtest/libqos/meson.build
> +++ b/tests/qtest/libqos/meson.build
> @@ -42,6 +42,7 @@ libqos_srcs = files('../libqtest.c',
>          'virtio-scsi.c',
>          'virtio-serial.c',
>          'virtio-iommu.c',
> +        'virtio-gpio.c',
>          'generic-pcihost.c',
>  
>          # qgraph machines:


-- 
Alex Bennée
Re: [RFC PATCH] tests/qtest: attempt to enable tests for virtio-gpio (!working)
Posted by Dr. David Alan Gilbert 2 years ago
* Alex Bennée (alex.bennee@linaro.org) wrote:
> 
> (expanding the CC list for help, anyone have a better idea about how
> vhost-user qtests should work/see obvious issues with this patch?)

How exactly does it fail?

DAve

> Alex Bennée <alex.bennee@linaro.org> writes:
> 
> > We don't have a virtio-gpio implementation in QEMU and only
> > support a vhost-user backend. The QEMU side of the code is minimal so
> > it should be enough to instantiate the device and pass some vhost-user
> > messages over the control socket. To do this we hook into the existing
> > vhost-user-test code and just add the bits required for gpio.
> >
> > Based-on: 20220118203833.316741-1-eric.auger@redhat.com
> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> > Cc: Viresh Kumar <viresh.kumar@linaro.org>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> >
> > ---
> >
> > This goes as far as to add things to the QOS tree but so far it's
> > failing to properly start QEMU with the chardev socket needed to
> > communicate between the mock vhost-user daemon and QEMU itself.
> > ---
> >  tests/qtest/libqos/virtio-gpio.h | 34 +++++++++++
> >  tests/qtest/libqos/virtio-gpio.c | 98 ++++++++++++++++++++++++++++++++
> >  tests/qtest/vhost-user-test.c    | 34 +++++++++++
> >  tests/qtest/libqos/meson.build   |  1 +
> >  4 files changed, 167 insertions(+)
> >  create mode 100644 tests/qtest/libqos/virtio-gpio.h
> >  create mode 100644 tests/qtest/libqos/virtio-gpio.c
> >
> > diff --git a/tests/qtest/libqos/virtio-gpio.h b/tests/qtest/libqos/virtio-gpio.h
> > new file mode 100644
> > index 0000000000..abe6967ae9
> > --- /dev/null
> > +++ b/tests/qtest/libqos/virtio-gpio.h
> > @@ -0,0 +1,34 @@
> > +/*
> > + * virtio-gpio structures
> > + *
> > + * Copyright (c) 2022 Linaro Ltd
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#ifndef TESTS_LIBQOS_VIRTIO_GPIO_H
> > +#define TESTS_LIBQOS_VIRTIO_GPIO_H
> > +
> > +#include "qgraph.h"
> > +#include "virtio.h"
> > +#include "virtio-pci.h"
> > +
> > +typedef struct QVhostUserGPIO QVhostUserGPIO;
> > +typedef struct QVhostUserGPIOPCI QVhostUserGPIOPCI;
> > +typedef struct QVhostUserGPIODevice QVhostUserGPIODevice;
> > +
> > +struct QVhostUserGPIO {
> > +    QVirtioDevice *vdev;
> > +};
> > +
> > +struct QVhostUserGPIOPCI {
> > +    QVirtioPCIDevice pci_vdev;
> > +    QVhostUserGPIO gpio;
> > +};
> > +
> > +struct QVhostUserGPIODevice {
> > +    QOSGraphObject obj;
> > +    QVhostUserGPIO gpio;
> > +};
> > +
> > +#endif
> > diff --git a/tests/qtest/libqos/virtio-gpio.c b/tests/qtest/libqos/virtio-gpio.c
> > new file mode 100644
> > index 0000000000..62c8074777
> > --- /dev/null
> > +++ b/tests/qtest/libqos/virtio-gpio.c
> > @@ -0,0 +1,98 @@
> > +/*
> > + * virtio-gpio nodes for testing
> > + *
> > + * Copyright (c) 2022 Linaro Ltd
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "libqtest.h"
> > +#include "qemu/module.h"
> > +#include "qgraph.h"
> > +#include "virtio-gpio.h"
> > +
> > +static void *qvirtio_gpio_get_driver(QVhostUserGPIO *v_gpio,
> > +                                       const char *interface)
> > +{
> > +    if (!g_strcmp0(interface, "vhost-user-gpio")) {
> > +        return v_gpio;
> > +    }
> > +    if (!g_strcmp0(interface, "virtio")) {
> > +        return v_gpio->vdev;
> > +    }
> > +
> > +    fprintf(stderr, "%s not present in virtio-gpio-device\n", interface);
> > +    g_assert_not_reached();
> > +}
> > +
> > +static void *qvirtio_gpio_device_get_driver(void *object,
> > +                                              const char *interface)
> > +{
> > +    QVhostUserGPIODevice *v_gpio = object;
> > +    return qvirtio_gpio_get_driver(&v_gpio->gpio, interface);
> > +}
> > +
> > +static void *virtio_gpio_device_create(void *virtio_dev,
> > +                                         QGuestAllocator *t_alloc,
> > +                                         void *addr)
> > +{
> > +    QVhostUserGPIODevice *virtio_device = g_new0(QVhostUserGPIODevice, 1);
> > +    QVhostUserGPIO *interface = &virtio_device->gpio;
> > +
> > +    interface->vdev = virtio_dev;
> > +
> > +    virtio_device->obj.get_driver = qvirtio_gpio_device_get_driver;
> > +
> > +    return &virtio_device->obj;
> > +}
> > +
> > +/* virtio-gpio-pci */
> > +static void *qvirtio_gpio_pci_get_driver(void *object, const char *interface)
> > +{
> > +    QVhostUserGPIOPCI *v_gpio = object;
> > +    if (!g_strcmp0(interface, "pci-device")) {
> > +        return v_gpio->pci_vdev.pdev;
> > +    }
> > +    return qvirtio_gpio_get_driver(&v_gpio->gpio, interface);
> > +}
> > +
> > +static void *virtio_gpio_pci_create(void *pci_bus, QGuestAllocator *t_alloc,
> > +                                      void *addr)
> > +{
> > +    QVhostUserGPIOPCI *virtio_spci = g_new0(QVhostUserGPIOPCI, 1);
> > +    QVhostUserGPIO *interface = &virtio_spci->gpio;
> > +    QOSGraphObject *obj = &virtio_spci->pci_vdev.obj;
> > +
> > +    virtio_pci_init(&virtio_spci->pci_vdev, pci_bus, addr);
> > +    interface->vdev = &virtio_spci->pci_vdev.vdev;
> > +
> > +    obj->get_driver = qvirtio_gpio_pci_get_driver;
> > +
> > +    return obj;
> > +}
> > +
> > +static void virtio_gpio_register_nodes(void)
> > +{
> > +    QPCIAddress addr = {
> > +        .devfn = QPCI_DEVFN(4, 0),
> > +    };
> > +
> > +    QOSGraphEdgeOptions edge_opts = { };
> > +
> > +    /* vhost-user-gpio-device */
> > +    edge_opts.extra_device_opts = "id=gpio0,chardev=vhgpio0";
> > +    qos_node_create_driver("vhost-user-gpio-device",
> > +                            virtio_gpio_device_create);
> > +    qos_node_consumes("vhost-user-gpio-device", "virtio-bus", &edge_opts);
> > +    qos_node_produces("vhost-user-gpio-device", "vhost-user-gpio");
> > +
> > +    /* virtio-gpio-pci */
> > +    edge_opts.extra_device_opts = "id=gpio0,addr=04.0,chardev=vhgpio0";
> > +    add_qpci_address(&edge_opts, &addr);
> > +    qos_node_create_driver("vhost-user-gpio-pci", virtio_gpio_pci_create);
> > +    qos_node_consumes("vhost-user-gpio-pci", "pci-bus", &edge_opts);
> > +    qos_node_produces("vhost-user-gpio-pci", "vhost-user-gpio");
> > +}
> > +
> > +libqos_init(virtio_gpio_register_nodes);
> > diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
> > index ee30f54796..4f8789424b 100644
> > --- a/tests/qtest/vhost-user-test.c
> > +++ b/tests/qtest/vhost-user-test.c
> > @@ -43,6 +43,7 @@
> >                          " -numa node,memdev=mem"
> >  #define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s%s"
> >  #define QEMU_CMD_NETDEV " -netdev vhost-user,id=hs0,chardev=%s,vhostforce=on"
> > +#define QEMU_CMD_GPIO   " -device vhost-user-gpio-pci,id=gpio0,chardev=%s,vhostforce=on"
> >  
> >  #define HUGETLBFS_MAGIC       0x958458f6
> >  
> > @@ -137,6 +138,7 @@ enum {
> >  
> >  enum {
> >      VHOST_USER_NET,
> > +    VHOST_USER_GPIO,
> >  };
> >  
> >  typedef struct TestServer {
> > @@ -194,6 +196,14 @@ static void append_vhost_net_opts(TestServer *s, GString *cmd_line,
> >                             chr_opts, s->chr_name);
> >  }
> >  
> > +static void append_vhost_gpio_opts(TestServer *s, GString *cmd_line,
> > +                             const char *chr_opts)
> > +{
> > +    g_string_append_printf(cmd_line, QEMU_CMD_CHR QEMU_CMD_GPIO,
> > +                           s->chr_name, s->socket_path,
> > +                           chr_opts, s->chr_name);
> > +}
> > +
> >  static void append_mem_opts(TestServer *server, GString *cmd_line,
> >                              int size, enum test_memfd memfd)
> >  {
> > @@ -1014,3 +1024,27 @@ static void register_vhost_user_test(void)
> >                   test_multiqueue, &opts);
> >  }
> >  libqos_init(register_vhost_user_test);
> > +
> > +static struct vhost_user_ops g_vu_gpio_ops = {
> > +    .type = VHOST_USER_GPIO,
> > +
> > +    .append_opts = append_vhost_gpio_opts,
> > +
> > +    .set_features = vu_net_set_features,
> > +    .get_protocol_features = vu_net_get_protocol_features,
> > +};
> > +
> > +static void register_vhost_gpio_test(void)
> > +{
> > +    QOSGraphTestOptions opts = {
> > +        .before = vhost_user_test_setup,
> > +        .subprocess = true,
> > +        .arg = &g_vu_gpio_ops,
> > +    };
> > +
> > +    qemu_add_opts(&qemu_chardev_opts);
> > +
> > +    qos_add_test("read-guest-mem/memfile",
> > +                 "vhost-user-gpio", test_read_guest_mem, &opts);
> > +}
> > +libqos_init(register_vhost_gpio_test);
> > diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build
> > index 8c8ee15553..f581330d8b 100644
> > --- a/tests/qtest/libqos/meson.build
> > +++ b/tests/qtest/libqos/meson.build
> > @@ -42,6 +42,7 @@ libqos_srcs = files('../libqtest.c',
> >          'virtio-scsi.c',
> >          'virtio-serial.c',
> >          'virtio-iommu.c',
> > +        'virtio-gpio.c',
> >          'generic-pcihost.c',
> >  
> >          # qgraph machines:
> 
> 
> -- 
> Alex Bennée
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Re: [RFC PATCH] tests/qtest: attempt to enable tests for virtio-gpio (!working)
Posted by Alex Bennée 2 years ago
"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Alex Bennée (alex.bennee@linaro.org) wrote:
>> 
>> (expanding the CC list for help, anyone have a better idea about how
>> vhost-user qtests should work/see obvious issues with this patch?)
>
> How exactly does it fail?

➜  env QTEST_QEMU_BINARY=./qemu-system-aarch64 QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon G_TEST_DBUS_DAEMON=/home/alex/lsrc/qemu.git/tests/dbus-v
mstate-daemon.sh QTEST_QEMU_IMG=./qemu-img MALLOC_PERTURB_=137 ./tests/qtest/qos-test -p /aarch64/virt/generic-pcihost/pci-bus-generic/pci-bus/vhost-user-gpio-pci/vhost-user-gpio/vhost-user-gpio-tests/read-guest-mem/memfile
# random seed: R02S5d7667675b4f6dd3b8559f8db621296c
# starting QEMU: exec ./qemu-system-aarch64 -qtest unix:/tmp/qtest-1245871.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-1245871.qmp,id=char0 -mon chardev=char0,mode=control -display none -machine none -accel qtest
# Start of aarch64 tests
# Start of virt tests
# Start of generic-pcihost tests
# Start of pci-bus-generic tests
# Start of pci-bus tests
# Start of vhost-user-gpio-pci tests
# Start of vhost-user-gpio tests
# Start of vhost-user-gpio-tests tests
# Start of read-guest-mem tests
# child process (/aarch64/virt/generic-pcihost/pci-bus-generic/pci-bus/vhost-user-gpio-pci/vhost-user-gpio/vhost-user-gpio-tests/read-guest-mem/memfile/subprocess [1245877])
 exit status: 1 (error)
# child process (/aarch64/virt/generic-pcihost/pci-bus-generic/pci-bus/vhost-user-gpio-pci/vhost-user-gpio/vhost-user-gpio-tests/read-guest-mem/memfile/subprocess [1245877]) stdout: ""
# child process (/aarch64/virt/generic-pcihost/pci-bus-generic/pci-bus/vhost-user-gpio-pci/vhost-user-gpio/vhost-user-gpio-tests/read-guest-mem/memfile/subprocess [1245877]) stderr: "qemu-system-aarch64: -device vhost-user-gpio-pci,id=gpio0,chardev=chr-vhost-user-test,vhostforce=on: Duplicate ID 'gpio0' for device\nsocket_accept failed: Resource temporarily unavailable\n**\nERROR:../../tests/qtest/libqtest.c:321:qtest_init_without_qmp_handshake: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0)\n"
**
ERROR:../../tests/qtest/qos-test.c:189:subprocess_run_one_test: child process (/aarch64/virt/generic-pcihost/pci-bus-generic/pci-bus/vhost-user-gpio-pci/vhost-user-gpio/vhost-user-gpio-tests/read-guest-mem/memfile/subprocess [1245877]) failed unexpectedly
Bail out! ERROR:../../tests/qtest/qos-test.c:189:subprocess_run_one_test: child process (/aarch64/virt/generic-pcihost/pci-bus-generic/pci-bus/vhost-user-gpio-pci/vhost-user-gpio/vhost-user-gpio-tests/read-guest-mem/memfile/subprocess [1245877]) failed unexpectedly
fish: “env QTEST_QEMU_BINARY=./qemu-sy…” terminated by signal SIGABRT
(Abort)

Although it would be nice if I could individually run qos-tests with all
the make machinery setting things up.


>
> DAve
>
>> Alex Bennée <alex.bennee@linaro.org> writes:
>> 
>> > We don't have a virtio-gpio implementation in QEMU and only
>> > support a vhost-user backend. The QEMU side of the code is minimal so
>> > it should be enough to instantiate the device and pass some vhost-user
>> > messages over the control socket. To do this we hook into the existing
>> > vhost-user-test code and just add the bits required for gpio.
>> >
>> > Based-on: 20220118203833.316741-1-eric.auger@redhat.com
>> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> > Cc: Viresh Kumar <viresh.kumar@linaro.org>
>> > Cc: Paolo Bonzini <pbonzini@redhat.com>
>> >
>> > ---
>> >
>> > This goes as far as to add things to the QOS tree but so far it's
>> > failing to properly start QEMU with the chardev socket needed to
>> > communicate between the mock vhost-user daemon and QEMU itself.
>> > ---
>> >  tests/qtest/libqos/virtio-gpio.h | 34 +++++++++++
>> >  tests/qtest/libqos/virtio-gpio.c | 98 ++++++++++++++++++++++++++++++++
>> >  tests/qtest/vhost-user-test.c    | 34 +++++++++++
>> >  tests/qtest/libqos/meson.build   |  1 +
>> >  4 files changed, 167 insertions(+)
>> >  create mode 100644 tests/qtest/libqos/virtio-gpio.h
>> >  create mode 100644 tests/qtest/libqos/virtio-gpio.c
>> >
>> > diff --git a/tests/qtest/libqos/virtio-gpio.h b/tests/qtest/libqos/virtio-gpio.h
>> > new file mode 100644
>> > index 0000000000..abe6967ae9
>> > --- /dev/null
>> > +++ b/tests/qtest/libqos/virtio-gpio.h
>> > @@ -0,0 +1,34 @@
>> > +/*
>> > + * virtio-gpio structures
>> > + *
>> > + * Copyright (c) 2022 Linaro Ltd
>> > + *
>> > + * SPDX-License-Identifier: GPL-2.0-or-later
>> > + */
>> > +
>> > +#ifndef TESTS_LIBQOS_VIRTIO_GPIO_H
>> > +#define TESTS_LIBQOS_VIRTIO_GPIO_H
>> > +
>> > +#include "qgraph.h"
>> > +#include "virtio.h"
>> > +#include "virtio-pci.h"
>> > +
>> > +typedef struct QVhostUserGPIO QVhostUserGPIO;
>> > +typedef struct QVhostUserGPIOPCI QVhostUserGPIOPCI;
>> > +typedef struct QVhostUserGPIODevice QVhostUserGPIODevice;
>> > +
>> > +struct QVhostUserGPIO {
>> > +    QVirtioDevice *vdev;
>> > +};
>> > +
>> > +struct QVhostUserGPIOPCI {
>> > +    QVirtioPCIDevice pci_vdev;
>> > +    QVhostUserGPIO gpio;
>> > +};
>> > +
>> > +struct QVhostUserGPIODevice {
>> > +    QOSGraphObject obj;
>> > +    QVhostUserGPIO gpio;
>> > +};
>> > +
>> > +#endif
>> > diff --git a/tests/qtest/libqos/virtio-gpio.c b/tests/qtest/libqos/virtio-gpio.c
>> > new file mode 100644
>> > index 0000000000..62c8074777
>> > --- /dev/null
>> > +++ b/tests/qtest/libqos/virtio-gpio.c
>> > @@ -0,0 +1,98 @@
>> > +/*
>> > + * virtio-gpio nodes for testing
>> > + *
>> > + * Copyright (c) 2022 Linaro Ltd
>> > + *
>> > + * SPDX-License-Identifier: GPL-2.0-or-later
>> > + */
>> > +
>> > +#include "qemu/osdep.h"
>> > +#include "libqtest.h"
>> > +#include "qemu/module.h"
>> > +#include "qgraph.h"
>> > +#include "virtio-gpio.h"
>> > +
>> > +static void *qvirtio_gpio_get_driver(QVhostUserGPIO *v_gpio,
>> > +                                       const char *interface)
>> > +{
>> > +    if (!g_strcmp0(interface, "vhost-user-gpio")) {
>> > +        return v_gpio;
>> > +    }
>> > +    if (!g_strcmp0(interface, "virtio")) {
>> > +        return v_gpio->vdev;
>> > +    }
>> > +
>> > +    fprintf(stderr, "%s not present in virtio-gpio-device\n", interface);
>> > +    g_assert_not_reached();
>> > +}
>> > +
>> > +static void *qvirtio_gpio_device_get_driver(void *object,
>> > +                                              const char *interface)
>> > +{
>> > +    QVhostUserGPIODevice *v_gpio = object;
>> > +    return qvirtio_gpio_get_driver(&v_gpio->gpio, interface);
>> > +}
>> > +
>> > +static void *virtio_gpio_device_create(void *virtio_dev,
>> > +                                         QGuestAllocator *t_alloc,
>> > +                                         void *addr)
>> > +{
>> > +    QVhostUserGPIODevice *virtio_device = g_new0(QVhostUserGPIODevice, 1);
>> > +    QVhostUserGPIO *interface = &virtio_device->gpio;
>> > +
>> > +    interface->vdev = virtio_dev;
>> > +
>> > +    virtio_device->obj.get_driver = qvirtio_gpio_device_get_driver;
>> > +
>> > +    return &virtio_device->obj;
>> > +}
>> > +
>> > +/* virtio-gpio-pci */
>> > +static void *qvirtio_gpio_pci_get_driver(void *object, const char *interface)
>> > +{
>> > +    QVhostUserGPIOPCI *v_gpio = object;
>> > +    if (!g_strcmp0(interface, "pci-device")) {
>> > +        return v_gpio->pci_vdev.pdev;
>> > +    }
>> > +    return qvirtio_gpio_get_driver(&v_gpio->gpio, interface);
>> > +}
>> > +
>> > +static void *virtio_gpio_pci_create(void *pci_bus, QGuestAllocator *t_alloc,
>> > +                                      void *addr)
>> > +{
>> > +    QVhostUserGPIOPCI *virtio_spci = g_new0(QVhostUserGPIOPCI, 1);
>> > +    QVhostUserGPIO *interface = &virtio_spci->gpio;
>> > +    QOSGraphObject *obj = &virtio_spci->pci_vdev.obj;
>> > +
>> > +    virtio_pci_init(&virtio_spci->pci_vdev, pci_bus, addr);
>> > +    interface->vdev = &virtio_spci->pci_vdev.vdev;
>> > +
>> > +    obj->get_driver = qvirtio_gpio_pci_get_driver;
>> > +
>> > +    return obj;
>> > +}
>> > +
>> > +static void virtio_gpio_register_nodes(void)
>> > +{
>> > +    QPCIAddress addr = {
>> > +        .devfn = QPCI_DEVFN(4, 0),
>> > +    };
>> > +
>> > +    QOSGraphEdgeOptions edge_opts = { };
>> > +
>> > +    /* vhost-user-gpio-device */
>> > +    edge_opts.extra_device_opts = "id=gpio0,chardev=vhgpio0";
>> > +    qos_node_create_driver("vhost-user-gpio-device",
>> > +                            virtio_gpio_device_create);
>> > +    qos_node_consumes("vhost-user-gpio-device", "virtio-bus", &edge_opts);
>> > +    qos_node_produces("vhost-user-gpio-device", "vhost-user-gpio");
>> > +
>> > +    /* virtio-gpio-pci */
>> > +    edge_opts.extra_device_opts = "id=gpio0,addr=04.0,chardev=vhgpio0";
>> > +    add_qpci_address(&edge_opts, &addr);
>> > +    qos_node_create_driver("vhost-user-gpio-pci", virtio_gpio_pci_create);
>> > +    qos_node_consumes("vhost-user-gpio-pci", "pci-bus", &edge_opts);
>> > +    qos_node_produces("vhost-user-gpio-pci", "vhost-user-gpio");
>> > +}
>> > +
>> > +libqos_init(virtio_gpio_register_nodes);
>> > diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
>> > index ee30f54796..4f8789424b 100644
>> > --- a/tests/qtest/vhost-user-test.c
>> > +++ b/tests/qtest/vhost-user-test.c
>> > @@ -43,6 +43,7 @@
>> >                          " -numa node,memdev=mem"
>> >  #define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s%s"
>> >  #define QEMU_CMD_NETDEV " -netdev vhost-user,id=hs0,chardev=%s,vhostforce=on"
>> > +#define QEMU_CMD_GPIO   " -device vhost-user-gpio-pci,id=gpio0,chardev=%s,vhostforce=on"
>> >  
>> >  #define HUGETLBFS_MAGIC       0x958458f6
>> >  
>> > @@ -137,6 +138,7 @@ enum {
>> >  
>> >  enum {
>> >      VHOST_USER_NET,
>> > +    VHOST_USER_GPIO,
>> >  };
>> >  
>> >  typedef struct TestServer {
>> > @@ -194,6 +196,14 @@ static void append_vhost_net_opts(TestServer *s, GString *cmd_line,
>> >                             chr_opts, s->chr_name);
>> >  }
>> >  
>> > +static void append_vhost_gpio_opts(TestServer *s, GString *cmd_line,
>> > +                             const char *chr_opts)
>> > +{
>> > +    g_string_append_printf(cmd_line, QEMU_CMD_CHR QEMU_CMD_GPIO,
>> > +                           s->chr_name, s->socket_path,
>> > +                           chr_opts, s->chr_name);
>> > +}
>> > +
>> >  static void append_mem_opts(TestServer *server, GString *cmd_line,
>> >                              int size, enum test_memfd memfd)
>> >  {
>> > @@ -1014,3 +1024,27 @@ static void register_vhost_user_test(void)
>> >                   test_multiqueue, &opts);
>> >  }
>> >  libqos_init(register_vhost_user_test);
>> > +
>> > +static struct vhost_user_ops g_vu_gpio_ops = {
>> > +    .type = VHOST_USER_GPIO,
>> > +
>> > +    .append_opts = append_vhost_gpio_opts,
>> > +
>> > +    .set_features = vu_net_set_features,
>> > +    .get_protocol_features = vu_net_get_protocol_features,
>> > +};
>> > +
>> > +static void register_vhost_gpio_test(void)
>> > +{
>> > +    QOSGraphTestOptions opts = {
>> > +        .before = vhost_user_test_setup,
>> > +        .subprocess = true,
>> > +        .arg = &g_vu_gpio_ops,
>> > +    };
>> > +
>> > +    qemu_add_opts(&qemu_chardev_opts);
>> > +
>> > +    qos_add_test("read-guest-mem/memfile",
>> > +                 "vhost-user-gpio", test_read_guest_mem, &opts);
>> > +}
>> > +libqos_init(register_vhost_gpio_test);
>> > diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build
>> > index 8c8ee15553..f581330d8b 100644
>> > --- a/tests/qtest/libqos/meson.build
>> > +++ b/tests/qtest/libqos/meson.build
>> > @@ -42,6 +42,7 @@ libqos_srcs = files('../libqtest.c',
>> >          'virtio-scsi.c',
>> >          'virtio-serial.c',
>> >          'virtio-iommu.c',
>> > +        'virtio-gpio.c',
>> >          'generic-pcihost.c',
>> >  
>> >          # qgraph machines:
>> 
>> 
>> -- 
>> Alex Bennée
>> 


-- 
Alex Bennée
Re: [PATCH 0/2] virtio: Add vhost-user-gpio device's support
Posted by Michael S. Tsirkin 2 years, 3 months ago
On Mon, Jan 17, 2022 at 10:11:21AM +0000, Alex Bennée wrote:
> 
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Wed, Jan 12, 2022 at 05:04:57PM +0530, Viresh Kumar wrote:
> >> Hello,
> >> 
> >> This patchset adds vhost-user-gpio device's support in Qemu. The support for the
> >> same has already been added to virtio specification and Linux Kernel.
> >> 
> >> A Rust based backend is also in progress and is tested against this patchset:
> >> 
> >> https://github.com/rust-vmm/vhost-device/pull/76
> >
> >
> > I'm reluctant to add this with no tests in tree.
> > Want to write a minimal libhost-user based backend?
> 
> This is going to be a problem going forward as we have more out-of-tree
> backends written as a first preference. While the first couple of vhost
> devices have C implementations in contrib before we worked on the rust
> version I think we are getting to the point of skipping a first C
> version for future devices.

Not sure what to do then. Integrate using submodules somehow?


> However I notice we have qtest/vhost-user-test.c so would that be enough
> to ensure we can instantiate the device and a basic vhost-user
> initialisation sequence doesn't cause it to crap out. This obviously
> won't be exercising the virtq processing itself but does that really
> exercise any of QEMU's boilerplate anyway?

we do have tests e.g. for network based on vhost-user, yes.


> > We also need some maintainers to step up.
> 
> You mean more reviewers for the vhost and virtio sections of QEMU's
> maintainers?
> 
> >
> >
> >> --
> >> Viresh
> >> 
> >> Viresh Kumar (2):
> >>   hw/virtio: add boilerplate for vhost-user-gpio device
> >>   hw/virtio: add vhost-user-gpio-pci boilerplate
> >> 
> >>  hw/virtio/Kconfig                   |   5 +
> >>  hw/virtio/meson.build               |   2 +
> >>  hw/virtio/vhost-user-gpio-pci.c     |  69 ++++++
> >>  hw/virtio/vhost-user-gpio.c         | 343 ++++++++++++++++++++++++++++
> >>  include/hw/virtio/vhost-user-gpio.h |  35 +++
> >>  5 files changed, 454 insertions(+)
> >>  create mode 100644 hw/virtio/vhost-user-gpio-pci.c
> >>  create mode 100644 hw/virtio/vhost-user-gpio.c
> >>  create mode 100644 include/hw/virtio/vhost-user-gpio.h
> >> 
> >> -- 
> >> 2.31.1.272.g89b43f80a514
> 
> 
> -- 
> Alex Bennée


Re: [PATCH 0/2] virtio: Add vhost-user-gpio device's support
Posted by Michael S. Tsirkin 2 years, 2 months ago
On Wed, Jan 12, 2022 at 05:04:57PM +0530, Viresh Kumar wrote:
> Hello,
> 
> This patchset adds vhost-user-gpio device's support in Qemu. The support for the
> same has already been added to virtio specification and Linux Kernel.
> 
> A Rust based backend is also in progress and is tested against this patchset:
> 
> https://github.com/rust-vmm/vhost-device/pull/76

So I'm waiting for v2 of this, right?

> --
> Viresh
> 
> Viresh Kumar (2):
>   hw/virtio: add boilerplate for vhost-user-gpio device
>   hw/virtio: add vhost-user-gpio-pci boilerplate
> 
>  hw/virtio/Kconfig                   |   5 +
>  hw/virtio/meson.build               |   2 +
>  hw/virtio/vhost-user-gpio-pci.c     |  69 ++++++
>  hw/virtio/vhost-user-gpio.c         | 343 ++++++++++++++++++++++++++++
>  include/hw/virtio/vhost-user-gpio.h |  35 +++
>  5 files changed, 454 insertions(+)
>  create mode 100644 hw/virtio/vhost-user-gpio-pci.c
>  create mode 100644 hw/virtio/vhost-user-gpio.c
>  create mode 100644 include/hw/virtio/vhost-user-gpio.h
> 
> -- 
> 2.31.1.272.g89b43f80a514


Re: [PATCH 0/2] virtio: Add vhost-user-gpio device's support
Posted by Alex Bennée 2 years, 2 months ago
"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Wed, Jan 12, 2022 at 05:04:57PM +0530, Viresh Kumar wrote:
>> Hello,
>> 
>> This patchset adds vhost-user-gpio device's support in Qemu. The support for the
>> same has already been added to virtio specification and Linux Kernel.
>> 
>> A Rust based backend is also in progress and is tested against this patchset:
>> 
>> https://github.com/rust-vmm/vhost-device/pull/76
>
> So I'm waiting for v2 of this, right?

If you can give any pointers for how to properly instantiate the stub in
qtest then we can spin v2 pretty quickly. I tried but it didn't work:

  Subject: [RFC PATCH] tests/qtest: attempt to enable tests for virtio-gpio (!working)
  Date: Fri, 21 Jan 2022 15:15:34 +0000
  Message-Id: <20220121151534.3654562-1-alex.bennee@linaro.org>
  X-Mailer: git-send-email 2.30.2
  In-Reply-To: <20220120040234.ehcjcdr6wrvlz3yc@vireshk-i7>
  References: <20220120040234.ehcjcdr6wrvlz3yc@vireshk-i7>

it's all a bit opaque as to why it didn't.

>
>> --
>> Viresh
>> 
>> Viresh Kumar (2):
>>   hw/virtio: add boilerplate for vhost-user-gpio device
>>   hw/virtio: add vhost-user-gpio-pci boilerplate
>> 
>>  hw/virtio/Kconfig                   |   5 +
>>  hw/virtio/meson.build               |   2 +
>>  hw/virtio/vhost-user-gpio-pci.c     |  69 ++++++
>>  hw/virtio/vhost-user-gpio.c         | 343 ++++++++++++++++++++++++++++
>>  include/hw/virtio/vhost-user-gpio.h |  35 +++
>>  5 files changed, 454 insertions(+)
>>  create mode 100644 hw/virtio/vhost-user-gpio-pci.c
>>  create mode 100644 hw/virtio/vhost-user-gpio.c
>>  create mode 100644 include/hw/virtio/vhost-user-gpio.h
>> 
>> -- 
>> 2.31.1.272.g89b43f80a514


-- 
Alex Bennée

Re: [PATCH 0/2] virtio: Add vhost-user-gpio device's support
Posted by Michael S. Tsirkin 2 years, 2 months ago
On Fri, Feb 04, 2022 at 01:52:47PM +0000, Alex Bennée wrote:
> 
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Wed, Jan 12, 2022 at 05:04:57PM +0530, Viresh Kumar wrote:
> >> Hello,
> >> 
> >> This patchset adds vhost-user-gpio device's support in Qemu. The support for the
> >> same has already been added to virtio specification and Linux Kernel.
> >> 
> >> A Rust based backend is also in progress and is tested against this patchset:
> >> 
> >> https://github.com/rust-vmm/vhost-device/pull/76
> >
> > So I'm waiting for v2 of this, right?
> 
> If you can give any pointers for how to properly instantiate the stub in
> qtest then we can spin v2 pretty quickly. I tried but it didn't work:
> 
>   Subject: [RFC PATCH] tests/qtest: attempt to enable tests for virtio-gpio (!working)
>   Date: Fri, 21 Jan 2022 15:15:34 +0000
>   Message-Id: <20220121151534.3654562-1-alex.bennee@linaro.org>
>   X-Mailer: git-send-email 2.30.2
>   In-Reply-To: <20220120040234.ehcjcdr6wrvlz3yc@vireshk-i7>
>   References: <20220120040234.ehcjcdr6wrvlz3yc@vireshk-i7>
> 
> it's all a bit opaque as to why it didn't.

A bit rushed short term, it this isn't resolved soon I'll try to help.
Maybe try CC other vhost-user maintainers (for blk,gpu etc) meanwhile.

> >
> >> --
> >> Viresh
> >> 
> >> Viresh Kumar (2):
> >>   hw/virtio: add boilerplate for vhost-user-gpio device
> >>   hw/virtio: add vhost-user-gpio-pci boilerplate
> >> 
> >>  hw/virtio/Kconfig                   |   5 +
> >>  hw/virtio/meson.build               |   2 +
> >>  hw/virtio/vhost-user-gpio-pci.c     |  69 ++++++
> >>  hw/virtio/vhost-user-gpio.c         | 343 ++++++++++++++++++++++++++++
> >>  include/hw/virtio/vhost-user-gpio.h |  35 +++
> >>  5 files changed, 454 insertions(+)
> >>  create mode 100644 hw/virtio/vhost-user-gpio-pci.c
> >>  create mode 100644 hw/virtio/vhost-user-gpio.c
> >>  create mode 100644 include/hw/virtio/vhost-user-gpio.h
> >> 
> >> -- 
> >> 2.31.1.272.g89b43f80a514
> 
> 
> -- 
> Alex Bennée


Re: [PATCH 0/2] virtio: Add vhost-user-gpio device's support
Posted by Alex Bennée 2 years ago
"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Fri, Feb 04, 2022 at 01:52:47PM +0000, Alex Bennée wrote:
>> 
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Wed, Jan 12, 2022 at 05:04:57PM +0530, Viresh Kumar wrote:
>> >> Hello,
>> >> 
>> >> This patchset adds vhost-user-gpio device's support in Qemu. The support for the
>> >> same has already been added to virtio specification and Linux Kernel.
>> >> 
>> >> A Rust based backend is also in progress and is tested against this patchset:
>> >> 
>> >> https://github.com/rust-vmm/vhost-device/pull/76
>> >
>> > So I'm waiting for v2 of this, right?
>> 
>> If you can give any pointers for how to properly instantiate the stub in
>> qtest then we can spin v2 pretty quickly. I tried but it didn't work:
>> 
>>   Subject: [RFC PATCH] tests/qtest: attempt to enable tests for virtio-gpio (!working)
>>   Date: Fri, 21 Jan 2022 15:15:34 +0000
>>   Message-Id: <20220121151534.3654562-1-alex.bennee@linaro.org>
>>   X-Mailer: git-send-email 2.30.2
>>   In-Reply-To: <20220120040234.ehcjcdr6wrvlz3yc@vireshk-i7>
>>   References: <20220120040234.ehcjcdr6wrvlz3yc@vireshk-i7>
>> 
>> it's all a bit opaque as to why it didn't.
>
> A bit rushed short term, it this isn't resolved soon I'll try to help.
> Maybe try CC other vhost-user maintainers (for blk,gpu etc) meanwhile.

I have expanded the CC list for this thread and the RFC patch to see if
anyone has pointers. I'll pull this series into my vhost cleanups series
for post 7.0.

-- 
Alex Bennée
Re: [PATCH 0/2] virtio: Add vhost-user-gpio device's support
Posted by Alex Bennée 2 years ago
Viresh Kumar <viresh.kumar@linaro.org> writes:

> Hello,
>
> This patchset adds vhost-user-gpio device's support in Qemu. The support for the
> same has already been added to virtio specification and Linux Kernel.
>
> A Rust based backend is also in progress and is tested against this patchset:
>
> https://github.com/rust-vmm/vhost-device/pull/76

Queued to virtio/virtio-cleanups-and-gpio-v2, thanks.

-- 
Alex Bennée