mbox series

[v1,00/13] various virtio docs, fixes and tweaks

Message ID 20220321153037.3622127-1-alex.bennee@linaro.org
Headers show
Series various virtio docs, fixes and tweaks | expand

Message

Alex Bennée March 21, 2022, 3:30 p.m. UTC
Hi,

This series is a sub-set of patches while I was trying to re-rev my
virtio-rpmb patches. It attempts to address a few things:

  - improve documentation for virtio/vhost/vhost-user
  - document some of the API
  - a hacky fix for F_CONFIG handling
  - putting VhostUserState on a diet, make VhostUserHostNotifier dynamic

In particular I've been trying to better understand how vhost-user
interactions are meant to work and why there are two different methods
for instantiating them. If my supposition is correct perhaps a number
of devices that don't have in-kernel vhost equivalents could be converted?

While working onthe VhostUserHostNotifier changes I found it quite
hard to trigger the code. Is this rarely used code or just requires
backends we don't see in the testing?

Alex Bennée (10):
  hw/virtio: move virtio-pci.h into shared include space
  virtio-pci: add notification trace points
  hw/virtio: add vhost_user_[read|write] trace points
  vhost-user.rst: add clarifying language about protocol negotiation
  libvhost-user: expose vu_request_to_string
  docs/devel: start documenting writing VirtIO devices
  include/hw: start documenting the vhost API
  contrib/vhost-user-blk: fix 32 bit build and enable
  hw/virtio/vhost-user: don't suppress F_CONFIG when supported
  virtio/vhost-user: dynamically assign VhostUserHostNotifiers

Paolo Bonzini (3):
  docs: vhost-user: clean up request/reply description
  docs: vhost-user: rewrite section on ring state machine
  docs: vhost-user: replace master/slave with front-end/back-end

 docs/devel/index-internals.rst            |   1 +
 docs/devel/virtio-backends.rst            | 214 +++++++++
 docs/interop/vhost-user-gpu.rst           |  10 +-
 docs/interop/vhost-user.rst               | 555 ++++++++++++----------
 meson.build                               |   2 +-
 include/hw/virtio/vhost-user.h            |  43 +-
 include/hw/virtio/vhost.h                 | 132 ++++-
 {hw => include/hw}/virtio/virtio-pci.h    |   0
 subprojects/libvhost-user/libvhost-user.h |   9 +
 contrib/vhost-user-blk/vhost-user-blk.c   |   6 +-
 hw/scsi/vhost-user-scsi.c                 |   1 +
 hw/virtio/vhost-scsi-pci.c                |   2 +-
 hw/virtio/vhost-user-blk-pci.c            |   2 +-
 hw/virtio/vhost-user-fs-pci.c             |   2 +-
 hw/virtio/vhost-user-i2c-pci.c            |   2 +-
 hw/virtio/vhost-user-input-pci.c          |   2 +-
 hw/virtio/vhost-user-rng-pci.c            |   2 +-
 hw/virtio/vhost-user-scsi-pci.c           |   2 +-
 hw/virtio/vhost-user-vsock-pci.c          |   2 +-
 hw/virtio/vhost-user.c                    | 133 ++++--
 hw/virtio/vhost-vsock-pci.c               |   2 +-
 hw/virtio/virtio-9p-pci.c                 |   2 +-
 hw/virtio/virtio-balloon-pci.c            |   2 +-
 hw/virtio/virtio-blk-pci.c                |   2 +-
 hw/virtio/virtio-input-host-pci.c         |   2 +-
 hw/virtio/virtio-input-pci.c              |   2 +-
 hw/virtio/virtio-iommu-pci.c              |   2 +-
 hw/virtio/virtio-net-pci.c                |   2 +-
 hw/virtio/virtio-pci.c                    |   5 +-
 hw/virtio/virtio-rng-pci.c                |   2 +-
 hw/virtio/virtio-scsi-pci.c               |   2 +-
 hw/virtio/virtio-serial-pci.c             |   2 +-
 subprojects/libvhost-user/libvhost-user.c |   2 +-
 contrib/vhost-user-blk/meson.build        |   3 +-
 hw/virtio/trace-events                    |  10 +-
 35 files changed, 831 insertions(+), 333 deletions(-)
 create mode 100644 docs/devel/virtio-backends.rst
 rename {hw => include/hw}/virtio/virtio-pci.h (100%)

