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

["Cédric Le Goater" <clg@redhat.com>]

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.