Currently we have realize() callback which is called before attachment.
But there are still some elements e.g., hwpt_id is not ready before
attachment. So we need a realize_late() callback to further initialize
them.
Currently, this callback is only useful for iommufd backend. For legacy
backend nothing needs to be initialized after attachment.
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
include/system/host_iommu_device.h | 17 +++++++++++++++++
hw/vfio/common.c | 17 ++++++++++++++---
2 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/include/system/host_iommu_device.h b/include/system/host_iommu_device.h
index 809cced4ba..df782598f2 100644
--- a/include/system/host_iommu_device.h
+++ b/include/system/host_iommu_device.h
@@ -66,6 +66,23 @@ struct HostIOMMUDeviceClass {
* Returns: true on success, false on failure.
*/
bool (*realize)(HostIOMMUDevice *hiod, void *opaque, Error **errp);
+ /**
+ * @realize_late: initialize host IOMMU device instance after attachment,
+ * some elements e.g., ioas are ready only after attachment.
+ * This callback initialize them.
+ *
+ * Optional callback.
+ *
+ * @hiod: pointer to a host IOMMU device instance.
+ *
+ * @opaque: pointer to agent device of this host IOMMU device,
+ * e.g., VFIO base device or VDPA device.
+ *
+ * @errp: pass an Error out when realize fails.
+ *
+ * Returns: true on success, false on failure.
+ */
+ bool (*realize_late)(HostIOMMUDevice *hiod, void *opaque, Error **errp);
/**
* @get_cap: check if a host IOMMU device capability is supported.
*
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index abbdc56b6d..e198b1e5a2 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1550,6 +1550,7 @@ bool vfio_attach_device(char *name, VFIODevice *vbasedev,
const VFIOIOMMUClass *ops =
VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));
HostIOMMUDevice *hiod = NULL;
+ HostIOMMUDeviceClass *hiod_ops = NULL;
if (vbasedev->iommufd) {
ops = VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
@@ -1560,16 +1561,26 @@ bool vfio_attach_device(char *name, VFIODevice *vbasedev,
if (!vbasedev->mdev) {
hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
+ hiod_ops = HOST_IOMMU_DEVICE_GET_CLASS(hiod);
vbasedev->hiod = hiod;
}
if (!ops->attach_device(name, vbasedev, as, errp)) {
- object_unref(hiod);
- vbasedev->hiod = NULL;
- return false;
+ goto err_attach;
+ }
+
+ if (hiod_ops && hiod_ops->realize_late &&
+ !hiod_ops->realize_late(hiod, vbasedev, errp)) {
+ ops->detach_device(vbasedev);
+ goto err_attach;
}
return true;
+
+err_attach:
+ object_unref(hiod);
+ vbasedev->hiod = NULL;
+ return false;
}
void vfio_detach_device(VFIODevice *vbasedev)
--
2.34.1On 2/19/25 09:22, Zhenzhong Duan wrote:
> Currently we have realize() callback which is called before attachment.
> But there are still some elements e.g., hwpt_id is not ready before
> attachment. So we need a realize_late() callback to further initialize
> them.
The relation between objects HostIOMMUDevice and VFIOIOMMU is starting
to look too complex for me.
I think it makes sense to realize HostIOMMUDevice after the device
is attached. Can't we move :
hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
vbasedev->hiod = hiod;
under ->attach_device() and also the call :
if (!vfio_device_hiod_realize(vbasedev, errp)) {
later in the ->attach_device() patch ?
hiod_legacy_vfio_realize() doesn't do much. We might need to rework
hiod_iommufd_vfio_realize() which queries the iommufd hw caps, later
used by intel-iommu.
Anyway, it is good time to cleanup our interfaces before adding more.
Thanks,
C.
> Currently, this callback is only useful for iommufd backend. For legacy
> backend nothing needs to be initialized after attachment.
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
> include/system/host_iommu_device.h | 17 +++++++++++++++++
> hw/vfio/common.c | 17 ++++++++++++++---
> 2 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/include/system/host_iommu_device.h b/include/system/host_iommu_device.h
> index 809cced4ba..df782598f2 100644
> --- a/include/system/host_iommu_device.h
> +++ b/include/system/host_iommu_device.h
> @@ -66,6 +66,23 @@ struct HostIOMMUDeviceClass {
> * Returns: true on success, false on failure.
> */
> bool (*realize)(HostIOMMUDevice *hiod, void *opaque, Error **errp);
> + /**
> + * @realize_late: initialize host IOMMU device instance after attachment,
> + * some elements e.g., ioas are ready only after attachment.
> + * This callback initialize them.
> + *
> + * Optional callback.
> + *
> + * @hiod: pointer to a host IOMMU device instance.
> + *
> + * @opaque: pointer to agent device of this host IOMMU device,
> + * e.g., VFIO base device or VDPA device.
> + *
> + * @errp: pass an Error out when realize fails.
> + *
> + * Returns: true on success, false on failure.
> + */
> + bool (*realize_late)(HostIOMMUDevice *hiod, void *opaque, Error **errp);
> /**
> * @get_cap: check if a host IOMMU device capability is supported.
> *
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index abbdc56b6d..e198b1e5a2 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1550,6 +1550,7 @@ bool vfio_attach_device(char *name, VFIODevice *vbasedev,
> const VFIOIOMMUClass *ops =
> VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));
> HostIOMMUDevice *hiod = NULL;
> + HostIOMMUDeviceClass *hiod_ops = NULL;
>
> if (vbasedev->iommufd) {
> ops = VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
> @@ -1560,16 +1561,26 @@ bool vfio_attach_device(char *name, VFIODevice *vbasedev,
>
> if (!vbasedev->mdev) {
> hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
> + hiod_ops = HOST_IOMMU_DEVICE_GET_CLASS(hiod);
> vbasedev->hiod = hiod;
> }
>
> if (!ops->attach_device(name, vbasedev, as, errp)) {
> - object_unref(hiod);
> - vbasedev->hiod = NULL;
> - return false;
> + goto err_attach;
> + }
> +
> + if (hiod_ops && hiod_ops->realize_late &&
> + !hiod_ops->realize_late(hiod, vbasedev, errp)) {
> + ops->detach_device(vbasedev);
> + goto err_attach;
> }
>
> return true;
> +
> +err_attach:
> + object_unref(hiod);
> + vbasedev->hiod = NULL;
> + return false;
> }
>
> void vfio_detach_device(VFIODevice *vbasedev)
On 4/7/25 13:19, Cédric Le Goater wrote:
> On 2/19/25 09:22, Zhenzhong Duan wrote:
>> Currently we have realize() callback which is called before attachment.
>> But there are still some elements e.g., hwpt_id is not ready before
>> attachment. So we need a realize_late() callback to further initialize
>> them.
>
> The relation between objects HostIOMMUDevice and VFIOIOMMU is starting
> to look too complex for me.
>
> I think it makes sense to realize HostIOMMUDevice after the device
> is attached. Can't we move :
>
> hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
> vbasedev->hiod = hiod;
>
> under ->attach_device() and also the call :
>
> if (!vfio_device_hiod_realize(vbasedev, errp)) {
>
> later in the ->attach_device() patch ?
>
> hiod_legacy_vfio_realize() doesn't do much. We might need to rework
> hiod_iommufd_vfio_realize() which queries the iommufd hw caps, later
> used by intel-iommu.
The only dependency I see on the IOMMUFD HostIOMMUDevice when attaching
the device to the container is in iommufd_cdev_autodomains_get(). The
flags for IOMMU_HWPT_ALLOC depends on the HW capability of the IOMMFD
backend and we rely on hiod_iommufd_vfio_realize() to have done the
query on the iommufd kernel device before.
Since this is not a hot path, I don't think it is a problem to add
a redundant call to iommufd_backend_get_device_info() in
iommufd_cdev_autodomains_get() and avoid the IOMMUFD HostIOMMUDevice
dependency. With that we can move the HostIOMMUDevice creation and
realize sequence at the end of the device attach sequence.
I think this makes the code cleaner when it comes to using the
vbasedev->hiod pointer too.
> Anyway, it is good time to cleanup our interfaces before adding more.
On that topic, I think
iommufd_cdev_attach_ioas_hwpt
iommufd_cdev_detach_ioas_hwpt
belong to IOMMUFD backend.
Thanks,
C.
>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH rfcv2 03/20] HostIOMMUDevice: Introduce realize_late
>callback
>
>On 4/7/25 13:19, Cédric Le Goater wrote:
>> On 2/19/25 09:22, Zhenzhong Duan wrote:
>>> Currently we have realize() callback which is called before attachment.
>>> But there are still some elements e.g., hwpt_id is not ready before
>>> attachment. So we need a realize_late() callback to further initialize
>>> them.
>>
>> The relation between objects HostIOMMUDevice and VFIOIOMMU is starting
>> to look too complex for me.
Agree.
>>
>> I think it makes sense to realize HostIOMMUDevice after the device
>> is attached. Can't we move :
>>
>> hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
>> vbasedev->hiod = hiod;
>>
>> under ->attach_device() and also the call :
>>
>> if (!vfio_device_hiod_realize(vbasedev, errp)) {
>>
>> later in the ->attach_device() patch ?
>>
>> hiod_legacy_vfio_realize() doesn't do much. We might need to rework
>> hiod_iommufd_vfio_realize() which queries the iommufd hw caps, later
>> used by intel-iommu.
>
>The only dependency I see on the IOMMUFD HostIOMMUDevice when attaching
>the device to the container is in iommufd_cdev_autodomains_get(). The
>flags for IOMMU_HWPT_ALLOC depends on the HW capability of the IOMMFD
>backend and we rely on hiod_iommufd_vfio_realize() to have done the
>query on the iommufd kernel device before.
>
>Since this is not a hot path, I don't think it is a problem to add
>a redundant call to iommufd_backend_get_device_info() in
>iommufd_cdev_autodomains_get() and avoid the IOMMUFD HostIOMMUDevice
>dependency. With that we can move the HostIOMMUDevice creation and
>realize sequence at the end of the device attach sequence.
Yes.
>
>I think this makes the code cleaner when it comes to using the
>vbasedev->hiod pointer too.
>
>> Anyway, it is good time to cleanup our interfaces before adding more.
OK, let me think about this further and write some patches to move .realize() after .attach_device().
will be based on vfio-next.
>
>On that topic, I think
>
> iommufd_cdev_attach_ioas_hwpt
> iommufd_cdev_detach_ioas_hwpt
>
>belong to IOMMUFD backend.
They are operation on VFIODevice, backends/iommufd.c are for operation on IOMMUFDBackend,
Do we need to move iommufd_cdev_attach/detach_ioas_hwpt to backends/iommufd.c which is VFIODevice agnostic?
Thanks
Zhenzhong
On 4/9/25 10:27, Duan, Zhenzhong wrote:
>
>
>> -----Original Message-----
>> From: Cédric Le Goater <clg@redhat.com>
>> Subject: Re: [PATCH rfcv2 03/20] HostIOMMUDevice: Introduce realize_late
>> callback
>>
>> On 4/7/25 13:19, Cédric Le Goater wrote:
>>> On 2/19/25 09:22, Zhenzhong Duan wrote:
>>>> Currently we have realize() callback which is called before attachment.
>>>> But there are still some elements e.g., hwpt_id is not ready before
>>>> attachment. So we need a realize_late() callback to further initialize
>>>> them.
>>>
>>> The relation between objects HostIOMMUDevice and VFIOIOMMU is starting
>>> to look too complex for me.
>
> Agree.
>
>>>
>>> I think it makes sense to realize HostIOMMUDevice after the device
>>> is attached. Can't we move :
>>>
>>> hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
>>> vbasedev->hiod = hiod;
>>>
>>> under ->attach_device() and also the call :
>>>
>>> if (!vfio_device_hiod_realize(vbasedev, errp)) {
>>>
>>> later in the ->attach_device() patch ?
>>>
>>> hiod_legacy_vfio_realize() doesn't do much. We might need to rework
>>> hiod_iommufd_vfio_realize() which queries the iommufd hw caps, later
>>> used by intel-iommu.
>>
>> The only dependency I see on the IOMMUFD HostIOMMUDevice when attaching
>> the device to the container is in iommufd_cdev_autodomains_get(). The
>> flags for IOMMU_HWPT_ALLOC depends on the HW capability of the IOMMFD
>> backend and we rely on hiod_iommufd_vfio_realize() to have done the
>> query on the iommufd kernel device before.
>>
>> Since this is not a hot path, I don't think it is a problem to add
>> a redundant call to iommufd_backend_get_device_info() in
>> iommufd_cdev_autodomains_get() and avoid the IOMMUFD HostIOMMUDevice
>> dependency. With that we can move the HostIOMMUDevice creation and
>> realize sequence at the end of the device attach sequence.
>
> Yes.
>
>>
>> I think this makes the code cleaner when it comes to using the
>> vbasedev->hiod pointer too.
>>
>>> Anyway, it is good time to cleanup our interfaces before adding more.
>
> OK, let me think about this further and write some patches to move .realize() after .attach_device().
> will be based on vfio-next.
I just updated the vfio-next branch with what should be in the next PR
for QEMU 10.1.
>
>>
>> On that topic, I think
>>
>> iommufd_cdev_attach_ioas_hwpt
>> iommufd_cdev_detach_ioas_hwpt
>>
>> belong to IOMMUFD backend.
>
> They are operation on VFIODevice, backends/iommufd.c are for operation on IOMMUFDBackend,
> Do we need to move iommufd_cdev_attach/detach_ioas_hwpt to backends/iommufd.c which is VFIODevice agnostic?
My mistake. I was confused with
int iommufd = vbasedev->iommufd->fd
and thought we could simply replace 'VFIODevice *' parameter with a
'IOMMUFDBackend *be' parameter but this is not the case.
Thanks,
C.
On 2/19/25 9:22 AM, Zhenzhong Duan wrote:
> Currently we have realize() callback which is called before attachment.
> But there are still some elements e.g., hwpt_id is not ready before
> attachment. So we need a realize_late() callback to further initialize
> them.
from the description it is not obvious why the realize() could not have
been called after the attach. Could you remind the reader what is the
reason?
Thanks
Eric
>
> Currently, this callback is only useful for iommufd backend. For legacy
> backend nothing needs to be initialized after attachment.
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
> include/system/host_iommu_device.h | 17 +++++++++++++++++
> hw/vfio/common.c | 17 ++++++++++++++---
> 2 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/include/system/host_iommu_device.h b/include/system/host_iommu_device.h
> index 809cced4ba..df782598f2 100644
> --- a/include/system/host_iommu_device.h
> +++ b/include/system/host_iommu_device.h
> @@ -66,6 +66,23 @@ struct HostIOMMUDeviceClass {
> * Returns: true on success, false on failure.
> */
> bool (*realize)(HostIOMMUDevice *hiod, void *opaque, Error **errp);
> + /**
> + * @realize_late: initialize host IOMMU device instance after attachment,
> + * some elements e.g., ioas are ready only after attachment.
> + * This callback initialize them.
> + *
> + * Optional callback.
> + *
> + * @hiod: pointer to a host IOMMU device instance.
> + *
> + * @opaque: pointer to agent device of this host IOMMU device,
> + * e.g., VFIO base device or VDPA device.
> + *
> + * @errp: pass an Error out when realize fails.
> + *
> + * Returns: true on success, false on failure.
> + */
> + bool (*realize_late)(HostIOMMUDevice *hiod, void *opaque, Error **errp);
> /**
> * @get_cap: check if a host IOMMU device capability is supported.
> *
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index abbdc56b6d..e198b1e5a2 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1550,6 +1550,7 @@ bool vfio_attach_device(char *name, VFIODevice *vbasedev,
> const VFIOIOMMUClass *ops =
> VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));
> HostIOMMUDevice *hiod = NULL;
> + HostIOMMUDeviceClass *hiod_ops = NULL;
>
> if (vbasedev->iommufd) {
> ops = VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
> @@ -1560,16 +1561,26 @@ bool vfio_attach_device(char *name, VFIODevice *vbasedev,
>
> if (!vbasedev->mdev) {
> hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
> + hiod_ops = HOST_IOMMU_DEVICE_GET_CLASS(hiod);
> vbasedev->hiod = hiod;
> }
>
> if (!ops->attach_device(name, vbasedev, as, errp)) {
> - object_unref(hiod);
> - vbasedev->hiod = NULL;
> - return false;
> + goto err_attach;
> + }
> +
> + if (hiod_ops && hiod_ops->realize_late &&
> + !hiod_ops->realize_late(hiod, vbasedev, errp)) {
> + ops->detach_device(vbasedev);
> + goto err_attach;
> }
>
> return true;
> +
> +err_attach:
> + object_unref(hiod);
> + vbasedev->hiod = NULL;
> + return false;
> }
>
> void vfio_detach_device(VFIODevice *vbasedev)
>-----Original Message----- >From: Eric Auger <eric.auger@redhat.com> >Subject: Re: [PATCH rfcv2 03/20] HostIOMMUDevice: Introduce realize_late >callback > > > > >On 2/19/25 9:22 AM, Zhenzhong Duan wrote: >> Currently we have realize() callback which is called before attachment. >> But there are still some elements e.g., hwpt_id is not ready before >> attachment. So we need a realize_late() callback to further initialize >> them. >from the description it is not obvious why the realize() could not have >been called after the attach. Could you remind the reader what is the >reason? Sure, will rephrase as below: " HostIOMMUDevice provides some elements to vIOMMU, but there are some which are ready after attachment, e.g., hwpt_id. Before create and attach to a new hwpt with IOMMU dirty tracking capability, we have to call realize() to get if hardware IOMMU supports dirty tracking capability. So moving realize() after attach() will not work here, we need a new callback realize_late() to further initialize those elements. Currently, this callback is only useful for iommufd backend. For legacy backend nothing needs to be initialized after attachment. " Thanks Zhenzhong
Hi Zhenzhong, On 2/28/25 9:16 AM, Duan, Zhenzhong wrote: > >> -----Original Message----- >> From: Eric Auger <eric.auger@redhat.com> >> Subject: Re: [PATCH rfcv2 03/20] HostIOMMUDevice: Introduce realize_late >> callback >> >> >> >> >> On 2/19/25 9:22 AM, Zhenzhong Duan wrote: >>> Currently we have realize() callback which is called before attachment. >>> But there are still some elements e.g., hwpt_id is not ready before >>> attachment. So we need a realize_late() callback to further initialize >>> them. > >from the description it is not obvious why the realize() could not have >> been called after the attach. Could you remind the reader what is the >> reason? > Sure, will rephrase as below: > > " HostIOMMUDevice provides some elements to vIOMMU, but there are some which > are ready after attachment, e.g., hwpt_id. > > Before create and attach to a new hwpt with IOMMU dirty tracking capability, > we have to call realize() to get if hardware IOMMU supports dirty tracking > capability. > > So moving realize() after attach() will not work here, we need a new callback > realize_late() to further initialize those elements. > > Currently, this callback is only useful for iommufd backend. For legacy > backend nothing needs to be initialized after attachment. " OK this helps me Thanks Eric > > Thanks > Zhenzhong >
© 2016 - 2026 Red Hat, Inc.