Comments

Michael S. Tsirkin March 22, 2022, 1:56 p.m. UTC | #1
On Mon, Mar 21, 2022 at 03:30:24PM +0000, Alex Bennée wrote:
> Hi,
> 
> This series is a sub-set of patches while I was trying to re-rev my
> virtio-rpmb patches. It attempts to address a few things:
> 
>   - improve documentation for virtio/vhost/vhost-user
>   - document some of the API
>   - a hacky fix for F_CONFIG handling
>   - putting VhostUserState on a diet, make VhostUserHostNotifier dynamic

So I think this is best deferred until after the release,
more of a cleanup than a bugfix.

I will tag this series, but please do remind me after the release
to help make sure it does not get lost.


> In particular I've been trying to better understand how vhost-user
> interactions are meant to work and why there are two different methods
> for instantiating them. If my supposition is correct perhaps a number
> of devices that don't have in-kernel vhost equivalents could be converted?

Hope I understand your question.  Well we started off with saying
vhost-user is just a backend, so should not affect the frontend device.
This is clean and makes migration work e.g. you can migrate between
different backends, but it makes adding features more work.


> While working onthe VhostUserHostNotifier changes I found it quite
> hard to trigger the code. Is this rarely used code or just requires
> backends we don't see in the testing?

Which function are you asking about exactly?

> Alex Bennée (10):
>   hw/virtio: move virtio-pci.h into shared include space
>   virtio-pci: add notification trace points
>   hw/virtio: add vhost_user_[read|write] trace points
>   vhost-user.rst: add clarifying language about protocol negotiation
>   libvhost-user: expose vu_request_to_string
>   docs/devel: start documenting writing VirtIO devices
>   include/hw: start documenting the vhost API
>   contrib/vhost-user-blk: fix 32 bit build and enable
>   hw/virtio/vhost-user: don't suppress F_CONFIG when supported
>   virtio/vhost-user: dynamically assign VhostUserHostNotifiers
> 
> Paolo Bonzini (3):
>   docs: vhost-user: clean up request/reply description
>   docs: vhost-user: rewrite section on ring state machine
>   docs: vhost-user: replace master/slave with front-end/back-end
> 
>  docs/devel/index-internals.rst            |   1 +
>  docs/devel/virtio-backends.rst            | 214 +++++++++
>  docs/interop/vhost-user-gpu.rst           |  10 +-
>  docs/interop/vhost-user.rst               | 555 ++++++++++++----------
>  meson.build                               |   2 +-
>  include/hw/virtio/vhost-user.h            |  43 +-
>  include/hw/virtio/vhost.h                 | 132 ++++-
>  {hw => include/hw}/virtio/virtio-pci.h    |   0
>  subprojects/libvhost-user/libvhost-user.h |   9 +
>  contrib/vhost-user-blk/vhost-user-blk.c   |   6 +-
>  hw/scsi/vhost-user-scsi.c                 |   1 +
>  hw/virtio/vhost-scsi-pci.c                |   2 +-
>  hw/virtio/vhost-user-blk-pci.c            |   2 +-
>  hw/virtio/vhost-user-fs-pci.c             |   2 +-
>  hw/virtio/vhost-user-i2c-pci.c            |   2 +-
>  hw/virtio/vhost-user-input-pci.c          |   2 +-
>  hw/virtio/vhost-user-rng-pci.c            |   2 +-
>  hw/virtio/vhost-user-scsi-pci.c           |   2 +-
>  hw/virtio/vhost-user-vsock-pci.c          |   2 +-
>  hw/virtio/vhost-user.c                    | 133 ++++--
>  hw/virtio/vhost-vsock-pci.c               |   2 +-
>  hw/virtio/virtio-9p-pci.c                 |   2 +-
>  hw/virtio/virtio-balloon-pci.c            |   2 +-
>  hw/virtio/virtio-blk-pci.c                |   2 +-
>  hw/virtio/virtio-input-host-pci.c         |   2 +-
>  hw/virtio/virtio-input-pci.c              |   2 +-
>  hw/virtio/virtio-iommu-pci.c              |   2 +-
>  hw/virtio/virtio-net-pci.c                |   2 +-
>  hw/virtio/virtio-pci.c                    |   5 +-
>  hw/virtio/virtio-rng-pci.c                |   2 +-
>  hw/virtio/virtio-scsi-pci.c               |   2 +-
>  hw/virtio/virtio-serial-pci.c             |   2 +-
>  subprojects/libvhost-user/libvhost-user.c |   2 +-
>  contrib/vhost-user-blk/meson.build        |   3 +-
>  hw/virtio/trace-events                    |  10 +-
>  35 files changed, 831 insertions(+), 333 deletions(-)
>  create mode 100644 docs/devel/virtio-backends.rst
>  rename {hw => include/hw}/virtio/virtio-pci.h (100%)
> 
> -- 
> 2.30.2
Alex Bennée March 22, 2022, 3:50 p.m. UTC | #2
"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Mon, Mar 21, 2022 at 03:30:24PM +0000, Alex Bennée wrote:
>> Hi,
>> 
>> This series is a sub-set of patches while I was trying to re-rev my
>> virtio-rpmb patches. It attempts to address a few things:
>> 
>>   - improve documentation for virtio/vhost/vhost-user
>>   - document some of the API
>>   - a hacky fix for F_CONFIG handling
>>   - putting VhostUserState on a diet, make VhostUserHostNotifier dynamic
>
> So I think this is best deferred until after the release,
> more of a cleanup than a bugfix.

