[PATCH v3 18/37] vfio/container: Implement attach/detach_device

Zhenzhong Duan posted 37 patches 1 year, 1 month ago
There is a newer version of this series
[PATCH v3 18/37] vfio/container: Implement attach/detach_device
Posted by Zhenzhong Duan 1 year, 1 month ago
From: Eric Auger <eric.auger@redhat.com>

No fucntional change intended.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/vfio/common.c    | 16 ++++++++++++++++
 hw/vfio/container.c | 12 +++++-------
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index d62c815d7f..64565b4ae9 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1500,3 +1500,19 @@ retry:
 
     return info;
 }
+
+int vfio_attach_device(char *name, VFIODevice *vbasedev,
+                       AddressSpace *as, Error **errp)
+{
+    const VFIOIOMMUOps *ops = &vfio_legacy_ops;
+
+    return ops->attach_device(name, vbasedev, as, errp);
+}
+
+void vfio_detach_device(VFIODevice *vbasedev)
+{
+    if (!vbasedev->bcontainer) {
+        return;
+    }
+    vbasedev->bcontainer->ops->detach_device(vbasedev);
+}
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 36c34683ad..c8ff0f2037 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -874,8 +874,8 @@ static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp)
  * @name and @vbasedev->name are likely to be different depending
  * on the type of the device, hence the need for passing @name
  */
