[PATCH v8 05/28] vfio: add vfio_prepare_device()

John Levon posted 28 patches 1 month, 2 weeks ago
[PATCH v8 05/28] vfio: add vfio_prepare_device()
Posted by John Levon 1 month, 2 weeks ago
Commonize some initialization code shared by the legacy and iommufd vfio
implementations (and later by vfio-user).

Signed-off-by: John Levon <john.levon@nutanix.com>
---
 hw/vfio/common.c              | 19 +++++++++++++++++++
 hw/vfio/container.c           | 14 +-------------
 hw/vfio/iommufd.c             |  9 +--------
 include/hw/vfio/vfio-common.h |  2 ++
 4 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index eefd735bc6..4434e0a0a2 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1569,6 +1569,25 @@ retry:
     return info;
 }
 
+void vfio_prepare_device(VFIODevice *vbasedev, VFIOContainerBase *bcontainer,
+                         VFIOGroup *group, struct vfio_device_info *info)
+{
+    vbasedev->group = group;
+
+    vbasedev->num_irqs = info->num_irqs;
+    vbasedev->num_regions = info->num_regions;
+    vbasedev->flags = info->flags;
+    vbasedev->reset_works = !!(info->flags & VFIO_DEVICE_FLAGS_RESET);
+
+    vbasedev->bcontainer = bcontainer;
+    QLIST_INSERT_HEAD(&bcontainer->device_list, vbasedev, container_next);
+    if (group) {
+        QLIST_INSERT_HEAD(&group->device_list, vbasedev, next);
+    }
+
+    QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next);
+}
+
 bool vfio_attach_device_by_iommu_type(const char *iommu_type, char *name,
                                       VFIODevice *vbasedev, AddressSpace *as,
                                       Error **errp)
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 82987063e5..37a3befbc5 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -876,17 +876,11 @@ static bool vfio_get_device(VFIOGroup *group, const char *name,
     }
 
     vbasedev->fd = fd;
-    vbasedev->group = group;
-    QLIST_INSERT_HEAD(&group->device_list, vbasedev, next);
 
-    vbasedev->num_irqs = info->num_irqs;
-    vbasedev->num_regions = info->num_regions;
-    vbasedev->flags = info->flags;
+    vfio_prepare_device(vbasedev, &group->container->bcontainer, group, info);
 
     trace_vfio_get_device(name, info->flags, info->num_regions, info->num_irqs);
 
-    vbasedev->reset_works = !!(info->flags & VFIO_DEVICE_FLAGS_RESET);
-
     return true;
 }
 
@@ -939,7 +933,6 @@ static bool vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev,
     int groupid = vfio_device_groupid(vbasedev, errp);
     VFIODevice *vbasedev_iter;
     VFIOGroup *group;
-    VFIOContainerBase *bcontainer;
 
     if (groupid < 0) {
         return false;
@@ -968,11 +961,6 @@ static bool vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev,
         return false;
     }
 
-    bcontainer = &group->container->bcontainer;
-    vbasedev->bcontainer = bcontainer;
-    QLIST_INSERT_HEAD(&bcontainer->device_list, vbasedev, container_next);
-    QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next);
-
     return true;
 }
 
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index e295f251c0..85c70eae37 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -604,14 +604,7 @@ found_container:
         iommufd_cdev_ram_block_discard_disable(false);
     }
 
-    vbasedev->group = 0;
-    vbasedev->num_irqs = dev_info.num_irqs;
-    vbasedev->num_regions = dev_info.num_regions;
-    vbasedev->flags = dev_info.flags;
-    vbasedev->reset_works = !!(dev_info.flags & VFIO_DEVICE_FLAGS_RESET);
-    vbasedev->bcontainer = bcontainer;
-    QLIST_INSERT_HEAD(&bcontainer->device_list, vbasedev, container_next);
-    QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next);
+    vfio_prepare_device(vbasedev, bcontainer, NULL, &dev_info);
 
     trace_iommufd_cdev_device_info(vbasedev->name, devfd, vbasedev->num_irqs,
                                    vbasedev->num_regions, vbasedev->flags);
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index c40f8de6bc..ae3ecbd9f6 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -250,6 +250,8 @@ void vfio_reset_handler(void *opaque);
 struct vfio_device_info *vfio_get_device_info(int fd);
 bool vfio_device_is_mdev(VFIODevice *vbasedev);
 bool vfio_device_hiod_realize(VFIODevice *vbasedev, Error **errp);