Sorry I should have made it clearer - I wasn't intending this for 7.0
but I also didn't want it bound up with the rpmb changes which will take
longer to land.

>
> I will tag this series, but please do remind me after the release
> to help make sure it does not get lost.
>
>
>> In particular I've been trying to better understand how vhost-user
>> interactions are meant to work and why there are two different methods
>> for instantiating them. If my supposition is correct perhaps a number
>> of devices that don't have in-kernel vhost equivalents could be converted?
>
> Hope I understand your question.  Well we started off with saying
> vhost-user is just a backend, so should not affect the frontend device.
> This is clean and makes migration work e.g. you can migrate between
> different backends, but it makes adding features more work.

This is covered in the doc patch, specifically:

  vhost_ops vs TYPE_VHOST_USER_BACKEND

>> While working onthe VhostUserHostNotifier changes I found it quite
>> hard to trigger the code. Is this rarely used code or just requires
>> backends we don't see in the testing?
>
> Which function are you asking about exactly?

  vhost_user_slave_handle_vring_host_notifier

which is the only place where a mapping is set up AFAICT.

>
>> Alex Bennée (10):
>>   hw/virtio: move virtio-pci.h into shared include space
>>   virtio-pci: add notification trace points
>>   hw/virtio: add vhost_user_[read|write] trace points
>>   vhost-user.rst: add clarifying language about protocol negotiation
>>   libvhost-user: expose vu_request_to_string
>>   docs/devel: start documenting writing VirtIO devices
>>   include/hw: start documenting the vhost API
>>   contrib/vhost-user-blk: fix 32 bit build and enable
>>   hw/virtio/vhost-user: don't suppress F_CONFIG when supported
>>   virtio/vhost-user: dynamically assign VhostUserHostNotifiers
>> 
>> Paolo Bonzini (3):
>>   docs: vhost-user: clean up request/reply description
>>   docs: vhost-user: rewrite section on ring state machine
>>   docs: vhost-user: replace master/slave with front-end/back-end
>> 
>>  docs/devel/index-internals.rst            |   1 +
>>  docs/devel/virtio-backends.rst            | 214 +++++++++
>>  docs/interop/vhost-user-gpu.rst           |  10 +-
>>  docs/interop/vhost-user.rst               | 555 ++++++++++++----------
>>  meson.build                               |   2 +-
>>  include/hw/virtio/vhost-user.h            |  43 +-
>>  include/hw/virtio/vhost.h                 | 132 ++++-
>>  {hw => include/hw}/virtio/virtio-pci.h    |   0
>>  subprojects/libvhost-user/libvhost-user.h |   9 +
>>  contrib/vhost-user-blk/vhost-user-blk.c   |   6 +-
>>  hw/scsi/vhost-user-scsi.c                 |   1 +
>>  hw/virtio/vhost-scsi-pci.c                |   2 +-
>>  hw/virtio/vhost-user-blk-pci.c            |   2 +-
>>  hw/virtio/vhost-user-fs-pci.c             |   2 +-
>>  hw/virtio/vhost-user-i2c-pci.c            |   2 +-
>>  hw/virtio/vhost-user-input-pci.c          |   2 +-
>>  hw/virtio/vhost-user-rng-pci.c            |   2 +-
>>  hw/virtio/vhost-user-scsi-pci.c           |   2 +-
>>  hw/virtio/vhost-user-vsock-pci.c          |   2 +-
>>  hw/virtio/vhost-user.c                    | 133 ++++--
>>  hw/virtio/vhost-vsock-pci.c               |   2 +-
>>  hw/virtio/virtio-9p-pci.c                 |   2 +-
>>  hw/virtio/virtio-balloon-pci.c            |   2 +-
>>  hw/virtio/virtio-blk-pci.c                |   2 +-
>>  hw/virtio/virtio-input-host-pci.c         |   2 +-
>>  hw/virtio/virtio-input-pci.c              |   2 +-
>>  hw/virtio/virtio-iommu-pci.c              |   2 +-
>>  hw/virtio/virtio-net-pci.c                |   2 +-
>>  hw/virtio/virtio-pci.c                    |   5 +-
>>  hw/virtio/virtio-rng-pci.c                |   2 +-
>>  hw/virtio/virtio-scsi-pci.c               |   2 +-
>>  hw/virtio/virtio-serial-pci.c             |   2 +-
>>  subprojects/libvhost-user/libvhost-user.c |   2 +-
>>  contrib/vhost-user-blk/meson.build        |   3 +-
>>  hw/virtio/trace-events                    |  10 +-
>>  35 files changed, 831 insertions(+), 333 deletions(-)
>>  create mode 100644 docs/devel/virtio-backends.rst
>>  rename {hw => include/hw}/virtio/virtio-pci.h (100%)
>> 
>> -- 
>> 2.30.2
Michael S. Tsirkin March 22, 2022, 4:13 p.m. UTC | #3
On Tue, Mar 22, 2022 at 03:50:28PM +0000, Alex Bennée wrote:
> 
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Mon, Mar 21, 2022 at 03:30:24PM +0000, Alex Bennée wrote:
> >> Hi,
> >> 
> >> This series is a sub-set of patches while I was trying to re-rev my
> >> virtio-rpmb patches. It attempts to address a few things:
> >> 
> >>   - improve documentation for virtio/vhost/vhost-user
> >>   - document some of the API
> >>   - a hacky fix for F_CONFIG handling
> >>   - putting VhostUserState on a diet, make VhostUserHostNotifier dynamic
> >
> > So I think this is best deferred until after the release,
> > more of a cleanup than a bugfix.
> 
> Sorry I should have made it clearer - I wasn't intending this for 7.0
> but I also didn't want it bound up with the rpmb changes which will take
> longer to land.

