| Message ID | 20231003101530.288864-14-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Prerequisite changes for IOMMUFD support | expand |
On 10/3/23 12:14, Eric Auger wrote: > From: Zhenzhong Duan <zhenzhong.duan@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.auger@redhat.com> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@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 ? 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);
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.duan@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.auger@redhat.com> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@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); >
Hello Eric, >>> @@ -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. the device-introspect-test needs it. No need to resend a v5, I can add it back in v4 if you are ok with that. Thanks, C.
Hi Cédric, On 10/4/23 18:55, Cédric Le Goater wrote: > the device-introspect-test needs it. No need to resend a v5, I can add it > back in v4 if you are ok with that. Ah OK. I am definitively OK for you to restore it if nothing else shows up inbetween Eric
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; + } + 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);