[PATCH v2 01/29] vfio: add more VFIOIOMMUClass docs

John Levon posted 29 patches 5 months, 4 weeks ago
Maintainers: John Levon <john.levon@nutanix.com>, Thanos Makatos <thanos.makatos@nutanix.com>, Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>
There is a newer version of this series
[PATCH v2 01/29] vfio: add more VFIOIOMMUClass docs
Posted by John Levon 5 months, 4 weeks ago
Add some additional doc comments for these class methods.

Signed-off-by: John Levon <john.levon@nutanix.com>
---
 include/hw/vfio/vfio-container-base.h | 64 ++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
index 3d392b0fd8..41c49b2aa5 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -115,13 +115,52 @@ OBJECT_DECLARE_TYPE(VFIOContainerBase, VFIOIOMMUClass, VFIO_IOMMU)
 struct VFIOIOMMUClass {
     ObjectClass parent_class;
 
-    /* basic feature */
+    /**
+     * @setup
+     *
+     * Perform basic setup of the container. Returns true on success, or false
+     * with @errp filled in on failure.
+     *
+     * @bcontainer: #VFIOContainerBase
+     * @errp: error filled in on failure
+     */
     bool (*setup)(VFIOContainerBase *bcontainer, Error **errp);
+
+    /**
+     * @listener_begin
+     *
+     * Called at the beginning of an address space update transaction.
+     * See #MemoryListener.
+     *
+     * @bcontainer: #VFIOContainerBase
+     */
     void (*listener_begin)(VFIOContainerBase *bcontainer);
+
+    /**
+     * @listener_commit
+     *
+     * Called at the end of an address space update transaction,
+     * See #MemoryListener.
+     *
+     * @bcontainer: #VFIOContainerBase
+     */
     void (*listener_commit)(VFIOContainerBase *bcontainer);
+
+    /**
+     * @dma_map
+     *
+     * Map an address range into the container.
+     *
+     * @bcontainer: #VFIOContainerBase to use
+     * @iova: start address to map
+     * @size: size of the range to map
+     * @vaddr: process virtual address of mapping
+     * @readonly: true if mapping should be readonly
+     */
     int (*dma_map)(const VFIOContainerBase *bcontainer,
                    hwaddr iova, ram_addr_t size,
                    void *vaddr, bool readonly);
+
     /**
      * @dma_unmap
      *
@@ -136,8 +175,31 @@ struct VFIOIOMMUClass {
     int (*dma_unmap)(const VFIOContainerBase *bcontainer,
                      hwaddr iova, ram_addr_t size,
                      IOMMUTLBEntry *iotlb, bool unmap_all);
+
+
+    /**
+     * @attach_device
+     *
+     * Associate the given device with a container and do some related
+     * initialization of the device context. Returns true on success, or false
+     * with @errp filled in.
+     *
+     * @name: name of the device
+     * @vbasedev: the device
+     * @as: address space to use
+     * @errp: error filled in on failure
+     */
     bool (*attach_device)(const char *name, VFIODevice *vbasedev,
                           AddressSpace *as, Error **errp);
+
+    /*
+     * @detach_device
+     *
+     * Detach the given device from its container and clean up any necessary
+     * state.
+     *
+     * @vbasedev: the device to disassociate
+     */
     void (*detach_device)(VFIODevice *vbasedev);
 
     /* migration feature */
-- 
2.43.0
Re: [PATCH v2 01/29] vfio: add more VFIOIOMMUClass docs
Posted by Cédric Le Goater 5 months, 4 weeks ago
On 5/20/25 17:03, John Levon wrote:
> Add some additional doc comments for these class methods.
> 
> Signed-off-by: John Levon <john.levon@nutanix.com>
> ---
>   include/hw/vfio/vfio-container-base.h | 64 ++++++++++++++++++++++++++-
>   1 file changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
> index 3d392b0fd8..41c49b2aa5 100644
> --- a/include/hw/vfio/vfio-container-base.h
> +++ b/include/hw/vfio/vfio-container-base.h
> @@ -115,13 +115,52 @@ OBJECT_DECLARE_TYPE(VFIOContainerBase, VFIOIOMMUClass, VFIO_IOMMU)
>   struct VFIOIOMMUClass {
>       ObjectClass parent_class;
>   
> -    /* basic feature */
> +    /**
> +     * @setup
> +     *
> +     * Perform basic setup of the container. 

We could add a few details, something like :

"IOMMU capabilities, page sizes and IOVA ranges initialization."


> Returns true on success, or false
> +     * with @errp filled in on failure.

I would prefer the return statement to be at the end. See query_dirty_bitmap.

> +     *
> +     * @bcontainer: #VFIOContainerBase
> +     * @errp: error filled in on failure
> +     */
>       bool (*setup)(VFIOContainerBase *bcontainer, Error **errp);
> +
> +    /**
> +     * @listener_begin
> +     *
> +     * Called at the beginning of an address space update transaction.
> +     * See #MemoryListener.
> +     *
> +     * @bcontainer: #VFIOContainerBase
> +     */
>       void (*listener_begin)(VFIOContainerBase *bcontainer);
> +
> +    /**
> +     * @listener_commit
> +     *
> +     * Called at the end of an address space update transaction,
> +     * See #MemoryListener.
> +     *
> +     * @bcontainer: #VFIOContainerBase
> +     */
>       void (*listener_commit)(VFIOContainerBase *bcontainer);
> +
> +    /**
> +     * @dma_map
> +     *
> +     * Map an address range into the container.
> +     *
> +     * @bcontainer: #VFIOContainerBase to use
> +     * @iova: start address to map
> +     * @size: size of the range to map
> +     * @vaddr: process virtual address of mapping
> +     * @readonly: true if mapping should be readonly

Returns zero to indicate success and negative for error.

In fact it returns -errno. Should be mentioned too.

> +     */
>       int (*dma_map)(const VFIOContainerBase *bcontainer,
>                      hwaddr iova, ram_addr_t size,
>                      void *vaddr, bool readonly);
> +
>       /**
>        * @dma_unmap
>        *
> @@ -136,8 +175,31 @@ struct VFIOIOMMUClass {
>       int (*dma_unmap)(const VFIOContainerBase *bcontainer,
>                        hwaddr iova, ram_addr_t size,
>                        IOMMUTLBEntry *iotlb, bool unmap_all);
> +
> +
> +    /**
> +     * @attach_device
> +     *
> +     * Associate the given device with a container and do some related
> +     * initialization of the device context. Returns true on success, or false
> +     * with @errp filled in.
> +     *
> +     * @name: name of the device
> +     * @vbasedev: the device
> +     * @as: address space to use
> +     * @errp: error filled in on failure

Return: true on success, else false setting @errp with error.

> +     */
>       bool (*attach_device)(const char *name, VFIODevice *vbasedev,>                             AddressSpace *as, Error **errp);
> +
> +    /*
> +     * @detach_device
> +     *
> +     * Detach the given device from its container and clean up any necessary
> +     * state.
> +     *
> +     * @vbasedev: the device to disassociate
> +     */
>       void (*detach_device)(VFIODevice *vbasedev);
>   
>       /* migration feature */

Please resend independently of the series.

Thanks,

C.