On 4/11/25 12:17, Zhenzhong Duan wrote:
Previously device attaching depends on realize() getting host iommu
capabilities to check dirty tracking support.
Now we save a caps copy in VFIODevice and check that copy for dirty
tracking support, there is no dependency any more, move realize()
call after attach_device() call in vfio_device_attach().
Drop vfio_device_hiod_realize() which looks redundant now.
Suggested-by: Cédric Le Goater <c...@redhat.com>
Suggested-by: Donald Dutile <ddut...@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
---
include/hw/vfio/vfio-device.h | 1 -
hw/vfio/container.c | 4 ----
hw/vfio/device.c | 28 +++++++++++-----------------
hw/vfio/iommufd.c | 4 ----
4 files changed, 11 insertions(+), 26 deletions(-)
diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
index 09a7af891a..14559733c6 100644
--- a/include/hw/vfio/vfio-device.h
+++ b/include/hw/vfio/vfio-device.h
@@ -124,7 +124,6 @@ bool vfio_device_irq_set_signaling(VFIODevice *vbasedev,
int index, int subindex
void vfio_device_reset_handler(void *opaque);
bool vfio_device_is_mdev(VFIODevice *vbasedev);
-bool vfio_device_hiod_realize(VFIODevice *vbasedev, Error **errp);
bool vfio_device_attach(char *name, VFIODevice *vbasedev,
AddressSpace *as, Error **errp);
void vfio_device_detach(VFIODevice *vbasedev);
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 23a3373470..676e88cef4 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -883,10 +883,6 @@ static bool vfio_legacy_attach_device(const char *name,
VFIODevice *vbasedev,
trace_vfio_device_attach(vbasedev->name, groupid);
- if (!vfio_device_hiod_realize(vbasedev, errp)) {
- return false;
- }
-
group = vfio_group_get(groupid, as, errp);
if (!group) {
return false;
diff --git a/hw/vfio/device.c b/hw/vfio/device.c
index 4de6948cf4..6154d3f443 100644
--- a/hw/vfio/device.c
+++ b/hw/vfio/device.c
@@ -347,17 +347,6 @@ bool vfio_device_is_mdev(VFIODevice *vbasedev)
return subsys && (strcmp(subsys, "/sys/bus/mdev") == 0);
}
-bool vfio_device_hiod_realize(VFIODevice *vbasedev, Error **errp)
-{
- HostIOMMUDevice *hiod = vbasedev->hiod;
-
- if (!hiod) {
- return true;
- }
-
- return HOST_IOMMU_DEVICE_GET_CLASS(hiod)->realize(hiod, vbasedev, errp);
-}
-
VFIODevice *vfio_get_vfio_device(Object *obj)
{
if (object_dynamic_cast(obj, TYPE_VFIO_PCI)) {
@@ -372,6 +361,7 @@ bool vfio_device_attach(char *name, VFIODevice *vbasedev,
{
const VFIOIOMMUClass *ops =
VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));
+ HostIOMMUDeviceClass *hiodc;
HostIOMMUDevice *hiod = NULL;
if (vbasedev->iommufd) {
@@ -380,16 +370,20 @@ bool vfio_device_attach(char *name, VFIODevice *vbasedev,
assert(ops);
+ if (!ops->attach_device(name, vbasedev, as, errp)) {
+ return false;
+ }
if (!vbasedev->mdev) {
hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
- vbasedev->hiod = hiod;
- }
+ hiodc = HOST_IOMMU_DEVICE_GET_CLASS(hiod);
- if (!ops->attach_device(name, vbasedev, as, errp)) {
- object_unref(hiod);
- vbasedev->hiod = NULL;
- return false;
+ if (!hiodc->realize(hiod, vbasedev, errp)) {
+ object_unref(hiod);
+ ops->detach_device(vbasedev);
+ return false;
+ }
+ vbasedev->hiod = hiod;
This is not what I meant. I was not clear enough. Sorry about that.
hiodc->realize can be called under each container backend: legacy
and iommufd. I don't see much much value to make it common and
it would remove the unref/detach sequence to handle errors.
Thanks,
C.
}
return true;
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 530cde6740..e05b472e35 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -502,10 +502,6 @@ static bool iommufd_cdev_attach(const char *name,
VFIODevice *vbasedev,
* FD to be connected and having a devid to be able to successfully call
* iommufd_backend_get_device_info().
*/
- if (!vfio_device_hiod_realize(vbasedev, errp)) {
- goto err_alloc_ioas;
- }
-
if (!iommufd_backend_get_device_info(vbasedev->iommufd, vbasedev->devid,
&caps->type, &caps->vendor_caps,
sizeof(VendorCaps), &caps->hw_caps,
