[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 03/23] vfio-user: add container IO ops vector
From: |
Alex Williamson |
Subject: |
Re: [PATCH v2 03/23] vfio-user: add container IO ops vector |
Date: |
Fri, 3 Feb 2023 15:33:33 -0700 |
On Fri, 3 Feb 2023 15:21:09 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:
> On Wed, 1 Feb 2023 21:55:39 -0800
> John Johnson <john.g.johnson@oracle.com> wrote:
>
> > Used for communication with VFIO driver
> > (prep work for vfio-user, which will communicate over a socket)
> >
> > Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
> > ---
> > include/hw/vfio/vfio-common.h | 24 ++++++++
> > hw/vfio/common.c | 128
> > ++++++++++++++++++++++++++++--------------
> > 2 files changed, 110 insertions(+), 42 deletions(-)
> >
> > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> > index e573f5a..953bc0f 100644
> > --- a/include/hw/vfio/vfio-common.h
> > +++ b/include/hw/vfio/vfio-common.h
> > @@ -75,6 +75,7 @@ typedef struct VFIOAddressSpace {
> > } VFIOAddressSpace;
> >
> > struct VFIOGroup;
> > +typedef struct VFIOContainerIO VFIOContainerIO;
> >
> > typedef struct VFIOContainer {
> > VFIOAddressSpace *space;
> > @@ -83,6 +84,7 @@ typedef struct VFIOContainer {
> > MemoryListener prereg_listener;
> > unsigned iommu_type;
> > Error *error;
> > + VFIOContainerIO *io;
> > bool initialized;
> > bool dirty_pages_supported;
> > uint64_t dirty_pgsizes;
> > @@ -154,6 +156,28 @@ struct VFIODeviceOps {
> > int (*vfio_load_config)(VFIODevice *vdev, QEMUFile *f);
> > };
> >
> > +#ifdef CONFIG_LINUX
> > +
> > +/*
> > + * The next 2 ops vectors are how Devices and Containers
> > + * communicate with the server. The default option is
> > + * through ioctl() to the kernel VFIO driver, but vfio-user
> > + * can use a socket to a remote process.
> > + */
> > +
> > +struct VFIOContainerIO {
> > + int (*dma_map)(VFIOContainer *container,
> > + struct vfio_iommu_type1_dma_map *map);
> > + int (*dma_unmap)(VFIOContainer *container,
> > + struct vfio_iommu_type1_dma_unmap *unmap,
> > + struct vfio_bitmap *bitmap);
> > + int (*dirty_bitmap)(VFIOContainer *container,
> > + struct vfio_iommu_type1_dirty_bitmap *bitmap,
> > + struct vfio_iommu_type1_dirty_bitmap_get *range);
> > +};
> > +
> > +#endif /* CONFIG_LINUX */
> > +
> > typedef struct VFIOGroup {
> > int fd;
> > int groupid;
> > diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> > index ace9562..9310a7f 100644
> > --- a/hw/vfio/common.c
> > +++ b/hw/vfio/common.c
> > @@ -58,6 +58,8 @@ static QLIST_HEAD(, VFIOAddressSpace) vfio_address_spaces
> > =
> > static int vfio_kvm_device_fd = -1;
> > #endif
> >
> > +static VFIOContainerIO vfio_cont_io_ioctl;
> > +
> > /*
> > * Common VFIO interrupt disable
> > */
> > @@ -432,12 +434,12 @@ static int vfio_dma_unmap_bitmap(VFIOContainer
> > *container,
> > goto unmap_exit;
> > }
> >
> > - ret = ioctl(container->fd, VFIO_IOMMU_UNMAP_DMA, unmap);
> > + ret = container->io->dma_unmap(container, unmap, bitmap);
>
> The bitmap arg doesn't make a lot of sense here, its use doesn't come
> around until vfio_user_dma_unmap() is added, but even then, it's
> readily available at &unmap->data with validity determined by
> unmap->flags. The eventual sanity test in vfio_user_dma_unmap() really
> only seems to prove the arg is unnecessary. Thanks,
The same comment really applies to .dirty_bitmap() with respect to the
range arg, but I think it's also important to note that both of these
are all but deprecated interfaces for the kernel. The migration
series[1] is pre-enabling these kernel interfaces to be removed by
adding support to QEMU to dirty all pages when support isn't present,
then we replace it with what we expect to be a longer term solution
with device dirty tracking support[2]. All the more reason not to make
these part of the internal API shared by kernel and user versions.
Thanks,
Alex
[1]https://lore.kernel.org/all/20230116141135.12021-1-avihaih@nvidia.com/
[2]https://lore.kernel.org/all/20230126184948.10478-1-avihaih@nvidia.com/
- [PATCH v2 00/23] vfio-user client, John Johnson, 2023/02/02
- [PATCH v2 03/23] vfio-user: add container IO ops vector, John Johnson, 2023/02/02
- [PATCH v2 02/23] vfio-user: add VFIO base abstract class, John Johnson, 2023/02/02
- [PATCH v2 04/23] vfio-user: add region cache, John Johnson, 2023/02/02
- [PATCH v2 01/23] vfio-user: introduce vfio-user protocol specification, John Johnson, 2023/02/02
- [PATCH v2 06/23] vfio-user: Define type vfio_user_pci_dev_info, John Johnson, 2023/02/02
- [PATCH v2 05/23] vfio-user: add device IO ops vector, John Johnson, 2023/02/02
- [PATCH v2 08/23] vfio-user: define socket receive functions, John Johnson, 2023/02/02
- [PATCH v2 07/23] vfio-user: connect vfio proxy to remote server, John Johnson, 2023/02/02
- [PATCH v2 10/23] vfio-user: get device info, John Johnson, 2023/02/02
- [PATCH v2 11/23] vfio-user: get region info, John Johnson, 2023/02/02