Hi, this is the 7th revision of the vfio-user client implementation.

First of all, thank you for your time reviewing the previous versions.

The vfio-user framework consists of 3 parts:
 1) The VFIO user protocol specification.
 2) A client - the VFIO device in QEMU that encapsulates VFIO messages
    and sends them to the server.
 3) A server - a remote process that emulates a device.

This patchset implements parts 1 and 2.

It has been tested against libvfio-user test servers as well as SPDK.

Contributors:

John G Johnson <john.g.johnson@oracle.com>
John Levon <john.levon@nutanix.com>
Thanos Makatos <thanos.makatos@nutanix.com>
Elena Ufimtseva <elena.ufimtseva@oracle.com>
Jagannathan Raman <jag.raman@oracle.com>

Changes from v6->v7:

 - removed BQL dropping - caused various different correctness issues
 - fixed incorrect list iteration in vfio_user_disconnect()
 - fixed short header read check
 - marked vbasedev->mdev as true to skip host iommu device setup

On Wed, Jan 08, 2025 at 11:50:06AM +0000, John Levon wrote:

> Hi, this is the 7th revision of the vfio-user client implementation.
> 
> First of all, thank you for your time reviewing the previous versions.
> 
> The vfio-user framework consists of 3 parts:
>  1) The VFIO user protocol specification.
>  2) A client - the VFIO device in QEMU that encapsulates VFIO messages
>     and sends them to the server.
>  3) A server - a remote process that emulates a device.
> 
> This patchset implements parts 1 and 2.
> 
> It has been tested against libvfio-user test servers as well as SPDK.

Ping, would appreciate any review comments

thanks
john

Hello John,

On 1/23/25 11:11, John Levon wrote:
> On Wed, Jan 08, 2025 at 11:50:06AM +0000, John Levon wrote:
> 
>> Hi, this is the 7th revision of the vfio-user client implementation.
>>
>> First of all, thank you for your time reviewing the previous versions.
>>
>> The vfio-user framework consists of 3 parts:
>>   1) The VFIO user protocol specification.
>>   2) A client - the VFIO device in QEMU that encapsulates VFIO messages
>>      and sends them to the server.
>>   3) A server - a remote process that emulates a device.
>>
>> This patchset implements parts 1 and 2.
>>
>> It has been tested against libvfio-user test servers as well as SPDK.
> 
> Ping, would appreciate any review comments

Well, among the reasons why we tend to push this at end of
the list are :

   - the respins are spaced over time (1 a year ?)
   - it's invasive in an already very complex subsystem
   - it's HUGE. See the diffstat below ...

I would introduce a new hw/vfio-user/ subsystem given the size.
The commit logs are short. Most are one liners, this is really
not much for such a big change.

Any how, I hope to take a look before the end of 10.0 cycle.

Thanks,

C.


  MAINTAINERS                           |   11
  docs/devel/index-internals.rst        |    1
  docs/devel/vfio-user.rst              | 1522 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  hw/vfio/ap.c                          |    4
  hw/vfio/ccw.c                         |    9
  hw/vfio/common.c                      |  131 +++++--
  hw/vfio/container-base.c              |    8
  hw/vfio/container.c                   |   78 +++-
  hw/vfio/helpers.c                     |  185 +++++++++-
  hw/vfio/igd.c                         |    8
  hw/vfio/iommufd.c                     |   31 +
  hw/vfio/meson.build                   |    5
  hw/vfio/pci.c                         |  595 ++++++++++++++++++++-------------
  hw/vfio/pci.h                         |   35 +
  hw/vfio/platform.c                    |    4
  hw/vfio/trace-events                  |   17
  hw/vfio/user-container.c              |  358 ++++++++++++++++++++
  hw/vfio/user-pci.c                    |  446 +++++++++++++++++++++++++
  hw/vfio/user-protocol.h               |  245 +++++++++++++
  hw/vfio/user.c                        | 1705 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  hw/vfio/user.h                        |  132 +++++++
  hw/virtio/vhost-vdpa.c                |    2
  include/exec/memory.h                 |    4
  include/hw/vfio/vfio-common.h         |   54 ++-
  include/hw/vfio/vfio-container-base.h |   11
  meson_options.txt                     |    2
  scripts/meson-buildoptions.sh         |    4
  system/memory.c                       |    7
  28 files changed, 5280 insertions(+), 334 deletions(-)

On Thu, Jan 23, 2025 at 02:50:16PM +0100, Cédric Le Goater wrote:

> > > Hi, this is the 7th revision of the vfio-user client implementation.
> 
> Well, among the reasons why we tend to push this at end of
> the list are :

Thanks for the steers. Just a note in case you're not aware - most of these
changes weren't authored by me, I'm in the position of picking up someone else's
series. I hope you'll bear with me in this respect.

>   - the respins are spaced over time (1 a year ?)

Understandable, I will certainly make time to be more responsive when I do get
review comments. The last spin got none, whereas the earlier ones sent out by
Oracle got a good amount of review.

You mentioned on IRC that it's worth me documenting previous reviews: I can
spelunk for that data, but it's worth pointing out that many of the patches have
changed significantly since previous reviews.

>   - it's invasive in an already very complex subsystem

As a qemu neophyte I would greatly appreciate suggestions on how to reduce this.
I think the user-container refactoring was a big one (we now barely touch the
other container types), but I'm definitely up for further suggestions.

>   - it's HUGE. See the diffstat below ...

Unavoidable right? The start of the series are generic preparation so could in
theory be merged separately. If you have other ways you'd prefer me to split
this up I'd be happy to do so.

FYI I tried sending out just the specification document (which reflects the
already-merged server side implementation) but did not get any reviews.

> I would introduce a new hw/vfio-user/ subsystem given the size.

Could you give me some further guidance? Are you talking about duplicating all
of the needed hw/vfio/pci.c code into hw/vfio-user/pci.c ?

Or do you just mean putting the new hw/vfio/user* files into hw/vfio-user/

> The commit logs are short. Most are one liners, this is really
> not much for such a big change.

The ones I've written have longer changelogs and I hope are better. I'll take
some time to rewrite the other authors' commit messages for the next cycle.

> Any how, I hope to take a look before the end of 10.0 cycle.

Thanks so much!

Would you prefer me to apply your above suggestions prior to you reviewing?

regards
john