Hi Cédric,
On 10/3/23 17:59, Cédric Le Goater wrote:
> On 10/3/23 12:14, Eric Auger wrote:
>> From: Zhenzhong Duan <zhenzhong.d...@intel.com>
>>
>> let's store the parent contaienr within the VFIODevice.
>> This simplifies the logic in vfio_viommu_preset() and
>> brings the benefice to hide the group specificity which
>> is useful for IOMMUFD migration.
>>
>> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>> ---
>> include/hw/vfio/vfio-common.h | 1 +
>> hw/vfio/common.c | 8 +++++++-
>> 2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/vfio/vfio-common.h
>> b/include/hw/vfio/vfio-common.h
>> index 8ca70dd821..bf12e40667 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -132,6 +132,7 @@ typedef struct VFIODevice {
>> QLIST_ENTRY(VFIODevice) next;
>> QLIST_ENTRY(VFIODevice) container_next;
>> struct VFIOGroup *group;
>> + VFIOContainer *container;
>> char *sysfsdev;
>> char *name;
>> DeviceState *dev;
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index ef9dc7c747..55f8a113ea 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -184,7 +184,7 @@ void vfio_unblock_multiple_devices_migration(void)
>> bool vfio_viommu_preset(VFIODevice *vbasedev)
>> {
>> - return vbasedev->group->container->space->as !=
>> &address_space_memory;
>> + return vbasedev->container->space->as != &address_space_memory;
>> }
>> static void vfio_set_migration_error(int err)
>> @@ -2655,6 +2655,7 @@ int vfio_attach_device(char *name, VFIODevice
>> *vbasedev,
>> }
>> container = group->container;
>> + vbasedev->container = container;
>> QLIST_INSERT_HEAD(&container->device_list, vbasedev,
>> container_next);
>> return ret;
>> @@ -2664,7 +2665,12 @@ void vfio_detach_device(VFIODevice *vbasedev)
>> {
>> VFIOGroup *group = vbasedev->group;
>> + if (!vbasedev->container) {
>> + return;
>> + }
>
> Can this happen ? Should it be an assert ?
I don't think so. Let me simply drop the check.
Thanks
Eric
>
> Thanks,
>
> C.
>
>
>> +
>> QLIST_REMOVE(vbasedev, container_next);
>> + vbasedev->container = NULL;
>> trace_vfio_detach_device(vbasedev->name, group->groupid);
>> vfio_put_base_device(vbasedev);
>> vfio_put_group(group);
>