+void vfio_prepare_device(VFIODevice *vbasedev, VFIOContainerBase *bcontainer,
+                         VFIOGroup *group, struct vfio_device_info *info);
 bool vfio_attach_device(char *name, VFIODevice *vbasedev,
                         AddressSpace *as, Error **errp);
 bool vfio_attach_device_by_iommu_type(const char *iommu_type, char *name,
-- 
2.34.1
Re: [PATCH v8 05/28] vfio: add vfio_prepare_device()
Posted by Cédric Le Goater 1 week ago
On 2/19/25 15:48, John Levon wrote:
> Commonize some initialization code shared by the legacy and iommufd vfio
> implementations (and later by vfio-user).
> 
> Signed-off-by: John Levon <john.levon@nutanix.com>
> ---
>   hw/vfio/common.c              | 19 +++++++++++++++++++
>   hw/vfio/container.c           | 14 +-------------
>   hw/vfio/iommufd.c             |  9 +--------
>   include/hw/vfio/vfio-common.h |  2 ++
>   4 files changed, 23 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index eefd735bc6..4434e0a0a2 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1569,6 +1569,25 @@ retry:
>       return info;
>   }
>   
> +void vfio_prepare_device(VFIODevice *vbasedev, VFIOContainerBase *bcontainer,
> +                         VFIOGroup *group, struct vfio_device_info *info)

I would prefer that the first version did not have a 'group' parameter.
Let's add it when needed.


Thanks,

C.




> +{
> +    vbasedev->group = group;
> +
> +    vbasedev->num_irqs = info->num_irqs;
> +    vbasedev->num_regions = info->num_regions;
> +    vbasedev->flags = info->flags;
> +    vbasedev->reset_works = !!(info->flags & VFIO_DEVICE_FLAGS_RESET);
> +
> +    vbasedev->bcontainer = bcontainer;
> +    QLIST_INSERT_HEAD(&bcontainer->device_list, vbasedev, container_next);
> +    if (group) {
> +        QLIST_INSERT_HEAD(&group->device_list, vbasedev, next);
> +    }
> +
> +    QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next);
> +}
> +
>   bool vfio_attach_device_by_iommu_type(const char *iommu_type, char *name,
>                                         VFIODevice *vbasedev, AddressSpace *as,
>                                         Error **errp)
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 82987063e5..37a3befbc5 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -876,17 +876,11 @@ static bool vfio_get_device(VFIOGroup *group, const char *name,
>       }
>   
>       vbasedev->fd = fd;
> -    vbasedev->group = group;
> -    QLIST_INSERT_HEAD(&group->device_list, vbasedev, next);
>   
> -    vbasedev->num_irqs = info->num_irqs;
> -    vbasedev->num_regions = info->num_regions;
> -    vbasedev->flags = info->flags;
> +    vfio_prepare_device(vbasedev, &group->container->bcontainer, group, info);
>   
>       trace_vfio_get_device(name, info->flags, info->num_regions, info->num_irqs);
>   
> -    vbasedev->reset_works = !!(info->flags & VFIO_DEVICE_FLAGS_RESET);
> -
>       return true;
>   }
>   
> @@ -939,7 +933,6 @@ static bool vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev,
>       int groupid = vfio_device_groupid(vbasedev, errp);
>       VFIODevice *vbasedev_iter;
>       VFIOGroup *group;
> -    VFIOContainerBase *bcontainer;
>   
>       if (groupid < 0) {
>           return false;
> @@ -968,11 +961,6 @@ static bool vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev,
>           return false;
>       }
>   
> -    bcontainer = &group->container->bcontainer;
> -    vbasedev->bcontainer = bcontainer;
> -    QLIST_INSERT_HEAD(&bcontainer->device_list, vbasedev, container_next);
> -    QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next);
> -
>       return true;
>   }
>   
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index e295f251c0..85c70eae37 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -604,14 +604,7 @@ found_container:
>           iommufd_cdev_ram_block_discard_disable(false);
>       }
>   
> -    vbasedev->group = 0;
> -    vbasedev->num_irqs = dev_info.num_irqs;
> -    vbasedev->num_regions = dev_info.num_regions;
> -    vbasedev->flags = dev_info.flags;
> -    vbasedev->reset_works = !!(dev_info.flags & VFIO_DEVICE_FLAGS_RESET);
> -    vbasedev->bcontainer = bcontainer;
> -    QLIST_INSERT_HEAD(&bcontainer->device_list, vbasedev, container_next);
> -    QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next);
> +    vfio_prepare_device(vbasedev, bcontainer, NULL, &dev_info);
>   
>       trace_iommufd_cdev_device_info(vbasedev->name, devfd, vbasedev->num_irqs,
>                                      vbasedev->num_regions, vbasedev->flags);
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index c40f8de6bc..ae3ecbd9f6 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -250,6 +250,8 @@ void vfio_reset_handler(void *opaque);
>   struct vfio_device_info *vfio_get_device_info(int fd);
>   bool vfio_device_is_mdev(VFIODevice *vbasedev);
>   bool vfio_device_hiod_realize(VFIODevice *vbasedev, Error **errp);
> +void vfio_prepare_device(VFIODevice *vbasedev, VFIOContainerBase *bcontainer,
> +                         VFIOGroup *group, struct vfio_device_info *info);
>   bool vfio_attach_device(char *name, VFIODevice *vbasedev,
>                           AddressSpace *as, Error **errp);
>   bool vfio_attach_device_by_iommu_type(const char *iommu_type, char *name,
Re: [PATCH v8 05/28] vfio: add vfio_prepare_device()
Posted by John Levon 1 week ago
On Thu, Apr 03, 2025 at 11:19:34AM +0200, Cédric Le Goater wrote:

> On 2/19/25 15:48, John Levon wrote:
> > Commonize some initialization code shared by the legacy and iommufd vfio
> > implementations (and later by vfio-user).
> > 
> > Signed-off-by: John Levon <john.levon@nutanix.com>
> > ---
> >   hw/vfio/common.c              | 19 +++++++++++++++++++
> >   hw/vfio/container.c           | 14 +-------------
> >   hw/vfio/iommufd.c             |  9 +--------
> >   include/hw/vfio/vfio-common.h |  2 ++
> >   4 files changed, 23 insertions(+), 21 deletions(-)
> > 
> > diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> > index eefd735bc6..4434e0a0a2 100644
> > --- a/hw/vfio/common.c
> > +++ b/hw/vfio/common.c
> > @@ -1569,6 +1569,25 @@ retry:
> >       return info;
> >   }
> > +void vfio_prepare_device(VFIODevice *vbasedev, VFIOContainerBase *bcontainer,
> > +                         VFIOGroup *group, struct vfio_device_info *info)
> 
> I would prefer that the first version did not have a 'group' parameter.
> Let's add it when needed.