-int vfio_attach_device(char *name, VFIODevice *vbasedev,
-                       AddressSpace *as, Error **errp)
+static int vfio_legacy_attach_device(char *name, VFIODevice *vbasedev,
+                                     AddressSpace *as, Error **errp)
 {
     int groupid = vfio_device_groupid(vbasedev, errp);
     VFIODevice *vbasedev_iter;
@@ -915,14 +915,10 @@ int vfio_attach_device(char *name, VFIODevice *vbasedev,
     return ret;
 }
 
-void vfio_detach_device(VFIODevice *vbasedev)
+static void vfio_legacy_detach_device(VFIODevice *vbasedev)
 {
     VFIOGroup *group = vbasedev->group;
 
-    if (!vbasedev->bcontainer) {
-        return;
-    }
-
     QLIST_REMOVE(vbasedev, global_next);
     QLIST_REMOVE(vbasedev, container_next);
     vbasedev->bcontainer = NULL;
@@ -934,6 +930,8 @@ void vfio_detach_device(VFIODevice *vbasedev)
 const VFIOIOMMUOps vfio_legacy_ops = {
     .dma_map = vfio_legacy_dma_map,
     .dma_unmap = vfio_legacy_dma_unmap,
+    .attach_device = vfio_legacy_attach_device,
+    .detach_device = vfio_legacy_detach_device,
     .set_dirty_page_tracking = vfio_legacy_set_dirty_page_tracking,
     .query_dirty_bitmap = vfio_legacy_query_dirty_bitmap,
 };
-- 
2.34.1


Re: [PATCH v3 18/37] vfio/container: Implement attach/detach_device
Posted by Cédric Le Goater 1 year, 1 month ago
Sorry, previous email was empty. Friday effect !

On 10/26/23 12:30, Zhenzhong Duan wrote:
> From: Eric Auger <eric.auger@redhat.com>
> 
> No fucntional change intended.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>   hw/vfio/common.c    | 16 ++++++++++++++++
>   hw/vfio/container.c | 12 +++++-------
>   2 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index d62c815d7f..64565b4ae9 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1500,3 +1500,19 @@ retry:
>   
>       return info;
>   }
> +
> +int vfio_attach_device(char *name, VFIODevice *vbasedev,
> +                       AddressSpace *as, Error **errp)
> +{
> +    const VFIOIOMMUOps *ops = &vfio_legacy_ops;

hmm, this looks wrong. please explain.

Thanks,

C.

> +
> +    return ops->attach_device(name, vbasedev, as, errp);
> +}
> +
> +void vfio_detach_device(VFIODevice *vbasedev)
> +{
> +    if (!vbasedev->bcontainer) {
> +        return;
> +    }
> +    vbasedev->bcontainer->ops->detach_device(vbasedev);
> +}
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 36c34683ad..c8ff0f2037 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -874,8 +874,8 @@ static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp)
>    * @name and @vbasedev->name are likely to be different depending
>    * on the type of the device, hence the need for passing @name
>    */
> -int vfio_attach_device(char *name, VFIODevice *vbasedev,
> -                       AddressSpace *as, Error **errp)
> +static int vfio_legacy_attach_device(char *name, VFIODevice *vbasedev,
> +                                     AddressSpace *as, Error **errp)
>   {
>       int groupid = vfio_device_groupid(vbasedev, errp);
>       VFIODevice *vbasedev_iter;
> @@ -915,14 +915,10 @@ int vfio_attach_device(char *name, VFIODevice *vbasedev,
>       return ret;
>   }
>   
> -void vfio_detach_device(VFIODevice *vbasedev)
> +static void vfio_legacy_detach_device(VFIODevice *vbasedev)
>   {
>       VFIOGroup *group = vbasedev->group;
>   
> -    if (!vbasedev->bcontainer) {
> -        return;
> -    }
> -
>       QLIST_REMOVE(vbasedev, global_next);
>       QLIST_REMOVE(vbasedev, container_next);
>       vbasedev->bcontainer = NULL;
> @@ -934,6 +930,8 @@ void vfio_detach_device(VFIODevice *vbasedev)
>   const VFIOIOMMUOps vfio_legacy_ops = {
>       .dma_map = vfio_legacy_dma_map,
>       .dma_unmap = vfio_legacy_dma_unmap,
> +    .attach_device = vfio_legacy_attach_device,
> +    .detach_device = vfio_legacy_detach_device,
>       .set_dirty_page_tracking = vfio_legacy_set_dirty_page_tracking,
>       .query_dirty_bitmap = vfio_legacy_query_dirty_bitmap,
>   };


RE: [PATCH v3 18/37] vfio/container: Implement attach/detach_device
Posted by Duan, Zhenzhong 1 year ago

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Saturday, October 28, 2023 12:07 AM
>Subject: Re: [PATCH v3 18/37] vfio/container: Implement attach/detach_device
>
>Sorry, previous email was empty. Friday effect !
>
>On 10/26/23 12:30, Zhenzhong Duan wrote:
>> From: Eric Auger <eric.auger@redhat.com>
>>
>> No fucntional change intended.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>   hw/vfio/common.c    | 16 ++++++++++++++++
>>   hw/vfio/container.c | 12 +++++-------
>>   2 files changed, 21 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index d62c815d7f..64565b4ae9 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -1500,3 +1500,19 @@ retry:
>>
>>       return info;
>>   }
>> +
>> +int vfio_attach_device(char *name, VFIODevice *vbasedev,
>> +                       AddressSpace *as, Error **errp)
>> +{
>> +    const VFIOIOMMUOps *ops = &vfio_legacy_ops;
>
>hmm, this looks wrong. please explain.

The final shape will be:

int vfio_attach_device(char *name, VFIODevice *vbasedev,
                       AddressSpace *as, Error **errp)
{
    const VFIOIOMMUOps *ops;

#ifdef CONFIG_IOMMUFD
    if (vbasedev->iommufd) {
        ops = &vfio_iommufd_ops;
    } else
#endif
    {
        ops = &vfio_legacy_ops;
    }
    return ops->attach_device(name, vbasedev, as, errp);
}

Depending on if iommufd is selected, different ops will be chosen.
Then corresponding attach_device callback is called to attach to
iommufd or legacy container.

Thanks
Zhenzhong


Re: [PATCH v3 18/37] vfio/container: Implement attach/detach_device
Posted by Cédric Le Goater 1 year, 1 month ago
On 10/26/23 12:30, Zhenzhong Duan wrote:
> From: Eric Auger <eric.auger@redhat.com>
> 
> No fucntional change intended.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>   hw/vfio/common.c    | 16 ++++++++++++++++
>   hw/vfio/container.c | 12 +++++-------
>   2 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index d62c815d7f..64565b4ae9 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1500,3 +1500,19 @@ retry:
>   
>       return info;
>   }
> +
> +int vfio_attach_device(char *name, VFIODevice *vbasedev,
> +                       AddressSpace *as, Error **errp)
> +{
> +    const VFIOIOMMUOps *ops = &vfio_legacy_ops;
> +
> +    return ops->attach_device(name, vbasedev, as, errp);
> +}
> +
> +void vfio_detach_device(VFIODevice *vbasedev)
> +{
> +    if (!vbasedev->bcontainer) {
> +        return;
> +    }
> +    vbasedev->bcontainer->ops->detach_device(vbasedev);
> +}
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 36c34683ad..c8ff0f2037 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -874,8 +874,8 @@ static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp)
>    * @name and @vbasedev->name are likely to be different depending
>    * on the type of the device, hence the need for passing @name
>    */
> -int vfio_attach_device(char *name, VFIODevice *vbasedev,
> -                       AddressSpace *as, Error **errp)
> +static int vfio_legacy_attach_device(char *name, VFIODevice *vbasedev,
> +                                     AddressSpace *as, Error **errp)
>   {
>       int groupid = vfio_device_groupid(vbasedev, errp);
>       VFIODevice *vbasedev_iter;
> @@ -915,14 +915,10 @@ int vfio_attach_device(char *name, VFIODevice *vbasedev,
>       return ret;
>   }
>   
> -void vfio_detach_device(VFIODevice *vbasedev)
> +static void vfio_legacy_detach_device(VFIODevice *vbasedev)
>   {
>       VFIOGroup *group = vbasedev->group;
>   
> -    if (!vbasedev->bcontainer) {
> -        return;
> -    }
> -
>       QLIST_REMOVE(vbasedev, global_next);
>       QLIST_REMOVE(vbasedev, container_next);
>       vbasedev->bcontainer = NULL;
> @@ -934,6 +930,8 @@ void vfio_detach_device(VFIODevice *vbasedev)
>   const VFIOIOMMUOps vfio_legacy_ops = {
>       .dma_map = vfio_legacy_dma_map,
>       .dma_unmap = vfio_legacy_dma_unmap,
> +    .attach_device = vfio_legacy_attach_device,
> +    .detach_device = vfio_legacy_detach_device,
>       .set_dirty_page_tracking = vfio_legacy_set_dirty_page_tracking,
>       .query_dirty_bitmap = vfio_legacy_query_dirty_bitmap,
>   };