>-----Original Message-----
>From: Joao Martins <joao.m.mart...@oracle.com>
>Subject: [PATCH v5 08/13] vfio/{iommufd,container}: Invoke
>HostIOMMUDevice::realize() during attach_device()
>
>Move the HostIOMMUDevice::realize() to be invoked during the attach of
>the device
>before we allocate IOMMUFD hardware pagetable objects (HWPT). This
>allows the use
>of the hw_caps obtained by IOMMU_GET_HW_INFO that essentially tell if
>the IOMMU
>behind the device supports dirty tracking.
>
>Note: The HostIOMMUDevice data from legacy backend is static and doesn't
>need any information from the (type1-iommu) backend to be initialized.
>In contrast however, the IOMMUFD HostIOMMUDevice data requires the
>iommufd FD to be connected and having a devid to be able to successfully
>GET_HW_INFO. This means vfio_device_hiod_realize() is called in
>different places within the backend .attach_device() implementation.
>
>Suggested-by: Cédric Le Goater <c...@redhat.cm>
>Signed-off-by: Joao Martins <joao.m.mart...@oracle.com>
Reviewed-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
Thanks
Zhenzhong
>---
> include/hw/vfio/vfio-common.h | 1 +
> hw/vfio/common.c | 16 ++++++----------
> hw/vfio/container.c | 4 ++++
> hw/vfio/helpers.c | 11 +++++++++++
> hw/vfio/iommufd.c | 4 ++++
> 5 files changed, 26 insertions(+), 10 deletions(-)
>
>diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-
>common.h
>index 1a96678f8c38..4e44b26d3c45 100644
>--- a/include/hw/vfio/vfio-common.h
>+++ b/include/hw/vfio/vfio-common.h
>@@ -242,6 +242,7 @@ void vfio_region_finalize(VFIORegion *region);
> 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);
> bool vfio_attach_device(char *name, VFIODevice *vbasedev,
> AddressSpace *as, Error **errp);
> void vfio_detach_device(VFIODevice *vbasedev);
>diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>index b0beed44116e..cc14f0e3fe24 100644
>--- a/hw/vfio/common.c
>+++ b/hw/vfio/common.c
>@@ -1544,7 +1544,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;
>+ HostIOMMUDevice *hiod = NULL;
>
> if (vbasedev->iommufd) {
> ops =
>VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUF
>D));
>@@ -1552,21 +1552,17 @@ bool vfio_attach_device(char *name,
>VFIODevice *vbasedev,
>
> assert(ops);
>
>- if (!ops->attach_device(name, vbasedev, as, errp)) {
>- return false;
>- }
>
>- if (vbasedev->mdev) {
>- return true;
>+ if (!vbasedev->mdev) {
>+ hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
>+ vbasedev->hiod = hiod;
> }
>
>- hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
>- if (!HOST_IOMMU_DEVICE_GET_CLASS(hiod)->realize(hiod, vbasedev,
>errp)) {
>+ if (!ops->attach_device(name, vbasedev, as, errp)) {
> object_unref(hiod);
>- ops->detach_device(vbasedev);
>+ vbasedev->hiod = NULL;
> return false;
> }
>- vbasedev->hiod = hiod;
>
> return true;
> }
>diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>index c27f448ba26e..adb302216e23 100644
>--- a/hw/vfio/container.c
>+++ b/hw/vfio/container.c
>@@ -917,6 +917,10 @@ static bool vfio_legacy_attach_device(const char
>*name, VFIODevice *vbasedev,
>
> trace_vfio_attach_device(vbasedev->name, groupid);
>
>+ if (!vfio_device_hiod_realize(vbasedev, errp)) {
>+ return false;
>+ }
>+
> group = vfio_get_group(groupid, as, errp);
> if (!group) {
> return false;
>diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
>index 7e23e9080c9d..ea15c79db0a3 100644
>--- a/hw/vfio/helpers.c
>+++ b/hw/vfio/helpers.c
>@@ -689,3 +689,14 @@ bool vfio_device_is_mdev(VFIODevice *vbasedev)
> subsys = realpath(tmp, NULL);
> 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);
>+}
>diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>index 7a10b1e90a6f..bb44d948c735 100644
>--- a/hw/vfio/iommufd.c
>+++ b/hw/vfio/iommufd.c
>@@ -403,6 +403,10 @@ static bool iommufd_cdev_attach(const char
>*name, VFIODevice *vbasedev,
>
> space = vfio_get_address_space(as);
>
>+ if (!vfio_device_hiod_realize(vbasedev, errp)) {
>+ return false;
>+ }
>+
> /* try to attach to an existing container in this space */
> QLIST_FOREACH(bcontainer, &space->containers, next) {
> container = container_of(bcontainer, VFIOIOMMUFDContainer,
>bcontainer);
>--
>2.17.2