I think you mean something like this in hw/vfio/container.c:

vfio_prepare_device(vbasedev, &group->container->bcontainer, info);

vbasedev->group = group;
QLIST_INSERT_HEAD(&group->device_list, vbasedev, next);

As it's the only consumer that actually wants a group.

thanks
john
Re: [PATCH v8 05/28] vfio: add vfio_prepare_device()
Posted by Cédric Le Goater 6 days ago
On 4/3/25 11:34, John Levon wrote:
> On Thu, Apr 03, 2025 at 11:19:34AM +0200, Cédric Le Goater wrote:
> 
>> On 2/19/25 15:48, John Levon wrote:
>>> Commonize some initialization code shared by the legacy and iommufd vfio
>>> implementations (and later by vfio-user).
>>>
>>> Signed-off-by: John Levon <john.levon@nutanix.com>
>>> ---
>>>    hw/vfio/common.c              | 19 +++++++++++++++++++
>>>    hw/vfio/container.c           | 14 +-------------
>>>    hw/vfio/iommufd.c             |  9 +--------
>>>    include/hw/vfio/vfio-common.h |  2 ++
>>>    4 files changed, 23 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>>> index eefd735bc6..4434e0a0a2 100644
>>> --- a/hw/vfio/common.c
>>> +++ b/hw/vfio/common.c
>>> @@ -1569,6 +1569,25 @@ retry:
>>>        return info;
>>>    }
>>> +void vfio_prepare_device(VFIODevice *vbasedev, VFIOContainerBase *bcontainer,
>>> +                         VFIOGroup *group, struct vfio_device_info *info)
>>
>> I would prefer that the first version did not have a 'group' parameter.
>> Let's add it when needed.
> 
> I think you mean something like this in hw/vfio/container.c:
> 
> vfio_prepare_device(vbasedev, &group->container->bcontainer, info);
> 
> vbasedev->group = group;
> QLIST_INSERT_HEAD(&group->device_list, vbasedev, next);
> 
> As it's the only consumer that actually wants a group.
yes. It's a vfio legacy only construct. We will see what to do
if vfio-user needs it.


Thanks,

C.




Re: [PATCH v8 05/28] vfio: add vfio_prepare_device()
Posted by John Levon 6 days ago
On Fri, Apr 04, 2025 at 05:41:54PM +0200, Cédric Le Goater wrote:

> > > > +void vfio_prepare_device(VFIODevice *vbasedev, VFIOContainerBase *bcontainer,
> > > > +                         VFIOGroup *group, struct vfio_device_info *info)
> > > 
> > > I would prefer that the first version did not have a 'group' parameter.
> > > Let's add it when needed.
> > 
> > I think you mean something like this in hw/vfio/container.c:
> > 
> > vfio_prepare_device(vbasedev, &group->container->bcontainer, info);
> > 
> > vbasedev->group = group;
> > QLIST_INSERT_HEAD(&group->device_list, vbasedev, next);
> > 
> > As it's the only consumer that actually wants a group.
> yes. It's a vfio legacy only construct. We will see what to do
> if vfio-user needs it.

vfio-user is not going to need it for sure, so sounds good to me.

regards
john