Makes sense, I was just making sure.

> >
> > I will tag this series, but please do remind me after the release
> > to help make sure it does not get lost.
> >
> >
> >> In particular I've been trying to better understand how vhost-user
> >> interactions are meant to work and why there are two different methods
> >> for instantiating them. If my supposition is correct perhaps a number
> >> of devices that don't have in-kernel vhost equivalents could be converted?
> >
> > Hope I understand your question.  Well we started off with saying
> > vhost-user is just a backend, so should not affect the frontend device.
> > This is clean and makes migration work e.g. you can migrate between
> > different backends, but it makes adding features more work.
> 
> This is covered in the doc patch, specifically:
> 
>   vhost_ops vs TYPE_VHOST_USER_BACKEND

I guess I misunderstood then - what was the question again?

> >> hard to trigger the code. Is this rarely used code or just requires
> >> backends we don't see in the testing?
> >
> > Which function are you asking about exactly?
> 
>   vhost_user_slave_handle_vring_host_notifier
> 
> which is the only place where a mapping is set up AFAICT.

Well it's mostly for use with vdpa, but I think vhost user bridge
is supposed to support that with a special flag (-H use host notifier).
It's for debugging purposes as it wastes lots of CPU polling
the notification memory.

> >
> >> Alex Bennée (10):
> >>   hw/virtio: move virtio-pci.h into shared include space
> >>   virtio-pci: add notification trace points
> >>   hw/virtio: add vhost_user_[read|write] trace points
> >>   vhost-user.rst: add clarifying language about protocol negotiation
> >>   libvhost-user: expose vu_request_to_string
> >>   docs/devel: start documenting writing VirtIO devices
> >>   include/hw: start documenting the vhost API
> >>   contrib/vhost-user-blk: fix 32 bit build and enable
> >>   hw/virtio/vhost-user: don't suppress F_CONFIG when supported
> >>   virtio/vhost-user: dynamically assign VhostUserHostNotifiers
> >> 
> >> Paolo Bonzini (3):
> >>   docs: vhost-user: clean up request/reply description
> >>   docs: vhost-user: rewrite section on ring state machine
> >>   docs: vhost-user: replace master/slave with front-end/back-end
> >> 
> >>  docs/devel/index-internals.rst            |   1 +
> >>  docs/devel/virtio-backends.rst            | 214 +++++++++
> >>  docs/interop/vhost-user-gpu.rst           |  10 +-
> >>  docs/interop/vhost-user.rst               | 555 ++++++++++++----------
> >>  meson.build                               |   2 +-
> >>  include/hw/virtio/vhost-user.h            |  43 +-
> >>  include/hw/virtio/vhost.h                 | 132 ++++-
> >>  {hw => include/hw}/virtio/virtio-pci.h    |   0
> >>  subprojects/libvhost-user/libvhost-user.h |   9 +
> >>  contrib/vhost-user-blk/vhost-user-blk.c   |   6 +-
> >>  hw/scsi/vhost-user-scsi.c                 |   1 +
> >>  hw/virtio/vhost-scsi-pci.c                |   2 +-
> >>  hw/virtio/vhost-user-blk-pci.c            |   2 +-
> >>  hw/virtio/vhost-user-fs-pci.c             |   2 +-
> >>  hw/virtio/vhost-user-i2c-pci.c            |   2 +-
> >>  hw/virtio/vhost-user-input-pci.c          |   2 +-
> >>  hw/virtio/vhost-user-rng-pci.c            |   2 +-
> >>  hw/virtio/vhost-user-scsi-pci.c           |   2 +-
> >>  hw/virtio/vhost-user-vsock-pci.c          |   2 +-
> >>  hw/virtio/vhost-user.c                    | 133 ++++--
> >>  hw/virtio/vhost-vsock-pci.c               |   2 +-
> >>  hw/virtio/virtio-9p-pci.c                 |   2 +-
> >>  hw/virtio/virtio-balloon-pci.c            |   2 +-
> >>  hw/virtio/virtio-blk-pci.c                |   2 +-
> >>  hw/virtio/virtio-input-host-pci.c         |   2 +-
> >>  hw/virtio/virtio-input-pci.c              |   2 +-
> >>  hw/virtio/virtio-iommu-pci.c              |   2 +-
> >>  hw/virtio/virtio-net-pci.c                |   2 +-
> >>  hw/virtio/virtio-pci.c                    |   5 +-
> >>  hw/virtio/virtio-rng-pci.c                |   2 +-
> >>  hw/virtio/virtio-scsi-pci.c               |   2 +-
> >>  hw/virtio/virtio-serial-pci.c             |   2 +-
> >>  subprojects/libvhost-user/libvhost-user.c |   2 +-
> >>  contrib/vhost-user-blk/meson.build        |   3 +-
> >>  hw/virtio/trace-events                    |  10 +-
> >>  35 files changed, 831 insertions(+), 333 deletions(-)
> >>  create mode 100644 docs/devel/virtio-backends.rst
> >>  rename {hw => include/hw}/virtio/virtio-pci.h (100%)
> >> 
> >> -- 
> >> 2.30.2
> 
> 
> -- 
> Alex Bennée
Michael S. Tsirkin May 13, 2022, 10:15 a.m. UTC | #4
On Mon, Mar 21, 2022 at 03:30:24PM +0000, Alex Bennée wrote:
>   contrib/vhost-user-blk: fix 32 bit build and enable

I applied up to this point. Pls respond to Philippe's comment.