mbox series

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

Message ID cover.1641987128.git.viresh.kumar@linaro.org
Headers show
Series virtio: Add vhost-user-gpio device's support | expand

Message

Viresh Kumar Jan. 12, 2022, 11:34 a.m. UTC
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

Comments

Michael S. Tsirkin Jan. 17, 2022, 8:47 a.m. UTC | #1
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
Alex Bennée Jan. 17, 2022, 10:11 a.m. UTC | #2
"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
Viresh Kumar Jan. 20, 2022, 4:02 a.m. UTC | #3
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.
Michael S. Tsirkin Jan. 20, 2022, 10:57 a.m. UTC | #4
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
Michael S. Tsirkin Jan. 20, 2022, 11:25 a.m. UTC | #5
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
Alex Bennée Jan. 20, 2022, 3:41 p.m. UTC | #6
"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.
Michael S. Tsirkin Feb. 4, 2022, 12:23 p.m. UTC | #7
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
Alex Bennée Feb. 4, 2022, 1:52 p.m. UTC | #8
"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
Michael S. Tsirkin Feb. 4, 2022, 2:06 p.m. UTC | #9
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
Alex Bennée March 31, 2022, 9:58 a.m. UTC | #10
"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 April 5, 2022, 10:55 a.m. UTC | #11
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.