1. Patchew
  2. QEMU
  3. [PATCH rfcv2 00/18] Check and sync host IOMMU cap/ecap with vIOMMU
  4. View patch

[PATCH rfcv2 04/18] vfio: Add host iommu device instance into VFIODevice

Zhenzhong Duan posted 18 patches 9 months, 4 weeks ago
Maintainers: Yi Liu <yi.l.liu@intel.com>, Eric Auger <eric.auger@redhat.com>, Zhenzhong Duan <zhenzhong.duan@intel.com>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Peter Xu <peterx@redhat.com>, Jason Wang <jasowang@redhat.com>, Thomas Huth <thuth@redhat.com>, Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>, Tony Krowiak <akrowiak@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Jason Herne <jjherne@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>
There is a newer version of this series
[PATCH rfcv2 04/18] vfio: Add host iommu device instance into VFIODevice
Posted by Zhenzhong Duan 9 months, 4 weeks ago 
Either IOMMULegacyDevice or IOMMUFDDevice into VFIODevice, neither
both.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/hw/vfio/vfio-common.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 8bfb9cbe94..1bbad003ee 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -32,6 +32,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/vfio/vfio-container-base.h"
 #include "sysemu/host_iommu_device.h"
+#include "sysemu/iommufd.h"
 
 #define VFIO_MSG_PREFIX "vfio %s: "
 
@@ -132,8 +133,18 @@ typedef struct VFIODevice {
     bool dirty_tracking;
     int devid;
     IOMMUFDBackend *iommufd;
+    union {
+        HostIOMMUDevice base_hdev;
+        IOMMULegacyDevice legacy_dev;
+        IOMMUFDDevice iommufd_dev;
+    };
 } VFIODevice;
 
+QEMU_BUILD_BUG_ON(offsetof(VFIODevice, legacy_dev.base) !=
+                  offsetof(VFIODevice, base_hdev));
+QEMU_BUILD_BUG_ON(offsetof(VFIODevice, iommufd_dev.base) !=
+                  offsetof(VFIODevice, base_hdev));
+
 struct VFIODeviceOps {
     void (*vfio_compute_needs_reset)(VFIODevice *vdev);
     int (*vfio_hot_reset_multi)(VFIODevice *vdev);
-- 
2.34.1
Re: [PATCH rfcv2 04/18] vfio: Add host iommu device instance into VFIODevice
Posted by Eric Auger 9 months, 1 week ago 

On 2/1/24 08:28, Zhenzhong Duan wrote:
> Either IOMMULegacyDevice or IOMMUFDDevice into VFIODevice, neither
> both.
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  include/hw/vfio/vfio-common.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 8bfb9cbe94..1bbad003ee 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -32,6 +32,7 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/vfio/vfio-container-base.h"
>  #include "sysemu/host_iommu_device.h"
> +#include "sysemu/iommufd.h"
>  
>  #define VFIO_MSG_PREFIX "vfio %s: "
>  
> @@ -132,8 +133,18 @@ typedef struct VFIODevice {
>      bool dirty_tracking;
>      int devid;
>      IOMMUFDBackend *iommufd;
> +    union {
> +        HostIOMMUDevice base_hdev;
> +        IOMMULegacyDevice legacy_dev;
> +        IOMMUFDDevice iommufd_dev;
I think you should rather have a HostIOMMUDevice handle.

host_iommu_device_init cb would allocate the right type of the derived object and you would store the base object pointer here.

Eric
> +    };
>  } VFIODevice;
>  
> +QEMU_BUILD_BUG_ON(offsetof(VFIODevice, legacy_dev.base) !=
> +                  offsetof(VFIODevice, base_hdev));
> +QEMU_BUILD_BUG_ON(offsetof(VFIODevice, iommufd_dev.base) !=
> +                  offsetof(VFIODevice, base_hdev));
> +
>  struct VFIODeviceOps {
>      void (*vfio_compute_needs_reset)(VFIODevice *vdev);
>      int (*vfio_hot_reset_multi)(VFIODevice *vdev);
RE: [PATCH rfcv2 04/18] vfio: Add host iommu device instance into VFIODevice
Posted by Duan, Zhenzhong 9 months ago 
Hi Eric,

>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: Re: [PATCH rfcv2 04/18] vfio: Add host iommu device instance into
>VFIODevice
>
>
>
>On 2/1/24 08:28, Zhenzhong Duan wrote:
>> Either IOMMULegacyDevice or IOMMUFDDevice into VFIODevice, neither
>> both.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  include/hw/vfio/vfio-common.h | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-
>common.h
>> index 8bfb9cbe94..1bbad003ee 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -32,6 +32,7 @@
>>  #include "sysemu/sysemu.h"
>>  #include "hw/vfio/vfio-container-base.h"
>>  #include "sysemu/host_iommu_device.h"
>> +#include "sysemu/iommufd.h"
>>
>>  #define VFIO_MSG_PREFIX "vfio %s: "
>>
>> @@ -132,8 +133,18 @@ typedef struct VFIODevice {
>>      bool dirty_tracking;
>>      int devid;
>>      IOMMUFDBackend *iommufd;
>> +    union {
>> +        HostIOMMUDevice base_hdev;
>> +        IOMMULegacyDevice legacy_dev;
>> +        IOMMUFDDevice iommufd_dev;
>I think you should rather have a HostIOMMUDevice handle.
>
>host_iommu_device_init cb would allocate the right type of the derived
>object and you would store the base object pointer here.

Sorry for late response, just back from vacation.

I didn't use a HostIOMMUDevice handle but a statically allocated one
Because in following patch:

'[PATCH rfcv2 05/18] vfio: Remove redundant iommufd and devid elements in VFIODevice'

iommufd and devid are removed from VFIODevice. I need a place to store
them early in attachment. The handle is dynamically allocated after
attachment and can't be used for that purpose.

I'll change to use HostIOMMUDevice handle and drop patch5 in rfcv3,
Let me know if you have other comments.

Thanks
Zhenzhong
Re: [PATCH rfcv2 04/18] vfio: Add host iommu device instance into VFIODevice
Posted by Eric Auger 9 months, 1 week ago 
Hi Zhenzhong,

On 2/1/24 08:28, Zhenzhong Duan wrote:
> Either IOMMULegacyDevice or IOMMUFDDevice into VFIODevice, neither
> both.
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  include/hw/vfio/vfio-common.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 8bfb9cbe94..1bbad003ee 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -32,6 +32,7 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/vfio/vfio-container-base.h"
>  #include "sysemu/host_iommu_device.h"
> +#include "sysemu/iommufd.h"
>  
>  #define VFIO_MSG_PREFIX "vfio %s: "
>  
> @@ -132,8 +133,18 @@ typedef struct VFIODevice {
>      bool dirty_tracking;
>      int devid;
>      IOMMUFDBackend *iommufd;
> +    union {
> +        HostIOMMUDevice base_hdev;
I don't think we want the base object above to be usable here

Thanks

Eric
> +        IOMMULegacyDevice legacy_dev;
> +        IOMMUFDDevice iommufd_dev;
> +    };
>  } VFIODevice;
>  
> +QEMU_BUILD_BUG_ON(offsetof(VFIODevice, legacy_dev.base) !=
> +                  offsetof(VFIODevice, base_hdev));
> +QEMU_BUILD_BUG_ON(offsetof(VFIODevice, iommufd_dev.base) !=
> +                  offsetof(VFIODevice, base_hdev));
> +
>  struct VFIODeviceOps {
>      void (*vfio_compute_needs_reset)(VFIODevice *vdev);
>      int (*vfio_hot_reset_multi)(VFIODevice *vdev);

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.